[llvm] r207338 - DAGCombiner: Simplify code a bit, make more transforms work with vectors.
Quentin Colombet
qcolombet at apple.com
Thu Jul 10 10:46:27 PDT 2014
Hi Benjamin,
I have narrowed down a failure of the factor test case (from the llvm test-suite: SingleSource/UnitTests/SignlessTypes) to this commit.
The failure happens at Os on a haswell machine running macosx.
I have reduced the problem to the attached test.ll and I proposed to fix that with the attached patch.
Basically the masks for the shuffle are wrong for vectors with 8 elements.
** To Reproduce **
Basically this test takes 8 numbers: 8, 9, …, 15 and divide each of them by 7 and print the result.
On a haswell machine
llc -mattr=-avx2 test.ll -o ok.s
llc -mattr=+avx2 test.ll -o ko.s
clang ok.s -arch x86_64 -o ok.out
clang ko.s -arch x86_64 -march core-avx2 -o ko.out
./ok.out
1
1
1
1
1
1
2
2
./ko.out
1
1
1
1
1
1
1 <-- 14/7 == 1!!!
2
** Feedback Needed **
Could you review the attached patch?
Moreover, because of this patch, vector-idiv.ll is failing. To me the output seems reasonable but could you take a look?
I’ll update the patch to match the new pattern if they are fine.
Thanks,
-Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lower_mullohi.patch
Type: application/octet-stream
Size: 4095 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140710/7bb86c3f/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.ll
Type: application/octet-stream
Size: 1802 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140710/7bb86c3f/attachment-0001.obj>
-------------- next part --------------
This commit (and more generally the custom lowering of MUL_LOHI)
> On May 2, 2014, at 5:42 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
>
> On 02.05.2014, at 13:48, Patrik Hägglund H <patrik.h.hagglund at ericsson.com> wrote:
>
>> Hi Benjamin,
>>
>> This commit is causing a regression for an llvm-stress test, as shown below.
>
> r207835 fixes the crash. Thanks for the test case!
>
> - Ben
>
>>
>> /Patrik Hägglund
>>
>> bin/llvm-stress -size 185 -seed 7564 | bin/llc -march=x86-64 -mcpu=corei7 -o /dev/null
>>
>> llc: ../lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5728: void llvm::SelectionDAG::ReplaceAllUsesWith(llvm::SDNode *, llvm::SDNode *): Assertion `(!From->hasAnyUseOfValue(i) || From->getValueType(i) == To->getValueType(i)) && "Cannot use this version of ReplaceAllUsesWith!"' failed.
>> 0 llc 0x00000000010b5a55 llvm::sys::PrintStackTrace(_IO_FILE*) + 37
>> 1 llc 0x00000000010b5e93
>> 2 libpthread.so.0 0x00007f7d65bcb7c0
>> 3 libc.so.6 0x00007f7d64ecfb35 gsignal + 53
>> 4 libc.so.6 0x00007f7d64ed1111 abort + 385
>> 5 libc.so.6 0x00007f7d64ec89f0 __assert_fail + 240
>> 6 llc 0x0000000000f3a524
>> 7 llc 0x0000000000ec4e45 llvm::SelectionDAG::Combine(llvm::CombineLevel, llvm::AliasAnalysis&, llvm::CodeGenOpt::Level) + 1573
>> 8 llc 0x0000000000fad035 llvm::SelectionDAGISel::CodeGenAndEmitDAG() + 901
>> 9 llc 0x0000000000fac368 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) + 6504
>> 10 llc 0x0000000000fa9c64 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) + 1332
>> 11 llc 0x0000000000c3261c llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 124
>> 12 llc 0x0000000000e1f20a llvm::FPPassManager::runOnFunction(llvm::Function&) + 362
>> 13 llc 0x0000000000e1f49b llvm::FPPassManager::runOnModule(llvm::Module&) + 43
>> 14 llc 0x0000000000e1fa37 llvm::legacy::PassManagerImpl::run(llvm::Module&) + 999
>> 15 llc 0x000000000056d835 main + 6549
>> 16 libc.so.6 0x00007f7d64ebbc16 __libc_start_main + 230
>> 17 llc 0x000000000056bdb9
>> Stack dump:
>> 0. Program arguments: bin/llc -march=x86-64 -mcpu=corei7 -o /dev/null
>> 1. Running pass 'Function Pass Manager' on module '<stdin>'.
>> 2. Running pass 'X86 DAG->DAG Instruction Selection' on function '@autogen_SD7564'
>> Abort
>>
>>
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Benjamin Kramer
>> Sent: den 27 april 2014 01:10
>> To: llvm-commits at cs.uiuc.edu
>> Subject: [llvm] r207338 - DAGCombiner: Simplify code a bit, make more transforms work with vectors.
>>
>> Author: d0k
>> Date: Sat Apr 26 18:09:49 2014
>> New Revision: 207338
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=207338&view=rev
>> Log:
>> DAGCombiner: Simplify code a bit, make more transforms work with vectors.
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> llvm/trunk/test/CodeGen/X86/vector-idiv.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=207338&r1=207337&r2=207338&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Sat Apr 26 18:09:49 2014
>> @@ -644,8 +644,13 @@ static ConstantSDNode *isConstOrConstSpl
>> if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(N))
>> return CN;
>>
>> - if (BuildVectorSDNode *BV = dyn_cast<BuildVectorSDNode>(N))
>> - return BV->getConstantSplatValue();
>> + if (BuildVectorSDNode *BV = dyn_cast<BuildVectorSDNode>(N)) {
>> + ConstantSDNode *CN = BV->getConstantSplatValue();
>> +
>> + // BuildVectors can truncate their operands. Ignore that case here.
>> + if (CN && CN->getValueType(0) == N.getValueType().getScalarType())
>> + return CN;
>> + }
>>
>> return nullptr;
>> }
>> @@ -1957,8 +1962,8 @@ SDValue DAGCombiner::visitMUL(SDNode *N)
>> SDValue DAGCombiner::visitSDIV(SDNode *N) {
>> SDValue N0 = N->getOperand(0);
>> SDValue N1 = N->getOperand(1);
>> - ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0.getNode());
>> - ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1.getNode());
>> + ConstantSDNode *N0C = isConstOrConstSplat(N0);
>> + ConstantSDNode *N1C = isConstOrConstSplat(N1);
>> EVT VT = N->getValueType(0);
>>
>> // fold vector ops
>> @@ -1985,25 +1990,15 @@ SDValue DAGCombiner::visitSDIV(SDNode *N
>> N0, N1);
>> }
>>
>> - const APInt *Divisor = nullptr;
>> - if (N1C) {
>> - Divisor = &N1C->getAPIntValue();
>> - } else if (N1.getValueType().isVector() &&
>> - N1->getOpcode() == ISD::BUILD_VECTOR) {
>> - BuildVectorSDNode *BV = cast<BuildVectorSDNode>(N->getOperand(1));
>> - if (ConstantSDNode *C = BV->getConstantSplatValue())
>> - Divisor = &C->getAPIntValue();
>> - }
>> -
>> // fold (sdiv X, pow2) -> simple ops after legalize
>> - if (Divisor && !!*Divisor &&
>> - (Divisor->isPowerOf2() || (-*Divisor).isPowerOf2())) {
>> + if (N1C && !N1C->isNullValue() && (N1C->getAPIntValue().isPowerOf2() ||
>> + (-N1C->getAPIntValue()).isPowerOf2())) {
>> // If dividing by powers of two is cheap, then don't perform the following
>> // fold.
>> if (TLI.isPow2DivCheap())
>> return SDValue();
>>
>> - unsigned lg2 = Divisor->countTrailingZeros();
>> + unsigned lg2 = N1C->getAPIntValue().countTrailingZeros();
>>
>> // Splat the sign bit into the register
>> SDValue SGN =
>> @@ -2025,7 +2020,7 @@ SDValue DAGCombiner::visitSDIV(SDNode *N
>>
>> // If we're dividing by a positive value, we're done. Otherwise, we must
>> // negate the result.
>> - if (Divisor->isNonNegative())
>> + if (N1C->getAPIntValue().isNonNegative())
>> return SRA;
>>
>> AddToWorkList(SRA.getNode());
>> @@ -2034,7 +2029,7 @@ SDValue DAGCombiner::visitSDIV(SDNode *N
>>
>> // if integer divide is expensive and we satisfy the requirements, emit an
>> // alternate sequence.
>> - if ((N1C || N1->getOpcode() == ISD::BUILD_VECTOR) && !TLI.isIntDivCheap()) {
>> + if (N1C && !TLI.isIntDivCheap()) {
>> SDValue Op = BuildSDIV(N);
>> if (Op.getNode()) return Op;
>> }
>> @@ -2052,8 +2047,8 @@ SDValue DAGCombiner::visitSDIV(SDNode *N
>> SDValue DAGCombiner::visitUDIV(SDNode *N) {
>> SDValue N0 = N->getOperand(0);
>> SDValue N1 = N->getOperand(1);
>> - ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0.getNode());
>> - ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1.getNode());
>> + ConstantSDNode *N0C = isConstOrConstSplat(N0);
>> + ConstantSDNode *N1C = isConstOrConstSplat(N1);
>> EVT VT = N->getValueType(0);
>>
>> // fold vector ops
>> @@ -2086,7 +2081,7 @@ SDValue DAGCombiner::visitUDIV(SDNode *N
>> }
>> }
>> // fold (udiv x, c) -> alternate
>> - if ((N1C || N1->getOpcode() == ISD::BUILD_VECTOR) && !TLI.isIntDivCheap()) {
>> + if (N1C && !TLI.isIntDivCheap()) {
>> SDValue Op = BuildUDIV(N);
>> if (Op.getNode()) return Op;
>> }
>> @@ -2104,8 +2099,8 @@ SDValue DAGCombiner::visitUDIV(SDNode *N
>> SDValue DAGCombiner::visitSREM(SDNode *N) {
>> SDValue N0 = N->getOperand(0);
>> SDValue N1 = N->getOperand(1);
>> - ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
>> - ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
>> + ConstantSDNode *N0C = isConstOrConstSplat(N0);
>> + ConstantSDNode *N1C = isConstOrConstSplat(N1);
>> EVT VT = N->getValueType(0);
>>
>> // fold (srem c1, c2) -> c1%c2
>> @@ -2146,8 +2141,8 @@ SDValue DAGCombiner::visitSREM(SDNode *N
>> SDValue DAGCombiner::visitUREM(SDNode *N) {
>> SDValue N0 = N->getOperand(0);
>> SDValue N1 = N->getOperand(1);
>> - ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
>> - ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
>> + ConstantSDNode *N0C = isConstOrConstSplat(N0);
>> + ConstantSDNode *N1C = isConstOrConstSplat(N1);
>> EVT VT = N->getValueType(0);
>>
>> // fold (urem c1, c2) -> c1%c2
>> @@ -11187,28 +11182,20 @@ SDValue DAGCombiner::SimplifySetCC(EVT V
>> /// multiplying by a magic number. See:
>> /// <http://the.wall.riscom.net/books/proc/ppc/cwg/code2.html>
>> SDValue DAGCombiner::BuildSDIV(SDNode *N) {
>> - const APInt *Divisor;
>> - if (N->getValueType(0).isVector()) {
>> - // Handle splat vectors.
>> - BuildVectorSDNode *BV = cast<BuildVectorSDNode>(N->getOperand(1));
>> - if (ConstantSDNode *C = BV->getConstantSplatValue())
>> - Divisor = &C->getAPIntValue();
>> - else
>> - return SDValue();
>> - } else {
>> - Divisor = &cast<ConstantSDNode>(N->getOperand(1))->getAPIntValue();
>> - }
>> + ConstantSDNode *C = isConstOrConstSplat(N->getOperand(1));
>> + if (!C)
>> + return SDValue();
>>
>> // Avoid division by zero.
>> - if (!*Divisor)
>> + if (!C->getAPIntValue())
>> return SDValue();
>>
>> std::vector<SDNode*> Built;
>> - SDValue S = TLI.BuildSDIV(N, *Divisor, DAG, LegalOperations, &Built);
>> + SDValue S =
>> + TLI.BuildSDIV(N, C->getAPIntValue(), DAG, LegalOperations, &Built);
>>
>> - for (std::vector<SDNode*>::iterator ii = Built.begin(), ee = Built.end();
>> - ii != ee; ++ii)
>> - AddToWorkList(*ii);
>> + for (SDNode *N : Built)
>> + AddToWorkList(N);
>> return S;
>> }
>>
>> @@ -11217,28 +11204,20 @@ SDValue DAGCombiner::BuildSDIV(SDNode *N
>> /// multiplying by a magic number. See:
>> /// <http://the.wall.riscom.net/books/proc/ppc/cwg/code2.html>
>> SDValue DAGCombiner::BuildUDIV(SDNode *N) {
>> - const APInt *Divisor;
>> - if (N->getValueType(0).isVector()) {
>> - // Handle splat vectors.
>> - BuildVectorSDNode *BV = cast<BuildVectorSDNode>(N->getOperand(1));
>> - if (ConstantSDNode *C = BV->getConstantSplatValue())
>> - Divisor = &C->getAPIntValue();
>> - else
>> - return SDValue();
>> - } else {
>> - Divisor = &cast<ConstantSDNode>(N->getOperand(1))->getAPIntValue();
>> - }
>> + ConstantSDNode *C = isConstOrConstSplat(N->getOperand(1));
>> + if (!C)
>> + return SDValue();
>>
>> // Avoid division by zero.
>> - if (!*Divisor)
>> + if (!C->getAPIntValue())
>> return SDValue();
>>
>> std::vector<SDNode*> Built;
>> - SDValue S = TLI.BuildUDIV(N, *Divisor, DAG, LegalOperations, &Built);
>> + SDValue S =
>> + TLI.BuildUDIV(N, C->getAPIntValue(), DAG, LegalOperations, &Built);
>>
>> - for (std::vector<SDNode*>::iterator ii = Built.begin(), ee = Built.end();
>> - ii != ee; ++ii)
>> - AddToWorkList(*ii);
>> + for (SDNode *N : Built)
>> + AddToWorkList(N);
>> return S;
>> }
>>
>>
>> Modified: llvm/trunk/test/CodeGen/X86/vector-idiv.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vector-idiv.ll?rev=207338&r1=207337&r2=207338&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/vector-idiv.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/vector-idiv.ll Sat Apr 26 18:09:49 2014
>> @@ -151,3 +151,38 @@ define <8 x i32> @test9(<8 x i32> %a) {
>> ; AVX: vpsrad $2
>> ; AVX: vpadd
>> }
>> +
>> +define <8 x i32> @test10(<8 x i32> %a) {
>> + %rem = urem <8 x i32> %a, <i32 7, i32 7, i32 7, i32 7,i32 7, i32 7, i32 7, i32 7>
>> + ret <8 x i32> %rem
>> +
>> +; AVX-LABEL: test10:
>> +; AVX: vpermd
>> +; AVX: vpmuludq
>> +; AVX: vshufps $-35
>> +; AVX: vpmuludq
>> +; AVX: vshufps $-35
>> +; AVX: vpsubd
>> +; AVX: vpsrld $1
>> +; AVX: vpadd
>> +; AVX: vpsrld $2
>> +; AVX: vpmulld
>> +}
>> +
>> +define <8 x i32> @test11(<8 x i32> %a) {
>> + %rem = srem <8 x i32> %a, <i32 7, i32 7, i32 7, i32 7,i32 7, i32 7, i32 7, i32 7>
>> + ret <8 x i32> %rem
>> +
>> +; AVX-LABEL: test11:
>> +; AVX: vpermd
>> +; AVX: vpmuldq
>> +; AVX: vshufps $-35
>> +; AVX: vpmuldq
>> +; AVX: vshufps $-35
>> +; AVX: vpshufd $-40
>> +; AVX: vpadd
>> +; AVX: vpsrld $31
>> +; AVX: vpsrad $2
>> +; AVX: vpadd
>> +; AVX: vpmulld
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list