[mlir] Use SymbolUserOpInterface in LLVM::AddressOfOp verifier

[Mogball]

Thanks!

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

1747

Somewhat unrelated: these helpers should probably be removed because they're an easy performance trap.

[ezhulenev]

rebase

[mehdi_amini]

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

1747

+1

Alternatively I would take a SymbolTable as argument instead.

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

 def UnnamedAddr : LLVM_EnumAttr<
     "UnnamedAddr",
     "::llvm::GlobalValue::UnnamedAddr",
     "LLVM GlobalValue UnnamedAddr",
     [UnnamedAddrNone, UnnamedAddrLocal, UnnamedAddrGlobal]> {
   let cppNamespace = "::mlir::LLVM";
 }
 
-def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof", [NoSideEffect]> {
+def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof",
+    [NoSideEffect, DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
   let arguments = (ins FlatSymbolRefAttr:$global_name);
   let results = (outs LLVM_AnyPointer:$res);
 
   let summary = "Creates a pointer pointing to a global or a function";
 
   let description = [{
     Creates an SSA value containing a pointer to a global variable or constant
     defined by `llvm.mlir.global`. The global value can be defined after its
let extraClassDeclaration = [{
     /// here.
     GlobalOp getGlobal();
 
     /// Return the llvm.func operation that is referenced here.
     LLVMFuncOp getFunction();
   }];
 
   let assemblyFormat = "$global_name attr-dict `:` type($res)";
-  let hasVerifier = 1;
 }
 
 def LLVM_MetadataOp : LLVM_Op<"metadata", [
    NoRegionArguments, SymbolTable, Symbol
 ]> {
   let arguments = (ins
     SymbolNameAttr:$sym_name
   );

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

LogicalResult ResumeOp::verify() {
   // No check for personality of function - landingpad op verifies it.
   return success();
 }
 
 //===----------------------------------------------------------------------===//
 // Verifier for LLVM::AddressOfOp.
 //===----------------------------------------------------------------------===//
 
-static Operation *lookupSymbolInModule(Operation *parent, StringRef name) {
-  Operation *module = parent;
+static Operation *parentLLVMModule(Operation *op) {
+  Operation *module = op->getParentOp();
   while (module && !satisfiesLLVMModule(module))
     module = module->getParentOp();
   assert(module && "unexpected operation outside of a module");
-  return mlir::SymbolTable::lookupSymbolIn(module, name);
+  return module;
 }
 
 GlobalOp AddressOfOp::getGlobal() {
   return dyn_cast_or_null<GlobalOp>(
-      lookupSymbolInModule((*this)->getParentOp(), getGlobalName()));
+      SymbolTable::lookupSymbolIn(parentLLVMModule(*this), getGlobalName()));
 }
 
 LLVMFuncOp AddressOfOp::getFunction() {
   return dyn_cast_or_null<LLVMFuncOp>(
-      lookupSymbolInModule((*this)->getParentOp(), getGlobalName()));
+      SymbolTable::lookupSymbolIn(parentLLVMModule(*this), getGlobalName()));
 }
 
-LogicalResult AddressOfOp::verify() {
+LogicalResult
+AddressOfOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   Operation *symbol =
-      lookupSymbolInModule((*this)->getParentOp(), getGlobalName());
+      symbolTable.lookupSymbolIn(parentLLVMModule(*this), getGlobalNameAttr());
 
   auto global = dyn_cast_or_null<GlobalOp>(symbol);
   auto function = dyn_cast_or_null<LLVMFuncOp>(symbol);
 
   if (!global && !function)
     return emitOpError(
         "must reference a global defined by 'llvm.mlir.global' or 'llvm.func'");
 

mlir/test/Dialect/LLVMIR/global.mlir

 
 // -----
 
 func.func @foo() {
   // The attribute parser will consume the first colon-type, so we put two of
   // them to trigger the attribute type mismatch error.
   // expected-error @+1 {{invalid kind of attribute specified}}
   llvm.mlir.addressof "foo" : i64 : !llvm.ptr<func<void ()>>
+  llvm.return
 }
 
 // -----
 
 func.func @foo() {
   // expected-error @+1 {{must reference a global defined by 'llvm.mlir.global'}}
   llvm.mlir.addressof @foo : !llvm.ptr<func<void ()>>
+  llvm.return
 }
 
 // -----
 
 llvm.mlir.global internal @foo(0: i32) : i32
 
 func.func @bar() {
   // expected-error @+1 {{the type must be a pointer to the type of the referenced global}}
   llvm.mlir.addressof @foo : !llvm.ptr<i64>
+  llvm.return
 }
 
 // -----
 
 llvm.func @foo()
 
 llvm.func @bar() {
   // expected-error @+1 {{the type must be a pointer to the type of the referenced function}}
   llvm.mlir.addressof @foo : !llvm.ptr<i8>
+  llvm.return
 }
 
 // -----
 
 // expected-error @+2 {{block with no terminator}}
 llvm.mlir.global internal @g() : i64 {
   %c = llvm.mlir.constant(42 : i64) : i64
 }

 }
 
 // -----
 
 llvm.mlir.global internal @g(32 : i64) {addr_space = 3: i32} : i64
 func.func @mismatch_addr_space_implicit_global() {
   // expected-error @+1 {{pointer address space must match address space of the referenced global}}
   llvm.mlir.addressof @g : !llvm.ptr<i64>
+  llvm.return
 }
 
 // -----
 
 llvm.mlir.global internal @g(32 : i64) {addr_space = 3: i32} : i64
 func.func @mismatch_addr_space() {
   // expected-error @+1 {{pointer address space must match address space of the referenced global}}
   llvm.mlir.addressof @g : !llvm.ptr<i64, 4>
+  llvm.return
 }
 // -----
 
 llvm.mlir.global internal @g(32 : i64) {addr_space = 3: i32} : i64
 
 func.func @mismatch_addr_space_opaque() {
   // expected-error @+1 {{pointer address space must match address space of the referenced global}}
   llvm.mlir.addressof @g : !llvm.ptr<4>
+  llvm.return
 }
 
 // -----
 
 llvm.func @ctor() {
   llvm.return
 }