[llvm] [SandboxIR] Added setVolatile member function to LoadInst and StoreInst (PR #101284)
Julius Alexandre via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 31 13:17:05 PDT 2024
https://github.com/medievalghoul updated https://github.com/llvm/llvm-project/pull/101284
>From fe6603fc2204d535255dba56bacd3d81a2ac4d77 Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Wed, 31 Jul 2024 00:22:12 -0400
Subject: [PATCH 1/4] [SandboxIR] Added setVolatile() function to LoadInst and
StoreInst
---
llvm/include/llvm/SandboxIR/SandboxIR.h | 9 +++++-
llvm/include/llvm/SandboxIR/Tracker.h | 19 +++++++++++++
llvm/lib/SandboxIR/Tracker.cpp | 28 ++++++++++++++++++
llvm/unittests/SandboxIR/SandboxIRTest.cpp | 33 ++++++++++++++++++++++
4 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 5b2330c35ee23..02c168302fd3c 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -776,6 +776,8 @@ class LoadInst final : public Instruction {
public:
/// Return true if this is a load from a volatile memory location.
bool isVolatile() const { return cast<llvm::LoadInst>(Val)->isVolatile(); }
+ /// Specify whether this is a volatile load or not.
+ void setVolatile(bool V) { return cast<llvm::LoadInst>(Val)->setVolatile(V); }
unsigned getUseOperandNo(const Use &Use) const final {
return getUseOperandNoDefault(Use);
@@ -825,6 +827,11 @@ class StoreInst final : public Instruction {
public:
/// Return true if this is a store from a volatile memory location.
bool isVolatile() const { return cast<llvm::StoreInst>(Val)->isVolatile(); }
+ /// Specify whether this is a volatile store or not.
+ void setVolatile(bool V) {
+ return cast<llvm::StoreInst>(Val)->setVolatile(V);
+ }
+
unsigned getUseOperandNo(const Use &Use) const final {
return getUseOperandNoDefault(Use);
}
@@ -1334,7 +1341,7 @@ class CastInst : public Instruction {
/// constructor directly.
CastInst(llvm::CastInst *CI, Context &Ctx)
: Instruction(ClassID::Cast, getCastOpcode(CI->getOpcode()), CI, Ctx) {}
- friend Context; // for SBCastInstruction()
+ friend Context; // for SBCastInstruction()
friend class PtrToInt; // For constructor.
Use getOperandUseInternal(unsigned OpIdx, bool Verify) const final {
return getOperandUseDefault(OpIdx, Verify);
diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h
index 64068461b9490..2b28c353f9173 100644
--- a/llvm/include/llvm/SandboxIR/Tracker.h
+++ b/llvm/include/llvm/SandboxIR/Tracker.h
@@ -213,6 +213,25 @@ class CallBrInstSetIndirectDest : public IRChangeBase {
#endif
};
+class SetVolatile : public IRChangeBase {
+ Instruction *Inst;
+ bool WasVolatile;
+ /// This could either be StoreInst or LoadInst
+ PointerUnion<StoreInst *, LoadInst *> InstUnion;
+
+public:
+ SetVolatile(Instruction *I, bool WasVolatile, Tracker &Tracker);
+ void revert() final;
+ void accept() final {}
+#ifndef NDEBUG
+ void dump(raw_ostream &OS) const final {
+ dumpCommon(OS);
+ OS << "SetVolatile";
+ }
+ LLVM_DUMP_METHOD void dump() const final;
+#endif
+};
+
class MoveInstr : public IRChangeBase {
/// The instruction that moved.
Instruction *MovedI;
diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp
index eae55d7b3d962..432151932571f 100644
--- a/llvm/lib/SandboxIR/Tracker.cpp
+++ b/llvm/lib/SandboxIR/Tracker.cpp
@@ -160,6 +160,34 @@ void CallBrInstSetIndirectDest::dump() const {
}
#endif
+SetVolatile::SetVolatile(Instruction *I, bool WasBool, Tracker &Tracker)
+ : IRChangeBase(Tracker), Inst(I) {
+ if (auto *Load = cast<llvm::LoadInst>(Inst)) {
+ WasVolatile = Load->isVolatile();
+ InstUnion = Load;
+ }
+ if (auto *Store = cast<llvm::StoreInst>(Inst)) {
+ WasVolatile = Store->isVolatile();
+ InstUnion = Store;
+ }
+}
+
+void SetVolatile::revert() {
+ if (llvm::LoadInst *Load = InstUnion.dyn_cast<llvm::LoadInst *>()) {
+ // It's a LoadInst
+ Load->setVolatile(WasVolatile);
+ } else if (llvm::StoreInst *Store = InstUnion.dyn_cast<llvm::StoreInst *>()) {
+ // It's a StoreInst
+ Store->setVolatile(WasVolatile);
+ }
+}
+#ifndef NDEBUG
+void SetVolatile::dump() const {
+ dump(dbgs());
+ dbgs() << "\n";
+}
+#endif
+
MoveInstr::MoveInstr(Instruction *MovedI, Tracker &Tracker)
: IRChangeBase(Tracker), MovedI(MovedI) {
if (auto *NextI = MovedI->getNextNode())
diff --git a/llvm/unittests/SandboxIR/SandboxIRTest.cpp b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
index 28c3aebeb1862..1c6d950955d09 100644
--- a/llvm/unittests/SandboxIR/SandboxIRTest.cpp
+++ b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
@@ -753,6 +753,7 @@ define void @foo(ptr %arg0, ptr %arg1) {
auto *Ld = cast<sandboxir::LoadInst>(&*It++);
auto *VLd = cast<sandboxir::LoadInst>(&*It++);
auto *Ret = cast<sandboxir::ReturnInst>(&*It++);
+ bool OrigVolatileValue;
// Check isVolatile()
EXPECT_FALSE(Ld->isVolatile());
@@ -767,7 +768,10 @@ define void @foo(ptr %arg0, ptr %arg1) {
sandboxir::LoadInst::create(Ld->getType(), Arg1, Align(8),
/*InsertBefore=*/Ret, Ctx, "NewLd");
EXPECT_FALSE(NewLd->isVolatile());
+ OrigVolatileValue = NewLd->isVolatile();
+ NewLd->setVolatile(true);
EXPECT_EQ(NewLd->getType(), Ld->getType());
+ NewLd->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewLd->getPointerOperand(), Arg1);
EXPECT_EQ(NewLd->getAlign(), 8);
EXPECT_EQ(NewLd->getName(), "NewLd");
@@ -778,12 +782,20 @@ define void @foo(ptr %arg0, ptr %arg1) {
/*IsVolatile=*/true, Ctx, "NewVLd");
EXPECT_TRUE(NewVLd->isVolatile());
+ OrigVolatileValue = NewVLd->isVolatile();
+ NewVLd->setVolatile(false);
+ EXPECT_FALSE(NewVLd->isVolatile());
+ NewVLd->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewVLd->getName(), "NewVLd");
// Check create(InsertAtEnd)
sandboxir::LoadInst *NewLdEnd =
sandboxir::LoadInst::create(Ld->getType(), Arg1, Align(8),
/*InsertAtEnd=*/BB, Ctx, "NewLdEnd");
EXPECT_FALSE(NewLdEnd->isVolatile());
+ OrigVolatileValue = NewLdEnd->isVolatile();
+ NewLdEnd->setVolatile(true);
+ EXPECT_TRUE(NewLdEnd->isVolatile());
+ NewLdEnd->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewLdEnd->getName(), "NewLdEnd");
EXPECT_EQ(NewLdEnd->getType(), Ld->getType());
EXPECT_EQ(NewLdEnd->getPointerOperand(), Arg1);
@@ -796,6 +808,10 @@ define void @foo(ptr %arg0, ptr %arg1) {
/*InsertAtEnd=*/BB,
/*IsVolatile=*/true, Ctx, "NewVLdEnd");
EXPECT_TRUE(NewVLdEnd->isVolatile());
+ OrigVolatileValue = NewVLdEnd->isVolatile();
+ NewVLdEnd->setVolatile(false);
+ EXPECT_FALSE(NewVLdEnd->isVolatile());
+ NewVLdEnd->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewVLdEnd->getName(), "NewVLdEnd");
EXPECT_EQ(NewVLdEnd->getType(), VLd->getType());
EXPECT_EQ(NewVLdEnd->getPointerOperand(), Arg1);
@@ -822,6 +838,7 @@ define void @foo(i8 %val, ptr %ptr) {
auto *St = cast<sandboxir::StoreInst>(&*It++);
auto *VSt = cast<sandboxir::StoreInst>(&*It++);
auto *Ret = cast<sandboxir::ReturnInst>(&*It++);
+ bool OrigVolatileValue;
// Check that the StoreInst has been created correctly.
EXPECT_FALSE(St->isVolatile());
@@ -836,6 +853,10 @@ define void @foo(i8 %val, ptr %ptr) {
sandboxir::StoreInst::create(Val, Ptr, Align(8),
/*InsertBefore=*/Ret, Ctx);
EXPECT_FALSE(NewSt->isVolatile());
+ OrigVolatileValue = NewSt->isVolatile();
+ NewSt->setVolatile(true);
+ EXPECT_TRUE(NewSt->isVolatile());
+ NewSt->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewSt->getType(), St->getType());
EXPECT_EQ(NewSt->getValueOperand(), Val);
EXPECT_EQ(NewSt->getPointerOperand(), Ptr);
@@ -847,6 +868,10 @@ define void @foo(i8 %val, ptr %ptr) {
/*InsertBefore=*/Ret,
/*IsVolatile=*/true, Ctx);
EXPECT_TRUE(NewVSt->isVolatile());
+ OrigVolatileValue = NewVSt->isVolatile();
+ NewVSt->setVolatile(false);
+ EXPECT_FALSE(NewVSt->isVolatile());
+ NewVSt->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewVSt->getType(), VSt->getType());
EXPECT_EQ(NewVSt->getValueOperand(), Val);
EXPECT_EQ(NewVSt->getPointerOperand(), Ptr);
@@ -857,6 +882,10 @@ define void @foo(i8 %val, ptr %ptr) {
sandboxir::StoreInst::create(Val, Ptr, Align(8),
/*InsertAtEnd=*/BB, Ctx);
EXPECT_FALSE(NewStEnd->isVolatile());
+ OrigVolatileValue = NewStEnd->isVolatile();
+ NewStEnd->setVolatile(true);
+ EXPECT_TRUE(NewStEnd->isVolatile());
+ NewStEnd->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewStEnd->getType(), St->getType());
EXPECT_EQ(NewStEnd->getValueOperand(), Val);
EXPECT_EQ(NewStEnd->getPointerOperand(), Ptr);
@@ -869,6 +898,10 @@ define void @foo(i8 %val, ptr %ptr) {
/*InsertAtEnd=*/BB,
/*IsVolatile=*/true, Ctx);
EXPECT_TRUE(NewVStEnd->isVolatile());
+ OrigVolatileValue = NewVStEnd->isVolatile();
+ NewVStEnd->setVolatile(false);
+ EXPECT_FALSE(NewVStEnd->isVolatile());
+ NewVStEnd->setVolatile(OrigVolatileValue);
EXPECT_EQ(NewVStEnd->getType(), VSt->getType());
EXPECT_EQ(NewVStEnd->getValueOperand(), Val);
EXPECT_EQ(NewVStEnd->getPointerOperand(), Ptr);
>From f097ecdc82be86bdd925275bbea804b267585904 Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Wed, 31 Jul 2024 00:27:40 -0400
Subject: [PATCH 2/4] simple fix
---
llvm/include/llvm/SandboxIR/SandboxIR.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 02c168302fd3c..34efad0afb304 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -828,9 +828,7 @@ class StoreInst final : public Instruction {
/// Return true if this is a store from a volatile memory location.
bool isVolatile() const { return cast<llvm::StoreInst>(Val)->isVolatile(); }
/// Specify whether this is a volatile store or not.
- void setVolatile(bool V) {
- return cast<llvm::StoreInst>(Val)->setVolatile(V);
- }
+ void setVolatile(bool V) { return cast<llvm::StoreInst>(Val)->setVolatile(V); }
unsigned getUseOperandNo(const Use &Use) const final {
return getUseOperandNoDefault(Use);
>From 63e2aa165c15db4bc9cdab4dacfaa338ea207adc Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Wed, 31 Jul 2024 00:37:41 -0400
Subject: [PATCH 3/4] clang format
---
llvm/include/llvm/SandboxIR/SandboxIR.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 34efad0afb304..02c168302fd3c 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -828,7 +828,9 @@ class StoreInst final : public Instruction {
/// Return true if this is a store from a volatile memory location.
bool isVolatile() const { return cast<llvm::StoreInst>(Val)->isVolatile(); }
/// Specify whether this is a volatile store or not.
- void setVolatile(bool V) { return cast<llvm::StoreInst>(Val)->setVolatile(V); }
+ void setVolatile(bool V) {
+ return cast<llvm::StoreInst>(Val)->setVolatile(V);
+ }
unsigned getUseOperandNo(const Use &Use) const final {
return getUseOperandNoDefault(Use);
>From 8b13e5b34bea270b4e5797994bdea58009da1a64 Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Wed, 31 Jul 2024 16:13:40 -0400
Subject: [PATCH 4/4] Added better constructs and along with a Tracker Test
---
llvm/include/llvm/SandboxIR/SandboxIR.h | 6 ++--
llvm/include/llvm/SandboxIR/Tracker.h | 5 ++--
llvm/lib/SandboxIR/SandboxIR.cpp | 17 ++++++++++++
llvm/lib/SandboxIR/Tracker.cpp | 22 ++++++++-------
llvm/unittests/SandboxIR/TrackerTest.cpp | 35 ++++++++++++++++++++++++
5 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 02c168302fd3c..c9cb1fb2e5ab9 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -777,7 +777,7 @@ class LoadInst final : public Instruction {
/// Return true if this is a load from a volatile memory location.
bool isVolatile() const { return cast<llvm::LoadInst>(Val)->isVolatile(); }
/// Specify whether this is a volatile load or not.
- void setVolatile(bool V) { return cast<llvm::LoadInst>(Val)->setVolatile(V); }
+ void setVolatile(bool V);
unsigned getUseOperandNo(const Use &Use) const final {
return getUseOperandNoDefault(Use);
@@ -828,9 +828,7 @@ class StoreInst final : public Instruction {
/// Return true if this is a store from a volatile memory location.
bool isVolatile() const { return cast<llvm::StoreInst>(Val)->isVolatile(); }
/// Specify whether this is a volatile store or not.
- void setVolatile(bool V) {
- return cast<llvm::StoreInst>(Val)->setVolatile(V);
- }
+ void setVolatile(bool V);
unsigned getUseOperandNo(const Use &Use) const final {
return getUseOperandNoDefault(Use);
diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h
index 2b28c353f9173..1fd0d89296c5f 100644
--- a/llvm/include/llvm/SandboxIR/Tracker.h
+++ b/llvm/include/llvm/SandboxIR/Tracker.h
@@ -214,13 +214,14 @@ class CallBrInstSetIndirectDest : public IRChangeBase {
};
class SetVolatile : public IRChangeBase {
- Instruction *Inst;
+ /// This holds the properties of whether
+ /// LoadInst or StoreInst was volatile
bool WasVolatile;
/// This could either be StoreInst or LoadInst
PointerUnion<StoreInst *, LoadInst *> InstUnion;
public:
- SetVolatile(Instruction *I, bool WasVolatile, Tracker &Tracker);
+ SetVolatile(Instruction *I, Tracker &Tracker);
void revert() final;
void accept() final {}
#ifndef NDEBUG
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index 010cc115a5cc3..6cc3346ed589d 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -610,6 +610,14 @@ void BranchInst::dump() const {
}
#endif // NDEBUG
+void LoadInst::setVolatile(bool V) {
+ auto &Tracker = Ctx.getTracker();
+ if (Tracker.isTracking())
+ Tracker.track(
+ std::make_unique<SetVolatile>(cast<Instruction>(this), Tracker));
+ cast<llvm::LoadInst>(Val)->setVolatile(V);
+}
+
LoadInst *LoadInst::create(Type *Ty, Value *Ptr, MaybeAlign Align,
Instruction *InsertBefore, Context &Ctx,
const Twine &Name) {
@@ -664,6 +672,15 @@ void LoadInst::dump() const {
dbgs() << "\n";
}
#endif // NDEBUG
+
+void StoreInst::setVolatile(bool V) {
+ auto &Tracker = Ctx.getTracker();
+ if (Tracker.isTracking())
+ Tracker.track(
+ std::make_unique<SetVolatile>(cast<Instruction>(this), Tracker));
+ cast<llvm::StoreInst>(Val)->setVolatile(V);
+}
+
StoreInst *StoreInst::create(Value *V, Value *Ptr, MaybeAlign Align,
Instruction *InsertBefore, Context &Ctx) {
return create(V, Ptr, Align, InsertBefore, /*IsVolatile=*/false, Ctx);
diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp
index 432151932571f..33b983602d880 100644
--- a/llvm/lib/SandboxIR/Tracker.cpp
+++ b/llvm/lib/SandboxIR/Tracker.cpp
@@ -160,26 +160,28 @@ void CallBrInstSetIndirectDest::dump() const {
}
#endif
-SetVolatile::SetVolatile(Instruction *I, bool WasBool, Tracker &Tracker)
- : IRChangeBase(Tracker), Inst(I) {
- if (auto *Load = cast<llvm::LoadInst>(Inst)) {
+SetVolatile::SetVolatile(Instruction *I, Tracker &Tracker)
+ : IRChangeBase(Tracker) {
+ if (auto *Load = cast<llvm::LoadInst>(I)) {
WasVolatile = Load->isVolatile();
InstUnion = Load;
- }
- if (auto *Store = cast<llvm::StoreInst>(Inst)) {
+ assert(Load && "This isn't a LoadInst");
+ } else if (auto *Store = cast<llvm::StoreInst>(I)) {
WasVolatile = Store->isVolatile();
InstUnion = Store;
+ assert(StoreInst && "This isn't a StoreInst");
+ } else {
+ llvm_unreachable("Expected LoadInst or StoreInst");
}
}
void SetVolatile::revert() {
- if (llvm::LoadInst *Load = InstUnion.dyn_cast<llvm::LoadInst *>()) {
- // It's a LoadInst
+ if (llvm::LoadInst *Load = InstUnion.dyn_cast<llvm::LoadInst *>())
Load->setVolatile(WasVolatile);
- } else if (llvm::StoreInst *Store = InstUnion.dyn_cast<llvm::StoreInst *>()) {
- // It's a StoreInst
+ else if (llvm::StoreInst *Store = InstUnion.dyn_cast<llvm::StoreInst *>())
Store->setVolatile(WasVolatile);
- }
+ else
+ llvm_unreachable("Expected LoadInst or StoreInst");
}
#ifndef NDEBUG
void SetVolatile::dump() const {
diff --git a/llvm/unittests/SandboxIR/TrackerTest.cpp b/llvm/unittests/SandboxIR/TrackerTest.cpp
index cd737d33dd193..5dde36f5e6eac 100644
--- a/llvm/unittests/SandboxIR/TrackerTest.cpp
+++ b/llvm/unittests/SandboxIR/TrackerTest.cpp
@@ -584,3 +584,38 @@ define void @foo(i8 %arg) {
Ctx.revert();
EXPECT_EQ(CallBr->getIndirectDest(0), OrigIndirectDest);
}
+
+TEST_F(TrackerTest, SetVolatile) {
+ parseIR(C, R"IR(
+define void @foo(ptr %arg0, i8 %val) {
+ %ld = load i8, ptr %arg0, align 64
+ store i8 %val, ptr %arg0, align 64
+ ret void
+}
+)IR");
+ Function &LLVMF = *M->getFunction("foo");
+ sandboxir::Context Ctx(C);
+
+ auto *F = Ctx.createFunction(&LLVMF);
+ unsigned ArgIdx = 0;
+ [[maybe_unused]] auto *Arg0 = F->getArg(ArgIdx++);
+ [[maybe_unused]] auto *Val = F->getArg(ArgIdx++);
+ auto *BB = &*F->begin();
+ auto It = BB->begin();
+ auto *Load = cast<sandboxir::LoadInst>(&*It++);
+ auto *Store = cast<sandboxir::StoreInst>(&*It++);
+ [[maybe_unused]] auto *Ret = cast<sandboxir::ReturnInst>(&*It++);
+
+ // Check setVolatile()
+ Ctx.save();
+ Load->setVolatile(true);
+ EXPECT_TRUE(Load->isVolatile());
+ Load->setVolatile(false);
+ EXPECT_FALSE(Load->isVolatile());
+
+ Ctx.save();
+ Store->setVolatile(true);
+ EXPECT_TRUE(Store->isVolatile());
+ Store->setVolatile(false);
+ EXPECT_FALSE(Store->isVolatile());
+}
More information about the llvm-commits
mailing list