[MLIR][LLVM] Fix llvm.mlir.global mismatching print and parser order (#138986)
`GlobalOp` was parsing `thread_local` after `unnamed_addr`, but printing in the reverse order.
While here, make `AliasOp` match the same behavior and share common parts of global and alias printing.
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index e17b9fd..fc74e3d 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2215,18 +2215,23 @@
result.addRegion();
}
-void GlobalOp::print(OpAsmPrinter &p) {
- p << ' ' << stringifyLinkage(getLinkage()) << ' ';
- StringRef visibility = stringifyVisibility(getVisibility_());
+template <typename OpType>
+static void printCommonGlobalAndAlias(OpAsmPrinter &p, OpType op) {
+ p << ' ' << stringifyLinkage(op.getLinkage()) << ' ';
+ StringRef visibility = stringifyVisibility(op.getVisibility_());
if (!visibility.empty())
p << visibility << ' ';
- if (getThreadLocal_())
+ if (op.getThreadLocal_())
p << "thread_local ";
- if (auto unnamedAddr = getUnnamedAddr()) {
+ if (auto unnamedAddr = op.getUnnamedAddr()) {
StringRef str = stringifyUnnamedAddr(*unnamedAddr);
if (!str.empty())
p << str << ' ';
}
+}
+
+void GlobalOp::print(OpAsmPrinter &p) {
+ printCommonGlobalAndAlias<GlobalOp>(p, *this);
if (getConstant())
p << "constant ";
p.printSymbolName(getSymName());
@@ -2309,16 +2314,16 @@
parseOptionalLLVMKeyword<LLVM::Visibility, int64_t>(
parser, result, LLVM::Visibility::Default)));
+ if (succeeded(parser.parseOptionalKeyword("thread_local")))
+ result.addAttribute(OpType::getThreadLocal_AttrName(result.name),
+ parser.getBuilder().getUnitAttr());
+
// Parse optional UnnamedAddr, default to None.
result.addAttribute(OpType::getUnnamedAddrAttrName(result.name),
parser.getBuilder().getI64IntegerAttr(
parseOptionalLLVMKeyword<UnnamedAddr, int64_t>(
parser, result, LLVM::UnnamedAddr::None)));
- if (succeeded(parser.parseOptionalKeyword("thread_local")))
- result.addAttribute(OpType::getThreadLocal_AttrName(result.name),
- parser.getBuilder().getUnitAttr());
-
return success();
}
@@ -2581,19 +2586,7 @@
}
void AliasOp::print(OpAsmPrinter &p) {
- p << ' ' << stringifyLinkage(getLinkage()) << ' ';
- StringRef visibility = stringifyVisibility(getVisibility_());
- if (!visibility.empty())
- p << visibility << ' ';
-
- if (std::optional<mlir::LLVM::UnnamedAddr> unnamedAddr = getUnnamedAddr()) {
- StringRef str = stringifyUnnamedAddr(*unnamedAddr);
- if (!str.empty())
- p << str << ' ';
- }
-
- if (getThreadLocal_())
- p << "thread_local ";
+ printCommonGlobalAndAlias<AliasOp>(p, *this);
p.printSymbolName(getSymName());
p.printOptionalAttrDict((*this)->getAttrs(),
diff --git a/mlir/test/Dialect/LLVMIR/alias.mlir b/mlir/test/Dialect/LLVMIR/alias.mlir
index efba248..7ce54f3 100644
--- a/mlir/test/Dialect/LLVMIR/alias.mlir
+++ b/mlir/test/Dialect/LLVMIR/alias.mlir
@@ -130,12 +130,12 @@
llvm.mlir.global private @g30(0 : i32) {dso_local} : i32
-llvm.mlir.alias private unnamed_addr thread_local @a30 {dso_local} : i32 {
+llvm.mlir.alias private thread_local unnamed_addr @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: llvm.mlir.alias private thread_local unnamed_addr @a30 {dso_local} : i32 {
// CHECK: %0 = llvm.mlir.addressof @g30 : !llvm.ptr
// CHECK: llvm.return %0 : !llvm.ptr
// CHECK: }
diff --git a/mlir/test/Dialect/LLVMIR/roundtrip.mlir b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
index 1fd24f3..2e6acc1 100644
--- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir
+++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
@@ -1042,3 +1042,6 @@
// CHECK-LABEL: llvm.func @callintrin_voidret
// CHECK-NEXT: llvm.call_intrinsic "llvm.aarch64.neon.st3.v8i8.p0"(%arg0, %arg1, %arg2, %arg3) : (vector<8xi8>, vector<8xi8>, vector<8xi8>, !llvm.ptr) -> ()
+
+llvm.mlir.global internal thread_local unnamed_addr @myglobal(-1 : i32) {addr_space = 0 : i32, alignment = 4 : i64, dso_local} : i32
+// CHECK: llvm.mlir.global internal thread_local unnamed_addr @myglobal(-1 : i32) {addr_space = 0 : i32, alignment = 4 : i64, dso_local} : i32
