llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Henrich Lauko (xlauko) <details> <summary>Changes</summary> Replace CIR_VisibilityAttr with DefaultValuedProp<EnumProp<CIR_VisibilityKind>> for global_visibility on GlobalOp and FuncOp. This removes the need for custom parse/print functions and simplifies callers to use direct enum values instead of wrapping/unwrapping VisibilityAttr. --- Full diff: https://github.com/llvm/llvm-project/pull/190488.diff 6 Files Affected: - (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+8-5) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+2-2) - (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+4-41) - (modified) clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp (+1-2) - (modified) clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp (+1-2) - (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+2-3) ``````````diff diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 55d919ae84a20..f72d891ecd941 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -2827,9 +2827,9 @@ def CIR_GlobalOp : CIR_Op<"global", [ // TODO: sym_visibility can possibly be represented by implementing the // necessary Symbol's interface in terms of linkage instead. let arguments = (ins SymbolNameAttr:$sym_name, - DefaultValuedAttr< - CIR_VisibilityAttr, - "VisibilityKind::Default" + DefaultValuedProp< + EnumProp<CIR_VisibilityKind>, + "cir::VisibilityKind::Default" >:$global_visibility, OptionalAttr<StrAttr>:$sym_visibility, TypeAttr:$sym_type, @@ -2851,7 +2851,7 @@ def CIR_GlobalOp : CIR_Op<"global", [ let assemblyFormat = [{ ($sym_visibility^)? - (custom<VisibilityAttr>($global_visibility)^)? + ($global_visibility^)? (`constant` $constant^)? $linkage (`comdat` $comdat^)? @@ -3667,7 +3667,10 @@ def CIR_FuncOp : CIR_Op<"func", [ let arguments = (ins SymbolNameAttr:$sym_name, - CIR_VisibilityAttr:$global_visibility, + DefaultValuedProp< + EnumProp<CIR_VisibilityKind>, + "cir::VisibilityKind::Default" + >:$global_visibility, TypeAttrOf<CIR_FuncType>:$function_type, UnitAttr:$builtin, UnitAttr:$coroutine, diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 2f4990cdc9add..fe0900c42a9cb 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -1016,7 +1016,7 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty, if (const SectionAttr *sa = d->getAttr<SectionAttr>()) gv.setSectionAttr(builder.getStringAttr(sa->getName())); } - gv.setGlobalVisibilityAttr(getGlobalVisibilityAttrFromDecl(d)); + gv.setGlobalVisibility(getGlobalVisibilityAttrFromDecl(d).getValue()); // Handle XCore specific ABI requirements. if (getTriple().getArch() == llvm::Triple::xcore) @@ -2612,7 +2612,7 @@ void CIRGenModule::setFunctionAttributes(GlobalDecl globalDecl, // recompute it here. This is a minimal fix for now. if (!isLocalLinkage(getFunctionLinkage(globalDecl))) { const Decl *decl = globalDecl.getDecl(); - func.setGlobalVisibilityAttr(getGlobalVisibilityAttrFromDecl(decl)); + func.setGlobalVisibility(getGlobalVisibilityAttrFromDecl(decl).getValue()); } // If we plan on emitting this inline builtin, we can't treat it as a builtin. diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index dbe064f03d119..8ccc83a25537b 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -207,33 +207,6 @@ static bool omitRegionTerm(mlir::Region &r) { return singleNonEmptyBlock && yieldsNothing(); } -void printVisibilityAttr(OpAsmPrinter &printer, - cir::VisibilityAttr &visibility) { - switch (visibility.getValue()) { - case cir::VisibilityKind::Hidden: - printer << "hidden"; - break; - case cir::VisibilityKind::Protected: - printer << "protected"; - break; - case cir::VisibilityKind::Default: - break; - } -} - -void printVisibilityAttr(OpAsmPrinter &printer, cir::GlobalOp, - cir::VisibilityAttr visibility) { - printVisibilityAttr(printer, visibility); -} - -mlir::OptionalParseResult parseVisibilityAttr(OpAsmParser &parser, - cir::VisibilityAttr &visibility) { - cir::VisibilityKind visibilityKind = - parseOptionalCIRKeyword(parser, cir::VisibilityKind::Default); - visibility = cir::VisibilityAttr::get(parser.getContext(), visibilityKind); - return mlir::success(); -} - //===----------------------------------------------------------------------===// // InlineKindAttr (FIXME: remove once FuncOp uses assembly format) //===----------------------------------------------------------------------===// @@ -1795,9 +1768,6 @@ void cir::GlobalOp::build( odsBuilder.createBlock(dtorRegion); dtorBuilder(odsBuilder, odsState.location); } - - odsState.addAttribute(getGlobalVisibilityAttrName(odsState.name), - cir::VisibilityAttr::get(odsBuilder.getContext())); } /// Given the region at `index`, or the parent operation if `index` is None, @@ -2061,8 +2031,6 @@ void cir::FuncOp::build(OpBuilder &builder, OperationState &result, result.addAttribute( getLinkageAttrNameString(), GlobalLinkageKindAttr::get(builder.getContext(), linkage)); - result.addAttribute(getGlobalVisibilityAttrName(result.name), - cir::VisibilityAttr::get(builder.getContext())); } ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) { @@ -2076,7 +2044,6 @@ ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) { mlir::StringAttr noProtoNameAttr = getNoProtoAttrName(state.name); mlir::StringAttr comdatNameAttr = getComdatAttrName(state.name); mlir::StringAttr visNameAttr = getSymVisibilityAttrName(state.name); - mlir::StringAttr visibilityNameAttr = getGlobalVisibilityAttrName(state.name); mlir::StringAttr dsoLocalNameAttr = getDsoLocalAttrName(state.name); mlir::StringAttr specialMemberAttr = getCxxSpecialMemberAttrName(state.name); @@ -2115,9 +2082,8 @@ ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) { parser.getBuilder().getStringAttr(visAttrStr)); } - cir::VisibilityAttr cirVisibilityAttr; - parseVisibilityAttr(parser, cirVisibilityAttr); - state.addAttribute(visibilityNameAttr, cirVisibilityAttr); + state.getOrAddProperties<cir::FuncOp::Properties>().global_visibility = + parseOptionalCIRKeyword(parser, cir::VisibilityKind::Default); if (parser.parseOptionalKeyword(dsoLocalNameAttr).succeeded()) state.addAttribute(dsoLocalNameAttr, parser.getBuilder().getUnitAttr()); @@ -2392,11 +2358,8 @@ void cir::FuncOp::print(OpAsmPrinter &p) { if (vis != mlir::SymbolTable::Visibility::Public) p << ' ' << vis; - cir::VisibilityAttr cirVisibilityAttr = getGlobalVisibilityAttr(); - if (!cirVisibilityAttr.isDefault()) { - p << ' '; - printVisibilityAttr(p, cirVisibilityAttr); - } + if (getGlobalVisibility() != cir::VisibilityKind::Default) + p << ' ' << stringifyVisibilityKind(getGlobalVisibility()); if (getDsoLocal()) p << " dso_local"; diff --git a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp index 14f7aca2bc136..abdc32951f857 100644 --- a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp @@ -200,8 +200,7 @@ void ItaniumEHLowering::ensureClangCallTerminate(mlir::Location loc) { auto funcOp = cir::FuncOp::create(builder, loc, "__clang_call_terminate", funcTy); funcOp.setLinkage(cir::GlobalLinkageKind::LinkOnceODRLinkage); - funcOp.setGlobalVisibilityAttr( - cir::VisibilityAttr::get(ctx, cir::VisibilityKind::Hidden)); + funcOp.setGlobalVisibility(cir::VisibilityKind::Hidden); mlir::Block *entryBlock = funcOp.addEntryBlock(); builder.setInsertionPointToStart(entryBlock); diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp index a67ce9850a838..4d0c7b179f260 100644 --- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp +++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp @@ -320,8 +320,7 @@ cir::GlobalOp LoweringPreparePass::buildRuntimeVariable( cir::GlobalLinkageKindAttr::get(builder.getContext(), linkage)); mlir::SymbolTable::setSymbolVisibility( g, mlir::SymbolTable::Visibility::Private); - g.setGlobalVisibilityAttr( - cir::VisibilityAttr::get(builder.getContext(), visibility)); + g.setGlobalVisibility(visibility); } return g; } diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index 40ee341afdc12..119269447ec37 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -2151,7 +2151,6 @@ void CIRToLLVMFuncOpLowering::lowerFuncAttributes( if (attr.getName() == mlir::SymbolTable::getSymbolAttrName() || attr.getName() == func.getFunctionTypeAttrName() || attr.getName() == getLinkageAttrNameString() || - attr.getName() == func.getGlobalVisibilityAttrName() || attr.getName() == func.getDsoLocalAttrName() || attr.getName() == func.getInlineKindAttrName() || attr.getName() == func.getSideEffectAttrName() || @@ -2329,8 +2328,8 @@ CIRToLLVMGlobalOpLowering::lowerGlobalAttributes( attributes.push_back(rewriter.getNamedAttr("section", sectionAttr)); mlir::LLVM::VisibilityAttr visibility = mlir::LLVM::VisibilityAttr::get( - getContext(), lowerCIRVisibilityToLLVMVisibility( - op.getGlobalVisibilityAttr().getValue())); + getContext(), + lowerCIRVisibilityToLLVMVisibility(op.getGlobalVisibility())); attributes.push_back(rewriter.getNamedAttr("visibility_", visibility)); if (op->getAttr(CUDAExternallyInitializedAttr::getMnemonic())) `````````` </details> https://github.com/llvm/llvm-project/pull/190488 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
