pmatos added subscribers: iliya-diyachkov, zuban32.
pmatos added a comment.
Thanks for the patch. Left a few comments. Others can take a better look, I am
sure. Maybe @iliya-diyachkov or @zuban32 can take a look as well.
================
Comment at: llvm/include/llvm/TargetParser/Triple.h:106
+ wasm32, // WebAssembly with 32-bit pointers
+ wasm64, // WebAssembly with 64-bit pointers
renderscript32, // 32-bit RenderScript
----------------
No need to reindent the whole block to add a single line.
================
Comment at: llvm/lib/TargetParser/Triple.cpp:74
+ case spirv:
+ return "spirv";
case spirv32: return "spirv32";
----------------
This should be in the same line.
================
Comment at: llvm/lib/TargetParser/Triple.cpp:387
+ .Case("spir64", spir64)
+ .Case("spirv", spirv)
+ .Case("spirv32", spirv32)
----------------
Same as above. I guess there's no need to reindent a whole block.
================
Comment at: llvm/lib/TargetParser/Triple.cpp:529
+ .Case("spir64", Triple::spir64)
+ .Cases("spirv", "spirv1.0", "spirv1.1", "spirv1.2", "spirv1.3",
+ "spirv1.4", "spirv1.5", Triple::spirv)
----------------
Whole block reindent.
================
Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+ EXPECT_TRUE(T.isSPIRV());
+
T.setArch(Triple::spirv32);
----------------
I am not well-versed in SPIRV for gfx but if we are using 32bits in SPIRV
logical, can't we reuse spirv32? Is there some sort of strong reason not to or
adding spirv for logical spirv as an alternative to spirv32 and spirv64 is just
easier?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155978/new/
https://reviews.llvm.org/D155978
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
