Skip to content

Commit 421a6bc

Browse files
asbDisasm
authored andcommitted
[RISCV] Quick fix for PR40333
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). llvm-svn: 351806
1 parent 8125338 commit 421a6bc

File tree

2 files changed

+32
-1
lines changed

2 files changed

+32
-1
lines changed

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,11 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
576576
!(Subtarget.hasStdExtM() && isVariableSDivUDivURem(Src)))
577577
break;
578578
SDLoc DL(N);
579-
return DCI.CombineTo(N, DAG.getNode(ISD::SIGN_EXTEND, DL, MVT::i64, Src));
579+
// Don't add the new node to the DAGCombiner worklist, in order to avoid
580+
// an infinite cycle due to SimplifyDemandedBits converting the
581+
// SIGN_EXTEND back to ANY_EXTEND.
582+
return DCI.CombineTo(N, DAG.getNode(ISD::SIGN_EXTEND, DL, MVT::i64, Src),
583+
false);
580584
}
581585
case RISCVISD::SplitF64: {
582586
// If the input to SplitF64 is just BuildPairF64 then the operation is

llvm/test/CodeGen/RISCV/pr40333.ll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
2+
; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
3+
; RUN: | FileCheck -check-prefix=RV64I %s
4+
5+
; This test case is significantly simplified from the submitted .ll but
6+
; demonstrates the same issue. At the time of this problem report, an infinite
7+
; loop would be created in DAGCombine, converting ANY_EXTEND to SIGN_EXTEND
8+
; and back again.
9+
10+
; TODO: This test case is also an example of where it would be cheaper to
11+
; select SRLW, but the current lowering strategy fails to do so.
12+
13+
define signext i8 @foo(i32 %a, i32 %b) nounwind {
14+
; RV64I-LABEL: foo:
15+
; RV64I: # %bb.0:
16+
; RV64I-NEXT: slli a1, a1, 32
17+
; RV64I-NEXT: srli a1, a1, 32
18+
; RV64I-NEXT: slli a0, a0, 32
19+
; RV64I-NEXT: srli a0, a0, 32
20+
; RV64I-NEXT: srl a0, a0, a1
21+
; RV64I-NEXT: slli a0, a0, 56
22+
; RV64I-NEXT: srai a0, a0, 56
23+
; RV64I-NEXT: ret
24+
%1 = lshr i32 %a, %b
25+
%2 = trunc i32 %1 to i8
26+
ret i8 %2
27+
}

0 commit comments

Comments
 (0)