[llvm] r246507 - [DAGCombine] Fixup SETCC legality checking
Hal Finkel via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 31 16:15:05 PDT 2015
Author: hfinkel
Date: Mon Aug 31 18:15:04 2015
New Revision: 246507
URL: http://llvm.org/viewvc/llvm-project?rev=246507&view=rev
Log:
[DAGCombine] Fixup SETCC legality checking
SETCC is one of those special node types for which operation actions (legality,
etc.) is keyed off of an operand type, not the node's value type. This makes
sense because the value type of a legal SETCC node is determined by its
operands' value type (via the TLI function getSetCCResultType). When the
SDAGBuilder creates SETCC nodes, it either creates them with an MVT::i1 value
type, or directly with the value type provided by TLI.getSetCCResultType.
The first problem being fixed here is that DAGCombine had several places
querying TLI.isOperationLegal on SETCC, but providing the return of
getSetCCResultType, instead of the operand type directly. This does not mean
what the author thought, and "luckily", most in-tree targets have SETCC with
Custom lowering, instead of marking them Legal, so these checks return false
anyway.
The second problem being fixed here is that two of the DAGCombines could create
SETCC nodes with arbitrary (integer) value types; specifically, those that
would simplify:
(setcc a, b, op1) and|or (setcc a, b, op2) -> setcc a, b, op3
(which is possible for some combinations of (op1, op2))
If the operands of the and|or node are actual setcc nodes, then this is not an
issue (because the and|or must share the same type), but, the relevant code in
DAGCombiner::visitANDLike and DAGCombiner::visitORLike actually calls
DAGCombiner::isSetCCEquivalent on each operand, and that function will
recognise setcc-like select_cc nodes with other return types. And, thus, when
creating new SETCC nodes, we need to be careful to respect the value-type
constraint. This is even true before type legalization, because it is quite
possible for the SELECT_CC node to have a legal type that does not happen to
match the corresponding TLI.getSetCCResultType type.
To be explicit, there is nothing that later fixes the value types of SETCC
nodes (if the type is legal, but does not happen to match
TLI.getSetCCResultType). Creating SETCCs with an MVT::i1 value type seems to
work only because, either MVT::i1 is not legal, or it is what
TLI.getSetCCResultType returns if it is legal. Fixing that is a larger change,
however. For the time being, restrict the relevant transformations to produce
only SETCC nodes with a value type matching TLI.getSetCCResultType (or MVT::i1
prior to type legalization).
Fixes PR24636.
Added:
llvm/trunk/test/CodeGen/PowerPC/pr24636.ll
Modified:
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=246507&r1=246506&r2=246507&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Aug 31 18:15:04 2015
@@ -2880,10 +2880,13 @@ SDValue DAGCombiner::visitANDLike(SDValu
if (Result != ISD::SETCC_INVALID &&
(!LegalOperations ||
(TLI.isCondCodeLegal(Result, LL.getSimpleValueType()) &&
- TLI.isOperationLegal(ISD::SETCC,
- getSetCCResultType(N0.getSimpleValueType())))))
- return DAG.getSetCC(SDLoc(LocReference), N0.getValueType(),
- LL, LR, Result);
+ TLI.isOperationLegal(ISD::SETCC, LL.getValueType())))) {
+ EVT CCVT = getSetCCResultType(LL.getValueType());
+ if (N0.getValueType() == CCVT ||
+ (!LegalOperations && N0.getValueType() == MVT::i1))
+ return DAG.getSetCC(SDLoc(LocReference), N0.getValueType(),
+ LL, LR, Result);
+ }
}
}
@@ -3538,10 +3541,13 @@ SDValue DAGCombiner::visitORLike(SDValue
if (Result != ISD::SETCC_INVALID &&
(!LegalOperations ||
(TLI.isCondCodeLegal(Result, LL.getSimpleValueType()) &&
- TLI.isOperationLegal(ISD::SETCC,
- getSetCCResultType(N0.getValueType())))))
- return DAG.getSetCC(SDLoc(LocReference), N0.getValueType(),
- LL, LR, Result);
+ TLI.isOperationLegal(ISD::SETCC, LL.getValueType())))) {
+ EVT CCVT = getSetCCResultType(LL.getValueType());
+ if (N0.getValueType() == CCVT ||
+ (!LegalOperations && N0.getValueType() == MVT::i1))
+ return DAG.getSetCC(SDLoc(LocReference), N0.getValueType(),
+ LL, LR, Result);
+ }
}
}
@@ -6059,7 +6065,8 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SD
if (!VT.isVector()) {
EVT SetCCVT = getSetCCResultType(N0.getOperand(0).getValueType());
- if (!LegalOperations || TLI.isOperationLegal(ISD::SETCC, SetCCVT)) {
+ if (!LegalOperations ||
+ TLI.isOperationLegal(ISD::SETCC, N0.getOperand(0).getValueType())) {
SDLoc DL(N);
ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();
SDValue SetCC = DAG.getSetCC(DL, SetCCVT,
@@ -13730,8 +13737,7 @@ SDValue DAGCombiner::SimplifySelectCC(SD
// Get a SetCC of the condition
// NOTE: Don't create a SETCC if it's not legal on this target.
if (!LegalOperations ||
- TLI.isOperationLegal(ISD::SETCC,
- LegalTypes ? getSetCCResultType(N0.getValueType()) : MVT::i1)) {
+ TLI.isOperationLegal(ISD::SETCC, N0.getValueType())) {
SDValue Temp, SCC;
// cast from setcc result type to select result type
if (LegalTypes) {
Added: llvm/trunk/test/CodeGen/PowerPC/pr24636.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/pr24636.ll?rev=246507&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/PowerPC/pr24636.ll (added)
+++ llvm/trunk/test/CodeGen/PowerPC/pr24636.ll Mon Aug 31 18:15:04 2015
@@ -0,0 +1,41 @@
+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+ at c = external global i32, align 4
+ at b = external global [1 x i32], align 4
+
+; Function Attrs: nounwind
+define void @fn2() #0 align 4 {
+ br i1 undef, label %.lr.ph, label %4
+
+; We used to crash because a bad DAGCombine was creating i32-typed SETCC nodes,
+; even when crbits are enabled.
+; CHECK-LABEL: @fn2
+; CHECK: blr
+
+.lr.ph: ; preds = %0
+ br i1 undef, label %.lr.ph.split, label %.preheader
+
+.preheader: ; preds = %.preheader, %.lr.ph
+ br i1 undef, label %.lr.ph.split, label %.preheader
+
+.lr.ph.split: ; preds = %.preheader, %.lr.ph
+ br i1 undef, label %._crit_edge, label %.lr.ph.split.split
+
+.lr.ph.split.split: ; preds = %.lr.ph.split.split, %.lr.ph.split
+ %1 = phi i32 [ %2, %.lr.ph.split.split ], [ undef, %.lr.ph.split ]
+ %2 = and i32 %1, and (i32 and (i32 and (i32 and (i32 and (i32 and (i32 and (i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32)), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32)), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32)), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32)), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32)), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1 x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32)), i32 zext (i1 select (i1 icmp eq ([1 x i32]* bitcast (i32* @c to [1
x i32]*), [1 x i32]* @b), i1 true, i1 false) to i32))
+ %3 = icmp slt i32 undef, 4
+ br i1 %3, label %.lr.ph.split.split, label %._crit_edge
+
+._crit_edge: ; preds = %.lr.ph.split.split, %.lr.ph.split
+ %.lcssa = phi i32 [ undef, %.lr.ph.split ], [ %2, %.lr.ph.split.split ]
+ br label %4
+
+; <label>:4 ; preds = %._crit_edge, %0
+ ret void
+}
+
+attributes #0 = { nounwind "target-cpu"="ppc64le" }
+
More information about the llvm-commits
mailing list