[llvm] [SandboxIR] Update visibility of IR constructors. (PR #98107)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 9 15:29:10 PDT 2024
https://github.com/vporpo updated https://github.com/llvm/llvm-project/pull/98107
>From 79343b9ffa147ee6771109ea1fdc3bfe60e17d5a Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vporpodas at google.com>
Date: Mon, 8 Jul 2024 20:15:01 -0700
Subject: [PATCH] [SandboxIR] Update visibility of IR constructors.
All SandboxIR objects are owned by sandboxir::Context and should be created
using sandboxir::Context's builder functions.
This patch protects the constructors to prohibit their use by the user.
---
llvm/include/llvm/SandboxIR/SandboxIR.h | 32 +++++++++-----
llvm/lib/SandboxIR/SandboxIR.cpp | 12 ++---
llvm/unittests/SandboxIR/SandboxIRTest.cpp | 51 ++++++++++++----------
3 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 8416e082aec3d..c84f25f6f5754 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -117,8 +117,9 @@ class Value {
void clearValue() { Val = nullptr; }
template <typename ItTy, typename SBTy> friend class LLVMOpUserItToSBTy;
-public:
Value(ClassID SubclassID, llvm::Value *Val, Context &Ctx);
+
+public:
virtual ~Value() = default;
ClassID getSubclassID() const { return SubclassID; }
@@ -146,9 +147,11 @@ class Value {
/// Argument of a sandboxir::Function.
class Argument : public sandboxir::Value {
-public:
Argument(llvm::Argument *Arg, sandboxir::Context &Ctx)
: sandboxir::Value(ClassID::Argument, Arg, Ctx) {}
+ friend class Context; // For constructor.
+
+public:
static bool classof(const sandboxir::Value *From) {
return From->getSubclassID() == ClassID::Argument;
}
@@ -168,8 +171,10 @@ class Argument : public sandboxir::Value {
};
class User : public Value {
-public:
+protected:
User(ClassID ID, llvm::Value *V, Context &Ctx) : Value(ID, V, Ctx) {}
+
+public:
/// For isa/dyn_cast.
static bool classof(const Value *From);
#ifndef NDEBUG
@@ -187,9 +192,11 @@ class User : public Value {
};
class Constant : public sandboxir::User {
-public:
Constant(llvm::Constant *C, sandboxir::Context &SBCtx)
: sandboxir::User(ClassID::Constant, C, SBCtx) {}
+ friend class Context; // For constructor.
+
+public:
/// For isa/dyn_cast.
static bool classof(const sandboxir::Value *From) {
return From->getSubclassID() == ClassID::Constant ||
@@ -263,11 +270,11 @@ class Instruction : public sandboxir::User {
#include "llvm/SandboxIR/SandboxIRValues.def"
};
+protected:
Instruction(ClassID ID, Opcode Opc, llvm::Instruction *I,
sandboxir::Context &SBCtx)
: sandboxir::User(ID, I, SBCtx), Opc(Opc) {}
-protected:
Opcode Opc;
public:
@@ -297,11 +304,13 @@ class Instruction : public sandboxir::User {
/// An LLLVM Instruction that has no SandboxIR equivalent class gets mapped to
/// an OpaqueInstr.
class OpaqueInst : public sandboxir::Instruction {
-public:
OpaqueInst(llvm::Instruction *I, sandboxir::Context &Ctx)
: sandboxir::Instruction(ClassID::Opaque, Opcode::Opaque, I, Ctx) {}
OpaqueInst(ClassID SubclassID, llvm::Instruction *I, sandboxir::Context &Ctx)
: sandboxir::Instruction(SubclassID, Opcode::Opaque, I, Ctx) {}
+ friend class Context; // For constructor.
+
+public:
static bool classof(const sandboxir::Value *From) {
return From->getSubclassID() == ClassID::Opaque;
}
@@ -326,11 +335,12 @@ class BasicBlock : public Value {
void buildBasicBlockFromLLVMIR(llvm::BasicBlock *LLVMBB);
friend class Context; // For `buildBasicBlockFromIR`
-public:
BasicBlock(llvm::BasicBlock *BB, Context &SBCtx)
: Value(ClassID::Block, BB, SBCtx) {
buildBasicBlockFromLLVMIR(BB);
}
+
+public:
~BasicBlock() = default;
/// For isa/dyn_cast.
static bool classof(const Value *From) {
@@ -385,7 +395,7 @@ class Context {
auto Pair = LLVMValueToValueMap.insert({LLVMArg, nullptr});
auto It = Pair.first;
if (Pair.second) {
- It->second = std::make_unique<Argument>(LLVMArg, *this);
+ It->second = std::unique_ptr<Argument>(new Argument(LLVMArg, *this));
return cast<Argument>(It->second.get());
}
return cast<Argument>(It->second.get());
@@ -422,10 +432,12 @@ class Function : public sandboxir::Value {
return *cast<BasicBlock>(Ctx.getValue(&LLVMBB));
}
};
-
-public:
+ /// Use Context::createFunction() instead.
Function(llvm::Function *F, sandboxir::Context &Ctx)
: sandboxir::Value(ClassID::Function, F, Ctx) {}
+ friend class Context; // For constructor.
+
+public:
/// For isa/dyn_cast.
static bool classof(const sandboxir::Value *From) {
return From->getSubclassID() == ClassID::Function;
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index bd615f0ee7654..f64b1145ebf43 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -233,11 +233,11 @@ Value *Context::getOrCreateValueInternal(llvm::Value *LLVMV, llvm::User *U) {
if (auto *C = dyn_cast<llvm::Constant>(LLVMV)) {
for (llvm::Value *COp : C->operands())
getOrCreateValueInternal(COp, C);
- It->second = std::make_unique<Constant>(C, *this);
+ It->second = std::unique_ptr<Constant>(new Constant(C, *this));
return It->second.get();
}
if (auto *Arg = dyn_cast<llvm::Argument>(LLVMV)) {
- It->second = std::make_unique<Argument>(Arg, *this);
+ It->second = std::unique_ptr<Argument>(new Argument(Arg, *this));
return It->second.get();
}
if (auto *BB = dyn_cast<llvm::BasicBlock>(LLVMV)) {
@@ -248,14 +248,14 @@ Value *Context::getOrCreateValueInternal(llvm::Value *LLVMV, llvm::User *U) {
return nullptr;
}
assert(isa<llvm::Instruction>(LLVMV) && "Expected Instruction");
- It->second =
- std::make_unique<OpaqueInst>(cast<llvm::Instruction>(LLVMV), *this);
+ It->second = std::unique_ptr<OpaqueInst>(
+ new OpaqueInst(cast<llvm::Instruction>(LLVMV), *this));
return It->second.get();
}
BasicBlock *Context::createBasicBlock(llvm::BasicBlock *LLVMBB) {
assert(getValue(LLVMBB) == nullptr && "Already exists!");
- auto NewBBPtr = std::make_unique<BasicBlock>(LLVMBB, *this);
+ auto NewBBPtr = std::unique_ptr<BasicBlock>(new BasicBlock(LLVMBB, *this));
auto *BB = cast<BasicBlock>(registerValue(std::move(NewBBPtr)));
// Create SandboxIR for BB's body.
BB->buildBasicBlockFromLLVMIR(LLVMBB);
@@ -271,7 +271,7 @@ Value *Context::getValue(llvm::Value *V) const {
Function *Context::createFunction(llvm::Function *F) {
assert(getValue(F) == nullptr && "Already exists!");
- auto NewFPtr = std::make_unique<Function>(F, *this);
+ auto NewFPtr = std::unique_ptr<Function>(new Function(F, *this));
// Create arguments.
for (auto &Arg : F->args())
getOrCreateArgument(&Arg);
diff --git a/llvm/unittests/SandboxIR/SandboxIRTest.cpp b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
index e523ae90966d7..161ee51432cd3 100644
--- a/llvm/unittests/SandboxIR/SandboxIRTest.cpp
+++ b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
@@ -35,19 +35,7 @@ struct SandboxIRTest : public testing::Test {
}
};
-TEST_F(SandboxIRTest, UserInstantiation) {
- parseIR(C, R"IR(
-define void @foo(i32 %v1) {
- ret void
-}
-)IR");
- Function &F = *M->getFunction("foo");
- auto *Ret = F.begin()->getTerminator();
- sandboxir::Context Ctx(C);
- [[maybe_unused]] sandboxir::User U(sandboxir::Value::ClassID::User, Ret, Ctx);
-}
-
-TEST_F(SandboxIRTest, FunctionArgumentConstantAndOpaqueInstInstantiation) {
+TEST_F(SandboxIRTest, ClassID) {
parseIR(C, R"IR(
define void @foo(i32 %v1) {
%add = add i32 %v1, 42
@@ -58,51 +46,66 @@ define void @foo(i32 %v1) {
llvm::BasicBlock *LLVMBB = &*LLVMF->begin();
llvm::Instruction *LLVMAdd = &*LLVMBB->begin();
auto *LLVMC = cast<llvm::Constant>(LLVMAdd->getOperand(1));
- auto *LLVMArg0 = LLVMF->getArg(0);
sandboxir::Context Ctx(C);
- sandboxir::Function F(LLVMF, Ctx);
- sandboxir::Argument Arg0(LLVMArg0, Ctx);
- sandboxir::Constant Const0(LLVMC, Ctx);
- sandboxir::OpaqueInst OpaqueI(LLVMAdd, Ctx);
+ sandboxir::Function *F = Ctx.createFunction(LLVMF);
+ sandboxir::Argument *Arg0 = F->getArg(0);
+ sandboxir::BasicBlock *BB = &*F->begin();
+ sandboxir::Instruction *AddI = &*BB->begin();
+ sandboxir::OpaqueInst *OpaqueI = cast<sandboxir::OpaqueInst>(AddI);
+ sandboxir::Constant *Const0 = cast<sandboxir::Constant>(Ctx.getValue(LLVMC));
EXPECT_TRUE(isa<sandboxir::Function>(F));
EXPECT_FALSE(isa<sandboxir::Function>(Arg0));
+ EXPECT_FALSE(isa<sandboxir::Function>(BB));
+ EXPECT_FALSE(isa<sandboxir::Function>(AddI));
EXPECT_FALSE(isa<sandboxir::Function>(Const0));
EXPECT_FALSE(isa<sandboxir::Function>(OpaqueI));
EXPECT_FALSE(isa<sandboxir::Argument>(F));
EXPECT_TRUE(isa<sandboxir::Argument>(Arg0));
+ EXPECT_FALSE(isa<sandboxir::Argument>(BB));
+ EXPECT_FALSE(isa<sandboxir::Argument>(AddI));
EXPECT_FALSE(isa<sandboxir::Argument>(Const0));
EXPECT_FALSE(isa<sandboxir::Argument>(OpaqueI));
EXPECT_TRUE(isa<sandboxir::Constant>(F));
EXPECT_FALSE(isa<sandboxir::Constant>(Arg0));
+ EXPECT_FALSE(isa<sandboxir::Constant>(BB));
+ EXPECT_FALSE(isa<sandboxir::Constant>(AddI));
EXPECT_TRUE(isa<sandboxir::Constant>(Const0));
EXPECT_FALSE(isa<sandboxir::Constant>(OpaqueI));
EXPECT_FALSE(isa<sandboxir::OpaqueInst>(F));
EXPECT_FALSE(isa<sandboxir::OpaqueInst>(Arg0));
+ EXPECT_FALSE(isa<sandboxir::OpaqueInst>(BB));
+ EXPECT_TRUE(isa<sandboxir::OpaqueInst>(AddI));
EXPECT_FALSE(isa<sandboxir::OpaqueInst>(Const0));
EXPECT_TRUE(isa<sandboxir::OpaqueInst>(OpaqueI));
EXPECT_FALSE(isa<sandboxir::Instruction>(F));
EXPECT_FALSE(isa<sandboxir::Instruction>(Arg0));
+ EXPECT_FALSE(isa<sandboxir::Instruction>(BB));
+ EXPECT_TRUE(isa<sandboxir::Instruction>(AddI));
EXPECT_FALSE(isa<sandboxir::Instruction>(Const0));
EXPECT_TRUE(isa<sandboxir::Instruction>(OpaqueI));
EXPECT_FALSE(isa<sandboxir::User>(F));
EXPECT_FALSE(isa<sandboxir::User>(Arg0));
+ EXPECT_FALSE(isa<sandboxir::User>(BB));
+ EXPECT_TRUE(isa<sandboxir::User>(AddI));
EXPECT_TRUE(isa<sandboxir::User>(Const0));
EXPECT_TRUE(isa<sandboxir::User>(OpaqueI));
#ifndef NDEBUG
- // The dump() functions should be very forgiving and should not crash even if
- // sandboxir has not been built properly.
- F.dump();
- Arg0.dump();
- Const0.dump();
- OpaqueI.dump();
+ std::string Buff;
+ raw_string_ostream BS(Buff);
+ F->dump(BS);
+ Arg0->dump(BS);
+ BB->dump(BS);
+ AddI->dump(BS);
+ Const0->dump(BS);
+ OpaqueI->dump(BS);
#endif
}
More information about the llvm-commits
mailing list