summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSanjay Patel <spatel@rotateright.com>2019-08-05 11:27:07 +0000
committerSanjay Patel <spatel@rotateright.com>2019-08-05 11:27:07 +0000
commiteaf13044bda2f58562a7e5f4ee762e70294299a9 (patch)
treea6ba175317a108d7e60824ee533f93a5b1a56c4b
parentc9051861cb29b0d539a42ece13cdf2b133aae19b (diff)
downloadbcm5719-llvm-eaf13044bda2f58562a7e5f4ee762e70294299a9.tar.gz
bcm5719-llvm-eaf13044bda2f58562a7e5f4ee762e70294299a9.zip
[DAGCombiner][x86] prevent infinite loop from truncate/extend transforms
The test case is based on the example from the post-commit thread for: https://reviews.llvm.org/rGc9171bd0a955 This replaces the x86-specific simple-type check from: rL367766 with a check in the DAGCombiner. Adding the check isn't strictly necessary after the fix from: rL367768 ...but it seems likely that we're heading for trouble if we are creating weird types in this transform. I combined the earlier legality check into the initial clause to simplify the code. So we should only try the trunc/sext transform at the earliest combine stage, but we limit the transform to simple types anyway because the TLI hook is probably too lax about what it considers a free truncate. llvm-svn: 367834
-rw-r--r--llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp9
-rw-r--r--llvm/lib/Target/X86/X86ISelLowering.cpp2
-rw-r--r--llvm/test/CodeGen/X86/trunc-and.ll24
3 files changed, 31 insertions, 4 deletions
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index f6b021c8fd1..77bc37ca5e9 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -7619,7 +7619,7 @@ SDValue DAGCombiner::visitSRA(SDNode *N) {
// We convert trunc/ext to opposing shifts in IR, but casts may be cheaper.
// sra (add (shl X, N1C), AddC), N1C -->
// sext (add (trunc X to (width - N1C)), AddC')
- if (!LegalOperations && N0.getOpcode() == ISD::ADD && N0.hasOneUse() && N1C &&
+ if (!LegalTypes && N0.getOpcode() == ISD::ADD && N0.hasOneUse() && N1C &&
N0.getOperand(0).getOpcode() == ISD::SHL &&
N0.getOperand(0).getOperand(1) == N1 && N0.getOperand(0).hasOneUse()) {
if (ConstantSDNode *AddC = isConstOrConstSplat(N0.getOperand(1))) {
@@ -7631,7 +7631,12 @@ SDValue DAGCombiner::visitSRA(SDNode *N) {
EVT TruncVT = EVT::getIntegerVT(Ctx, OpSizeInBits - ShiftAmt);
if (VT.isVector())
TruncVT = EVT::getVectorVT(Ctx, TruncVT, VT.getVectorNumElements());
- if (isTypeLegal(TruncVT) && TLI.isTruncateFree(VT, TruncVT)) {
+
+ // TODO: The simple type check probably belongs in the default hook
+ // implementation and/or target-specific overrides (because
+ // non-simple types likely require masking when legalized), but that
+ // restriction may conflict with other transforms.
+ if (TruncVT.isSimple() && TLI.isTruncateFree(VT, TruncVT)) {
SDLoc DL(N);
SDValue Trunc = DAG.getZExtOrTrunc(Shl.getOperand(0), DL, TruncVT);
SDValue ShiftC = DAG.getConstant(AddC->getAPIntValue().lshr(ShiftAmt).
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index b4473ef6737..899c6afa7d0 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -28846,8 +28846,6 @@ bool X86TargetLowering::isLegalStoreImmediate(int64_t Imm) const {
bool X86TargetLowering::isTruncateFree(EVT VT1, EVT VT2) const {
if (!VT1.isInteger() || !VT2.isInteger())
return false;
- if (!VT1.isSimple() || !VT2.isSimple())
- return false;
unsigned NumBits1 = VT1.getSizeInBits();
unsigned NumBits2 = VT2.getSizeInBits();
return NumBits1 > NumBits2;
diff --git a/llvm/test/CodeGen/X86/trunc-and.ll b/llvm/test/CodeGen/X86/trunc-and.ll
index 28a55c257bd..09fe6413d27 100644
--- a/llvm/test/CodeGen/X86/trunc-and.ll
+++ b/llvm/test/CodeGen/X86/trunc-and.ll
@@ -24,3 +24,27 @@ define i16 @PR40793(<8 x i16> %t1) {
declare <2 x double> @llvm.fabs.v2f64(<2 x double>)
+; This would infinite loop by trying to truncate and any_extend.
+
+%struct.anon = type { [9 x i8], [3 x i8] }
+
+@b = common local_unnamed_addr global %struct.anon zeroinitializer, align 4
+
+define i32 @d() {
+; CHECK-LABEL: d:
+; CHECK: # %bb.0:
+; CHECK-NEXT: movzbl b+{{.*}}(%rip), %ecx
+; CHECK-NEXT: andl $7, %ecx
+; CHECK-NEXT: movl $d, %eax
+; CHECK-NEXT: addl %ecx, %eax
+; CHECK-NEXT: # kill: def $eax killed $eax killed $rax
+; CHECK-NEXT: retq
+ %bf.load = load i72, i72* bitcast (%struct.anon* @b to i72*), align 4
+ %bf.lshr = lshr i72 %bf.load, 64
+ %t0 = trunc i72 %bf.lshr to i64
+ %bf.cast = and i64 %t0, 7
+ %add.ptr = getelementptr i8, i8* bitcast (i32 ()* @d to i8*), i64 %bf.cast
+ %t1 = ptrtoint i8* %add.ptr to i64
+ %t2 = trunc i64 %t1 to i32
+ ret i32 %t2
+}
OpenPOWER on IntegriCloud