]> granicus.if.org Git - llvm/commitdiff
[RISCV] Quick fix for PR40333
authorAlex Bradbury <asb@lowrisc.org>
Tue, 22 Jan 2019 12:11:53 +0000 (12:11 +0000)
committerAlex Bradbury <asb@lowrisc.org>
Tue, 22 Jan 2019 12:11:53 +0000 (12:11 +0000)
Avoid the infinite loop caused by the target DAG combine converting ANYEXT to
SIGNEXT and the target-independent DAG combine logic converting back to
ANYEXT. Do this by not adding the new node to the worklist.

Committing directly as this definitely doesn't make the problem any worse, and
I intend to follow-up with a patch that avoids this custom combiner logic
altogether and just lowers the i32 operations to a target-specific
SelectionDAG node. This should be easier to reason about and improve codegen
quality in some cases (though may miss out on some later DAG combines).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351806 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/RISCV/RISCVISelLowering.cpp
test/CodeGen/RISCV/pr40333.ll [new file with mode: 0644]

index 8a32d957ec599b5cb01128f3d446be6c514fe126..95943f3dbaf9fb7d73a01a5a98d1f16ed22fe5be 100644 (file)
@@ -575,7 +575,11 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
         !(Subtarget.hasStdExtM() && isVariableSDivUDivURem(Src)))
       break;
     SDLoc DL(N);
-    return DCI.CombineTo(N, DAG.getNode(ISD::SIGN_EXTEND, DL, MVT::i64, Src));
+    // Don't add the new node to the DAGCombiner worklist, in order to avoid
+    // an infinite cycle due to SimplifyDemandedBits converting the
+    // SIGN_EXTEND back to ANY_EXTEND.
+    return DCI.CombineTo(N, DAG.getNode(ISD::SIGN_EXTEND, DL, MVT::i64, Src),
+                         false);
   }
   case RISCVISD::SplitF64: {
     // If the input to SplitF64 is just BuildPairF64 then the operation is
diff --git a/test/CodeGen/RISCV/pr40333.ll b/test/CodeGen/RISCV/pr40333.ll
new file mode 100644 (file)
index 0000000..3f7ae8d
--- /dev/null
@@ -0,0 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64I %s
+
+; This test case is significantly simplified from the submitted .ll but
+; demonstrates the same issue. At the time of this problem report, an infinite
+; loop would be created in DAGCombine, converting ANY_EXTEND to SIGN_EXTEND
+; and back again.
+
+; TODO: This test case is also an example of where it would be cheaper to
+; select SRLW, but the current lowering strategy fails to do so.
+
+define signext i8 @foo(i32 %a, i32 %b) nounwind {
+; RV64I-LABEL: foo:
+; RV64I:       # %bb.0:
+; RV64I-NEXT:    slli a1, a1, 32
+; RV64I-NEXT:    srli a1, a1, 32
+; RV64I-NEXT:    slli a0, a0, 32
+; RV64I-NEXT:    srli a0, a0, 32
+; RV64I-NEXT:    srl a0, a0, a1
+; RV64I-NEXT:    slli a0, a0, 56
+; RV64I-NEXT:    srai a0, a0, 56
+; RV64I-NEXT:    ret
+ %1 = lshr i32 %a, %b
+ %2 = trunc i32 %1 to i8
+ ret i8 %2
+}