[DAGCombiner][AArch64] Make combineCarryDiamond avoid creating UADDO_CARRY with carry in larger than setcc result type. #89121
Conversation
|
@llvm/pr-subscribers-llvm-selectiondag @llvm/pr-subscribers-backend-aarch64 Author: Craig Topper (topperc) ChangesIn the attach test case we were creating a UADDO_CARRY with i1 carry out and i41 carry in. i41 exceeds is larger than the setcc result type for AArch64 which is i32. i41 needs to be promoted to i64 since it is larger than i32. The type legalizer tried to use promoteTargetBoolean, but that can only promote from a type smaller than setcc result type. The easiest fix here is to force the carryin type to match the carryout type at the type of creation. This should ensure the node won't exceeed setcc result type as long as the output type doesn't. I think we should explore requiring the types to match for this node. Fixes #88966 Full diff: https://github.com/llvm/llvm-project/pull/89121.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index c36b1cc9039c26..be724f41992c2e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -3520,6 +3520,8 @@ static SDValue combineCarryDiamond(SelectionDAG &DAG, const TargetLowering &TLI,
return SDValue();
SDLoc DL(N);
+ CarryIn = DAG.getBoolExtOrTrunc(CarryIn, DL, Carry1->getValueType(1),
+ Carry1->getValueType(0));
SDValue Merged =
DAG.getNode(NewOp, DL, Carry1->getVTList(), Carry0.getOperand(0),
Carry0.getOperand(1), CarryIn);
diff --git a/llvm/test/CodeGen/AArch64/pr88966.ll b/llvm/test/CodeGen/AArch64/pr88966.ll
new file mode 100644
index 00000000000000..1e20f60957e0ac
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/pr88966.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc < %s -mtriple=aarch64 | FileCheck %s
+
+define i32 @f(ptr %0, i41 %1, ptr %2) {
+; CHECK-LABEL: f:
+; CHECK: // %bb.0:
+; CHECK-NEXT: and w9, w1, #0x1
+; CHECK-NEXT: mov w10, #1 // =0x1
+; CHECK-NEXT: mov x8, x0
+; CHECK-NEXT: cmp w9, #1
+; CHECK-NEXT: mov w0, wzr
+; CHECK-NEXT: adc x9, xzr, x10
+; CHECK-NEXT: str x9, [x2]
+; CHECK-NEXT: str xzr, [x8]
+; CHECK-NEXT: ret
+ %new0 = and i41 %1, 1
+ %last = trunc i41 %new0 to i1
+ %4 = add i64 0, 1
+ %5 = zext i1 %last to i64
+ %6 = add i64 %4, %5
+ %7 = icmp ult i64 %4, 0
+ %8 = icmp ult i64 %6, %4
+ %9 = and i1 %7, %8
+ %10 = zext i1 %9 to i64
+ store i64 %6, ptr %2, align 8
+ store i64 %10, ptr %0, align 8
+ ret i32 0
+}
|
llvm/test/CodeGen/AArch64/pr88966.ll
Outdated
| ; CHECK-NEXT: ret | ||
| %new0 = and i41 %1, 1 | ||
| %last = trunc i41 %new0 to i1 | ||
| %4 = add i64 0, 1 |
There was a problem hiding this comment.
https://llvm.org/docs/TestingGuide.html#best-practices-for-regression-tests
It's a pain to deal with these anytime there's an IR change
…CARRY with carry in larger than setcc result type. In the attach test case we were creating a UADDO_CARRY with i1 carry out and i41 carry in. i41 exceeds is larger than the setcc result type for AArch64 which is i32. i41 needs to be promoted to i64 since it is larger than i32. The type legalizer tried to use promoteTargetBoolean, but that can only promote from a type smaller than setcc result type. The easiest fix here is to force the carryin type to match the carryout type at the type of creation. This should ensure the node won't exceeed setcc result type as long as the output type doesn't. I think we should explore requiring the types to match for this node. Fixes llvm#88966
I assumed that was already the case, I don't think it makes sense to permit it to be different |
We still have failures. We need to change type legalization to promote the source when it promotes the dest. Doing that still left a failure on a SystemZ test that I haven't investigated yet. |
In the attach test case we were creating a UADDO_CARRY with i1 carry out and i41 carry in. i41 exceeds is larger than the setcc result type for AArch64 which is i32. i41 needs to be promoted to i64 since it is larger than i32. The type legalizer tried to use promoteTargetBoolean, but that can only promote from a type smaller than setcc result type.
The easiest fix here is to force the carryin type to match the carryout type at the type of creation. This should ensure the node won't exceeed setcc result type as long as the output type doesn't.
I think we should explore requiring the types to match for this node.
Fixes #88966