[SystemZ] Fix off-by-one error in backend#200141
Conversation
|
@llvm/pr-subscribers-backend-systemz Author: Dominik Steenken (dominik-steenken) ChangesWhen combineCCMask is called on a TM node with two constant operands, and all of the bits in the mask are active, the existing APInt bit access goes off the overall length of the integer by one. This commit fixes that by using the index value of the leftmost active bit, rather than the number of active bits. Full diff: https://github.com/llvm/llvm-project/pull/200141.diff 1 Files Affected:
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index 26cc921b6b169..c0e671b31fb1a 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -8838,7 +8838,7 @@ static bool combineCCMask(SDValue &CCReg, int &CCValid, int &CCMask,
auto Result = Op0APVal & Op1APVal;
bool AllOnes = Result == Op1APVal;
bool AllZeros = Result == 0;
- bool IsLeftMostBitSet = Result[Op1APVal.getActiveBits()] != 0;
+ bool IsLeftMostBitSet = Result[Op1APVal.getActiveBits() - 1] != 0;
return AllZeros ? 0 : AllOnes ? 3 : IsLeftMostBitSet ? 2 : 1;
};
SDValue Op0 = CCNode->getOperand(0);
|
|
Good catch, thanks! Can we add a test case? |
|
I've added a test case derived from the reduction of the csmith-generated program that triggered the error. Since this is just meant to test whether the assert is triggered, it does not include any |
Thanks! Can we still add (auto-generated) CHECK lines - it would be good to also verify we're in fact getting correct output from this logic ... |
When combineCCMask is called on a TM node with two constant operands, and all of the bits in the mask are active, the existing APInt bit access goes off the overall length of the integer by one. This commit fixes that by using the index value of the leftmost active bit, rather than the number of active bits.
b99ffca to
edee83a
Compare
github-project-automation
Bot
moved this from Needs Triage
to Done
in LLVM Release Status
|
/cherry-pick 78eca55 |
|
/pull-request #200384 |
When combineCCMask is called on a TM node with two constant operands, and all of the bits in the mask are active, the existing APInt bit access goes off the overall length of the integer by one. This commit fixes that by using the index value of the leftmost active bit, rather than the number of active bits. (cherry picked from commit 78eca55)
When combineCCMask is called on a TM node with two constant operands, and all of the bits in the mask are active, the existing APInt bit access goes off the overall length of the integer by one. This commit fixes that by using the index value of the leftmost active bit, rather than the number of active bits. (cherry picked from commit 78eca55)
3 participants
When combineCCMask is called on a TM node with two constant operands, and all of the bits in the mask are active, the existing APInt bit access goes off the overall length of the integer by one. This commit fixes that by using the index value of the leftmost active bit, rather than the number of active bits.