From 8ca2ed22b24f052173a882e1a061fdbb7f476647 Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Tue, 6 Sep 2016 23:02:23 +0000 Subject: [PATCH] [DAGCombine] More fixups to SETCC legality checking (visitANDLike/visitORLike) I might have called this "r246507, the sequel". It fixes the same issue, as the issue has cropped up in a few more places. The underlying problem is that isSetCCEquivalent can pick up select_cc nodes with a result type that is not legal for a setcc node to have, and if we use that type to create new setcc nodes, nothing fixes that (and so we've violated the contract that the infrastructure has with the backend regarding setcc node types). Fixes PR30276. For convenience, here's the commit message from r246507, which explains the problem is greater detail: [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. llvm-svn: 280767 --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 74 ++++++++++++++++---------- llvm/test/CodeGen/PowerPC/setcclike-or-comb.ll | 31 +++++++++++ 2 files changed, 77 insertions(+), 28 deletions(-) create mode 100644 llvm/test/CodeGen/PowerPC/setcclike-or-comb.ll diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index ab47e1a..ad9351e 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -2881,25 +2881,34 @@ SDValue DAGCombiner::visitANDLike(SDValue N0, SDValue N1, LL.getValueType().isInteger()) { // fold (and (seteq X, 0), (seteq Y, 0)) -> (seteq (or X, Y), 0) if (isNullConstant(LR) && Op1 == ISD::SETEQ) { - SDValue ORNode = DAG.getNode(ISD::OR, SDLoc(N0), - LR.getValueType(), LL, RL); - AddToWorklist(ORNode.getNode()); - return DAG.getSetCC(SDLoc(LocReference), VT, ORNode, LR, Op1); + EVT CCVT = getSetCCResultType(LR.getValueType()); + if (VT == CCVT || (!LegalOperations && VT == MVT::i1)) { + SDValue ORNode = DAG.getNode(ISD::OR, SDLoc(N0), + LR.getValueType(), LL, RL); + AddToWorklist(ORNode.getNode()); + return DAG.getSetCC(SDLoc(LocReference), VT, ORNode, LR, Op1); + } } if (isAllOnesConstant(LR)) { // fold (and (seteq X, -1), (seteq Y, -1)) -> (seteq (and X, Y), -1) if (Op1 == ISD::SETEQ) { - SDValue ANDNode = DAG.getNode(ISD::AND, SDLoc(N0), - LR.getValueType(), LL, RL); - AddToWorklist(ANDNode.getNode()); - return DAG.getSetCC(SDLoc(LocReference), VT, ANDNode, LR, Op1); + EVT CCVT = getSetCCResultType(LR.getValueType()); + if (VT == CCVT || (!LegalOperations && VT == MVT::i1)) { + SDValue ANDNode = DAG.getNode(ISD::AND, SDLoc(N0), + LR.getValueType(), LL, RL); + AddToWorklist(ANDNode.getNode()); + return DAG.getSetCC(SDLoc(LocReference), VT, ANDNode, LR, Op1); + } } // fold (and (setgt X, -1), (setgt Y, -1)) -> (setgt (or X, Y), -1) if (Op1 == ISD::SETGT) { - SDValue ORNode = DAG.getNode(ISD::OR, SDLoc(N0), - LR.getValueType(), LL, RL); - AddToWorklist(ORNode.getNode()); - return DAG.getSetCC(SDLoc(LocReference), VT, ORNode, LR, Op1); + EVT CCVT = getSetCCResultType(LR.getValueType()); + if (VT == CCVT || (!LegalOperations && VT == MVT::i1)) { + SDValue ORNode = DAG.getNode(ISD::OR, SDLoc(N0), + LR.getValueType(), LL, RL); + AddToWorklist(ORNode.getNode()); + return DAG.getSetCC(SDLoc(LocReference), VT, ORNode, LR, Op1); + } } } } @@ -2908,14 +2917,17 @@ SDValue DAGCombiner::visitANDLike(SDValue N0, SDValue N1, Op0 == Op1 && LL.getValueType().isInteger() && Op0 == ISD::SETNE && ((isNullConstant(LR) && isAllOnesConstant(RR)) || (isAllOnesConstant(LR) && isNullConstant(RR)))) { - SDLoc DL(N0); - SDValue ADDNode = DAG.getNode(ISD::ADD, DL, LL.getValueType(), - LL, DAG.getConstant(1, DL, - LL.getValueType())); - AddToWorklist(ADDNode.getNode()); - return DAG.getSetCC(SDLoc(LocReference), VT, ADDNode, - DAG.getConstant(2, DL, LL.getValueType()), - ISD::SETUGE); + EVT CCVT = getSetCCResultType(LL.getValueType()); + if (VT == CCVT || (!LegalOperations && VT == MVT::i1)) { + SDLoc DL(N0); + SDValue ADDNode = DAG.getNode(ISD::ADD, DL, LL.getValueType(), + LL, DAG.getConstant(1, DL, + LL.getValueType())); + AddToWorklist(ADDNode.getNode()); + return DAG.getSetCC(SDLoc(LocReference), VT, ADDNode, + DAG.getConstant(2, DL, LL.getValueType()), + ISD::SETUGE); + } } // canonicalize equivalent to ll == rl if (LL == RR && LR == RL) { @@ -3636,18 +3648,24 @@ SDValue DAGCombiner::visitORLike(SDValue N0, SDValue N1, SDNode *LocReference) { // fold (or (setne X, 0), (setne Y, 0)) -> (setne (or X, Y), 0) // fold (or (setlt X, 0), (setlt Y, 0)) -> (setne (or X, Y), 0) if (isNullConstant(LR) && (Op1 == ISD::SETNE || Op1 == ISD::SETLT)) { - SDValue ORNode = DAG.getNode(ISD::OR, SDLoc(LR), - LR.getValueType(), LL, RL); - AddToWorklist(ORNode.getNode()); - return DAG.getSetCC(SDLoc(LocReference), VT, ORNode, LR, Op1); + EVT CCVT = getSetCCResultType(LR.getValueType()); + if (VT == CCVT || (!LegalOperations && VT == MVT::i1)) { + SDValue ORNode = DAG.getNode(ISD::OR, SDLoc(LR), + LR.getValueType(), LL, RL); + AddToWorklist(ORNode.getNode()); + return DAG.getSetCC(SDLoc(LocReference), VT, ORNode, LR, Op1); + } } // fold (or (setne X, -1), (setne Y, -1)) -> (setne (and X, Y), -1) // fold (or (setgt X, -1), (setgt Y -1)) -> (setgt (and X, Y), -1) if (isAllOnesConstant(LR) && (Op1 == ISD::SETNE || Op1 == ISD::SETGT)) { - SDValue ANDNode = DAG.getNode(ISD::AND, SDLoc(LR), - LR.getValueType(), LL, RL); - AddToWorklist(ANDNode.getNode()); - return DAG.getSetCC(SDLoc(LocReference), VT, ANDNode, LR, Op1); + EVT CCVT = getSetCCResultType(LR.getValueType()); + if (VT == CCVT || (!LegalOperations && VT == MVT::i1)) { + SDValue ANDNode = DAG.getNode(ISD::AND, SDLoc(LR), + LR.getValueType(), LL, RL); + AddToWorklist(ANDNode.getNode()); + return DAG.getSetCC(SDLoc(LocReference), VT, ANDNode, LR, Op1); + } } } // canonicalize equivalent to ll == rl diff --git a/llvm/test/CodeGen/PowerPC/setcclike-or-comb.ll b/llvm/test/CodeGen/PowerPC/setcclike-or-comb.ll new file mode 100644 index 0000000..8394d30 --- /dev/null +++ b/llvm/test/CodeGen/PowerPC/setcclike-or-comb.ll @@ -0,0 +1,31 @@ +; RUN: llc -O0 -fast-isel=0 < %s | FileCheck %s +target datalayout = "e-m:e-i64:64-n32:64" +target triple = "powerpc64le-unknown-linux-gnu" + +@a = external global i32, align 4 +@b = external global i32, align 4 + +; Function Attrs: nounwind +define void @fn1() #0 { +entry: + %0 = load i32, i32* @a, align 4 + %cmp = icmp ne i32 %0, 1 + %conv = zext i1 %cmp to i32 + %1 = load i32, i32* @b, align 4 + %cmp1 = icmp ne i32 0, %1 + %conv2 = zext i1 %cmp1 to i32 + %or = or i32 %conv, %conv2 + %xor = xor i32 1, %or + %call = call signext i32 @fn2(i32 signext %xor) + %conv4 = zext i1 undef to i32 + store i32 %conv4, i32* @b, align 4 + ret void + +; CHECK-LABEL: @fn1 +; CHECK: blr +} + +declare signext i32 @fn2(i32 signext) + +attributes #0 = { nounwind "target-cpu"="ppc64le" } + -- 2.7.4