Fix i1 array global crash in NVPTXAsmPrinter. #92506
Conversation
See the test file. At head, this crashes with ``` assertion failed at llvm/lib/Support/APInt.cpp:492 in uint64_t llvm::APInt::extractBitsAsZExtValue(unsigned int, unsigned int) const: bitPosition < BitWidth && (numBits + bitPosition) <= BitWidth && "Illegal bit extraction" ```
|
Cal you provide more details on why exactly that assertion got triggered with |
| @@ -1847,9 +1847,13 @@ void NVPTXAsmPrinter::bufferLEByte(const Constant *CPV, int Bytes, | |||
| auto AddIntToBuffer = [AggBuffer, Bytes](const APInt &Val) { | |||
| size_t NumBytes = (Val.getBitWidth() + 7) / 8; | |||
| SmallVector<unsigned char, 16> Buf(NumBytes); | |||
| for (unsigned I = 0; I < NumBytes; ++I) { | |||
| for (unsigned I = 0; I < NumBytes - 1; ++I) { | |||
There was a problem hiding this comment.
This could certainly use a comment or two about what's going on, because it's not obvious. At all.
Why exactly are we special casing the last byte?
There was a problem hiding this comment.
It's explained in the PR description, also look at the test. If the type's bitwidth is not a multiple of 8, the old code crashes.
There was a problem hiding this comment.
PR description only shows the expression for the failed assertion: bitPosition < BitWidth && (numBits + bitPosition) <= BitWidth, but it was not clear which part of that assertion has failed.
The substance of the fix LGTM.
That said, If I ever need to look at this code in the future, I'm pretty sure I'll have to scratch my head, again, pondering what's going on here. Hence, my suggestion that a comment would be helpful. We already have way too many puzzles, we do not need more.
There was a problem hiding this comment.
Sounds good. Sent a follow-up.
This is a follow-up to llvm#92506.
This is a follow-up to llvm#92506.
This is a follow-up to #92506.
This is a follow-up to llvm#92506.
See the test file. At head, this crashes with