From 5f8ba4bcf114df3c4fc734c95cdb5f5c7a23d360 Mon Sep 17 00:00:00 2001 From: Fangyi Zhou Date: Thu, 22 May 2025 17:31:12 +0100 Subject: [PATCH 1/2] [CIR] Add getter/setter for CIR visibility in CIRGlobalValueInterface --- .../clang/CIR/Interfaces/CIROpInterfaces.h | 1 + .../clang/CIR/Interfaces/CIROpInterfaces.td | 196 ++++++++++-------- clang/lib/CIR/Interfaces/CIROpInterfaces.cpp | 6 - 3 files changed, 105 insertions(+), 98 deletions(-) diff --git a/clang/include/clang/CIR/Interfaces/CIROpInterfaces.h b/clang/include/clang/CIR/Interfaces/CIROpInterfaces.h index 86064619af7d..0389f1db4df5 100644 --- a/clang/include/clang/CIR/Interfaces/CIROpInterfaces.h +++ b/clang/include/clang/CIR/Interfaces/CIROpInterfaces.h @@ -17,6 +17,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Mangle.h" +#include "clang/CIR/Dialect/IR/CIRAttrs.h" #include "clang/CIR/Dialect/IR/CIROpsEnums.h" namespace cir {} // namespace cir diff --git a/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td b/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td index 6667058c72b2..9927b8d2e402 100644 --- a/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td +++ b/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td @@ -46,120 +46,132 @@ let cppNamespace = "::cir" in { def CIRGlobalValueInterface : OpInterface<"CIRGlobalValueInterface", [Symbol]> { - let methods = [ - InterfaceMethod<"", - "bool", "hasExternalLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + let methods = + [InterfaceMethod<"", "bool", "hasExternalLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isExternalLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasAvailableExternallyLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasAvailableExternallyLinkage", (ins), + [{}], + /*defaultImplementation=*/[{ return cir::isAvailableExternallyLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasLinkOnceLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasLinkOnceLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isLinkOnceLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasLinkOnceAnyLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasLinkOnceAnyLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isLinkOnceAnyLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasLinkOnceODRLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasLinkOnceODRLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isLinkOnceODRLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasWeakLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasWeakLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isWeakLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasWeakAnyLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasWeakAnyLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isWeakAnyLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasWeakODRLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasWeakODRLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isWeakODRLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasInternalLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasInternalLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isInternalLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasPrivateLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasPrivateLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isPrivateLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasLocalLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasLocalLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isLocalLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasExternalWeakLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasExternalWeakLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isExternalWeakLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "hasCommonLinkage", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasCommonLinkage", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isCommonLinkage($_op.getLinkage()); - }] - >, - InterfaceMethod<"", - "bool", "isDeclarationForLinker", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "isDeclarationForLinker", (ins), [{}], + /*defaultImplementation=*/[{ if ($_op.hasAvailableExternallyLinkage()) return true; return $_op.isDeclaration(); - }] - >, - InterfaceMethod<"", - "bool", "hasComdat", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "hasComdat", (ins), [{}], + /*defaultImplementation=*/[{ return $_op.getComdat(); - }] - >, - InterfaceMethod<"", - "void", "setDSOLocal", (ins "bool":$val), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "void", "setDSOLocal", (ins "bool":$val), [{}], + /*defaultImplementation=*/[{ $_op.setDsolocal(val); - }] - >, - InterfaceMethod<"", - "bool", "isDSOLocal", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "isDSOLocal", (ins), [{}], + /*defaultImplementation=*/[{ return $_op.getDsolocal(); - }] - >, - InterfaceMethod<"", - "bool", "isWeakForLinker", (ins), [{}], - /*defaultImplementation=*/[{ + }]>, + InterfaceMethod<"", "bool", "isWeakForLinker", (ins), [{}], + /*defaultImplementation=*/[{ return cir::isWeakForLinker($_op.getLinkage()); - }] - > + }]>, + InterfaceMethod<"Gets the CIR visibility of this global variable.", + "::cir::VisibilityKind", "getCIRVisibility", (ins), + [{}], + /*defaultImplementation=*/[{ + return $_op.getGlobalVisibility().getValue(); + }]>, + InterfaceMethod< + "Returns true if this global variable has default CIR visibility.", + "bool", "hasDefaultVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + return $_op.getGlobalVisibility().isDefault(); + }]>, + InterfaceMethod< + "Returns true if this global variable has hidden CIR visibility.", + "bool", "hasHiddenVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + return $_op.getGlobalVisibility().isHidden(); + }]>, + InterfaceMethod<"Returns true if this global variable has protected " + "CIR visibility.", + "bool", "hasProtectedVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + return $_op.getGlobalVisibility().isProtected(); + }]>, + InterfaceMethod<"Sets the CIR visibility of this global variable.", + "void", "setGlobalVisibility", + (ins "::cir::VisibilityKind":$vis), [{}], + /*defaultImplementation=*/[{ + $_op.setGlobalVisibilityAttr(::cir::VisibilityAttr::get($_op.getContext(), vis)); + }]>, + InterfaceMethod< + "Sets the CIR visibility of this global variable to be default.", + "void", "setDefaultCIRVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + setGlobalVisibility(::cir::VisibilityKind::Default); + }]>, + InterfaceMethod< + "Sets the CIR visibility of this global variable to be hidden.", + "void", "setHiddenCIRVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + setGlobalVisibility(::cir::VisibilityKind::Hidden); + }]>, + InterfaceMethod< + "Sets the CIR visibility of this global variable to be protected.", + "void", "setProtectedCIRVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + setGlobalVisibility(::cir::VisibilityKind::Protected); + }]>, ]; let extraClassDeclaration = [{ - bool hasDefaultVisibility(); bool canBenefitFromLocalAlias(); }]; } diff --git a/clang/lib/CIR/Interfaces/CIROpInterfaces.cpp b/clang/lib/CIR/Interfaces/CIROpInterfaces.cpp index cdaf01a177c5..2ed102b1b671 100644 --- a/clang/lib/CIR/Interfaces/CIROpInterfaces.cpp +++ b/clang/lib/CIR/Interfaces/CIROpInterfaces.cpp @@ -17,12 +17,6 @@ using namespace cir; #include "clang/CIR/MissingFeatures.h" -bool CIRGlobalValueInterface::hasDefaultVisibility() { - assert(!cir::MissingFeatures::hiddenVisibility()); - assert(!cir::MissingFeatures::protectedVisibility()); - return isPublic() || isPrivate(); -} - bool CIRGlobalValueInterface::canBenefitFromLocalAlias() { assert(!cir::MissingFeatures::supportIFuncAttr()); // hasComdat here should be isDeduplicateComdat, but as far as clang codegen From ae9b78d327f57c272ee71e8262e961b11783714a Mon Sep 17 00:00:00 2001 From: Fangyi Zhou Date: Wed, 28 May 2025 17:31:47 +0100 Subject: [PATCH 2/2] [CIR] add a helper function to deduce MLIR visibility --- .../include/clang/CIR/Dialect/IR/CIRDialect.h | 2 + .../clang/CIR/Dialect/IR/CIROpsEnums.h | 38 +++++++++++++++++++ .../clang/CIR/Interfaces/CIROpInterfaces.td | 10 +++++ clang/lib/CIR/CodeGen/CIRGenDecl.cpp | 4 +- clang/lib/CIR/CodeGen/CIRGenModule.cpp | 32 +++------------- clang/lib/CIR/CodeGen/CIRGenModule.h | 4 +- 6 files changed, 61 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIRDialect.h b/clang/include/clang/CIR/Dialect/IR/CIRDialect.h index be928d1ee19b..b5463379114d 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRDialect.h +++ b/clang/include/clang/CIR/Dialect/IR/CIRDialect.h @@ -18,6 +18,7 @@ #include "mlir/IR/BuiltinTypes.h" #include "mlir/IR/Dialect.h" #include "mlir/IR/OpDefinition.h" +#include "mlir/IR/SymbolTable.h" #include "mlir/Interfaces/CallInterfaces.h" #include "mlir/Interfaces/ControlFlowInterfaces.h" #include "mlir/Interfaces/FunctionInterfaces.h" @@ -90,6 +91,7 @@ class SameFirstSecondOperandAndResultType namespace cir { void buildTerminatedBody(mlir::OpBuilder &builder, mlir::Location loc); + } // namespace cir #define GET_OP_CLASSES diff --git a/clang/include/clang/CIR/Dialect/IR/CIROpsEnums.h b/clang/include/clang/CIR/Dialect/IR/CIROpsEnums.h index 802d517f9202..89ceb65a8efe 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROpsEnums.h +++ b/clang/include/clang/CIR/Dialect/IR/CIROpsEnums.h @@ -15,6 +15,7 @@ #define MLIR_DIALECT_CIR_CIROPSENUMS_H_ #include "mlir/IR/BuiltinAttributes.h" +#include "mlir/IR/SymbolTable.h" #include "clang/CIR/Dialect/IR/CIROpsEnums.h.inc" namespace cir { @@ -126,6 +127,43 @@ template inline bool isValidCIRAtomicOrderingCABI(Int I) { I <= (Int)cir::MemOrder::SequentiallyConsistent; } +/// This function is used to deduce the MLIR visibility of a CIR symbol based on +/// CIR linkage and visibility. +static inline mlir::SymbolTable::Visibility +deduceMLIRVisibility(cir::GlobalLinkageKind linkage, + cir::VisibilityKind visibilityKind) { + if (visibilityKind != cir::VisibilityKind::Default) { + switch (visibilityKind) { + case cir::VisibilityKind::Hidden: + return mlir::SymbolTable::Visibility::Private; + case cir::VisibilityKind::Protected: + llvm::errs() << "visibility not implemented for protected visibility\n"; + llvm_unreachable("protected visibility NYI"); + default: + llvm_unreachable("unknown visibility kind"); + } + } + switch (linkage) { + case cir::GlobalLinkageKind::InternalLinkage: + case cir::GlobalLinkageKind::PrivateLinkage: + return mlir::SymbolTable::Visibility::Private; + case cir::GlobalLinkageKind::ExternalLinkage: + case cir::GlobalLinkageKind::ExternalWeakLinkage: + case cir::GlobalLinkageKind::LinkOnceODRLinkage: + case cir::GlobalLinkageKind::AvailableExternallyLinkage: + case cir::GlobalLinkageKind::CommonLinkage: + case cir::GlobalLinkageKind::WeakAnyLinkage: + case cir::GlobalLinkageKind::WeakODRLinkage: + return mlir::SymbolTable::Visibility::Public; + default: { + llvm::errs() << "visibility not implemented for '" + << stringifyGlobalLinkageKind(linkage) << "'\n"; + assert(0 && "not implemented"); + } + } + llvm_unreachable("linkage should be handled above!"); +} + } // namespace cir #endif // MLIR_DIALECT_CIR_CIROPSENUMS_H_ diff --git a/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td b/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td index 9927b8d2e402..e8710c33f6b0 100644 --- a/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td +++ b/clang/include/clang/CIR/Interfaces/CIROpInterfaces.td @@ -170,6 +170,16 @@ let cppNamespace = "::cir" in { /*defaultImplementation=*/[{ setGlobalVisibility(::cir::VisibilityKind::Protected); }]>, + InterfaceMethod<"Updates the MLIR visibility of this global variable " + "based on the CIR visiblity and linkage.", + "void", "updateMLIRVisibility", (ins), [{}], + /*defaultImplementation=*/[{ + mlir::SymbolTable::setSymbolVisibility( + $_op, ::cir::deduceMLIRVisibility( + $_op.getLinkage(), + $_op.getGlobalVisibility().getValue()) + ); + }]>, ]; let extraClassDeclaration = [{ bool canBenefitFromLocalAlias(); diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp index f04631e22aee..1a442b028fde 100644 --- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp @@ -23,6 +23,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/ExprCXX.h" #include "clang/CIR/Dialect/IR/CIRDataLayout.h" +#include "clang/CIR/Dialect/IR/CIRDialect.h" #include "clang/CIR/Dialect/IR/CIROpsEnums.h" #include "clang/CIR/Dialect/IR/CIRTypes.h" #include "clang/CIR/MissingFeatures.h" @@ -485,7 +486,8 @@ CIRGenModule::getOrCreateStaticVarDecl(const VarDecl &D, cir::GlobalOp GV = builder.createVersionedGlobal( getModule(), getLoc(D.getLocation()), Name, LTy, false, Linkage, AS); // TODO(cir): infer visibility from linkage in global op builder. - GV.setVisibility(getMLIRVisibilityFromCIRLinkage(Linkage)); + GV.setVisibility( + cir::deduceMLIRVisibility(Linkage, cir::VisibilityKind::Default)); GV.setInitialValueAttr(Init); GV.setAlignment(getASTContext().getDeclAlign(&D).getAsAlign().value()); diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index b36185f4f36a..a13a5b388a76 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -1561,7 +1561,8 @@ void CIRGenModule::emitGlobalVarDefinition(const clang::VarDecl *d, if (const SectionAttr *sa = d->getAttr()) gv.setSectionAttr(builder.getStringAttr(sa->getName())); - gv.setGlobalVisibilityAttr(getGlobalVisibilityAttrFromDecl(d)); + const auto globalVisibilityAttr = getGlobalVisibilityAttrFromDecl(d); + gv.setGlobalVisibilityAttr(globalVisibilityAttr); // TODO(cir): // GV->setAlignment(getContext().getDeclAlign(D).getAsAlign()); @@ -1591,7 +1592,8 @@ void CIRGenModule::emitGlobalVarDefinition(const clang::VarDecl *d, // Set CIR linkage and DLL storage class. gv.setLinkage(linkage); // FIXME(cir): setLinkage should likely set MLIR's visibility automatically. - gv.setVisibility(getMLIRVisibilityFromCIRLinkage(linkage)); + gv.setVisibility( + cir::deduceMLIRVisibility(linkage, globalVisibilityAttr.getValue())); // TODO(cir): handle DLL storage classes in CIR? if (d->hasAttr()) assert(!cir::MissingFeatures::setDLLStorageClass()); @@ -2243,30 +2245,8 @@ CIRGenModule::getMLIRVisibility(cir::GlobalOp op) { // definitions). if (op.isDeclaration()) return mlir::SymbolTable::Visibility::Private; - return getMLIRVisibilityFromCIRLinkage(op.getLinkage()); -} - -mlir::SymbolTable::Visibility -CIRGenModule::getMLIRVisibilityFromCIRLinkage(cir::GlobalLinkageKind glk) { - switch (glk) { - case cir::GlobalLinkageKind::InternalLinkage: - case cir::GlobalLinkageKind::PrivateLinkage: - return mlir::SymbolTable::Visibility::Private; - case cir::GlobalLinkageKind::ExternalLinkage: - case cir::GlobalLinkageKind::ExternalWeakLinkage: - case cir::GlobalLinkageKind::LinkOnceODRLinkage: - case cir::GlobalLinkageKind::AvailableExternallyLinkage: - case cir::GlobalLinkageKind::CommonLinkage: - case cir::GlobalLinkageKind::WeakAnyLinkage: - case cir::GlobalLinkageKind::WeakODRLinkage: - return mlir::SymbolTable::Visibility::Public; - default: { - llvm::errs() << "visibility not implemented for '" - << stringifyGlobalLinkageKind(glk) << "'\n"; - assert(0 && "not implemented"); - } - } - llvm_unreachable("linkage should be handled above!"); + return cir::deduceMLIRVisibility(op.getLinkage(), + op.getGlobalVisibility().getValue()); } cir::VisibilityKind CIRGenModule::getGlobalVisibilityKindFromClangVisibility( diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h index e96220ea395a..0e2f04b18fb6 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.h +++ b/clang/lib/CIR/CodeGen/CIRGenModule.h @@ -827,9 +827,9 @@ class CIRGenModule : public CIRGenTypeCache { bool IsConstantVariable); void setFunctionLinkage(GlobalDecl GD, cir::FuncOp f) { auto L = getFunctionLinkage(GD); + auto V = f.getGlobalVisibility().getValue(); f.setLinkageAttr(cir::GlobalLinkageKindAttr::get(&getMLIRContext(), L)); - mlir::SymbolTable::setSymbolVisibility(f, - getMLIRVisibilityFromCIRLinkage(L)); + mlir::SymbolTable::setSymbolVisibility(f, cir::deduceMLIRVisibility(L, V)); } cir::GlobalLinkageKind getCIRLinkageVarDefinition(const VarDecl *VD,