[mlir][target][LLVMIR] Change translation order to translate non-LLVM ops before function bodies.

[fmorac]

Summary

Convert function bodies after all other operations, breaking possible declaration-reference issues between top non-LLVM Ops and non-LLVM ops inside function bodies.

Example:

mydialect.global @myglobal : i32
llvm.func @bar(...) {
...
%address = mydialect.global_address @myglobal : llvm.ptr
...
}

With the previous scheme mydialect.global_address always got translated before mydialect.global, this change ensures mydialect.global gets translated first.

[mehdi_amini]

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

1406

Typo: function

1407

I don't understand the explanation in the comment.

I would think that the issue with translation is that we need to have declaration before we introduce references to these, but I can be mistaken about the issue at play here?

[mehdi_amini]

That LGTM when the comment will be fixed :)

[fmorac]

Changed the description in the comment.

[fmorac]

Updated the comment.

[mehdi_amini]

LG!

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

mlir::translateModuleToLLVMIR(Operation *module, llvm::LLVMContext &llvmContext,
   if (failed(translator.convertFunctionSignatures()))
     return nullptr;
   if (failed(translator.convertGlobals()))
     return nullptr;
   if (failed(translator.createAliasScopeMetadata()))
     return nullptr;
   if (failed(translator.createTBAAMetadata()))
     return nullptr;
-  if (failed(translator.convertFunctions()))
-    return nullptr;
 
   // Convert other top-level operations if possible.
   llvm::IRBuilder<> llvmBuilder(llvmContext);
   for (Operation &o : getModuleBody(module).getOperations()) {
     if (!isa<LLVM::LLVMFuncOp, LLVM::GlobalOp, LLVM::GlobalCtorsOp,
              LLVM::GlobalDtorsOp, LLVM::ComdatOp>(&o) &&
         !o.hasTrait<OpTrait::IsTerminator>() &&
         failed(translator.convertOperation(o, llvmBuilder))) {
       return nullptr;
     }
   }
 
+  // Operations in function bodies with symbolic references must be converted
+  // after the top-level operations they refer to are declared, so we do it
+  // last.
+  if (failed(translator.convertFunctions()))
+    return nullptr;
+
   // Convert module itself.
   if (failed(translator.convertOperation(*module, llvmBuilder)))
     return nullptr;
 
   if (llvm::verifyModule(*translator.llvmModule, &llvm::errs()))
     return nullptr;
 
   return std::move(translator.llvmModule);
 }