[llvm] [CodeGen][MachineLoop] Fix getLoopID (PR #137820)

Vito Kortbeek via llvm-commits llvm-commits at lists.llvm.org
Fri May 23 02:39:21 PDT 2025


================
@@ -181,48 +181,32 @@ MachineLoopInfo::findLoopPreheader(MachineLoop *L, bool SpeculativePreheader,
 
 MDNode *MachineLoop::getLoopID() const {
   MDNode *LoopID = nullptr;
-  if (const auto *MBB = findLoopControlBlock()) {
-    // If there is a single latch block, then the metadata
-    // node is attached to its terminating instruction.
+
+  // Go through the latch blocks and check the terminator for the metadata
+  SmallVector<MachineBasicBlock *, 4> LatchesBlocks;
+  getLoopLatches(LatchesBlocks);
+  for (const auto *MBB : LatchesBlocks) {
     const auto *BB = MBB->getBasicBlock();
     if (!BB)
       return nullptr;
-    if (const auto *TI = BB->getTerminator())
-      LoopID = TI->getMetadata(LLVMContext::MD_loop);
-  } else if (const auto *MBB = getHeader()) {
-    // There seem to be multiple latch blocks, so we have to
-    // visit all predecessors of the loop header and check
-    // their terminating instructions for the metadata.
-    if (const auto *Header = MBB->getBasicBlock()) {
-      // Walk over all blocks in the loop.
-      for (const auto *MBB : this->blocks()) {
-        const auto *BB = MBB->getBasicBlock();
-        if (!BB)
-          return nullptr;
-        const auto *TI = BB->getTerminator();
-        if (!TI)
-          return nullptr;
-        MDNode *MD = nullptr;
-        // Check if this terminating instruction jumps to the loop header.
-        for (const auto *Succ : successors(TI)) {
-          if (Succ == Header) {
-            // This is a jump to the header - gather the metadata from it.
-            MD = TI->getMetadata(LLVMContext::MD_loop);
-            break;
-          }
-        }
-        if (!MD)
-          continue;
-        if (!LoopID)
-          LoopID = MD;
-        else if (MD != LoopID)
-          return nullptr;
-      }
-    }
+    const auto *TI = BB->getTerminator();
+    if (!TI)
+      return nullptr;
+    MDNode *MD = TI->getMetadata(LLVMContext::MD_loop);
+
+    if (!MD)
+      return nullptr;
+
+    if (!LoopID)
+      LoopID = MD;
+    else if (MD != LoopID)
+      return nullptr;
   }
-  if (LoopID &&
-      (LoopID->getNumOperands() == 0 || LoopID->getOperand(0) != LoopID))
-    LoopID = nullptr;
+
+  if (!LoopID || LoopID->getNumOperands() == 0 ||
----------------
kortbeek-snps wrote:

I was indeed just moving code.

However, I don't find any checks for this in the verifier, but I'm not too familiar with it. If I just experimentally remove the `LoopID` operands and run `opt -passes=verify code-align-loops.ll -disable-output` (or `llc`), I get an assert during the LLVMIR parsing: 
`llvm/lib/IR/DebugInfo.cpp:536: MDNode *stripDebugLocFromLoopID(MDNode *): Assertion !N->operands().empty() && "Missing self reference?"' failed.`

I do agree that it feels a bit out of place to check the validity of the metadata here, but since `LoopInfo.cpp::Loop::getLoopID()` does the same, It might not be too strange?

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


More information about the llvm-commits mailing list