[PATCH] D127504: [NVPTX] Use the mask() operator to initialize packed structs with pointers

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp:1197
           if (aggBuffer.numSymbols) {
-            if (static_cast<const NVPTXTargetMachine &>(TM).is64Bit()) {
+            if (Packed) {
+              if (!STI.hasMaskOperator())
----------------
tra wrote:
> What will happen if we have an aggregate with a mix of packed and unpacked fields and we only want to init with a pointer located in the unpacked part of it? Will something like this trigger the error?
> ```
> %ti = type <{ i8, i32 }>
> %to = type { i8, %ti, i32, void ()* }
> @n = addrspace(1) global %to {
>     i8 12,
>     %ti <{ i8 56, i32 78 }>,
>     i32 34,
>     void()* @func
>     }
> ```
> 
> It's one of the test cases below with another pointer added to non-packed top-level struct.
> 
Unfortunately, yes, that would trigger the error. On the other hand, supporting that case would require a deeper analysis of the initialization expression itself, not only the type, because there can be `ptrtoint` operators. I'm not sure if we need that complication, considering that CUDA 11.1 is almost 2 years old already.


================
Comment at: llvm/test/CodeGen/NVPTX/packed-aggr.ll:50
+    %ti <{ void()* @func }>,
+; CHECK-SAME:   0xFF(func), 0xFF00(func), 0xFF0000(func), 0xFF000000(func),
+; CHECK64-SAME: 0xFF00000000(func), 0xFF0000000000(func), 0xFF000000000000(func), 0xFF00000000000000(func),
----------------
tra wrote:
> Interesting. Right now we're printing '0' for the function pointer, which is indeed not good at all. https://godbolt.org/z/KEexEar5o
That's true. Currently, the implementation does not expect pointers to be unaligned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127504