Skip to content

Commit

Permalink
[SandboxIR] Update visibility of IR constructors. (#98107)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vporpo authored Jul 9, 2024
1 parent d283627 commit 2d8b282
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 40 deletions.
32 changes: 22 additions & 10 deletions llvm/include/llvm/SandboxIR/SandboxIR.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand All @@ -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 ||
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/SandboxIR/SandboxIR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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);
Expand All @@ -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);
Expand Down
51 changes: 27 additions & 24 deletions llvm/unittests/SandboxIR/SandboxIRTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down

0 comments on commit 2d8b282

Please sign in to comment.