From 821bba7fda8acb85a173f731a09e6818fd0b58e2 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Sun, 1 Mar 2015 00:09:35 +0000 Subject: [PATCH] avoid infinite looping when folding vector multiplies of constants (PR22698) We were missing a check for the following fold in DAGCombiner: // fold (fmul (fmul x, c1), c2) -> (fmul x, (fmul c1, c2)) If 'x' is also a constant, then we shouldn't do anything. Otherwise, we could end up swapping the operands back and forth forever. This should fix: http://llvm.org/bugs/show_bug.cgi?id=22698 Differential Revision: http://reviews.llvm.org/D7917 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@230884 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 19 +++++++++---- test/CodeGen/X86/fmul-combines.ll | 34 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 8722c624797..9e2b5afde1e 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -7453,14 +7453,23 @@ SDValue DAGCombiner::visitFMUL(SDNode *N) { // Fold scalars or any vector constants (not just splats). // This fold is done in general by InstCombine, but extra fmul insts // may have been generated during lowering. + SDValue N00 = N0.getOperand(0); SDValue N01 = N0.getOperand(1); auto *BV1 = dyn_cast(N1); + auto *BV00 = dyn_cast(N00); auto *BV01 = dyn_cast(N01); - if ((N1CFP && isConstOrConstSplatFP(N01)) || - (BV1 && BV01 && BV1->isConstant() && BV01->isConstant())) { - SDLoc SL(N); - SDValue MulConsts = DAG.getNode(ISD::FMUL, SL, VT, N01, N1); - return DAG.getNode(ISD::FMUL, SL, VT, N0.getOperand(0), MulConsts); + + // Check 1: Make sure that the first operand of the inner multiply is NOT + // a constant. Otherwise, we may induce infinite looping. + if (!(isConstOrConstSplatFP(N00) || (BV00 && BV00->isConstant()))) { + // Check 2: Make sure that the second operand of the inner multiply and + // the second operand of the outer multiply are constants. + if ((N1CFP && isConstOrConstSplatFP(N01)) || + (BV1 && BV01 && BV1->isConstant() && BV01->isConstant())) { + SDLoc SL(N); + SDValue MulConsts = DAG.getNode(ISD::FMUL, SL, VT, N01, N1); + return DAG.getNode(ISD::FMUL, SL, VT, N00, MulConsts); + } } } diff --git a/test/CodeGen/X86/fmul-combines.ll b/test/CodeGen/X86/fmul-combines.ll index 703651153c1..7d75611e133 100644 --- a/test/CodeGen/X86/fmul-combines.ll +++ b/test/CodeGen/X86/fmul-combines.ll @@ -103,6 +103,40 @@ define <4 x float> @fmul_v4f32_two_consts_no_splat_multiple_use(<4 x float> %x) ret <4 x float> %a } +; PR22698 - http://llvm.org/bugs/show_bug.cgi?id=22698 +; Make sure that we don't infinite loop swapping constants back and forth. + +define <4 x float> @PR22698_splats(<4 x float> %a) #0 { + %mul1 = fmul fast <4 x float> , + %mul2 = fmul fast <4 x float> , %mul1 + %mul3 = fmul fast <4 x float> %a, %mul2 + ret <4 x float> %mul3 + +; CHECK: float 2.400000e+01 +; CHECK: float 2.400000e+01 +; CHECK: float 2.400000e+01 +; CHECK: float 2.400000e+01 +; CHECK-LABEL: PR22698_splats: +; CHECK: mulps +; CHECK: ret +} + +; Same as above, but verify that non-splat vectors are handled correctly too. +define <4 x float> @PR22698_no_splats(<4 x float> %a) #0 { + %mul1 = fmul fast <4 x float> , + %mul2 = fmul fast <4 x float> , %mul1 + %mul3 = fmul fast <4 x float> %a, %mul2 + ret <4 x float> %mul3 + +; CHECK: float 4.500000e+01 +; CHECK: float 1.200000e+02 +; CHECK: float 2.310000e+02 +; CHECK: float 3.840000e+02 +; CHECK-LABEL: PR22698_no_splats: +; CHECK: mulps +; CHECK: ret +} + ; CHECK-LABEL: fmul_c2_c4_f32: ; CHECK-NOT: addss ; CHECK: mulss -- 2.34.1