[llvm] 9b1e953 - [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X transforms
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 8 13:06:30 PDT 2020
Rather than removing them, could we make the transform explicitly
dependent on X being provably non-poison? If nothing else, having the
conditional transform gives an obvious place for a comment explaining
the subtly herein. :)
Philip
On 7/8/20 12:53 PM, Craig Topper via llvm-commits wrote:
> Author: Craig Topper
> Date: 2020-07-08T12:53:05-07:00
> New Revision: 9b1e95329af7bb005275f18225b2c130ec3ea98d
>
> URL: https://github.com/llvm/llvm-project/commit/9b1e95329af7bb005275f18225b2c130ec3ea98d
> DIFF: https://github.com/llvm/llvm-project/commit/9b1e95329af7bb005275f18225b2c130ec3ea98d.diff
>
> LOG: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X transforms
>
> As noted here https://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html and by alive2, this transform isn't valid. If X is poison this potentially propagates poison when it shouldn't.
>
> This same transform still exists in DAGCombiner.
>
> Differential Revision: https://reviews.llvm.org/D83360
>
> Added:
>
>
> Modified:
> clang/test/CodeGen/arm-mve-intrinsics/dup.c
> llvm/lib/Analysis/InstructionSimplify.cpp
> llvm/test/Transforms/InstCombine/select.ll
> llvm/test/Transforms/InstSimplify/select.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/clang/test/CodeGen/arm-mve-intrinsics/dup.c b/clang/test/CodeGen/arm-mve-intrinsics/dup.c
> index 283c08257005..b443917cb258 100644
> --- a/clang/test/CodeGen/arm-mve-intrinsics/dup.c
> +++ b/clang/test/CodeGen/arm-mve-intrinsics/dup.c
> @@ -242,7 +242,8 @@ uint32x4_t test_vdupq_m_n_u32(uint32x4_t inactive, uint32_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <8 x half> undef, half [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <8 x half> [[DOTSPLATINSERT]], <8 x half> undef, <8 x i32> zeroinitializer
> -// CHECK-NEXT: ret <8 x half> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x half> [[DOTSPLAT]], <8 x half> undef
> +// CHECK-NEXT: ret <8 x half> [[TMP2]]
> //
> float16x8_t test_vdupq_x_n_f16(float16_t a, mve_pred16_t p)
> {
> @@ -255,7 +256,8 @@ float16x8_t test_vdupq_x_n_f16(float16_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <4 x float> undef, float [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <4 x float> [[DOTSPLATINSERT]], <4 x float> undef, <4 x i32> zeroinitializer
> -// CHECK-NEXT: ret <4 x float> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x float> [[DOTSPLAT]], <4 x float> undef
> +// CHECK-NEXT: ret <4 x float> [[TMP2]]
> //
> float32x4_t test_vdupq_x_n_f32(float32_t a, mve_pred16_t p)
> {
> @@ -268,7 +270,8 @@ float32x4_t test_vdupq_x_n_f32(float32_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <16 x i8> undef, i8 [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <16 x i8> [[DOTSPLATINSERT]], <16 x i8> undef, <16 x i32> zeroinitializer
> -// CHECK-NEXT: ret <16 x i8> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <16 x i1> [[TMP1]], <16 x i8> [[DOTSPLAT]], <16 x i8> undef
> +// CHECK-NEXT: ret <16 x i8> [[TMP2]]
> //
> int8x16_t test_vdupq_x_n_s8(int8_t a, mve_pred16_t p)
> {
> @@ -281,7 +284,8 @@ int8x16_t test_vdupq_x_n_s8(int8_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <8 x i16> undef, i16 [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <8 x i16> [[DOTSPLATINSERT]], <8 x i16> undef, <8 x i32> zeroinitializer
> -// CHECK-NEXT: ret <8 x i16> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x i16> [[DOTSPLAT]], <8 x i16> undef
> +// CHECK-NEXT: ret <8 x i16> [[TMP2]]
> //
> int16x8_t test_vdupq_x_n_s16(int16_t a, mve_pred16_t p)
> {
> @@ -294,7 +298,8 @@ int16x8_t test_vdupq_x_n_s16(int16_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <4 x i32> undef, i32 [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <4 x i32> [[DOTSPLATINSERT]], <4 x i32> undef, <4 x i32> zeroinitializer
> -// CHECK-NEXT: ret <4 x i32> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i32> [[DOTSPLAT]], <4 x i32> undef
> +// CHECK-NEXT: ret <4 x i32> [[TMP2]]
> //
> int32x4_t test_vdupq_x_n_s32(int32_t a, mve_pred16_t p)
> {
> @@ -307,7 +312,8 @@ int32x4_t test_vdupq_x_n_s32(int32_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <16 x i8> undef, i8 [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <16 x i8> [[DOTSPLATINSERT]], <16 x i8> undef, <16 x i32> zeroinitializer
> -// CHECK-NEXT: ret <16 x i8> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <16 x i1> [[TMP1]], <16 x i8> [[DOTSPLAT]], <16 x i8> undef
> +// CHECK-NEXT: ret <16 x i8> [[TMP2]]
> //
> uint8x16_t test_vdupq_x_n_u8(uint8_t a, mve_pred16_t p)
> {
> @@ -320,7 +326,8 @@ uint8x16_t test_vdupq_x_n_u8(uint8_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <8 x i16> undef, i16 [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <8 x i16> [[DOTSPLATINSERT]], <8 x i16> undef, <8 x i32> zeroinitializer
> -// CHECK-NEXT: ret <8 x i16> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x i16> [[DOTSPLAT]], <8 x i16> undef
> +// CHECK-NEXT: ret <8 x i16> [[TMP2]]
> //
> uint16x8_t test_vdupq_x_n_u16(uint16_t a, mve_pred16_t p)
> {
> @@ -333,7 +340,8 @@ uint16x8_t test_vdupq_x_n_u16(uint16_t a, mve_pred16_t p)
> // CHECK-NEXT: [[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[TMP0]])
> // CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <4 x i32> undef, i32 [[A:%.*]], i32 0
> // CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <4 x i32> [[DOTSPLATINSERT]], <4 x i32> undef, <4 x i32> zeroinitializer
> -// CHECK-NEXT: ret <4 x i32> [[DOTSPLAT]]
> +// CHECK-NEXT: [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i32> [[DOTSPLAT]], <4 x i32> undef
> +// CHECK-NEXT: ret <4 x i32> [[TMP2]]
> //
> uint32x4_t test_vdupq_x_n_u32(uint32_t a, mve_pred16_t p)
> {
>
> diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
> index d3bdf9d6aafd..8cd5d2034586 100644
> --- a/llvm/lib/Analysis/InstructionSimplify.cpp
> +++ b/llvm/lib/Analysis/InstructionSimplify.cpp
> @@ -4118,11 +4118,6 @@ static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
> if (TrueVal == FalseVal)
> return TrueVal;
>
> - if (isa<UndefValue>(TrueVal)) // select ?, undef, X -> X
> - return FalseVal;
> - if (isa<UndefValue>(FalseVal)) // select ?, X, undef -> X
> - return TrueVal;
> -
> // Deal with partial undef vector constants: select ?, VecC, VecC' --> VecC''
> Constant *TrueC, *FalseC;
> if (TrueVal->getType()->isVectorTy() && match(TrueVal, m_Constant(TrueC)) &&
>
> diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
> index 381a77bb8d78..f990a58f984c 100644
> --- a/llvm/test/Transforms/InstCombine/select.ll
> +++ b/llvm/test/Transforms/InstCombine/select.ll
> @@ -2273,3 +2273,43 @@ exit:
> %sel = select i1 %cond, i32 %phi, i32 %A
> ret i32 %sel
> }
> +
> +; Negative tests to ensure we don't remove selects with undef true/false values.
> +; See https://bugs.llvm.org/show_bug.cgi?id=31633
> +; https://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html
> +; https://reviews.llvm.org/D83360
> +define i32 @false_undef(i1 %cond, i32 %x) {
> +; CHECK-LABEL: @false_undef(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], i32 [[X:%.*]], i32 undef
> +; CHECK-NEXT: ret i32 [[S]]
> +;
> + %s = select i1 %cond, i32 %x, i32 undef
> + ret i32 %s
> +}
> +
> +define i32 @true_undef(i1 %cond, i32 %x) {
> +; CHECK-LABEL: @true_undef(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], i32 undef, i32 [[X:%.*]]
> +; CHECK-NEXT: ret i32 [[S]]
> +;
> + %s = select i1 %cond, i32 undef, i32 %x
> + ret i32 %s
> +}
> +
> +define <2 x i32> @false_undef_vec(i1 %cond, <2 x i32> %x) {
> +; CHECK-LABEL: @false_undef_vec(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> [[X:%.*]], <2 x i32> undef
> +; CHECK-NEXT: ret <2 x i32> [[S]]
> +;
> + %s = select i1 %cond, <2 x i32> %x, <2 x i32> undef
> + ret <2 x i32> %s
> +}
> +
> +define <2 x i32> @true_undef_vec(i1 %cond, <2 x i32> %x) {
> +; CHECK-LABEL: @true_undef_vec(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> undef, <2 x i32> [[X:%.*]]
> +; CHECK-NEXT: ret <2 x i32> [[S]]
> +;
> + %s = select i1 %cond, <2 x i32> undef, <2 x i32> %x
> + ret <2 x i32> %s
> +}
>
> diff --git a/llvm/test/Transforms/InstSimplify/select.ll b/llvm/test/Transforms/InstSimplify/select.ll
> index 139f5e3c3c23..81fc3ff186cd 100644
> --- a/llvm/test/Transforms/InstSimplify/select.ll
> +++ b/llvm/test/Transforms/InstSimplify/select.ll
> @@ -750,3 +750,43 @@ define i1 @y_might_be_poison(float %x, float %y) {
> %c3 = select i1 %c1, i1 %c2, i1 false
> ret i1 %c3
> }
> +
> +; Negative tests to ensure we don't remove selects with undef true/false values.
> +; See https://bugs.llvm.org/show_bug.cgi?id=31633
> +; https://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html
> +; https://reviews.llvm.org/D83360
> +define i32 @false_undef(i1 %cond, i32 %x) {
> +; CHECK-LABEL: @false_undef(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], i32 [[X:%.*]], i32 undef
> +; CHECK-NEXT: ret i32 [[S]]
> +;
> + %s = select i1 %cond, i32 %x, i32 undef
> + ret i32 %s
> +}
> +
> +define i32 @true_undef(i1 %cond, i32 %x) {
> +; CHECK-LABEL: @true_undef(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], i32 undef, i32 [[X:%.*]]
> +; CHECK-NEXT: ret i32 [[S]]
> +;
> + %s = select i1 %cond, i32 undef, i32 %x
> + ret i32 %s
> +}
> +
> +define <2 x i32> @false_undef_vec(i1 %cond, <2 x i32> %x) {
> +; CHECK-LABEL: @false_undef_vec(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> [[X:%.*]], <2 x i32> undef
> +; CHECK-NEXT: ret <2 x i32> [[S]]
> +;
> + %s = select i1 %cond, <2 x i32> %x, <2 x i32> undef
> + ret <2 x i32> %s
> +}
> +
> +define <2 x i32> @true_undef_vec(i1 %cond, <2 x i32> %x) {
> +; CHECK-LABEL: @true_undef_vec(
> +; CHECK-NEXT: [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> undef, <2 x i32> [[X:%.*]]
> +; CHECK-NEXT: ret <2 x i32> [[S]]
> +;
> + %s = select i1 %cond, <2 x i32> undef, <2 x i32> %x
> + ret <2 x i32> %s
> +}
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list