[llvm] 0fe91ad - [InstCombine] foldSelectFunnelShift - block poison in funnel shift value

[Roman Lebedev via llvm-commits]

On Sun, Nov 8, 2020 at 3:59 PM Simon Pilgrim via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Simon Pilgrim
> Date: 2020-11-08T12:58:30Z
> New Revision: 0fe91ad463fea9d08cbcd640a62aa9ca2d8d05e0
>
> URL: https://github.com/llvm/llvm-project/commit/0fe91ad463fea9d08cbcd640a62aa9ca2d8d05e0
> DIFF: https://github.com/llvm/llvm-project/commit/0fe91ad463fea9d08cbcd640a62aa9ca2d8d05e0.diff
>
> LOG: [InstCombine] foldSelectFunnelShift - block poison in funnel shift value
>
> As raised by @nlopes on D90382 - if this is not a rotate then the select was blocking poison from the 'shift-by-zero' non-TVal, but a funnel shift won't - so freeze it.
>
> Added:
>
>
> Modified:
>     llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
>     llvm/test/Transforms/InstCombine/funnel.ll
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
> index 55aa085fa876..ea2a02a1bba7 100644
> --- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
> +++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
> @@ -2374,6 +2374,15 @@ static Instruction *foldSelectFunnelShift(SelectInst &Sel,
>        Pred != ICmpInst::ICMP_EQ)
>      return nullptr;
>
> +  // If this is not a rotate then the select was blocking poison from the
> +  // 'shift-by-zero' non-TVal, but a funnel shift won't - so freeze it.
> +  if (SV0 != SV1) {
> +    if (IsFshl && !llvm::isGuaranteedNotToBePoison(SV1))
> +      SV1 = Builder.CreateFreeze(SV1);
> +    else if (!IsFshl && !llvm::isGuaranteedNotToBePoison(SV0))
> +      SV0 = Builder.CreateFreeze(SV0);
> +  }
I would think, in situations like this, we should always just freeze,
unconditionally, because we'll revisit that freeze, and drop it if we can,
or won't if we can't. Point being, are we saving anything
by that isGuaranteedNotToBePoison() here?


>    // This is a funnel/rotate that avoids shift-by-bitwidth UB in a suboptimal way.
>    // Convert to funnel shift intrinsic.
>    Intrinsic::ID IID = IsFshl ? Intrinsic::fshl : Intrinsic::fshr;
>
> diff  --git a/llvm/test/Transforms/InstCombine/funnel.ll b/llvm/test/Transforms/InstCombine/funnel.ll
> index 40e28185d540..0aed0bd3005b 100644
> --- a/llvm/test/Transforms/InstCombine/funnel.ll
> +++ b/llvm/test/Transforms/InstCombine/funnel.ll
> @@ -285,7 +285,8 @@ define i8 @fshr_commute_8bit(i32 %x, i32 %y, i32 %shift) {
>
>  define i8 @fshr_select(i8 %x, i8 %y, i8 %shamt) {
>  ; CHECK-LABEL: @fshr_select(
> -; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.fshr.i8(i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[SHAMT:%.*]])
> +; CHECK-NEXT:    [[TMP1:%.*]] = freeze i8 [[X:%.*]]
> +; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.fshr.i8(i8 [[TMP1]], i8 [[Y:%.*]], i8 [[SHAMT:%.*]])
>  ; CHECK-NEXT:    ret i8 [[R]]
>  ;
>    %cmp = icmp eq i8 %shamt, 0
> @@ -301,7 +302,8 @@ define i8 @fshr_select(i8 %x, i8 %y, i8 %shamt) {
>
>  define i16 @fshl_select(i16 %x, i16 %y, i16 %shamt) {
>  ; CHECK-LABEL: @fshl_select(
> -; CHECK-NEXT:    [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[Y:%.*]], i16 [[SHAMT:%.*]])
> +; CHECK-NEXT:    [[TMP1:%.*]] = freeze i16 [[Y:%.*]]
> +; CHECK-NEXT:    [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[TMP1]], i16 [[SHAMT:%.*]])
>  ; CHECK-NEXT:    ret i16 [[R]]
>  ;
>    %cmp = icmp eq i16 %shamt, 0
> @@ -317,7 +319,8 @@ define i16 @fshl_select(i16 %x, i16 %y, i16 %shamt) {
>
>  define <2 x i64> @fshl_select_vector(<2 x i64> %x, <2 x i64> %y, <2 x i64> %shamt) {
>  ; CHECK-LABEL: @fshl_select_vector(
> -; CHECK-NEXT:    [[R:%.*]] = call <2 x i64> @llvm.fshl.v2i64(<2 x i64> [[Y:%.*]], <2 x i64> [[X:%.*]], <2 x i64> [[SHAMT:%.*]])
> +; CHECK-NEXT:    [[TMP1:%.*]] = freeze <2 x i64> [[X:%.*]]
> +; CHECK-NEXT:    [[R:%.*]] = call <2 x i64> @llvm.fshl.v2i64(<2 x i64> [[Y:%.*]], <2 x i64> [[TMP1]], <2 x i64> [[SHAMT:%.*]])
>  ; CHECK-NEXT:    ret <2 x i64> [[R]]
>  ;
>    %cmp = icmp eq <2 x i64> %shamt, zeroinitializer
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits