[PATCH] D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 8 10:03:23 PDT 2022


MaskRay added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVBaseInfo.cpp:380
+  switch (e) {
+  case ExecutionMode::Invocations:
+    return "Invocations";
----------------
iliya-diyachkov wrote:
> MaskRay wrote:
> > Can we use X macros for the long list?
> Initially all these functions were generated using macros. Then Renato suggested getting rid of them and we agreed to use expanded macros for now. Later we'll re-implement all the functions with tablegen (this work is in progress so I expect it to be completed in several months).
> 
> I have already fixed a number of issues in these functions after expanding the macros. I'm not sure that now we really need to replace them with new macros.
Ah, ok. @rengolin I feel that X macros may be more readable and less error-prone than open coding here.

TableGen can generate such strings as well, but it will add one step, bring a little abstraction code, and is probably not conciser than macros here (AIUI one line for one enumeration value). WDYT?


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

https://reviews.llvm.org/D116464



More information about the llvm-commits mailing list