From 14ef04c1fb57f73807e9e5030b9b729adeb1cd85 Mon Sep 17 00:00:00 2001 From: vporpo Date: Tue, 9 Jul 2024 16:53:21 -0700 Subject: [PATCH] [SandboxIR] Update visibility of IR constructors. (#98107) 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 8416e082aec3db..c84f25f6f57541 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 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(LLVMArg, *this); + It->second = std::unique_ptr(new Argument(LLVMArg, *this)); return cast(It->second.get()); } return cast(It->second.get()); @@ -422,10 +432,12 @@ class Function : public sandboxir::Value { return *cast(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 bd615f0ee76543..f64b1145ebf43d 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(LLVMV)) { for (llvm::Value *COp : C->operands()) getOrCreateValueInternal(COp, C); - It->second = std::make_unique(C, *this); + It->second = std::unique_ptr(new Constant(C, *this)); return It->second.get(); } if (auto *Arg = dyn_cast(LLVMV)) { - It->second = std::make_unique(Arg, *this); + It->second = std::unique_ptr(new Argument(Arg, *this)); return It->second.get(); } if (auto *BB = dyn_cast(LLVMV)) { @@ -248,14 +248,14 @@ Value *Context::getOrCreateValueInternal(llvm::Value *LLVMV, llvm::User *U) { return nullptr; } assert(isa(LLVMV) && "Expected Instruction"); - It->second = - std::make_unique(cast(LLVMV), *this); + It->second = std::unique_ptr( + new OpaqueInst(cast(LLVMV), *this)); return It->second.get(); } BasicBlock *Context::createBasicBlock(llvm::BasicBlock *LLVMBB) { assert(getValue(LLVMBB) == nullptr && "Already exists!"); - auto NewBBPtr = std::make_unique(LLVMBB, *this); + auto NewBBPtr = std::unique_ptr(new BasicBlock(LLVMBB, *this)); auto *BB = cast(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(F, *this); + auto NewFPtr = std::unique_ptr(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 e523ae90966d79..161ee51432cd33 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(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(AddI); + sandboxir::Constant *Const0 = cast(Ctx.getValue(LLVMC)); EXPECT_TRUE(isa(F)); EXPECT_FALSE(isa(Arg0)); + EXPECT_FALSE(isa(BB)); + EXPECT_FALSE(isa(AddI)); EXPECT_FALSE(isa(Const0)); EXPECT_FALSE(isa(OpaqueI)); EXPECT_FALSE(isa(F)); EXPECT_TRUE(isa(Arg0)); + EXPECT_FALSE(isa(BB)); + EXPECT_FALSE(isa(AddI)); EXPECT_FALSE(isa(Const0)); EXPECT_FALSE(isa(OpaqueI)); EXPECT_TRUE(isa(F)); EXPECT_FALSE(isa(Arg0)); + EXPECT_FALSE(isa(BB)); + EXPECT_FALSE(isa(AddI)); EXPECT_TRUE(isa(Const0)); EXPECT_FALSE(isa(OpaqueI)); EXPECT_FALSE(isa(F)); EXPECT_FALSE(isa(Arg0)); + EXPECT_FALSE(isa(BB)); + EXPECT_TRUE(isa(AddI)); EXPECT_FALSE(isa(Const0)); EXPECT_TRUE(isa(OpaqueI)); EXPECT_FALSE(isa(F)); EXPECT_FALSE(isa(Arg0)); + EXPECT_FALSE(isa(BB)); + EXPECT_TRUE(isa(AddI)); EXPECT_FALSE(isa(Const0)); EXPECT_TRUE(isa(OpaqueI)); EXPECT_FALSE(isa(F)); EXPECT_FALSE(isa(Arg0)); + EXPECT_FALSE(isa(BB)); + EXPECT_TRUE(isa(AddI)); EXPECT_TRUE(isa(Const0)); EXPECT_TRUE(isa(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 }