Address last round of comments

Author: bcardosolopes

mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h

@@ -148,8 +148,8 @@ class ModuleTranslation {
   /// Maps a blockaddress operation to its corresponding placeholder LLVM
   /// value.
   void mapBlockTag(BlockAddressAttr attr, BlockTagOp blockTag) {
-    // Attempts to map already mapped block labels are fine given labels are
-    // verified to be unique.
+    // Attempts to map already mapped block labels which is fine if the given
+    // labels are verified to be unique.
     blockTagMapping[attr] = blockTag;
   }
 

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

@@ -3862,8 +3862,10 @@ LLVMFuncOp BlockAddressOp::getFunction(SymbolTableCollection &symbolTable) {
 }
 
 BlockTagOp BlockAddressOp::getBlockTagOp() {
-  auto funcOp = cast<LLVMFuncOp>(mlir::SymbolTable::lookupNearestSymbolFrom(
+  auto funcOp = dyn_cast<LLVMFuncOp>(mlir::SymbolTable::lookupNearestSymbolFrom(
       parentLLVMModule(*this), getBlockAddr().getFunction()));
+  if (!funcOp)
+    return nullptr;
 
   BlockTagOp blockTagOp = nullptr;
   funcOp.walk([&](LLVM::BlockTagOp labelOp) {

mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp

@@ -726,8 +726,9 @@ struct LLVMInlinerInterface : public DialectInlinerInterface {
   }
 
   bool isLegalToInline(Operation *op, Region *, bool, IRMapping &) const final {
-    // The inliner cannot handle variadic function arguments nor blocktag
-    // operations.
+    // The inliner cannot handle variadic function arguments and blocktag
+    // operations prevent inlining since they the blockaddress operations
+    // reference them via the callee symbol.
     return !(isa<LLVM::VaStartOp>(op) || isa<LLVM::BlockTagOp>(op));
   }
 

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -2440,9 +2440,9 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) {
     // Skip unreachable blocks.
     if (!reachable.contains(&basicBlock)) {
       if (basicBlock.hasAddressTaken())
-        emitError(funcOp.getLoc())
-            << "unreachable block '" << basicBlock.getName()
-            << "' with address taken";
+        return emitError(funcOp.getLoc())
+               << "unreachable block '" << basicBlock.getName()
+               << "' with address taken";
       continue;
     }
     Region &body = funcOp.getBody();

mlir/test/Dialect/LLVMIR/blockaddress-canonicalize.mlir

@@ -1,12 +1,21 @@
 // RUN: mlir-opt %s -pass-pipeline='builtin.module(llvm.func(canonicalize{region-simplify=aggressive}))' -split-input-file | FileCheck %s
 
+llvm.mlir.global private @x() {addr_space = 0 : i32, dso_local} : !llvm.ptr {
+  %0 = llvm.blockaddress <function = @ba, tag = <id = 2>> : !llvm.ptr
+  llvm.return %0 : !llvm.ptr
+}
+
 // CHECK-LABEL: llvm.func @ba()
 llvm.func @ba() -> !llvm.ptr {
   %0 = llvm.blockaddress <function = @ba, tag = <id = 1>> : !llvm.ptr
   llvm.br ^bb1
 ^bb1:
   // CHECK: llvm.blocktag <id = 1>
   llvm.blocktag <id = 1>
+  llvm.br ^bb2
+^bb2:
+  // CHECK: llvm.blocktag <id = 2>
+  llvm.blocktag <id = 2>
   llvm.return %0 : !llvm.ptr
 }
 

mlir/test/Dialect/LLVMIR/inlining.mlir

@@ -700,11 +700,12 @@ func.func @func(%arg0 : i32) -> i32  {
   llvm.blocktag <id = 1>
   llvm.return %arg0 : i32
 }
+
 // CHECK-LABEL: @llvm_ret
-// CHECK-NOT: llvm.blocktag
-// CHECK: %[[R:.*]] = call
-// CHECK: return %[[R]]
 func.func @llvm_ret(%arg0 : i32) -> i32 {
+  // CHECK-NOT: llvm.blocktag
+  // CHECK: %[[R:.*]] = call
   %res = call @func(%arg0) : (i32) -> (i32)
+  // CHECK: return %[[R]]
   return %res : i32
 }

mlir/test/Target/LLVMIR/blockaddress.mlir

@@ -14,8 +14,8 @@ llvm.func @fn() {
 
 // CHECK: @g = private global ptr blockaddress(@fn, %1)
 // CHECK: define void @fn() {
-// CHECK:   br label %1
-// CHECK: 1:
+// CHECK:   br label %[[RET:.*]]
+// CHECK: [[RET]]:
 // CHECK:   ret void
 // CHECK: }
 
@@ -30,7 +30,7 @@ llvm.func @blockaddr0() -> !llvm.ptr {
 }
 
 // CHECK: define ptr @blockaddr0() {
-// CHECK:   br label %1
-// CHECK: 1:
+// CHECK:   br label %[[RET:.*]]
+// CHECK: [[RET]]:
 // CHECK:   ret ptr blockaddress(@blockaddr0, %1)
 // CHECK: }