pmatos added inline comments.
================
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
----------------
Keenuts wrote:
> 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.
I know what you did and it makes sense - I did the same myself while working on
another backend but lets try not to change such a large amount of lines in a
patch focused on something else. I think it's certainly better to propose this
change in a single NFC patch if we think that clang-format output is better
than the existing formatting.
================
Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+ EXPECT_TRUE(T.isSPIRV());
+
T.setArch(Triple::spirv32);
----------------
Keenuts wrote:
> 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.
>
>
This is a tricky one tbh - I would have to do a bit more research to have a
more insighful reply here. Maybe @mpaszkowski or @Anastasia can add more.
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
