-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[DAG] SimplifyDemandedBits - ICMP_SLT(X,0) - only sign mask of X is required #164946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
5cf0b09
cd33ea9
4fe479e
c52a02e
6276068
b5f73f5
82aa5ac
232ff84
1b67549
f7def1c
3ebf72d
0bb1be8
a79eda0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1754,24 +1754,31 @@ bool TargetLowering::SimplifyDemandedBits( | |
| SDValue Op0 = Op.getOperand(0); | ||
| SDValue Op1 = Op.getOperand(1); | ||
| ISD::CondCode CC = cast<CondCodeSDNode>(Op.getOperand(2))->get(); | ||
| // If (1) we only need the sign-bit, (2) the setcc operands are the same | ||
| // width as the setcc result, and (3) the result of a setcc conforms to 0 or | ||
| // -1, we may be able to bypass the setcc. | ||
| if (DemandedBits.isSignMask() && | ||
| Op0.getScalarValueSizeInBits() == BitWidth && | ||
| getBooleanContents(Op0.getValueType()) == | ||
| BooleanContent::ZeroOrNegativeOneBooleanContent) { | ||
| // If we're testing X < 0, then this compare isn't needed - just use X! | ||
| // FIXME: We're limiting to integer types here, but this should also work | ||
| // if we don't care about FP signed-zero. The use of SETLT with FP means | ||
| // that we don't care about NaNs. | ||
| if (CC == ISD::SETLT && Op1.getValueType().isInteger() && | ||
| (isNullConstant(Op1) || ISD::isBuildVectorAllZeros(Op1.getNode()))) | ||
| return TLO.CombineTo(Op, Op0); | ||
|
|
||
| // TODO: Should we check for other forms of sign-bit comparisons? | ||
| // Examples: X <= -1, X >= 0 | ||
| } | ||
| // If we're testing X < 0 or X >= 0 then we only need the sign mask of the previous | ||
| // result | ||
| // FIXME: We're limiting to integer types here, but this should also work | ||
| // if we don't care about FP signed-zero. The use of SETLT with FP means | ||
| // that we don't care about NaNs. | ||
| if ((CC == ISD::SETLT || CC == ISD::SETGE) && Op1.getValueType().isInteger() && | ||
| (isNullConstant(Op1) || ISD::isBuildVectorAllZeros(Op1.getNode()))) { | ||
| KnownBits KnownOp0; | ||
| bool Changed = false; | ||
| if (SimplifyDemandedBits( | ||
| Op0, APInt::getSignMask(Op0.getScalarValueSizeInBits()), | ||
| DemandedElts, KnownOp0, TLO, Depth + 1)) | ||
| Changed = true; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just return true instead of allowing multiple optimizations. I think we will end up revisiting. Do any other places in this file do multiple optimizations like this? |
||
| // If (1) we only need the sign-bit, (2) the setcc operands are the same | ||
| // width as the setcc result, and (3) the result of a setcc conforms to 0 | ||
| // or -1, we may be able to bypass the setcc. | ||
| if (DemandedBits.isSignMask() && | ||
| Op0.getScalarValueSizeInBits() == BitWidth && | ||
| getBooleanContents(Op0.getValueType()) == | ||
| BooleanContent::ZeroOrNegativeOneBooleanContent) | ||
| Changed |= TLO.CombineTo(Op, Op0); | ||
|
||
| return Changed; | ||
| } | ||
| // TODO: Should we check for other forms of sign-bit comparisons? | ||
| // Example: X <= -1, X > -1 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this FIXME still relevant? |
||
| if (getBooleanContents(Op0.getValueType()) == | ||
| TargetLowering::ZeroOrOneBooleanContent && | ||
| BitWidth > 1) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3553,7 +3553,8 @@ define i32 @bittest_31_slt0_i32(i32 %x, i1 %y) { | |
| ; | ||
| ; RV64-LABEL: bittest_31_slt0_i32: | ||
| ; RV64: # %bb.0: | ||
| ; RV64-NEXT: srliw a0, a0, 31 | ||
| ; RV64-NEXT: slli a0, a0, 32 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a regression. I'll take a closer look when I get a chance. |
||
| ; RV64-NEXT: srli a0, a0, 63 | ||
| ; RV64-NEXT: and a0, a0, a1 | ||
| ; RV64-NEXT: ret | ||
| %cmp = icmp slt i32 %x, 0 | ||
|
|
@@ -3565,14 +3566,14 @@ define i32 @bittest_31_slt0_i32(i32 %x, i1 %y) { | |
| define i32 @bittest_63_slt0_i64(i32 %x, i1 %y) { | ||
| ; RV32-LABEL: bittest_63_slt0_i64: | ||
| ; RV32: # %bb.0: | ||
| ; RV32-NEXT: srai a0, a0, 31 | ||
| ; RV32-NEXT: srli a0, a0, 31 | ||
| ; RV32-NEXT: and a0, a0, a1 | ||
| ; RV32-NEXT: ret | ||
| ; | ||
| ; RV64-LABEL: bittest_63_slt0_i64: | ||
| ; RV64: # %bb.0: | ||
| ; RV64-NEXT: srliw a0, a0, 31 | ||
| ; RV64-NEXT: slli a0, a0, 32 | ||
| ; RV64-NEXT: srli a0, a0, 63 | ||
| ; RV64-NEXT: and a0, a0, a1 | ||
| ; RV64-NEXT: ret | ||
| %ext = sext i32 %x to i64 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scalar and vector cases shouldn't require separate handling like this. I think there's a utility for this, but these constant check functions are an inconsistent mess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced the checks with a single isNullOrNullSplat() check