diff options
author | Sanjay Patel <spatel@rotateright.com> | 2019-08-05 11:27:07 +0000 |
---|---|---|
committer | Sanjay Patel <spatel@rotateright.com> | 2019-08-05 11:27:07 +0000 |
commit | eaf13044bda2f58562a7e5f4ee762e70294299a9 (patch) | |
tree | a6ba175317a108d7e60824ee533f93a5b1a56c4b | |
parent | c9051861cb29b0d539a42ece13cdf2b133aae19b (diff) | |
download | bcm5719-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.cpp | 9 | ||||
-rw-r--r-- | llvm/lib/Target/X86/X86ISelLowering.cpp | 2 | ||||
-rw-r--r-- | llvm/test/CodeGen/X86/trunc-and.ll | 24 |
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 +} |