[llvm] [IR] Make instr-create fns with Instruction *InsertBefore non-default (PR #86132)

via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 21 08:25:32 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-ir

Author: Stephen Tozer (SLTozer)

<details>
<summary>Changes</summary>

This patch changes each of the methods that create instructions and may insert it at the same time - generally `InstrType::Create(...)` or constructors - such that the overload called when the insert-position argument is omitted will be the candidate that takes a `BasicBlock *InsertAtEnd` argument, rather than an `Instruction *InsertBefore` argument as is currently default in all cases. This is to move us towards deprecating the insert-before-instruction versions, for the sake of preventing debug-info errors creeping in from the RemoveDIs project; for more details on this motivation, see the relevant [discourse thread](https://discourse.llvm.org/t/psa-instruction-constructors-changing-to-iterator-only-insertion/77845) and the [pull request](https://github.com/llvm/llvm-project/pull/85980) that would delete the insert-before-instruction functions.

Unclear who's best placed to review this, but since this patch should be NFC - even for downstream consumers - it's relatively low-stakes; for the subsequent patch that deprecates the Instruction versions, I'll put out a RFC on discourse.

---

Patch is 98.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86132.diff


4 Files Affected:

- (modified) llvm/include/llvm/IR/InstrTypes.h (+135-70) 
- (modified) llvm/include/llvm/IR/Instruction.h (+2-2) 
- (modified) llvm/include/llvm/IR/Instructions.h (+289-282) 
- (modified) llvm/lib/IR/Instructions.cpp (+97) 


``````````diff
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index e8c2cba8418dc8..4d5ccbbbb46f3d 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -60,11 +60,11 @@ class UnaryInstruction : public Instruction {
     Op<0>() = V;
   }
   UnaryInstruction(Type *Ty, unsigned iType, Value *V,
-                   Instruction *IB = nullptr)
+                   Instruction *IB)
     : Instruction(Ty, iType, &Op<0>(), 1, IB) {
     Op<0>() = V;
   }
-  UnaryInstruction(Type *Ty, unsigned iType, Value *V, BasicBlock *IAE)
+  UnaryInstruction(Type *Ty, unsigned iType, Value *V, BasicBlock *IAE = nullptr)
     : Instruction(Ty, iType, &Op<0>(), 1, IAE) {
     Op<0>() = V;
   }
@@ -132,16 +132,16 @@ class UnaryOperator : public UnaryInstruction {
   /// Instruction is allowed to be a dereferenced end iterator.
   ///
   static UnaryOperator *Create(UnaryOps Op, Value *S,
-                               const Twine &Name = Twine(),
-                               Instruction *InsertBefore = nullptr);
+                               const Twine &Name,
+                               Instruction *InsertBefore);
 
   /// 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
@@ -180,13 +180,22 @@ class UnaryOperator : public UnaryInstruction {
 
   static UnaryOperator *
   CreateWithCopiedFlags(UnaryOps Opc, Value *V, Instruction *CopyO,
-                        const Twine &Name = "",
-                        Instruction *InsertBefore = nullptr) {
+                        const Twine &Name,
+                        Instruction *InsertBefore) {
     UnaryOperator *UO = Create(Opc, V, Name, InsertBefore);
     UO->copyIRFlags(CopyO);
     return UO;
   }
 
+  static UnaryOperator *
+  CreateWithCopiedFlags(UnaryOps Opc, Value *V, Instruction *CopyO,
+                        const Twine &Name = "",
+                        BasicBlock *InsertAtEnd = nullptr) {
+    UnaryOperator *UO = Create(Opc, V, Name, InsertAtEnd);
+    UO->copyIRFlags(CopyO);
+    return UO;
+  }
+
   static UnaryOperator *CreateFNegFMF(Value *Op, Instruction *FMFSource,
                                       const Twine &Name,
                                       BasicBlock::iterator InsertBefore) {
@@ -195,12 +204,19 @@ class UnaryOperator : public UnaryInstruction {
   }
 
   static UnaryOperator *CreateFNegFMF(Value *Op, Instruction *FMFSource,
-                                      const Twine &Name = "",
-                                      Instruction *InsertBefore = nullptr) {
+                                      const Twine &Name,
+                                      Instruction *InsertBefore) {
     return CreateWithCopiedFlags(Instruction::FNeg, Op, FMFSource, Name,
                                  InsertBefore);
   }
 
+  static UnaryOperator *CreateFNegFMF(Value *Op, Instruction *FMFSource,
+                                      const Twine &Name = "",
+                                      BasicBlock *InsertAtEnd = nullptr) {
+    return CreateWithCopiedFlags(Instruction::FNeg, Op, FMFSource, Name,
+                                 InsertAtEnd);
+  }
+
   UnaryOps getOpcode() const {
     return static_cast<UnaryOps>(Instruction::getOpcode());
   }
@@ -256,15 +272,15 @@ class BinaryOperator : public Instruction {
   /// 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);
+                                const Twine &Name,
+                                Instruction *InsertBefore);
 
   /// 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
@@ -304,13 +320,22 @@ class BinaryOperator : public Instruction {
 
   static BinaryOperator *
   CreateWithCopiedFlags(BinaryOps Opc, Value *V1, Value *V2, Value *CopyO,
-                        const Twine &Name = "",
-                        Instruction *InsertBefore = nullptr) {
+                        const Twine &Name,
+                        Instruction *InsertBefore) {
     BinaryOperator *BO = Create(Opc, V1, V2, Name, InsertBefore);
     BO->copyIRFlags(CopyO);
     return BO;
   }
 
+  static BinaryOperator *
+  CreateWithCopiedFlags(BinaryOps Opc, Value *V1, Value *V2, Value *CopyO,
+                        const Twine &Name = "",
+                        BasicBlock *InsertAtEnd = nullptr) {
+    BinaryOperator *BO = Create(Opc, V1, V2, Name, InsertAtEnd);
+    BO->copyIRFlags(CopyO);
+    return BO;
+  }
+
   static BinaryOperator *CreateFAddFMF(Value *V1, Value *V2,
                                        Instruction *FMFSource,
                                        const Twine &Name = "") {
@@ -472,22 +497,22 @@ class BinaryOperator : public Instruction {
                                    BasicBlock *InsertAtEnd = nullptr);
   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);
+                                      Instruction *InsertBefore);
+  static BinaryOperator *CreateNSWNeg(Value *Op, const Twine &Name = "",
+                                      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);
+                                      Instruction *InsertBefore);
+  static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name = "",
+                                      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);
+                                   Instruction *InsertBefore);
+  static BinaryOperator *CreateNot(Value *Op, const Twine &Name = "",
+                                   BasicBlock *InsertAtEnd = nullptr);
 
   BinaryOps getOpcode() const {
     return static_cast<BinaryOps>(Instruction::getOpcode());
@@ -587,13 +612,13 @@ class CastInst : public UnaryInstruction {
   }
   /// Constructor with insert-before-instruction semantics for subclasses
   CastInst(Type *Ty, unsigned iType, Value *S,
-           const Twine &NameStr = "", Instruction *InsertBefore = nullptr)
+           const Twine &NameStr, Instruction *InsertBefore)
     : 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)
+           const Twine &NameStr = "", BasicBlock *InsertAtEnd = nullptr)
     : UnaryInstruction(Ty, iType, S, InsertAtEnd) {
     setName(NameStr);
   }
@@ -622,8 +647,8 @@ class CastInst : public UnaryInstruction {
     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
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< 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
@@ -635,8 +660,8 @@ class CastInst : public UnaryInstruction {
     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
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Create a ZExt or BitCast cast instruction
@@ -651,16 +676,16 @@ class CastInst : public UnaryInstruction {
   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
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< 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
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Create a SExt or BitCast cast instruction
@@ -675,24 +700,24 @@ class CastInst : public UnaryInstruction {
   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
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< 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
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The 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
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Create a BitCast, AddrSpaceCast or a PtrToInt cast instruction.
@@ -707,16 +732,16 @@ class CastInst : public UnaryInstruction {
   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
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< 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
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Create a BitCast or an AddrSpaceCast cast instruction.
@@ -731,8 +756,8 @@ class CastInst : public UnaryInstruction {
   static CastInst *CreatePointerBitCastOrAddrSpaceCast(
     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
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< Place to insert the instruction
   );
 
   /// Create a BitCast, a PtrToInt, or an IntToPTr cast instruction.
@@ -748,6 +773,19 @@ class CastInst : public UnaryInstruction {
     BasicBlock::iterator InsertBefore ///< Place to insert the instruction
   );
 
+  /// Create a BitCast, a PtrToInt, or an IntToPTr cast instruction.
+  ///
+  /// If the value is a pointer type and the destination an integer type,
+  /// creates a PtrToInt cast. If the value is an integer type and the
+  /// destination a pointer type, creates an IntToPtr cast. Otherwise, creates
+  /// a bitcast.
+  static CastInst *CreateBitOrPointerCast(
+    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 ///< Place to insert the instruction
+  );
+
   /// Create a BitCast, a PtrToInt, or an IntToPTr cast instruction.
   ///
   /// If the value is a pointer type and the destination an integer type,
@@ -758,7 +796,7 @@ class CastInst : public UnaryInstruction {
     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
+    BasicBlock *InsertAtEnd = nullptr ///< Place to insert the instruction
   );
 
   /// Create a ZExt, BitCast, or Trunc for int -> int casts.
@@ -775,8 +813,8 @@ class CastInst : public UnaryInstruction {
     Value *S,                ///< The pointer value to be casted (operand 0)
     Type *Ty,          ///< The type to which cast should be made
     bool isSigned,           ///< Whether to regard S as signed or not
-    const Twine &Name = "", ///< Name for the instruction
-    Instruction *InsertBefore = nullptr ///< Place to insert the instruction
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< Place to insert the instruction
   );
 
   /// Create a ZExt, BitCast, or Trunc for int -> int casts.
@@ -784,8 +822,8 @@ class CastInst : public UnaryInstruction {
     Value *S,                ///< The integer value to be casted (operand 0)
     Type *Ty,          ///< The integer type to which operand is casted
     bool isSigned,           ///< Whether to regard S as signed or not
-    const Twine &Name, ///< The name for the instruction
-    BasicBlock *InsertAtEnd  ///< The block to insert the instruction into
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Create an FPExt, BitCast, or FPTrunc for fp -> fp casts
@@ -800,16 +838,16 @@ class CastInst : public UnaryInstruction {
   static CastInst *CreateFPCast(
     Value *S,                ///< The floating point value to be casted
     Type *Ty,          ///< The floating point type to cast to
-    const Twine &Name = "", ///< Name for the instruction
-    Instruction *InsertBefore = nullptr ///< Place to insert the instruction
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< Place to insert the instruction
   );
 
   /// Create an FPExt, BitCast, or FPTrunc for fp -> fp casts
   static CastInst *CreateFPCast(
     Value *S,                ///< The floating point value to be casted
     Type *Ty,          ///< The floating point type to cast to
-    const Twine &Name, ///< The name for the instruction
-    BasicBlock *InsertAtEnd  ///< The block to insert the instruction into
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Create a Trunc or BitCast cast instruction
@@ -824,16 +862,16 @@ class CastInst : public UnaryInstruction {
   static CastInst *CreateTruncOrBitCast(
     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
+    const Twine &Name, ///< Name for the instruction
+    Instruction *InsertBefore ///< Place to insert the instruction
   );
 
   /// Create a Trunc or BitCast cast instruction
   static CastInst *CreateTruncOrBitCast(
     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
+    const Twine &Name = "", ///< The name for the instruction
+    BasicBlock *InsertAtEnd = nullptr  ///< The block to insert the instruction into
   );
 
   /// Check whether a bitcast between these types is valid
@@ -1021,13 +1059,13 @@ class CmpInst : public Instruction {
           Instruction *FlagsSource = nullptr);
 
   CmpInst(Type *ty, Instruction::OtherOps op, Predicate pred,
-          Value *LHS, Value *RHS, const Twine &Name = "",
-          Instruction *InsertBefore = nullptr,
+          Value *LHS, Value *RHS, const Twine &Name,
+          Instruction *InsertBefore,
           Instruction *FlagsSource = nullptr);
 
   CmpInst(Type *ty, Instruction::OtherOps op, Predicate pred,
-          Value *LHS, Value *RHS, const Twine &Name,
-          BasicBlock *InsertAtEnd);
+          Value *LHS, Value *RHS, const Twine &Name = "",
+          BasicBlock *InsertAtEnd = nullptr);
 
 public:
   // allocate space for exactly two operands
@@ -1048,15 +1086,15 @@ class CmpInst : public Instruction {
   /// Create a CmpInst
   static CmpInst *Create(OtherOps Op,
                          Predicate predicate, Value *S1,
-                         Value *S2, const Twine &Name = "",
-                         Instruction *InsertBefore = nullptr);
+                         Value *S2, const Twine &Name,
+                         Instruct...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/86132


More information about the llvm-commits mailing list