Keenuts marked 4 inline comments as done.
Keenuts added a comment.
Thanks for the review š
Reverted clang-format patch, and answered inline.
================
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
----------------
pmatos wrote:
> No need to reindent the whole block to add a single line.
IIRC it I did a clang-format because the buildbot complained the format was not
right.
Reverted the clang-format commit (which seems better, I agree), and I'll see if
the bots complains.
================
Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+ EXPECT_TRUE(T.isSPIRV());
+
T.setArch(Triple::spirv32);
----------------
pmatos wrote:
> 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?
This is a very valid question! And I'm not sure of the best option.
My understanding is: in logical SPIR-V, there are no notion of pointer size, or
architecture size. We cannot offset pointers by a sizeof(), or do such things.
But the options I had didn't seem to allow this kind of "undefined
architecture".
I chose 32bits here because the IDs are 32bit. But pointer addresses are not,
so returning 32bit is not quite right either.
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
