[RemoveDIs][NFC] Remove Instruction constructors that insert prior to Instructions #85980
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Stephen Tozer (SLTozer) ChangesThis patch was written almost entirely by @jmorse, but he's on holiday for a while so I'll be handling it. For background reference, see this discourse post: As a step to ensure the correctness of the RemoveDIs project, we're looking to remove all the constructors/static-creation methods for Instructions that take an Removing these constructors will be a pain for downstream developers, since it will result in a lot of build errors wherever the Instruction-taking constructors are used; the good news is that it should generally be simple to fix these, instructions for which are in the above discourse post. Although this is a fairly disruptive change up-front, once it's landed and uses of the old constructors have been updated, it should continue to ensure that debug info is handled correctly without very much/any thought required going onwards. Patch is 166.27 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85980.diff 5 Files Affected:
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index fed21b992e3d10..74476dd773b822 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -59,12 +59,8 @@ class UnaryInstruction : public Instruction {
Op<0>() = V;
}
UnaryInstruction(Type *Ty, unsigned iType, Value *V,
- Instruction *IB = nullptr)
- : Instruction(Ty, iType, &Op<0>(), 1, IB) {
- Op<0>() = V;
- }
- UnaryInstruction(Type *Ty, unsigned iType, Value *V, BasicBlock *IAE)
- : Instruction(Ty, iType, &Op<0>(), 1, IAE) {
+ BasicBlock *IAE = nullptr)
+ : Instruction(Ty, iType, &Op<0>(), 1, IAE) {
Op<0>() = V;
}
@@ -107,8 +103,6 @@ class UnaryOperator : public UnaryInstruction {
protected:
UnaryOperator(UnaryOps iType, Value *S, Type *Ty,
const Twine &Name, BasicBlock::iterator InsertBefore);
- UnaryOperator(UnaryOps iType, Value *S, Type *Ty,
- const Twine &Name, Instruction *InsertBefore);
UnaryOperator(UnaryOps iType, Value *S, Type *Ty,
const Twine &Name, BasicBlock *InsertAtEnd);
@@ -125,22 +119,13 @@ class UnaryOperator : public UnaryInstruction {
static UnaryOperator *Create(UnaryOps Op, Value *S, const Twine &Name,
BasicBlock::iterator InsertBefore);
- /// Construct a unary instruction, given the opcode and an operand.
- /// Optionally (if InstBefore is specified) insert the instruction
- /// into a BasicBlock right before the specified instruction. The specified
- /// Instruction is allowed to be a dereferenced end iterator.
- ///
- static UnaryOperator *Create(UnaryOps Op, Value *S,
- const Twine &Name = Twine(),
- Instruction *InsertBefore = nullptr);
-
/// Construct a unary instruction, given the opcode and an operand.
/// Also automatically insert this instruction to the end of the
/// BasicBlock specified.
///
static UnaryOperator *Create(UnaryOps Op, Value *S,
- const Twine &Name,
- BasicBlock *InsertAtEnd);
+ const Twine &Name = Twine(),
+ BasicBlock *InsertAtEnd = nullptr);
/// These methods just forward to Create, and are useful when you
/// statically know what type of instruction you're going to create. These
@@ -156,12 +141,6 @@ class UnaryOperator : public UnaryInstruction {
return Create(Instruction::OPC, V, Name, BB);\
}
#include "llvm/IR/Instruction.def"
-#define HANDLE_UNARY_INST(N, OPC, CLASS) \
- static UnaryOperator *Create##OPC(Value *V, const Twine &Name, \
- Instruction *I) {\
- return Create(Instruction::OPC, V, Name, I);\
- }
-#include "llvm/IR/Instruction.def"
#define HANDLE_UNARY_INST(N, OPC, CLASS) \
static UnaryOperator *Create##OPC(Value *V, const Twine &Name, \
BasicBlock::iterator It) {\
@@ -177,11 +156,10 @@ class UnaryOperator : public UnaryInstruction {
return UO;
}
- static UnaryOperator *
- CreateWithCopiedFlags(UnaryOps Opc, Value *V, Instruction *CopyO,
- const Twine &Name = "",
- Instruction *InsertBefore = nullptr) {
- UnaryOperator *UO = Create(Opc, V, Name, InsertBefore);
+ static UnaryOperator *CreateWithCopiedFlags(UnaryOps Opc, Value *V,
+ Instruction *CopyO,
+ const Twine &Name = "") {
+ UnaryOperator *UO = Create(Opc, V, Name);
UO->copyIRFlags(CopyO);
return UO;
}
@@ -194,10 +172,8 @@ class UnaryOperator : public UnaryInstruction {
}
static UnaryOperator *CreateFNegFMF(Value *Op, Instruction *FMFSource,
- const Twine &Name = "",
- Instruction *InsertBefore = nullptr) {
- return CreateWithCopiedFlags(Instruction::FNeg, Op, FMFSource, Name,
- InsertBefore);
+ const Twine &Name = "") {
+ return CreateWithCopiedFlags(Instruction::FNeg, Op, FMFSource, Name);
}
UnaryOps getOpcode() const {
@@ -223,8 +199,6 @@ class BinaryOperator : public Instruction {
protected:
BinaryOperator(BinaryOps iType, Value *S1, Value *S2, Type *Ty,
const Twine &Name, BasicBlock::iterator InsertBefore);
- BinaryOperator(BinaryOps iType, Value *S1, Value *S2, Type *Ty,
- const Twine &Name, Instruction *InsertBefore);
BinaryOperator(BinaryOps iType, Value *S1, Value *S2, Type *Ty,
const Twine &Name, BasicBlock *InsertAtEnd);
@@ -249,21 +223,13 @@ class BinaryOperator : public Instruction {
const Twine &Name,
BasicBlock::iterator InsertBefore);
- /// Construct a binary instruction, given the opcode and the two
- /// operands. Optionally (if InstBefore is specified) insert the instruction
- /// into a BasicBlock right before the specified instruction. The specified
- /// Instruction is allowed to be a dereferenced end iterator.
- ///
- static BinaryOperator *Create(BinaryOps Op, Value *S1, Value *S2,
- const Twine &Name = Twine(),
- Instruction *InsertBefore = nullptr);
-
/// Construct a binary instruction, given the opcode and the two
/// operands. Also automatically insert this instruction to the end of the
/// BasicBlock specified.
///
static BinaryOperator *Create(BinaryOps Op, Value *S1, Value *S2,
- const Twine &Name, BasicBlock *InsertAtEnd);
+ const Twine &Name = Twine(),
+ BasicBlock *InsertAtEnd = nullptr);
/// These methods just forward to Create, and are useful when you
/// statically know what type of instruction you're going to create. These
@@ -280,12 +246,6 @@ class BinaryOperator : public Instruction {
return Create(Instruction::OPC, V1, V2, Name, BB);\
}
#include "llvm/IR/Instruction.def"
-#define HANDLE_BINARY_INST(N, OPC, CLASS) \
- static BinaryOperator *Create##OPC(Value *V1, Value *V2, \
- const Twine &Name, Instruction *I) {\
- return Create(Instruction::OPC, V1, V2, Name, I);\
- }
-#include "llvm/IR/Instruction.def"
#define HANDLE_BINARY_INST(N, OPC, CLASS) \
static BinaryOperator *Create##OPC(Value *V1, Value *V2, \
const Twine &Name, BasicBlock::iterator It) {\
@@ -301,11 +261,10 @@ class BinaryOperator : public Instruction {
return BO;
}
- static BinaryOperator *
- CreateWithCopiedFlags(BinaryOps Opc, Value *V1, Value *V2, Value *CopyO,
- const Twine &Name = "",
- Instruction *InsertBefore = nullptr) {
- BinaryOperator *BO = Create(Opc, V1, V2, Name, InsertBefore);
+ static BinaryOperator *CreateWithCopiedFlags(BinaryOps Opc, Value *V1,
+ Value *V2, Value *CopyO,
+ const Twine &Name = "") {
+ BinaryOperator *BO = Create(Opc, V1, V2, Name, nullptr);
BO->copyIRFlags(CopyO);
return BO;
}
@@ -348,12 +307,6 @@ class BinaryOperator : public Instruction {
BO->setHasNoSignedWrap(true);
return BO;
}
- static BinaryOperator *CreateNSW(BinaryOps Opc, Value *V1, Value *V2,
- const Twine &Name, Instruction *I) {
- BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
- BO->setHasNoSignedWrap(true);
- return BO;
- }
static BinaryOperator *CreateNSW(BinaryOps Opc, Value *V1, Value *V2,
const Twine &Name, BasicBlock::iterator It) {
BinaryOperator *BO = Create(Opc, V1, V2, Name, It);
@@ -373,12 +326,6 @@ class BinaryOperator : public Instruction {
BO->setHasNoUnsignedWrap(true);
return BO;
}
- static BinaryOperator *CreateNUW(BinaryOps Opc, Value *V1, Value *V2,
- const Twine &Name, Instruction *I) {
- BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
- BO->setHasNoUnsignedWrap(true);
- return BO;
- }
static BinaryOperator *CreateNUW(BinaryOps Opc, Value *V1, Value *V2,
const Twine &Name, BasicBlock::iterator It) {
BinaryOperator *BO = Create(Opc, V1, V2, Name, It);
@@ -398,12 +345,6 @@ class BinaryOperator : public Instruction {
BO->setIsExact(true);
return BO;
}
- static BinaryOperator *CreateExact(BinaryOps Opc, Value *V1, Value *V2,
- const Twine &Name, Instruction *I) {
- BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
- BO->setIsExact(true);
- return BO;
- }
static BinaryOperator *CreateExact(BinaryOps Opc, Value *V1, Value *V2,
const Twine &Name,
BasicBlock::iterator It) {
@@ -417,9 +358,6 @@ class BinaryOperator : public Instruction {
static inline BinaryOperator *CreateDisjoint(BinaryOps Opc, Value *V1,
Value *V2, const Twine &Name,
BasicBlock *BB);
- static inline BinaryOperator *CreateDisjoint(BinaryOps Opc, Value *V1,
- Value *V2, const Twine &Name,
- Instruction *I);
static inline BinaryOperator *CreateDisjoint(BinaryOps Opc, Value *V1,
Value *V2, const Twine &Name,
BasicBlock::iterator It);
@@ -433,10 +371,6 @@ class BinaryOperator : public Instruction {
Value *V1, Value *V2, const Twine &Name, BasicBlock *BB) { \
return Create##NUWNSWEXACT(Instruction::OPC, V1, V2, Name, BB); \
} \
- static BinaryOperator *Create##NUWNSWEXACT##OPC( \
- Value *V1, Value *V2, const Twine &Name, Instruction *I) { \
- return Create##NUWNSWEXACT(Instruction::OPC, V1, V2, Name, I); \
- } \
static BinaryOperator *Create##NUWNSWEXACT##OPC( \
Value *V1, Value *V2, const Twine &Name, BasicBlock::iterator It) { \
return Create##NUWNSWEXACT(Instruction::OPC, V1, V2, Name, It); \
@@ -472,21 +406,15 @@ class BinaryOperator : public Instruction {
static BinaryOperator *CreateNSWNeg(Value *Op, const Twine &Name,
BasicBlock::iterator InsertBefore);
static BinaryOperator *CreateNSWNeg(Value *Op, const Twine &Name = "",
- Instruction *InsertBefore = nullptr);
- static BinaryOperator *CreateNSWNeg(Value *Op, const Twine &Name,
- BasicBlock *InsertAtEnd);
+ BasicBlock *InsertAtEnd = nullptr);
static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name,
BasicBlock::iterator InsertBefore);
static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name = "",
- Instruction *InsertBefore = nullptr);
- static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name,
- BasicBlock *InsertAtEnd);
+ BasicBlock *InsertAtEnd = nullptr);
static BinaryOperator *CreateNot(Value *Op, const Twine &Name,
BasicBlock::iterator InsertBefore);
static BinaryOperator *CreateNot(Value *Op, const Twine &Name = "",
- Instruction *InsertBefore = nullptr);
- static BinaryOperator *CreateNot(Value *Op, const Twine &Name,
- BasicBlock *InsertAtEnd);
+ BasicBlock *InsertAtEnd = nullptr);
BinaryOps getOpcode() const {
return static_cast<BinaryOps>(Instruction::getOpcode());
@@ -551,13 +479,6 @@ BinaryOperator *BinaryOperator::CreateDisjoint(BinaryOps Opc, Value *V1,
cast<PossiblyDisjointInst>(BO)->setIsDisjoint(true);
return BO;
}
-BinaryOperator *BinaryOperator::CreateDisjoint(BinaryOps Opc, Value *V1,
- Value *V2, const Twine &Name,
- Instruction *I) {
- BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
- cast<PossiblyDisjointInst>(BO)->setIsDisjoint(true);
- return BO;
-}
BinaryOperator *BinaryOperator::CreateDisjoint(BinaryOps Opc, Value *V1,
Value *V2, const Twine &Name,
BasicBlock::iterator It) {
@@ -584,16 +505,10 @@ class CastInst : public UnaryInstruction {
: UnaryInstruction(Ty, iType, S, InsertBefore) {
setName(NameStr);
}
- /// Constructor with insert-before-instruction semantics for subclasses
- CastInst(Type *Ty, unsigned iType, Value *S,
- const Twine &NameStr = "", Instruction *InsertBefore = nullptr)
- : UnaryInstruction(Ty, iType, S, InsertBefore) {
- setName(NameStr);
- }
/// Constructor with insert-at-end-of-block semantics for subclasses
- CastInst(Type *Ty, unsigned iType, Value *S,
- const Twine &NameStr, BasicBlock *InsertAtEnd)
- : UnaryInstruction(Ty, iType, S, InsertAtEnd) {
+ CastInst(Type *Ty, unsigned iType, Value *S, const Twine &NameStr = "",
+ BasicBlock *InsertAtEnd = nullptr)
+ : UnaryInstruction(Ty, iType, S, InsertAtEnd) {
setName(NameStr);
}
@@ -613,29 +528,17 @@ class CastInst : public UnaryInstruction {
);
/// Provides a way to construct any of the CastInst subclasses using an
/// opcode instead of the subclass's constructor. The opcode must be in the
- /// CastOps category (Instruction::isCast(opcode) returns true). This
- /// constructor has insert-before-instruction semantics to automatically
- /// insert the new CastInst before InsertBefore (if it is non-null).
- /// Construct any of the CastInst subclasses
- static CastInst *Create(
- Instruction::CastOps, ///< The opcode of the cast instruction
- Value *S, ///< The value to be casted (operand 0)
- Type *Ty, ///< The type to which cast should be made
- const Twine &Name = "", ///< Name for the instruction
- Instruction *InsertBefore = nullptr ///< Place to insert the instruction
- );
- /// Provides a way to construct any of the CastInst subclasses using an
- /// opcode instead of the subclass's constructor. The opcode must be in the
/// CastOps category. This constructor has insert-at-end-of-block semantics
/// to automatically insert the new CastInst at the end of InsertAtEnd (if
/// its non-null).
/// Construct any of the CastInst subclasses
- static CastInst *Create(
- Instruction::CastOps, ///< The opcode for the cast instruction
- Value *S, ///< The value to be casted (operand 0)
- Type *Ty, ///< The type to which operand is casted
- const Twine &Name, ///< The name for the instruction
- BasicBlock *InsertAtEnd ///< The block to insert the instruction into
+ static CastInst *
+ Create(Instruction::CastOps, ///< The opcode for the cast instruction
+ Value *S, ///< The value to be casted (operand 0)
+ Type *Ty, ///< The type to which operand is casted
+ const Twine &Name = "", ///< The name for the instruction
+ BasicBlock *InsertAtEnd =
+ nullptr ///< Block to insert the instruction into
);
/// Create a ZExt or BitCast cast instruction
@@ -647,19 +550,12 @@ class CastInst : public UnaryInstruction {
);
/// Create a ZExt or BitCast cast instruction
- static CastInst *CreateZExtOrBitCast(
- Value *S, ///< The value to be casted (operand 0)
- Type *Ty, ///< The type to which cast should be made
- const Twine &Name = "", ///< Name for the instruction
- Instruction *InsertBefore = nullptr ///< Place to insert the instruction
- );
-
- /// Create a ZExt or BitCast cast instruction
- static CastInst *CreateZExtOrBitCast(
- Value *S, ///< The value to be casted (operand 0)
- Type *Ty, ///< The type to which operand is casted
- const Twine &Name, ///< The name for the instruction
- BasicBlock *InsertAtEnd ///< The block to insert the instruction into
+ static CastInst *
+ CreateZExtOrBitCast(Value *S, ///< The value to be casted (operand 0)
+ Type *Ty, ///< The type to which operand is casted
+ const Twine &Name = "", ///< The name for the instruction
+ BasicBlock *InsertAtEnd =
+ nullptr ///< Block to insert the instruction into
);
/// Create a SExt or BitCast cast instruction
@@ -671,27 +567,21 @@ class CastInst : public UnaryInstruction {
);
/// Create a SExt or BitCast cast instruction
- static CastInst *CreateSExtOrBitCast(
- Value *S, ///< The value to be casted (operand 0)
- Type *Ty, ///< The type to which cast should be made
- const Twine &Name = "", ///< Name for the instruction
- Instruction *InsertBefore = nullptr ///< Place to insert the instruction
- );
-
- /// Create a SExt or BitCast cast instruction
- static CastInst *CreateSExtOrBitCast(
- Value *S, ///< The value to be casted (operand 0)
- Type *Ty, ///< The type to which operand is casted
- const Twine &Name, ///< The name for the instruction
- BasicBlock *InsertAtEnd ///< The block to insert the instruction into
+ static CastInst *
+ CreateSExtOrBitCast(Value *S, ///< The value to be casted (operand 0)
+ Type *Ty, ///< The type to which operand is casted
+ const Twine &Name = "", ///< The name for the instruction
+ BasicBlock *InsertAtEnd =
+ nullptr ///< Block to insert the instruction into
);
/// Create a BitCast AddrSpaceCast, or a PtrToInt cast instruction.
- static CastInst *CreatePointerCast(
- Value *S, ///< The pointer value to be casted (operand 0)
- Type *Ty, ///< The type to which operand is casted
- const Twine &Name, ///< The name for the instruction
- BasicBlock *InsertAtEnd ///< The block to insert the instruction into
+ static CastInst *
+ CreatePointerCast(Value *S, ///< The pointer value to be casted (operand 0)
+ Type *Ty, ///< The type to which operand is casted
+ const Twine &Name = "", ///< The name for the instruction
+ BasicBlock *InsertAtEnd =
+ nullptr ///< block to insert the instruction into
);
/// Create a BitCast, AddrSpaceCast or a PtrToInt cast instruction.
@@ -702,20 +592,13 @@ class CastInst : public UnaryInstruction {
BasicBlock::iterator InsertBefore ///< Place to insert the instruction
);
- /// Create a BitCast, AddrSpaceCast or a PtrToInt cast instruction.
- static CastInst *CreatePointerCast(
- Value *S, ///< The pointer value to be casted (operand 0)
- Type *Ty, ///< The type to which cast should be made
- const Twine &Name = "", ///< Name for the instruction
- Instruction *InsertBefore = nullptr ///< Place to insert the instruction
- );
-
/// Create a BitCast or an AddrSpaceCast cast instruction.
static CastInst *CreatePointerBitCastOrAddrSpaceCast(
- Value *S, ///< The pointer value to be casted (operand 0)
- Type *Ty, ///< The type to which operand is casted
- const Twine &Name, ///< The name for the instruction
- BasicBlock *InsertAtEnd ///< The block to insert the instruction into
+ Value *S, ///< The pointer value to be casted (operand 0)
+ Type *Ty, ...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff d93cfd8dab577b09a8d01ef10a279b02572e4814 f2b457ea13f2e1eb0ebac81f000bb67c19f40d63 -- llvm/include/llvm/IR/InstrTypes.h llvm/include/llvm/IR/Instruction.h llvm/include/llvm/IR/Instructions.h llvm/lib/IR/Instruction.cpp llvm/lib/IR/Instructions.cppView the diff from clang-format here.diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index 74476dd773..90760f75f1 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -101,8 +101,8 @@ class UnaryOperator : public UnaryInstruction {
void AssertOK();
protected:
- UnaryOperator(UnaryOps iType, Value *S, Type *Ty,
- const Twine &Name, BasicBlock::iterator InsertBefore);
+ UnaryOperator(UnaryOps iType, Value *S, Type *Ty, const Twine &Name,
+ BasicBlock::iterator InsertBefore);
UnaryOperator(UnaryOps iType, Value *S, Type *Ty,
const Twine &Name, BasicBlock *InsertAtEnd);
|
This was referenced Mar 20, 2024
Closed
|
This patch on hold for now, as it seems likely that for the next release we'll be deprecating these functions instead; see discussion here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch was written entirely by @jmorse, but he's on holiday for a while so I'll be handling it. For background reference, see this discourse post: https://discourse.llvm.org/t/psa-instruction-constructors-changing-to-iterator-only-insertion/77845
As a step to ensure the correctness of the RemoveDIs project, we're looking to remove all the constructors/static-creation methods for Instructions that take an
Instruction*to insert before. The benefit from this is that it prevents us from accidentally clearing the head-insertion bit in instruction iterators, which is important for preventing incorrect debug info placement, which can result in errors (example here.Removing these constructors will be a pain for downstream developers, since it will result in a lot of build errors wherever the Instruction-taking constructors are used; the good news is that it should generally be simple to fix these, instructions for which are in the above discourse post. Although this is a fairly disruptive change up-front, once it's landed and uses of the old constructors have been updated, it should continue to ensure that debug info is handled correctly without very much/any thought required going onwards.
Note: If this patch is applied directly to LLVM there will be build errors, as since it was written more uses have emerged (and some may have been missed in the first pass); there is a separate review that removes these instances here, so pick that as well if you're testing this locally.