[PATCH] D153755: [GlobalISel] Generalize `InstructionSelector` Match Tables

Pierre van Houtryve via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 12 04:02:43 PDT 2023


Pierre-vh added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h:456
   CodeGenCoverage *CoverageInfo = nullptr;
   GISelKnownBits *KnownBits = nullptr;
   MachineFunction *MF = nullptr;
----------------
foad wrote:
> Hi @Pierre-vh, after merging this patch into a downstream branch I find I have to write this in some of our C++ combines:
> ```
>   struct KnownBits Op0KnownBits = KnownBits->getKnownBits(Reg0);
> ```
> This is a bit ugly because of the name clash between the KnownBits struct and this KnownBits field. I guess this is not a new problem, since you did not change this line, but it is now affecting more code.
> 
> Do you think it would make sense to change the name of this field?
Sure, I wouldn't mind changing the name. Should we just name it `KB`?
Do you want me to make a patch?

Those fields really ought to be cleaned-up some day anyway. Once the older backend is removed I plan to do a pass over this code and eliminate as much redundancy as possible. Things aren't a good state right now because we still need to allow downstream users to use the older combiner backend. e.g. the whole "CombinerInfo" class is useless with the new combiner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153755



More information about the llvm-commits mailing list