[llvm] 6f34839 - [instcombine] propagate freeze through single use poison producing flag instruction
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 12 20:39:36 PDT 2021
Since no else got to this before I did, I fixed the reported problem in
4c5702c (instead of reverting).
Philip
On 10/12/21 4:29 PM, Philip Reames wrote:
> AFK and there was a problem reported post commit. Can I ask someone to revert on my behalf?
>
> Philip
>
>> On Oct 12, 2021, at 1:52 PM, Philip Reames via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>>
>> Author: Philip Reames
>> Date: 2021-10-12T13:52:41-07:00
>> New Revision: 6f348394079f8baa8f33bf68a50457c108e2305f
>>
>> URL: https://github.com/llvm/llvm-project/commit/6f348394079f8baa8f33bf68a50457c108e2305f
>> DIFF: https://github.com/llvm/llvm-project/commit/6f348394079f8baa8f33bf68a50457c108e2305f.diff
>>
>> LOG: [instcombine] propagate freeze through single use poison producing flag instruction
>>
>> If we have an instruction which produces poison only when flags are specified on the instruction, then we know that freezing the operands and dropping flags is equivalent to freezing the result. If we know those flags don't result in any undefined behavior being executed, then there's no point in preserving the flags as we gain no knowledge by having them.
>>
>> This patch extends the existing propagation logic which sinks freeze to single potential non-poison operands to allow dropping of flags when we know the freeze is the sole use of the instruction with poison flags.
>>
>> The main value is that we tend to sink freezes towards the phi in IV cycles where the incoming value to the phi is the freeze of an IV increment. This will in turn (in a future patch), let us fold the freeze through the phi into the loop preheader. Motivated by eliminating need for CanonicalizeFreezeInLoops for the clearly profitable cases from onephi.ll test case in the test directory.
>>
>> Differential Revision: https://reviews.llvm.org/D111675
>>
>> Added:
>>
>>
>> Modified:
>> llvm/include/llvm/Analysis/ValueTracking.h
>> llvm/lib/Analysis/ValueTracking.cpp
>> llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
>> llvm/test/Transforms/InstCombine/freeze.ll
>>
>> Removed:
>>
>>
>>
>> ################################################################################
>> diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
>> index e386c46662453..3ba84a99e3406 100644
>> --- a/llvm/include/llvm/Analysis/ValueTracking.h
>> +++ b/llvm/include/llvm/Analysis/ValueTracking.h
>> @@ -637,10 +637,16 @@ constexpr unsigned MaxAnalysisRecursionDepth = 6;
>> /// true. If Op raises immediate UB but never creates poison or undef
>> /// (e.g. sdiv I, 0), canCreatePoison returns false.
>> ///
>> + /// \p ConsiderFlags controls whether poison producing flags on the
>> + /// instruction are considered. This can be used to see if the instruction
>> + /// could still introduce undef or poison even without poison generating flags
>> + /// which might be on the instruction. (i.e. could the result of
>> + /// Op->dropPoisonGeneratingFlags() still create poison or undef)
>> + ///
>> /// canCreatePoison returns true if Op can create poison from non-poison
>> /// operands.
>> - bool canCreateUndefOrPoison(const Operator *Op);
>> - bool canCreatePoison(const Operator *Op);
>> + bool canCreateUndefOrPoison(const Operator *Op, bool ConsiderFlags = true);
>> + bool canCreatePoison(const Operator *Op, bool ConsiderFlags = true);
>>
>> /// Return true if V is poison given that ValAssumedPoison is already poison.
>> /// For example, if ValAssumedPoison is `icmp X, 10` and V is `icmp X, 5`,
>>
>> diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
>> index 2c98e8f6fb336..e5b1b2423845c 100644
>> --- a/llvm/lib/Analysis/ValueTracking.cpp
>> +++ b/llvm/lib/Analysis/ValueTracking.cpp
>> @@ -4948,19 +4948,22 @@ bool llvm::isOverflowIntrinsicNoWrap(const WithOverflowInst *WO,
>> return llvm::any_of(GuardingBranches, AllUsesGuardedByBranch);
>> }
>>
>> -static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly) {
>> - // See whether I has flags that may create poison
>> - if (const auto *OvOp = dyn_cast<OverflowingBinaryOperator>(Op)) {
>> - if (OvOp->hasNoSignedWrap() || OvOp->hasNoUnsignedWrap())
>> - return true;
>> - }
>> - if (const auto *ExactOp = dyn_cast<PossiblyExactOperator>(Op))
>> - if (ExactOp->isExact())
>> - return true;
>> - if (const auto *FP = dyn_cast<FPMathOperator>(Op)) {
>> - auto FMF = FP->getFastMathFlags();
>> - if (FMF.noNaNs() || FMF.noInfs())
>> - return true;
>> +static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly,
>> + bool ConsiderFlags) {
>> + if (ConsiderFlags) {
>> + // See whether I has flags that may create poison
>> + if (const auto *OvOp = dyn_cast<OverflowingBinaryOperator>(Op)) {
>> + if (OvOp->hasNoSignedWrap() || OvOp->hasNoUnsignedWrap())
>> + return true;
>> + }
>> + if (const auto *ExactOp = dyn_cast<PossiblyExactOperator>(Op))
>> + if (ExactOp->isExact())
>> + return true;
>> + if (const auto *FP = dyn_cast<FPMathOperator>(Op)) {
>> + auto FMF = FP->getFastMathFlags();
>> + if (FMF.noNaNs() || FMF.noInfs())
>> + return true;
>> + }
>> }
>>
>> unsigned Opcode = Op->getOpcode();
>> @@ -5061,12 +5064,12 @@ static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly) {
>> }
>> }
>>
>> -bool llvm::canCreateUndefOrPoison(const Operator *Op) {
>> - return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/false);
>> +bool llvm::canCreateUndefOrPoison(const Operator *Op, bool ConsiderFlags) {
>> + return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/false, ConsiderFlags);
>> }
>>
>> -bool llvm::canCreatePoison(const Operator *Op) {
>> - return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true);
>> +bool llvm::canCreatePoison(const Operator *Op, bool ConsiderFlags) {
>> + return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true, ConsiderFlags);
>> }
>>
>> static bool directlyImpliesPoison(const Value *ValAssumedPoison,
>>
>> diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
>> index d1d6be962edb7..bc4a088d6f2fa 100644
>> --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
>> +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
>> @@ -3544,8 +3544,14 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
>> // While we could change the other users of OrigOp to use freeze(OrigOp), that
>> // potentially reduces their optimization potential, so let's only do this iff
>> // the OrigOp is only used by the freeze.
>> - if (!OrigOpInst || !OrigOpInst->hasOneUse() || isa<PHINode>(OrigOp) ||
>> - canCreateUndefOrPoison(dyn_cast<Operator>(OrigOp)))
>> + if (!OrigOpInst || !OrigOpInst->hasOneUse() || isa<PHINode>(OrigOp))
>> + return nullptr;
>> +
>> + // We can't push the freeze through an instruction which can itself create
>> + // poison. If the only source of new poison is flags, we can simply
>> + // strip them (since we know the only use is the freeze and nothing can
>> + // benefit from them.)
>> + if (canCreateUndefOrPoison(cast<Operator>(OrigOp), /*ConsiderFlags*/ false))
>> return nullptr;
>>
>> // If operand is guaranteed not to be poison, there is no need to add freeze
>> @@ -3561,6 +3567,8 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
>> return nullptr;
>> }
>>
>> + OrigOpInst->dropPoisonGeneratingFlags();
>> +
>> // If all operands are guaranteed to be non-poison, we can drop freeze.
>> if (!MaybePoisonOperand)
>> return OrigOp;
>>
>> diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
>> index e6cd4e07a7e63..981d671fbcfe9 100644
>> --- a/llvm/test/Transforms/InstCombine/freeze.ll
>> +++ b/llvm/test/Transforms/InstCombine/freeze.ll
>> @@ -121,10 +121,10 @@ define i1 @early_freeze_test2(i32* %ptr) {
>>
>> define i32 @early_freeze_test3(i32 %v1) {
>> ; CHECK-LABEL: @early_freeze_test3(
>> -; CHECK-NEXT: [[V2:%.*]] = shl i32 [[V1:%.*]], 1
>> -; CHECK-NEXT: [[V3:%.*]] = add nuw i32 [[V2]], 2
>> -; CHECK-NEXT: [[V3_FR:%.*]] = freeze i32 [[V3]]
>> -; CHECK-NEXT: [[V4:%.*]] = or i32 [[V3_FR]], 1
>> +; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1:%.*]]
>> +; CHECK-NEXT: [[V2:%.*]] = shl i32 [[V1_FR]], 1
>> +; CHECK-NEXT: [[V3:%.*]] = add i32 [[V2]], 2
>> +; CHECK-NEXT: [[V4:%.*]] = or i32 [[V3]], 1
>> ; CHECK-NEXT: ret i32 [[V4]]
>> ;
>> %v2 = shl i32 %v1, 1
>> @@ -223,9 +223,9 @@ end:
>>
>> define i32 @propagate_drop_flags_add(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_flags_add(
>> -; CHECK-NEXT: [[V1:%.*]] = add nuw nsw i32 [[ARG:%.*]], 2
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = add i32 [[ARG_FR]], 2
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = add nsw nuw i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -234,9 +234,8 @@ define i32 @propagate_drop_flags_add(i32 %arg) {
>>
>> define i32 @propagate_drop_flags_add_foldaway(i32 noundef %arg) {
>> ; CHECK-LABEL: @propagate_drop_flags_add_foldaway(
>> -; CHECK-NEXT: [[V1:%.*]] = add nuw nsw i32 [[ARG:%.*]], 2
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[V1:%.*]] = add i32 [[ARG:%.*]], 2
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = add nsw nuw i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -245,9 +244,9 @@ define i32 @propagate_drop_flags_add_foldaway(i32 noundef %arg) {
>>
>> define i32 @propagate_drop_flags_sub(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_flags_sub(
>> -; CHECK-NEXT: [[V1:%.*]] = add nsw i32 [[ARG:%.*]], -2
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = add i32 [[ARG_FR]], -2
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = sub nsw nuw i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -256,9 +255,9 @@ define i32 @propagate_drop_flags_sub(i32 %arg) {
>>
>> define i32 @propagate_drop_flags_mul(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_flags_mul(
>> -; CHECK-NEXT: [[V1:%.*]] = shl nuw nsw i32 [[ARG:%.*]], 1
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = shl i32 [[ARG_FR]], 1
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = mul nsw nuw i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -267,9 +266,9 @@ define i32 @propagate_drop_flags_mul(i32 %arg) {
>>
>> define i32 @propagate_drop_flags_udiv(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_flags_udiv(
>> -; CHECK-NEXT: [[V1:%.*]] = lshr exact i32 [[ARG:%.*]], 1
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = lshr i32 [[ARG_FR]], 1
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = udiv exact i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -278,9 +277,9 @@ define i32 @propagate_drop_flags_udiv(i32 %arg) {
>>
>> define i32 @propagate_drop_flags_sdiv(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_flags_sdiv(
>> -; CHECK-NEXT: [[V1:%.*]] = ashr exact i32 [[ARG:%.*]], 1
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = ashr i32 [[ARG_FR]], 1
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = sdiv exact i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -289,9 +288,9 @@ define i32 @propagate_drop_flags_sdiv(i32 %arg) {
>>
>> define i32 @propagate_drop_shl1(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_shl1(
>> -; CHECK-NEXT: [[V1:%.*]] = shl nuw nsw i32 [[ARG:%.*]], 2
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = shl i32 [[ARG_FR]], 2
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = shl nsw nuw i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -311,9 +310,9 @@ define i32 @propagate_drop_shl2(i32 %arg, i32 %unknown) {
>>
>> define i32 @propagate_drop_ashr1(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_ashr1(
>> -; CHECK-NEXT: [[V1:%.*]] = ashr exact i32 [[ARG:%.*]], 2
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = ashr i32 [[ARG_FR]], 2
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = ashr exact i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>> @@ -333,9 +332,9 @@ define i32 @propagate_drop_ashr2(i32 %arg, i32 %unknown) {
>>
>> define i32 @propagate_drop_lshr1(i32 %arg) {
>> ; CHECK-LABEL: @propagate_drop_lshr1(
>> -; CHECK-NEXT: [[V1:%.*]] = lshr exact i32 [[ARG:%.*]], 2
>> -; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
>> -; CHECK-NEXT: ret i32 [[V1_FR]]
>> +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
>> +; CHECK-NEXT: [[V1:%.*]] = lshr i32 [[ARG_FR]], 2
>> +; CHECK-NEXT: ret i32 [[V1]]
>> ;
>> %v1 = lshr exact i32 %arg, 2
>> %v1.fr = freeze i32 %v1
>>
>>
>>
>> _______________________________________________
>> 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