[PATCH] D142083: [X86][NFC] Move MemoryFoldTable2Addr MemoryFoldTable0~4 into X86InstrFoldTables.def

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 1 00:40:58 PST 2023


craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86MemFoldTables.inc:2
+static const X86MemoryFoldTableEntry MemoryFoldTable2Addr[] = {
+{X86::ADD16ri8_DB,X86::ADD16mi8,TB_NO_REVERSE},
+{X86::ADD16ri_DB,X86::ADD16mi,TB_NO_REVERSE},
----------------
yubing wrote:
> skan wrote:
> > RKSimon wrote:
> > > pengfei wrote:
> > > > skan wrote:
> > > > > pengfei wrote:
> > > > > > Can we format this file, or at least add two spaces for it?
> > > > > > Can we format this file, or at least add two spaces for it?
> > > > > 
> > > > > You mean one space after the first and the second comma?
> > > > No, just
> > > > ```
> > > > static const X86MemoryFoldTableEntry MemoryFoldTable2Addr[] = {
> > > >   {X86::ADD16ri8_DB,X86::ADD16mi8,TB_NO_REVERSE},
> > > > ```
> > > column alignment would be even better
> > Column aligment for huge array may bring issue for future development. For example, if we add a instruction with a long name, e.g `X86::AAA_BBB_CCC_DDD_123_mi`, then we have to format all the array to achive column aligment again.
> Column alignment seems more reasonable, auto-gen mechanism can use OS.PadToColumn(40); if there is a longer instruction name in the future, we can make padding larger.
PadToColumn is kind of expensive. It requires updating a column counter during every print operation. Or at least it was on X86GenDAGISel.inc years ago before I removed it.

Since you’re printing a table it’s probably better to just print each string with padding. That should be cheaper than tracking a column. left_justify in Format.h might work


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142083/new/

https://reviews.llvm.org/D142083



More information about the llvm-commits mailing list