[llvm] [LIR][SCEVExpander] Restore original flags when aborting transform (PR #82362)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 21 00:21:39 PST 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/82362
>From 7880decce8175a30d4c35d971dbd706d74bc92c3 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 20 Feb 2024 14:41:01 +0100
Subject: [PATCH 1/2] [LIR][SCEVExpander] Restore original flags when aborting
transform
SCEVExpanderCleaner will currently remove instructions created
by SCEVExpander, but not restore poison generating flags that it
may have dropped. As such, running LIR can currently spuriously
drop flags without performing any transforms.
Fix this by keeping track of original instruction flags in
SCEVExpander.
Fixes https://github.com/llvm/llvm-project/issues/82337.
---
.../Utils/ScalarEvolutionExpander.h | 20 +++++++++
.../Utils/ScalarEvolutionExpander.cpp | 42 +++++++++++++++++++
llvm/test/Transforms/LoopIdiom/pr82337.ll | 6 +--
3 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
index fa10443f14bb74..9de0996fb1e30d 100644
--- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
+++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
@@ -41,6 +41,17 @@ struct SCEVOperand {
const SCEV* S;
};
+struct PoisonFlags {
+ unsigned NUW : 1;
+ unsigned NSW : 1;
+ unsigned Exact : 1;
+ unsigned Disjoint : 1;
+ unsigned NNeg : 1;
+
+ PoisonFlags(const Instruction *I);
+ void apply(Instruction *I);
+};
+
/// This class uses information about analyze scalars to rewrite expressions
/// in canonical form.
///
@@ -48,6 +59,8 @@ struct SCEVOperand {
/// and destroy it when finished to allow the release of the associated
/// memory.
class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
+ friend class SCEVExpanderCleaner;
+
ScalarEvolution &SE;
const DataLayout &DL;
@@ -70,6 +83,10 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
/// InsertedValues/InsertedPostIncValues.
SmallPtrSet<Value *, 16> ReusedValues;
+ /// Original flags of instructions for which they were modified. Used
+ /// by SCEVExpanderCleaner to undo changes.
+ DenseMap<AssertingVH<Instruction>, PoisonFlags> OrigFlags;
+
// The induction variables generated.
SmallVector<WeakVH, 2> InsertedIVs;
@@ -188,6 +205,7 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
InsertedValues.clear();
InsertedPostIncValues.clear();
ReusedValues.clear();
+ OrigFlags.clear();
ChainedPhis.clear();
InsertedIVs.clear();
}
@@ -491,6 +509,8 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
void rememberInstruction(Value *I);
+ void rememberFlags(Instruction *I);
+
bool isNormalAddRecExprPHI(PHINode *PN, Instruction *IncV, const Loop *L);
bool isExpandedAddRecExprPHI(PHINode *PN, Instruction *IncV, const Loop *L);
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index fbe1dba5b8d4e4..90c800f78c9807 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -43,6 +43,37 @@ cl::opt<unsigned> llvm::SCEVCheapExpansionBudget(
using namespace PatternMatch;
+PoisonFlags::PoisonFlags(const Instruction *I) {
+ NUW = false;
+ NSW = false;
+ Exact = false;
+ Disjoint = false;
+ NNeg = false;
+ if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I)) {
+ NUW = OBO->hasNoUnsignedWrap();
+ NSW = OBO->hasNoUnsignedWrap();
+ }
+ if (auto *PEO = dyn_cast<PossiblyExactOperator>(I))
+ Exact = PEO->isExact();
+ if (auto *PDI = dyn_cast<PossiblyDisjointInst>(I))
+ Disjoint = PDI->isDisjoint();
+ if (auto *PNI = dyn_cast<PossiblyNonNegInst>(I))
+ NNeg = PNI->hasNonNeg();
+}
+
+void PoisonFlags::apply(Instruction *I) {
+ if (isa<OverflowingBinaryOperator>(I)) {
+ I->setHasNoUnsignedWrap(NUW);
+ I->setHasNoUnsignedWrap(NSW);
+ }
+ if (isa<PossiblyExactOperator>(I))
+ I->setIsExact(Exact);
+ if (auto *PDI = dyn_cast<PossiblyDisjointInst>(I))
+ PDI->setIsDisjoint(Disjoint);
+ if (auto *PNI = dyn_cast<PossiblyNonNegInst>(I))
+ PNI->setNonNeg(NNeg);
+}
+
/// ReuseOrCreateCast - Arrange for there to be a cast of V to Ty at IP,
/// reusing an existing cast if a suitable one (= dominating IP) exists, or
/// creating a new one.
@@ -724,6 +755,7 @@ bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos,
auto FixupPoisonFlags = [this](Instruction *I) {
// Drop flags that are potentially inferred from old context and infer flags
// in new context.
+ rememberFlags(I);
I->dropPoisonGeneratingFlags();
if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
if (auto Flags = SE.getStrengthenedNoWrapFlagsFromBinOp(OBO)) {
@@ -1481,6 +1513,7 @@ Value *SCEVExpander::expand(const SCEV *S) {
V = fixupLCSSAFormFor(V);
} else {
for (Instruction *I : DropPoisonGeneratingInsts) {
+ rememberFlags(I);
I->dropPoisonGeneratingFlagsAndMetadata();
// See if we can re-infer from first principles any of the flags we just
// dropped.
@@ -1521,6 +1554,11 @@ void SCEVExpander::rememberInstruction(Value *I) {
DoInsert(I);
}
+void SCEVExpander::rememberFlags(Instruction *I) {
+ // If we already have flags for the instruction, keep the existing ones.
+ OrigFlags.try_emplace(I, PoisonFlags(I));
+}
+
void SCEVExpander::replaceCongruentIVInc(
PHINode *&Phi, PHINode *&OrigPhi, Loop *L, const DominatorTree *DT,
SmallVectorImpl<WeakTrackingVH> &DeadInsts) {
@@ -2318,6 +2356,10 @@ void SCEVExpanderCleaner::cleanup() {
if (ResultUsed)
return;
+ // Restore original poison flags.
+ for (auto [I, Flags] : Expander.OrigFlags)
+ Flags.apply(I);
+
auto InsertedInstructions = Expander.getAllInsertedInstructions();
#ifndef NDEBUG
SmallPtrSet<Instruction *, 8> InsertedSet(InsertedInstructions.begin(),
diff --git a/llvm/test/Transforms/LoopIdiom/pr82337.ll b/llvm/test/Transforms/LoopIdiom/pr82337.ll
index e8a6e1704f7c17..da9eb14af3f0a4 100644
--- a/llvm/test/Transforms/LoopIdiom/pr82337.ll
+++ b/llvm/test/Transforms/LoopIdiom/pr82337.ll
@@ -1,15 +1,15 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
; RUN: opt -S -passes=loop-idiom < %s | FileCheck %s
-; FIXME: The poison flags should be preserved, as no transform takes place.
+; The poison flags should be preserved, as no transform takes place.
define void @test(ptr %p.end, ptr %p.start) {
; CHECK-LABEL: define void @test(
; CHECK-SAME: ptr [[P_END:%.*]], ptr [[P_START:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[P_END_INT:%.*]] = ptrtoint ptr [[P_END]] to i64
; CHECK-NEXT: [[P_START_INT:%.*]] = ptrtoint ptr [[P_START]] to i64
-; CHECK-NEXT: [[DIST:%.*]] = sub i64 [[P_END_INT]], [[P_START_INT]]
-; CHECK-NEXT: [[LEN:%.*]] = lshr i64 [[DIST]], 5
+; CHECK-NEXT: [[DIST:%.*]] = sub nuw i64 [[P_END_INT]], [[P_START_INT]]
+; CHECK-NEXT: [[LEN:%.*]] = lshr exact i64 [[DIST]], 5
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P_END]], [[P_START]]
; CHECK-NEXT: br i1 [[CMP]], label [[EXIT:%.*]], label [[PREHEADER:%.*]]
; CHECK: preheader:
>From 43d0a0089a97bd949cf2d8e923d7967c345ab269 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 21 Feb 2024 09:21:05 +0100
Subject: [PATCH 2/2] fix typos
---
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index 90c800f78c9807..0f67cc3ff4faca 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -51,7 +51,7 @@ PoisonFlags::PoisonFlags(const Instruction *I) {
NNeg = false;
if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I)) {
NUW = OBO->hasNoUnsignedWrap();
- NSW = OBO->hasNoUnsignedWrap();
+ NSW = OBO->hasNoSignedWrap();
}
if (auto *PEO = dyn_cast<PossiblyExactOperator>(I))
Exact = PEO->isExact();
@@ -64,7 +64,7 @@ PoisonFlags::PoisonFlags(const Instruction *I) {
void PoisonFlags::apply(Instruction *I) {
if (isa<OverflowingBinaryOperator>(I)) {
I->setHasNoUnsignedWrap(NUW);
- I->setHasNoUnsignedWrap(NSW);
+ I->setHasNoSignedWrap(NSW);
}
if (isa<PossiblyExactOperator>(I))
I->setIsExact(Exact);
More information about the llvm-commits
mailing list