circt icon indicating copy to clipboard operation
circt copied to clipboard

Adopt "pretty accessors" in MLIR

Open lattner opened this issue 4 years ago • 5 comments

MLIR mainline added the ability to synthesize get/setters with a properly camel cased and get/set-prefixed name, see this patch and this announcement.

We should do a merge from MLIR mainline to get this, and then adopt it in the CIRCT dialects. I don't think we need to do the multistage adoption thing, we can just jump directly to _Prefixed

lattner avatar Oct 16 '21 20:10 lattner

Should this be closed or is it still available?

thatTPR avatar Oct 22 '21 19:10 thatTPR

I think the Merge was done, but we haven't adopted the new names yet.

lattner avatar Oct 24 '21 00:10 lattner

There were still a few things missing in MLIR-tablegen to generate printers from the assembly-format etc. with the new prefixes which were addressed in https://github.com/llvm/llvm-project/commit/6a99423390995f874ee4e345157523f4139ccbef We should bump the LLVM version to that first. It might also make sense to wait until MLIR switches the default to 'Both' here, then we can avoid setting that flag separately for every dialect.

maerhart avatar Oct 24 '21 09:10 maerhart

Aha, right, I thought we already had that, thanks.

lattner avatar Oct 24 '21 19:10 lattner

Dialects that have adopted prefixed accessors so far:

  • [ ] Calyx
  • [x] Comb
  • [ ] ESI
  • [x] FIRRTL
  • [ ] FSM
  • [ ] Handshake
  • [x] HW
  • [ ] HWArith
  • [x] LLHD
  • [x] Moore
  • [ ] MSFT
  • [x] Seq
  • [ ] StaticLogic
  • [x] SV

maerhart avatar Jul 14 '22 06:07 maerhart