HazyFish
commented
1 year ago cc @DataCorrupted
Open HazyFish opened 1 year ago
HazyFish
commented
1 year ago cc @DataCorrupted
junaire
commented
1 year ago I think the problem is here (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp#L1817-L1823):
size_t NumBytes = (Val.getBitWidth() + 7) / 8;
SmallVector<unsigned char, 16> Buf(NumBytes);
for (unsigned I = 0; I < NumBytes; ++I) {
Buf[I] = Val.extractBitsAsZExtValue(8, I * 8);
}We calculate how many bytes the value contains (rounding up), so i20 got 3 bytes, then we try to zero extend that. However, since the last byte doesn't contain enough bits, so we got a crash.
I can work on this if someone can tell me what the expected behavior is :)
DataCorrupted
commented
1 year ago Thanks for the analysis. If that is correct root cause, you can fix it with one line of code.
Just explicitly pad zeros to the head of the Val by zext it Val->zext(NumBytes << 3); before this for loop.
DataCorrupted
commented
1 year ago @Artem-B @junaire After some digging I found the root cause of the crash, but I need some help fixing it.
In short, the size of AggBuffer is aligned as a whole, while the element is not aligned. A <4 x i20> has 80 bits thus getting 10 bytes. However, when coping a constant into the buffer(shown below), we treat each element type as byte aligned, thus <4 x i20> would require 4 x 3 = 12bytes.
This difference is the root cause of the problem.
To fix this I can modify AggBuffer to allow insertion of bits, but I am not sure if you will feel like too hacky, as AggBuffer is also designed to be byte aligned.
junaire
commented
1 year ago @Artem-B @junaire After some digging I found the root cause of the crash, but I need some help fixing it.
In short, the size of
AggBufferis aligned as a whole, while the element is not aligned. A<4 x i20>has 80 bits thus getting 10 bytes. However, when coping a constant into the buffer(shown below), we treat each element type as byte aligned, thus<4 x i20>would require 4 x 3 = 12bytes.This difference is the root cause of the problem. To fix this I can modify
AggBufferto allow insertion of bits, but I am not sure if you will feel like too hacky, asAggBufferis also designed to be byte aligned.
Hi, I actually tried this before (squash all bits into the AggBuffer without considering their alignment). Besides, I also tried to preallocate enough space for AggBuffer so every element and their alignment could fit. However, unfortunately, both strategies failed (verifier crashes, same backtrace above) :( I bet there's something different with the NVPTX target, so I'd like to know @Artem-B 's ideas...
Artem-B
commented
1 year ago Caveat: I didn't look into the details yet, will do once I'm back at work. The stuff below are my general thoughts on this.
Non-power-of-2 sized integers (let's call them PO2) are not handled particularly well in NVPTX in general. I vaguely recall that we promote scalars up to the next PO2 size, but vectors of such oddly-sized integers are likely to have problems, as you've discovered.
The problem does not exist when targeting x86_64, aarch64, riscv64, or wasm64
That's not a very strong argument.
v16i20 work? Yes. v16i20 not working a bug? Not necessarily. Finding IR which works on one back-end, but not the other is not that uncommon. Not all back-ends support everything representable in IR. Sometimes intentionally, sometimes because there was no need. This case is probably the latter. What to do:
TBH, I'm not even quite sure what would be the right way to store such a vector in memory.
16 x 3bytes as one byte is the smallest addressable memory bit?16 x i32 as i32 would be much more efficient for loads/stores?How does x64 deal with this? One of the implicit assumptions NVPTX users rely on in practice is that in-memory layout matches that of the host and the host sort of happens to be x64, even though these days NVIDIA GPUs are used on PPC and ARM as well. Making sure that we store such a vector the same way we do on x64 would probably make most sense.
Description
When targeting
nvptxornvptx64, the following code containing v16i20 global crashes backend withbitPosition < BitWidth && (numBits + bitPosition) <= BitWidth && "Illegal bit extraction"failed during passNVPTX Assembly Printerin debug build;corrupted size vs. prev_sizeduringmallocin release build.The problem does not exist when targeting
x86_64,aarch64,riscv64, orwasm64.Minimal Reproduction
https://godbolt.org/z/jnaqz5qq3
Code
Stack Trace (Debug)
Stack Trace (Release)