From d9d0163b55f5687dc613d029a410728ee4d7ba5d Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Thu, 8 May 2025 11:17:18 -0700 Subject: [PATCH] [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. --- mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 37 +++++++++------------- mlir/test/Dialect/LLVMIR/alias.mlir | 4 +-- mlir/test/Dialect/LLVMIR/roundtrip.mlir | 33 +++++++++++++++++++ 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index 22cb5f4cd88a4..336023fab9ea9 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -2196,18 +2196,23 @@ void GlobalOp::build(OpBuilder &builder, OperationState &result, Type type, result.addRegion(); } -void GlobalOp::print(OpAsmPrinter &p) { - p << ' ' << stringifyLinkage(getLinkage()) << ' '; - StringRef visibility = stringifyVisibility(getVisibility_()); +template +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(p, *this); if (getConstant()) p << "constant "; p.printSymbolName(getSymName()); @@ -2271,16 +2276,16 @@ static ParseResult parseCommonGlobalAndAlias(OpAsmParser &parser, parseOptionalLLVMKeyword( 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( parser, result, LLVM::UnnamedAddr::None))); - if (succeeded(parser.parseOptionalKeyword("thread_local"))) - result.addAttribute(OpType::getThreadLocal_AttrName(result.name), - parser.getBuilder().getUnitAttr()); - return success(); } @@ -2543,19 +2548,7 @@ void AliasOp::build(OpBuilder &builder, OperationState &result, Type type, } void AliasOp::print(OpAsmPrinter &p) { - p << ' ' << stringifyLinkage(getLinkage()) << ' '; - StringRef visibility = stringifyVisibility(getVisibility_()); - if (!visibility.empty()) - p << visibility << ' '; - - if (std::optional unnamedAddr = getUnnamedAddr()) { - StringRef str = stringifyUnnamedAddr(*unnamedAddr); - if (!str.empty()) - p << str << ' '; - } - - if (getThreadLocal_()) - p << "thread_local "; + printCommonGlobalAndAlias(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 efba248534862..7ce54f35a5557 100644 --- a/mlir/test/Dialect/LLVMIR/alias.mlir +++ b/mlir/test/Dialect/LLVMIR/alias.mlir @@ -130,12 +130,12 @@ llvm.mlir.global internal constant @g3() : !llvm.ptr { 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 d0aa65d14a176..cf9b3acbde2d3 100644 --- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir +++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir @@ -1002,3 +1002,36 @@ llvm.func @intrinsic_call_arg_attrs_bundles(%arg0: i32) -> i32 { %0 = llvm.call_intrinsic "llvm.riscv.sha256sig0"(%arg0) ["adazdazd"()] {constant} : (i32 {llvm.signext}) -> (i32) llvm.return %0 : i32 } + +llvm.mlir.global private @blockaddr_global() {addr_space = 0 : i32, dso_local} : !llvm.ptr { + %0 = llvm.blockaddress > : !llvm.ptr + llvm.return %0 : !llvm.ptr +} + +// CHECK: llvm.mlir.global private @blockaddr_global() {{.*}} +// CHECK-NEXT: %{{.*}} = llvm.blockaddress > : !llvm.ptr +// CHECK-NEXT: llvm.return %{{.*}} : !llvm.ptr + +llvm.func @blockaddr_fn() { + llvm.br ^bb1 +^bb1: + llvm.blocktag + llvm.return +} + +// CHECK-LABEL: llvm.func @blockaddr_fn +// CHECK-NEXT: llvm.br ^bb1 +// CHECK-NEXT:^bb1: +// CHECK-NEXT: llvm.blocktag + +llvm.func @callintrin_voidret(%arg0: vector<8xi8>, %arg1: vector<8xi8>, %arg2: vector<8xi8>, %arg3: !llvm.ptr) { + llvm.call_intrinsic "llvm.aarch64.neon.st3.v8i8.p0"(%arg0, %arg1, %arg2, %arg3) : (vector<8xi8>, vector<8xi8>, vector<8xi8>, !llvm.ptr) -> () + llvm.return +} +llvm.func @llvm.aarch64.neon.st3.v8i8.p0(vector<8xi8>, vector<8xi8>, vector<8xi8>, !llvm.ptr) + +// 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