address more reviews

Author: bcardosolopes

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

@@ -2470,13 +2470,16 @@ void AliasOp::print(OpAsmPrinter &p) {
   StringRef visibility = stringifyVisibility(getVisibility_());
   if (!visibility.empty())
     p << visibility << ' ';
-  if (getThreadLocal_())
-    p << "thread_local ";
+
   if (std::optional<mlir::LLVM::UnnamedAddr> unnamedAddr = getUnnamedAddr()) {
     StringRef str = stringifyUnnamedAddr(*unnamedAddr);
     if (!str.empty())
       p << str << ' ';
   }
+
+  if (getThreadLocal_())
+    p << "thread_local ";
+
   p.printSymbolName(getSymName());
   p.printOptionalAttrDict((*this)->getAttrs(),
                           {SymbolTable::getSymbolAttrName(),
@@ -2555,7 +2558,7 @@ LogicalResult AliasOp::verify() {
 
 LogicalResult AliasOp::verifyRegions() {
   Block &b = getInitializerBlock();
-  ReturnOp ret = cast<ReturnOp>(b.getTerminator());
+  auto ret = cast<ReturnOp>(b.getTerminator());
   if (ret.getNumOperands() == 0 ||
       !isa<LLVM::LLVMPointerType>(ret.getOperand(0).getType()))
     return emitOpError("initializer region must always return a pointer");

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -987,9 +987,8 @@ LogicalResult ModuleImport::convertAlias(llvm::GlobalAlias *alias) {
     return failure();
   builder.create<ReturnOp>(aliasOp.getLoc(), *initializer);
 
-  if (alias->hasAtLeastLocalUnnamedAddr()) {
+  if (alias->hasAtLeastLocalUnnamedAddr())
     aliasOp.setUnnamedAddr(convertUnnamedAddrFromLLVM(alias->getUnnamedAddr()));
-  }
   aliasOp.setVisibility_(convertVisibilityFromLLVM(alias->getVisibility()));
 
   return success();

mlir/test/Dialect/LLVMIR/alias.mlir

@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -split-input-file --verify-roundtrip | FileCheck %s
 
 llvm.func internal @callee() -> !llvm.ptr attributes {dso_local} {
   %0 = llvm.mlir.zero : !llvm.ptr
@@ -125,3 +125,17 @@ llvm.mlir.global internal constant @g3() {addr_space = 0 : i32, dso_local} : !ll
 // CHECK:   %[[ADDR:.*]] = llvm.mlir.addressof @a2 : !llvm.ptr
 // CHECK:   llvm.return %[[ADDR]] : !llvm.ptr
 // CHECK: }
+
+// -----
+
+llvm.mlir.global private @g30(0 : i32) {dso_local} : i32
+
+llvm.mlir.alias private unnamed_addr thread_local @a30 {dso_local} : i32 {
+  %0 = llvm.mlir.addressof @g30 : !llvm.ptr
+  llvm.return %0 : !llvm.ptr
+}
+
+// CHECK: llvm.mlir.alias private unnamed_addr thread_local @a30 {dso_local} : i32 {
+// CHECK:   %0 = llvm.mlir.addressof @g30 : !llvm.ptr
+// CHECK:   llvm.return %0 : !llvm.ptr
+// CHECK: }