[Mlir-commits] [mlir] [mlir][LLVMIR] Fix loop follow-up metadata import and export (PR #223376)

[Tobias Gysi]

================
@@ -253,13 +247,17 @@ LoopMetadataConversion::lookupMDNodes(StringRef name) {
 
 FailureOr<LoopAnnotationAttr>
 LoopMetadataConversion::lookupFollowupNode(StringRef name) {
-  auto node = lookupMDNode(name);
-  if (failed(node))
-    return failure();
-  if (*node == nullptr)
+  const llvm::MDNode *followup = lookupAndEraseProperty(name);
+  if (!followup)
     return LoopAnnotationAttr(nullptr);
 
-  return loopAnnotationImporter.translateLoopAnnotation(*node, loc);
+  LoopAnnotationAttr attr =
+      LoopMetadataConversion(followup, loc, loopAnnotationImporter, activeNodes,
+                             InputKind::Followup)
+          .convert();
----------------
gysit wrote:

nit:

Did you consider having two separate conversion signatures on LoopMetadataConversion:
1. convert
2. convertFollowup

That way the input kind specific logic including allocating the active nodes could be in the corresponding function while the shared parts could be moved into some convertImpl function.

I didn't think it through but such a change may encapsulate things a bit better? 


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