<div dir="ltr">I think I see the problem. I'll see if I can fix this.<div><br clear="all"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Sep 8, 2019 at 12:04 PM Fedor Sergeev via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Roman,<br>
<br>
Despite this being kinda NFC we see it causing regressions.<br>
Here is a small reproducer that asserts while running through llc <br>
(testcase shown below and also attached):<br>
<br>
] cat reduced.ll<br>
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"<br>
target triple = "x86_64-unknown-linux-gnu"<br>
<br>
define void @TestvMeth(i32 %0, i64 %1) gc "statepoint-example" !prof !1 {<br>
bci_0:<br>
%token418 = call token (i64, i32, i8 * (i64, i32, i32, i32)*, i32, <br>
i32, ...) @llvm.experimental.gc.statepoint.p0f_p1i8i64i32i32i32f(i64 <br>
2882400000, i32 0, i8 * (i64, i32, i32, i32)* nonnull @newarray, i32 4, <br>
i32 0, i64 undef, i32 10, i32 10, i32 400, i32 0, i32 35, i32 0, i32 1, <br>
i32 0, i32 43, i32 1, i32 13, i32 0, i32 3, i32 400, i32 3, i32 %0, i32 <br>
4, i64 %1, i32 7, i8* null, i32 3, i32 -11464, i32 7, i8* null, i32 3, <br>
i32 -243, i32 3, i32 14, i32 3, i32 117, i32 3, i32 -13, i32 3, i32 -15, <br>
i32 3, i32 -210, i32 3, i32 541, i32 7, i8* null)<br>
%v2 = load atomic float, float * undef unordered, align 4<br>
%v3 = load <4 x i32>, <4 x i32> * undef, align 4<br>
%v4 = add <4 x i32> %v3, <i32 1, i32 1, i32 1, i32 1><br>
store <4 x i32> %v4, <4 x i32> * undef, align 4<br>
%v5 = fadd float %v2, 1.500000e+01<br>
store atomic float %v5, float * undef unordered, align 4<br>
unreachable<br>
}<br>
<br>
declare i32* @personality_function()<br>
declare i8 * @newarray(i64, i32, i32, i32)<br>
declare token @llvm.experimental.gc.statepoint.p0f_p1i8i64i32i32i32f(i64 <br>
immarg, i32 immarg, i8 * (i64, i32, i32, i32)*, i32 immarg, i32 immarg, ...)<br>
<br>
!1 = !{!"function_entry_count", i64 32768}<br>
] bin/llc reduced.ll<br>
llc: <br>
/home/fsergeev/ws/llvm-upstream/llvm-mono/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1149: <br>
void llvm::SelectionDAGISel::DoInstructionSelection(): Assertion <br>
`Op->getNodeId() != -1 && "Node has already selected predecessor node<br>
"' failed.<br>
...<br>
<br>
It stops asserting when I revert this commit locally.<br>
<br>
regards,<br>
Fedor.<br>
<br>
On 8/29/19 1:50 PM, Roman Lebedev via llvm-commits wrote:<br>
> Author: lebedevri<br>
> Date: Thu Aug 29 03:50:09 2019<br>
> New Revision: 370327<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=370327&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=370327&view=rev</a><br>
> Log:<br>
> [X86][CodeGen][NFC] Delay `combineIncDecVector()` from DAGCombine to X86DAGToDAGISel<br>
><br>
> Summary:<br>
> We were previously doing it in DAGCombine.<br>
> But we also want to do `sub %x, C` -> `add %x, (sub 0, C)` for vectors in DAGCombine.<br>
> So if we had `sub %x, -1`, we'll transform it to `add %x, 1`,<br>
> which `combineIncDecVector()` will immediately transform back into `sub %x, -1`,<br>
> and here we go again...<br>
><br>
> I've marked this as NFC since not a single test changes,<br>
> but since that 'changes' DAGCombine, probably this isn't fully NFC.<br>
><br>
> Reviewers: RKSimon, craig.topper, spatel<br>
><br>
> Reviewed By: craig.topper<br>
><br>
> Subscribers: hiraditya, llvm-commits<br>
><br>
> Tags: #llvm<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D62327" rel="noreferrer" target="_blank">https://reviews.llvm.org/D62327</a><br>
><br>
> Modified:<br>
> llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp<br>
> llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
> llvm/trunk/lib/Target/X86/X86ISelLowering.h<br>
> llvm/trunk/test/CodeGen/X86/stack-folding-int-avx1.ll<br>
> llvm/trunk/test/CodeGen/X86/stack-folding-int-sse42.ll<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp?rev=370327&r1=370326&r2=370327&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp?rev=370327&r1=370326&r2=370327&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp Thu Aug 29 03:50:09 2019<br>
> @@ -507,6 +507,7 @@ namespace {<br>
> bool shrinkAndImmediate(SDNode *N);<br>
> bool isMaskZeroExtended(SDNode *N) const;<br>
> bool tryShiftAmountMod(SDNode *N);<br>
> + bool combineIncDecVector(SDNode *Node);<br>
> bool tryShrinkShlLogicImm(SDNode *N);<br>
> bool tryVPTESTM(SDNode *Root, SDValue Setcc, SDValue Mask);<br>
> <br>
> @@ -3775,6 +3776,49 @@ bool X86DAGToDAGISel::tryShrinkShlLogicI<br>
> return true;<br>
> }<br>
> <br>
> +/// Convert vector increment or decrement to sub/add with an all-ones constant:<br>
> +/// add X, <1, 1...> --> sub X, <-1, -1...><br>
> +/// sub X, <1, 1...> --> add X, <-1, -1...><br>
> +/// The all-ones vector constant can be materialized using a pcmpeq instruction<br>
> +/// that is commonly recognized as an idiom (has no register dependency), so<br>
> +/// that's better/smaller than loading a splat 1 constant.<br>
> +bool X86DAGToDAGISel::combineIncDecVector(SDNode *Node) {<br>
> + assert((Node->getOpcode() == ISD::ADD || Node->getOpcode() == ISD::SUB) &&<br>
> + "Unexpected opcode for increment/decrement transform");<br>
> +<br>
> + EVT VT = Node->getValueType(0);<br>
> + assert(VT.isVector() && "Should only be called for vectors.");<br>
> +<br>
> + SDValue X = Node->getOperand(0);<br>
> + SDValue OneVec = Node->getOperand(1);<br>
> +<br>
> + APInt SplatVal;<br>
> + if (!X86::isConstantSplat(OneVec, SplatVal) || !SplatVal.isOneValue())<br>
> + return false;<br>
> +<br>
> + SDLoc DL(Node);<br>
> + SDValue AllOnesVec;<br>
> +<br>
> + APInt Ones = APInt::getAllOnesValue(32);<br>
> + assert(VT.getSizeInBits() % 32 == 0 &&<br>
> + "Expected bit count to be a multiple of 32");<br>
> + unsigned NumElts = VT.getSizeInBits() / 32;<br>
> + assert(NumElts > 0 && "Expected to get non-empty vector.");<br>
> + AllOnesVec =<br>
> + CurDAG->getConstant(Ones, DL, MVT::getVectorVT(MVT::i32, NumElts));<br>
> + insertDAGNode(*CurDAG, X, AllOnesVec);<br>
> +<br>
> + AllOnesVec = CurDAG->getBitcast(VT, AllOnesVec);<br>
> + insertDAGNode(*CurDAG, X, AllOnesVec);<br>
> +<br>
> + unsigned NewOpcode = Node->getOpcode() == ISD::ADD ? ISD::SUB : ISD::ADD;<br>
> + SDValue NewNode = CurDAG->getNode(NewOpcode, DL, VT, X, AllOnesVec);<br>
> +<br>
> + ReplaceNode(Node, NewNode.getNode());<br>
> + SelectCode(NewNode.getNode());<br>
> + return true;<br>
> +}<br>
> +<br>
> /// If the high bits of an 'and' operand are known zero, try setting the<br>
> /// high bits of an 'and' constant operand to produce a smaller encoding by<br>
> /// creating a small, sign-extended negative immediate rather than a large<br>
> @@ -4347,6 +4391,10 @@ void X86DAGToDAGISel::Select(SDNode *Nod<br>
> LLVM_FALLTHROUGH;<br>
> case ISD::ADD:<br>
> case ISD::SUB: {<br>
> + if ((Opcode == ISD::ADD || Opcode == ISD::SUB) && NVT.isVector() &&<br>
> + combineIncDecVector(Node))<br>
> + return;<br>
> +<br>
> // Try to avoid folding immediates with multiple uses for optsize.<br>
> // This code tries to select to register form directly to avoid going<br>
> // through the isel table which might fold the immediate. We can't change<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=370327&r1=370326&r2=370327&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=370327&r1=370326&r2=370327&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Thu Aug 29 03:50:09 2019<br>
> @@ -6215,7 +6215,9 @@ static bool getTargetConstantBitsFromNod<br>
> return false;<br>
> }<br>
> <br>
> -static bool isConstantSplat(SDValue Op, APInt &SplatVal) {<br>
> +namespace llvm {<br>
> +namespace X86 {<br>
> +bool isConstantSplat(SDValue Op, APInt &SplatVal) {<br>
> APInt UndefElts;<br>
> SmallVector<APInt, 16> EltBits;<br>
> if (getTargetConstantBitsFromNode(Op, Op.getScalarValueSizeInBits(),<br>
> @@ -6238,6 +6240,8 @@ static bool isConstantSplat(SDValue Op,<br>
> <br>
> return false;<br>
> }<br>
> +} // namespace X86<br>
> +} // namespace llvm<br>
> <br>
> static bool getTargetShuffleMaskIndices(SDValue MaskNode,<br>
> unsigned MaskEltSizeInBits,<br>
> @@ -18115,7 +18119,7 @@ static SDValue LowerFunnelShift(SDValue<br>
> std::swap(Op0, Op1);<br>
> <br>
> APInt APIntShiftAmt;<br>
> - if (isConstantSplat(Amt, APIntShiftAmt)) {<br>
> + if (X86::isConstantSplat(Amt, APIntShiftAmt)) {<br>
> uint64_t ShiftAmt = APIntShiftAmt.urem(VT.getScalarSizeInBits());<br>
> return DAG.getNode(IsFSHR ? X86ISD::VSHRD : X86ISD::VSHLD, DL, VT,<br>
> Op0, Op1, DAG.getConstant(ShiftAmt, DL, MVT::i8));<br>
> @@ -25337,7 +25341,7 @@ static SDValue LowerScalarImmediateShift<br>
> <br>
> // Optimize shl/srl/sra with constant shift amount.<br>
> APInt APIntShiftAmt;<br>
> - if (!isConstantSplat(Amt, APIntShiftAmt))<br>
> + if (!X86::isConstantSplat(Amt, APIntShiftAmt))<br>
> return SDValue();<br>
> <br>
> // If the shift amount is out of range, return undef.<br>
> @@ -43750,39 +43754,6 @@ static SDValue combineLoopSADPattern(SDN<br>
> return DAG.getNode(ISD::ADD, DL, VT, Sad, OtherOp, Flags);<br>
> }<br>
> <br>
> -/// Convert vector increment or decrement to sub/add with an all-ones constant:<br>
> -/// add X, <1, 1...> --> sub X, <-1, -1...><br>
> -/// sub X, <1, 1...> --> add X, <-1, -1...><br>
> -/// The all-ones vector constant can be materialized using a pcmpeq instruction<br>
> -/// that is commonly recognized as an idiom (has no register dependency), so<br>
> -/// that's better/smaller than loading a splat 1 constant.<br>
> -static SDValue combineIncDecVector(SDNode *N, SelectionDAG &DAG,<br>
> - TargetLowering::DAGCombinerInfo &DCI) {<br>
> - assert((N->getOpcode() == ISD::ADD || N->getOpcode() == ISD::SUB) &&<br>
> - "Unexpected opcode for increment/decrement transform");<br>
> -<br>
> - // Delay this until legalize ops to avoid interfering with early DAG combines<br>
> - // that may expect canonical adds.<br>
> - // FIXME: We may want to consider moving this to custom lowering or all the<br>
> - // way to isel, but lets start here.<br>
> - if (DCI.isBeforeLegalizeOps())<br>
> - return SDValue();<br>
> -<br>
> - // Pseudo-legality check: getOnesVector() expects one of these types, so bail<br>
> - // out and wait for legalization if we have an unsupported vector length.<br>
> - EVT VT = N->getValueType(0);<br>
> - if (!VT.is128BitVector() && !VT.is256BitVector() && !VT.is512BitVector())<br>
> - return SDValue();<br>
> -<br>
> - APInt SplatVal;<br>
> - if (!isConstantSplat(N->getOperand(1), SplatVal) || !SplatVal.isOneValue())<br>
> - return SDValue();<br>
> -<br>
> - SDValue AllOnesVec = getOnesVector(VT, DAG, SDLoc(N));<br>
> - unsigned NewOpcode = N->getOpcode() == ISD::ADD ? ISD::SUB : ISD::ADD;<br>
> - return DAG.getNode(NewOpcode, SDLoc(N), VT, N->getOperand(0), AllOnesVec);<br>
> -}<br>
> -<br>
> static SDValue matchPMADDWD(SelectionDAG &DAG, SDValue Op0, SDValue Op1,<br>
> const SDLoc &DL, EVT VT,<br>
> const X86Subtarget &Subtarget) {<br>
> @@ -44045,9 +44016,6 @@ static SDValue combineAdd(SDNode *N, Sel<br>
> HADDBuilder);<br>
> }<br>
> <br>
> - if (SDValue V = combineIncDecVector(N, DAG, DCI))<br>
> - return V;<br>
> -<br>
> return combineAddOrSubToADCOrSBB(N, DAG);<br>
> }<br>
> <br>
> @@ -44176,9 +44144,6 @@ static SDValue combineSub(SDNode *N, Sel<br>
> HSUBBuilder);<br>
> }<br>
> <br>
> - if (SDValue V = combineIncDecVector(N, DAG, DCI))<br>
> - return V;<br>
> -<br>
> // Try to create PSUBUS if SUB's argument is max/min<br>
> if (SDValue V = combineSubToSubus(N, DAG, Subtarget))<br>
> return V;<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.h?rev=370327&r1=370326&r2=370327&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.h?rev=370327&r1=370326&r2=370327&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.h (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.h Thu Aug 29 03:50:09 2019<br>
> @@ -683,6 +683,9 @@ namespace llvm {<br>
> bool isCalleePop(CallingConv::ID CallingConv,<br>
> bool is64Bit, bool IsVarArg, bool GuaranteeTCO);<br>
> <br>
> + /// If Op is a constant whose elements are all the same constant or<br>
> + /// undefined, return true and return the constant value in \p SplatVal.<br>
> + bool isConstantSplat(SDValue Op, APInt &SplatVal);<br>
> } // end namespace X86<br>
> <br>
> //===--------------------------------------------------------------------===//<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/stack-folding-int-avx1.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/stack-folding-int-avx1.ll?rev=370327&r1=370326&r2=370327&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/stack-folding-int-avx1.ll?rev=370327&r1=370326&r2=370327&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/stack-folding-int-avx1.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/stack-folding-int-avx1.ll Thu Aug 29 03:50:09 2019<br>
> @@ -540,8 +540,8 @@ define <16 x i8> @stack_fold_pandn(<16 x<br>
> ; CHECK-NEXT: #APP<br>
> ; CHECK-NEXT: nop<br>
> ; CHECK-NEXT: #NO_APP<br>
> -; CHECK-NEXT: vpcmpeqd %xmm1, %xmm1, %xmm1<br>
> ; CHECK-NEXT: vpandn {{[-0-9]+}}(%r{{[sb]}}p), %xmm0, %xmm0 # 16-byte Folded Reload<br>
> +; CHECK-NEXT: vpcmpeqd %xmm1, %xmm1, %xmm1<br>
> ; CHECK-NEXT: vpsubb %xmm1, %xmm0, %xmm0<br>
> ; CHECK-NEXT: retq<br>
> %1 = tail call <2 x i64> asm sideeffect "nop", "=x,~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15},~{flags}"()<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/stack-folding-int-sse42.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/stack-folding-int-sse42.ll?rev=370327&r1=370326&r2=370327&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/stack-folding-int-sse42.ll?rev=370327&r1=370326&r2=370327&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/stack-folding-int-sse42.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/stack-folding-int-sse42.ll Thu Aug 29 03:50:09 2019<br>
> @@ -724,8 +724,8 @@ define <16 x i8> @stack_fold_pandn(<16 x<br>
> ; CHECK-NEXT: #APP<br>
> ; CHECK-NEXT: nop<br>
> ; CHECK-NEXT: #NO_APP<br>
> -; CHECK-NEXT: pcmpeqd %xmm1, %xmm1<br>
> ; CHECK-NEXT: pandn {{[-0-9]+}}(%r{{[sb]}}p), %xmm0 # 16-byte Folded Reload<br>
> +; CHECK-NEXT: pcmpeqd %xmm1, %xmm1<br>
> ; CHECK-NEXT: psubb %xmm1, %xmm0<br>
> ; CHECK-NEXT: retq<br>
> %1 = tail call <2 x i64> asm sideeffect "nop", "=x,~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15},~{flags}"()<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>