llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Vatsal Khosla (VatsalKhosla) <details> <summary>Changes</summary> Fix InlineAsmOp parser/printer roundtrip for cir.asm and avoid null operand_attrs entries that crash alias printing during --verify-roundtrip. - Parse attr-dict before optional result arrow to match print order. - Use non-null sentinel attributes for non-maybe_memory operands and check UnitAttr explicitly. - Keep lowering semantics by treating only UnitAttr as maybe_memory marker. - Update inline-asm CIR IR test to run with --verify-roundtrip and add an attr+result coverage case. --- Full diff: https://github.com/llvm/llvm-project/pull/186588.diff 4 Files Affected: - (modified) clang/lib/CIR/CodeGen/CIRGenAsm.cpp (+1-1) - (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+5-5) - (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+1-1) - (modified) clang/test/CIR/IR/inline-asm.cir (+15-1) ``````````diff diff --git a/clang/lib/CIR/CodeGen/CIRGenAsm.cpp b/clang/lib/CIR/CodeGen/CIRGenAsm.cpp index c53ac0d6e2312..bf3d0fad2c169 100644 --- a/clang/lib/CIR/CodeGen/CIRGenAsm.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenAsm.cpp @@ -574,7 +574,7 @@ mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &s) { // We need to add an attribute for every arg since later, during // the lowering to LLVM IR the attributes will be assigned to the // CallInsn argument by index, i.e. we can't skip null type here - operandAttrs.push_back(mlir::Attribute()); + operandAttrs.push_back(mlir::DictionaryAttr::get(&getMLIRContext())); } } assert(args.size() == operandAttrs.size() && diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 8d2990af5de8c..5f13f16b6e2ee 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -3607,7 +3607,7 @@ void cir::InlineAsmOp::print(OpAsmPrinter &p) { [&](Value value) { p.printOperand(value); p << " : " << value.getType(); - if (*attrIt) + if (mlir::isa<mlir::UnitAttr>(*attrIt)) p << " (maybe_memory)"; attrIt++; }); @@ -3727,7 +3727,7 @@ ParseResult cir::InlineAsmOp::parse(OpAsmParser &parser, size++; if (parser.parseOptionalLParen().failed()) { - operandAttrs.push_back(mlir::Attribute()); + operandAttrs.push_back(mlir::DictionaryAttr::get(ctxt)); return mlir::success(); } @@ -3770,11 +3770,11 @@ ParseResult cir::InlineAsmOp::parse(OpAsmParser &parser, if (parser.parseOptionalKeyword("side_effects").succeeded()) result.attributes.set("side_effects", UnitAttr::get(ctxt)); - if (parser.parseOptionalArrow().succeeded() && - parser.parseType(resType).failed()) + if (parser.parseOptionalAttrDict(result.attributes).failed()) return mlir::failure(); - if (parser.parseOptionalAttrDict(result.attributes).failed()) + if (parser.parseOptionalArrow().succeeded() && + parser.parseType(resType).failed()) return mlir::failure(); result.attributes.set("asm_flavor", AsmFlavorAttr::get(ctxt, *flavor)); diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index 8bcd040382ab3..3ee16edd1cc3e 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -4663,7 +4663,7 @@ mlir::LogicalResult CIRToLLVMInlineAsmOpLowering::matchAndRewrite( // CIR operand type. for (auto const &[cirOpAttr, cirOp] : zip(op.getOperandAttrs(), cirOperands)) { - if (!cirOpAttr) { + if (!mlir::isa<mlir::UnitAttr>(cirOpAttr)) { opAttrs.push_back(mlir::Attribute()); continue; } diff --git a/clang/test/CIR/IR/inline-asm.cir b/clang/test/CIR/IR/inline-asm.cir index fb1f6315a7d72..907c7944670e2 100644 --- a/clang/test/CIR/IR/inline-asm.cir +++ b/clang/test/CIR/IR/inline-asm.cir @@ -1,4 +1,4 @@ -// RUN: cir-opt %s | FileCheck %s +// RUN: cir-opt %s --verify-roundtrip | FileCheck %s !s32i = !cir.int<s, 32> !u32i = !cir.int<u, 32> @@ -109,4 +109,18 @@ cir.func @f7(%arg0: !u32i) -> !u32i { %3 = cir.load align(4) %0 : !cir.ptr<!u32i>, !u32i cir.return %3 : !u32i } + +cir.func @f8() -> !s32i { + // CHECK: %[[RES:.*]] = cir.asm(x86_att, + // CHECK: out = [], + // CHECK: in = [], + // CHECK: in_out = [], + // CHECK: {"movl $$7, $0" "=r,~{dirflag},~{fpsr},~{flags}"}) side_effects {test.attr = 7 : i32} -> !s32i + %0 = cir.asm(x86_att, + out = [], + in = [], + in_out = [], + {"movl $$7, $0" "=r,~{dirflag},~{fpsr},~{flags}"}) side_effects {test.attr = 7 : i32} -> !s32i + cir.return %0 : !s32i +} } `````````` </details> https://github.com/llvm/llvm-project/pull/186588 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
