Skip to content

[NFC][DebugInfo] Mop up final instruction-insertion call sites #124289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Jan 24, 2025

These are the final places in the monorepo that make use of instruction insertion for methods like insertBefore and moveBefore. As part of the RemoveDIs project, instead use iterators for insertion. (see: https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939 ).

These are the final places in the monorepo that make use of instruction
insertion for methods like insertBefore and moveBefore. As part of the
RemoveDIs project, instead use iterators for insertion.
@llvmbot
Copy link
Member

llvmbot commented Jan 24, 2025

@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-flang-openmp

Author: Jeremy Morse (jmorse)

Changes

These are the final places in the monorepo that make use of instruction insertion for methods like insertBefore and moveBefore. As part of the RemoveDIs project, instead use iterators for insertion. (see: https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939 ).


Full diff: https://github.com/llvm/llvm-project/pull/124289.diff

3 Files Affected:

  • (modified) llvm/lib/SandboxIR/Instruction.cpp (+1-1)
  • (modified) llvm/lib/SandboxIR/Tracker.cpp (+2-2)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+1-1)
diff --git a/llvm/lib/SandboxIR/Instruction.cpp b/llvm/lib/SandboxIR/Instruction.cpp
index cc961418600e3f..956047cf87b6bb 100644
--- a/llvm/lib/SandboxIR/Instruction.cpp
+++ b/llvm/lib/SandboxIR/Instruction.cpp
@@ -129,7 +129,7 @@ void Instruction::insertBefore(Instruction *BeforeI) {
 
   // Insert the LLVM IR Instructions in program order.
   for (llvm::Instruction *I : getLLVMInstrs())
-    I->insertBefore(BeforeTopI);
+    I->insertBefore(BeforeTopI->getIterator());
 }
 
 void Instruction::insertAfter(Instruction *AfterI) {
diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp
index 27ed37aa9bdd37..5fa9f181055ca2 100644
--- a/llvm/lib/SandboxIR/Tracker.cpp
+++ b/llvm/lib/SandboxIR/Tracker.cpp
@@ -175,7 +175,7 @@ void EraseFromParent::revert(Tracker &Tracker) {
   // Place the bottom-most instruction first.
   auto [Operands, BotLLVMI] = InstrData[0];
   if (auto *NextLLVMI = dyn_cast<llvm::Instruction *>(NextLLVMIOrBB)) {
-    BotLLVMI->insertBefore(NextLLVMI);
+    BotLLVMI->insertBefore(NextLLVMI->getIterator());
   } else {
     auto *LLVMBB = cast<llvm::BasicBlock *>(NextLLVMIOrBB);
     BotLLVMI->insertInto(LLVMBB, LLVMBB->end());
@@ -185,7 +185,7 @@ void EraseFromParent::revert(Tracker &Tracker) {
 
   // Go over the rest of the instructions and stack them on top.
   for (auto [Operands, LLVMI] : drop_begin(InstrData)) {
-    LLVMI->insertBefore(BotLLVMI);
+    LLVMI->insertBefore(BotLLVMI->getIterator());
     for (auto [OpNum, Op] : enumerate(Operands))
       LLVMI->setOperand(OpNum, Op);
     BotLLVMI = LLVMI;
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 3fcdefa8a2f673..eb873fd1b7f6fe 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -3828,7 +3828,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
           if (insn->getFunction() == func) {
             auto *load = builder.CreateLoad(mapData.BasePointers[i]->getType(),
                                             mapData.BasePointers[i]);
-            load->moveBefore(insn);
+            load->moveBefore(insn->getIterator());
             user->replaceUsesOfWith(mapData.OriginalValue[i], load);
           }
         }

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jmorse jmorse merged commit 749443a into llvm:main Jan 27, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants