[llvm] 7e18cd8 - [InstCombine] Whitelist non-refining folds in SimplifyWithOpReplaced
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 23 09:55:31 PDT 2021
Is there a high level description of the problem AllowRefinement is
trying to solve somewhere? I don't follow the intent of this, and would
like to read up on the background.
Philip
On 3/22/21 2:13 PM, Nikita Popov via llvm-commits wrote:
> Author: Nikita Popov
> Date: 2021-03-22T22:12:56+01:00
> New Revision: 7e18cd887cd402e3d5465c57c218079e4df65231
>
> URL: https://github.com/llvm/llvm-project/commit/7e18cd887cd402e3d5465c57c218079e4df65231
> DIFF: https://github.com/llvm/llvm-project/commit/7e18cd887cd402e3d5465c57c218079e4df65231.diff
>
> LOG: [InstCombine] Whitelist non-refining folds in SimplifyWithOpReplaced
>
> This is an alternative to D98391/D98585, playing things more
> conservatively. If AllowRefinement == false, then we don't use
> InstSimplify methods at all, and instead explicitly implement a
> small number of non-refining folds. Most cases are handled by
> constant folding, and I only had to add three folds to cover
> our unit tests / test-suite. While this may lose some optimization
> power, I think it is safer to approach from this direction, given
> how many issues this code has already caused.
>
> Differential Revision: https://reviews.llvm.org/D99027
>
> Added:
>
>
> Modified:
> llvm/include/llvm/Analysis/InstructionSimplify.h
> llvm/lib/Analysis/InstructionSimplify.cpp
> llvm/test/Transforms/InstCombine/minmax-fold.ll
> llvm/test/Transforms/InstCombine/select.ll
> llvm/test/Transforms/InstSimplify/pr49495.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/Analysis/InstructionSimplify.h b/llvm/include/llvm/Analysis/InstructionSimplify.h
> index 17d6f30a35cb..dda90e826bba 100644
> --- a/llvm/include/llvm/Analysis/InstructionSimplify.h
> +++ b/llvm/include/llvm/Analysis/InstructionSimplify.h
> @@ -294,8 +294,8 @@ Value *SimplifyInstruction(Instruction *I, const SimplifyQuery &Q,
>
> /// See if V simplifies when its operand Op is replaced with RepOp. If not,
> /// return null.
> -/// AllowRefinement specifies whether the simplification can be a refinement,
> -/// or whether it needs to be strictly identical.
> +/// AllowRefinement specifies whether the simplification can be a refinement
> +/// (e.g. 0 instead of poison), or whether it needs to be strictly identical.
> Value *SimplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
> const SimplifyQuery &Q, bool AllowRefinement);
>
>
> diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
> index 4d7e281312ba..1dc7499c85c8 100644
> --- a/llvm/lib/Analysis/InstructionSimplify.cpp
> +++ b/llvm/lib/Analysis/InstructionSimplify.cpp
> @@ -3936,18 +3936,33 @@ static Value *SimplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
> transform(I->operands(), NewOps.begin(),
> [&](Value *V) { return V == Op ? RepOp : V; });
>
> - // Consider:
> - // %cmp = icmp eq i32 %x, 2147483647
> - // %add = add nsw i32 %x, 1
> - // %sel = select i1 %cmp, i32 -2147483648, i32 %add
> - //
> - // We can't replace %sel with %add unless we strip away the flags (which will
> - // be done in InstCombine).
> - // TODO: This is unsound, because it only catches some forms of refinement.
> - if (!AllowRefinement && canCreatePoison(cast<Operator>(I)))
> - return nullptr;
> + if (!AllowRefinement) {
> + // General InstSimplify functions may refine the result, e.g. by returning
> + // a constant for a potentially poison value. To avoid this, implement only
> + // a few non-refining but profitable transforms here.
> +
> + if (auto *BO = dyn_cast<BinaryOperator>(I)) {
> + unsigned Opcode = BO->getOpcode();
> + // id op x -> x, x op id -> x
> + if (NewOps[0] == ConstantExpr::getBinOpIdentity(Opcode, I->getType()))
> + return NewOps[1];
> + if (NewOps[1] == ConstantExpr::getBinOpIdentity(Opcode, I->getType(),
> + /* RHS */ true))
> + return NewOps[0];
> +
> + // x & x -> x, x | x -> x
> + if ((Opcode == Instruction::And || Opcode == Instruction::Or) &&
> + NewOps[0] == NewOps[1])
> + return NewOps[0];
> + }
>
> - if (MaxRecurse) {
> + if (auto *GEP = dyn_cast<GetElementPtrInst>(I)) {
> + // getelementptr x, 0 -> x
> + if (NewOps.size() == 2 && match(NewOps[1], m_Zero()) &&
> + !GEP->isInBounds())
> + return NewOps[0];
> + }
> + } else if (MaxRecurse) {
> // The simplification queries below may return the original value. Consider:
> // %div = udiv i32 %arg, %arg2
> // %mul = mul nsw i32 %div, %arg2
> @@ -3986,6 +4001,18 @@ static Value *SimplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
> return nullptr;
> }
>
> + // Consider:
> + // %cmp = icmp eq i32 %x, 2147483647
> + // %add = add nsw i32 %x, 1
> + // %sel = select i1 %cmp, i32 -2147483648, i32 %add
> + //
> + // We can't replace %sel with %add unless we strip away the flags (which
> + // will be done in InstCombine).
> + // TODO: This may be unsound, because it only catches some forms of
> + // refinement.
> + if (!AllowRefinement && canCreatePoison(cast<Operator>(I)))
> + return nullptr;
> +
> if (CmpInst *C = dyn_cast<CmpInst>(I))
> return ConstantFoldCompareInstOperands(C->getPredicate(), ConstOps[0],
> ConstOps[1], Q.DL, Q.TLI);
>
> diff --git a/llvm/test/Transforms/InstCombine/minmax-fold.ll b/llvm/test/Transforms/InstCombine/minmax-fold.ll
> index 78126302f9b9..7aa3a3992449 100644
> --- a/llvm/test/Transforms/InstCombine/minmax-fold.ll
> +++ b/llvm/test/Transforms/InstCombine/minmax-fold.ll
> @@ -1080,7 +1080,7 @@ define i37 @add_umax_constant_limit(i37 %x) {
>
> define i37 @add_umax_simplify(i37 %x) {
> ; CHECK-LABEL: @add_umax_simplify(
> -; CHECK-NEXT: [[A:%.*]] = add i37 [[X:%.*]], 42
> +; CHECK-NEXT: [[A:%.*]] = add nuw i37 [[X:%.*]], 42
> ; CHECK-NEXT: ret i37 [[A]]
> ;
> %a = add nuw i37 %x, 42
>
> diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
> index ad1d32999556..f98a369c144b 100644
> --- a/llvm/test/Transforms/InstCombine/select.ll
> +++ b/llvm/test/Transforms/InstCombine/select.ll
> @@ -902,7 +902,9 @@ define i32 @test56(i16 %x) {
> define i32 @test57(i32 %x, i32 %y) {
> ; CHECK-LABEL: @test57(
> ; CHECK-NEXT: [[AND:%.*]] = and i32 [[X:%.*]], [[Y:%.*]]
> -; CHECK-NEXT: ret i32 [[AND]]
> +; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[X]], 0
> +; CHECK-NEXT: [[DOTAND:%.*]] = select i1 [[TOBOOL]], i32 0, i32 [[AND]]
> +; CHECK-NEXT: ret i32 [[DOTAND]]
> ;
> %and = and i32 %x, %y
> %tobool = icmp eq i32 %x, 0
>
> diff --git a/llvm/test/Transforms/InstSimplify/pr49495.ll b/llvm/test/Transforms/InstSimplify/pr49495.ll
> index f085de3b4a22..b3eca968c060 100644
> --- a/llvm/test/Transforms/InstSimplify/pr49495.ll
> +++ b/llvm/test/Transforms/InstSimplify/pr49495.ll
> @@ -4,9 +4,11 @@
> ; The first comparison (a != b) should not be dropped
> define i1 @test1(i8* %a, i8* %b) {
> ; CHECK-LABEL: @test1(
> -; CHECK-NEXT: [[A2:%.*]] = getelementptr inbounds i8, i8* [[A:%.*]], i64 -1
> -; CHECK-NEXT: [[COND2:%.*]] = icmp ugt i8* [[A2]], [[B:%.*]]
> -; CHECK-NEXT: ret i1 [[COND2]]
> +; CHECK-NEXT: [[COND1:%.*]] = icmp ne i8* [[A:%.*]], [[B:%.*]]
> +; CHECK-NEXT: [[A2:%.*]] = getelementptr inbounds i8, i8* [[A]], i64 -1
> +; CHECK-NEXT: [[COND2:%.*]] = icmp ugt i8* [[A2]], [[B]]
> +; CHECK-NEXT: [[RES:%.*]] = select i1 [[COND1]], i1 [[COND2]], i1 false
> +; CHECK-NEXT: ret i1 [[RES]]
> ;
> %cond1 = icmp ne i8* %a, %b
> %a2 = getelementptr inbounds i8, i8* %a, i64 -1
> @@ -18,9 +20,11 @@ define i1 @test1(i8* %a, i8* %b) {
> ; The first comparison (a != b) should not be dropped
> define i1 @test2(i32 %a, i32 %b) {
> ; CHECK-LABEL: @test2(
> -; CHECK-NEXT: [[A2:%.*]] = add nuw i32 [[A:%.*]], 1
> -; CHECK-NEXT: [[COND2:%.*]] = icmp ult i32 [[A2]], [[B:%.*]]
> -; CHECK-NEXT: ret i1 [[COND2]]
> +; CHECK-NEXT: [[COND1:%.*]] = icmp ne i32 [[A:%.*]], [[B:%.*]]
> +; CHECK-NEXT: [[A2:%.*]] = add nuw i32 [[A]], 1
> +; CHECK-NEXT: [[COND2:%.*]] = icmp ult i32 [[A2]], [[B]]
> +; CHECK-NEXT: [[RES:%.*]] = select i1 [[COND1]], i1 [[COND2]], i1 false
> +; CHECK-NEXT: ret i1 [[RES]]
> ;
> %cond1 = icmp ne i32 %a, %b
> %a2 = add nuw i32 %a, 1
>
>
>
> _______________________________________________
> 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