From d361f76f78e6bb1223e81be24db6ba95fae709ad Mon Sep 17 00:00:00 2001 From: Carl Ritson Date: Tue, 23 Jun 2020 21:38:24 +0900 Subject: [PATCH] Revert "[InstCombine] canonicalize bitcast after insertelement into undef" This reverts commit 856cc60bc1ad07b5cba1ab81160c1c3ef8ff4c23. The change is causing inappropriate reordering of floating point operations in edge cases. Change-Id: Ic474948b53d563e73d2a7af89c601476b704d967 --- .../InstCombine/InstCombineVectorOps.cpp | 19 +--------- .../InstCombine/bitcast-vec-canon.ll | 38 ++++--------------- 2 files changed, 9 insertions(+), 48 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp index ff70347569abce..f05ebdbb3d9a1b 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp @@ -1050,26 +1050,9 @@ Instruction *InstCombiner::visitInsertElementInst(InsertElementInst &IE) { VecOp, ScalarOp, IdxOp, SQ.getWithInstruction(&IE))) return replaceInstUsesWith(IE, V); - // If the scalar is bitcast and inserted into undef, do the insert in the - // source type followed by bitcast. - // TODO: Generalize for insert into any constant, not just undef? - Value *ScalarSrc; - if (match(VecOp, m_Undef()) && - match(ScalarOp, m_OneUse(m_BitCast(m_Value(ScalarSrc)))) && - (ScalarSrc->getType()->isIntegerTy() || - ScalarSrc->getType()->isFloatingPointTy())) { - // inselt undef, (bitcast ScalarSrc), IdxOp --> - // bitcast (inselt undef, ScalarSrc, IdxOp) - Type *ScalarTy = ScalarSrc->getType(); - Type *VecTy = VectorType::get(ScalarTy, IE.getType()->getElementCount()); - UndefValue *NewUndef = UndefValue::get(VecTy); - Value *NewInsElt = Builder.CreateInsertElement(NewUndef, ScalarSrc, IdxOp); - return new BitCastInst(NewInsElt, IE.getType()); - } - // If the vector and scalar are both bitcast from the same element type, do // the insert in that source type followed by bitcast. - Value *VecSrc; + Value *VecSrc, *ScalarSrc; if (match(VecOp, m_BitCast(m_Value(VecSrc))) && match(ScalarOp, m_BitCast(m_Value(ScalarSrc))) && (VecOp->hasOneUse() || ScalarOp->hasOneUse()) && diff --git a/llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll b/llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll index e90f60c5f67e91..73aa226bd58656 100644 --- a/llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll +++ b/llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll @@ -70,12 +70,10 @@ entry: ret double %1 } -; FP source is ok. - define <3 x i64> @bitcast_inselt_undef(double %x, i32 %idx) { ; CHECK-LABEL: @bitcast_inselt_undef( -; CHECK-NEXT: [[TMP1:%.*]] = insertelement <3 x double> undef, double [[X:%.*]], i32 [[IDX:%.*]] -; CHECK-NEXT: [[I:%.*]] = bitcast <3 x double> [[TMP1]] to <3 x i64> +; CHECK-NEXT: [[XB:%.*]] = bitcast double [[X:%.*]] to i64 +; CHECK-NEXT: [[I:%.*]] = insertelement <3 x i64> undef, i64 [[XB]], i32 [[IDX:%.*]] ; CHECK-NEXT: ret <3 x i64> [[I]] ; %xb = bitcast double %x to i64 @@ -83,12 +81,10 @@ define <3 x i64> @bitcast_inselt_undef(double %x, i32 %idx) { ret <3 x i64> %i } -; Integer source is ok; index is anything. - define <3 x float> @bitcast_inselt_undef_fp(i32 %x, i567 %idx) { ; CHECK-LABEL: @bitcast_inselt_undef_fp( -; CHECK-NEXT: [[TMP1:%.*]] = insertelement <3 x i32> undef, i32 [[X:%.*]], i567 [[IDX:%.*]] -; CHECK-NEXT: [[I:%.*]] = bitcast <3 x i32> [[TMP1]] to <3 x float> +; CHECK-NEXT: [[XB:%.*]] = bitcast i32 [[X:%.*]] to float +; CHECK-NEXT: [[I:%.*]] = insertelement <3 x float> undef, float [[XB]], i567 [[IDX:%.*]] ; CHECK-NEXT: ret <3 x float> [[I]] ; %xb = bitcast i32 %x to float @@ -96,21 +92,8 @@ define <3 x float> @bitcast_inselt_undef_fp(i32 %x, i567 %idx) { ret <3 x float> %i } -define @bitcast_inselt_undef_vscale(i32 %x, i567 %idx) { -; CHECK-LABEL: @bitcast_inselt_undef_vscale( -; CHECK-NEXT: [[TMP1:%.*]] = insertelement undef, i32 [[X:%.*]], i567 [[IDX:%.*]] -; CHECK-NEXT: [[I:%.*]] = bitcast [[TMP1]] to -; CHECK-NEXT: ret [[I]] -; - %xb = bitcast i32 %x to float - %i = insertelement undef, float %xb, i567 %idx - ret %i -} - declare void @use(i64) -; Negative test - extra use prevents canonicalization - define <3 x i64> @bitcast_inselt_undef_extra_use(double %x, i32 %idx) { ; CHECK-LABEL: @bitcast_inselt_undef_extra_use( ; CHECK-NEXT: [[XB:%.*]] = bitcast double [[X:%.*]] to i64 @@ -124,8 +107,6 @@ define <3 x i64> @bitcast_inselt_undef_extra_use(double %x, i32 %idx) { ret <3 x i64> %i } -; Negative test - source type must be scalar - define <3 x i64> @bitcast_inselt_undef_vec_src(<2 x i32> %x, i32 %idx) { ; CHECK-LABEL: @bitcast_inselt_undef_vec_src( ; CHECK-NEXT: [[XB:%.*]] = bitcast <2 x i32> [[X:%.*]] to i64 @@ -137,8 +118,6 @@ define <3 x i64> @bitcast_inselt_undef_vec_src(<2 x i32> %x, i32 %idx) { ret <3 x i64> %i } -; Negative test - source type must be scalar - define <3 x i64> @bitcast_inselt_undef_from_mmx(x86_mmx %x, i32 %idx) { ; CHECK-LABEL: @bitcast_inselt_undef_from_mmx( ; CHECK-NEXT: [[XB:%.*]] = bitcast x86_mmx [[X:%.*]] to i64 @@ -150,13 +129,12 @@ define <3 x i64> @bitcast_inselt_undef_from_mmx(x86_mmx %x, i32 %idx) { ret <3 x i64> %i } -; Reduce number of casts - define <2 x i64> @PR45748(double %x, double %y) { ; CHECK-LABEL: @PR45748( -; CHECK-NEXT: [[TMP1:%.*]] = insertelement <2 x double> undef, double [[X:%.*]], i32 0 -; CHECK-NEXT: [[TMP2:%.*]] = insertelement <2 x double> [[TMP1]], double [[Y:%.*]], i32 1 -; CHECK-NEXT: [[I1:%.*]] = bitcast <2 x double> [[TMP2]] to <2 x i64> +; CHECK-NEXT: [[XB:%.*]] = bitcast double [[X:%.*]] to i64 +; CHECK-NEXT: [[I0:%.*]] = insertelement <2 x i64> undef, i64 [[XB]], i32 0 +; CHECK-NEXT: [[YB:%.*]] = bitcast double [[Y:%.*]] to i64 +; CHECK-NEXT: [[I1:%.*]] = insertelement <2 x i64> [[I0]], i64 [[YB]], i32 1 ; CHECK-NEXT: ret <2 x i64> [[I1]] ; %xb = bitcast double %x to i64