Skip to content

[flang] Create TBAA subtree for COMMON block variables. #153918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[flang] Create TBAA subtree for COMMON block variables. #153918

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b15ca4
[flang] Create TBAA subtree for COMMON block variables.
vzakhari Aug 16, 2025
675ebed
Recognize equivalenced variables.
vzakhari Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions flang/include/flang/Optimizer/Analysis/TBAAForest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ struct TBAATree {

mlir::LLVM::TBAATypeDescriptorAttr getRoot() const { return parent; }

/// For the given name, get or create a subtree in the current
/// subtree. For example, this is used for creating subtrees
/// inside the "global data" subtree for the COMMON block variables
/// belonging to the same COMMON block.
SubtreeState &getOrCreateNamedSubtree(mlir::StringAttr name);

private:
SubtreeState(mlir::MLIRContext *ctx, std::string name,
mlir::LLVM::TBAANodeAttr grandParent)
Expand All @@ -57,6 +63,9 @@ struct TBAATree {
const std::string parentId;
mlir::MLIRContext *const context;
mlir::LLVM::TBAATypeDescriptorAttr parent;
// A map of named sub-trees, e.g. sub-trees of the COMMON blocks
// placed under the "global data" root.
llvm::DenseMap<mlir::StringAttr, SubtreeState> namedSubtrees;
};

/// A subtree for POINTER/TARGET variables data.
Expand Down Expand Up @@ -131,22 +140,29 @@ class TBAAForrest {
// responsibility to provide unique name for the scope.
// If the scope string is empty, returns the TBAA tree for the
// "root" scope of the given function.
inline const TBAATree &getFuncTreeWithScope(mlir::func::FuncOp func,
llvm::StringRef scope) {
inline TBAATree &getMutableFuncTreeWithScope(mlir::func::FuncOp func,
llvm::StringRef scope) {
mlir::StringAttr name = func.getSymNameAttr();
if (!scope.empty())
name = mlir::StringAttr::get(name.getContext(),
llvm::Twine(name) + " - " + scope);
return getFuncTree(name);
}

inline const TBAATree &getFuncTreeWithScope(mlir::func::FuncOp func,
llvm::StringRef scope) {
return getMutableFuncTreeWithScope(func, scope);
}

private:
const TBAATree &getFuncTree(mlir::StringAttr symName) {
TBAATree &getFuncTree(mlir::StringAttr symName) {
if (!separatePerFunction)
symName = mlir::StringAttr::get(symName.getContext(), "");
if (!trees.contains(symName))
trees.insert({symName, TBAATree::buildTree(symName)});
return trees.at(symName);
auto it = trees.find(symName);
assert(it != trees.end());
return it->second;
}

// Should each function use a different tree?
Expand Down
7 changes: 6 additions & 1 deletion flang/include/flang/Optimizer/Builder/FIRBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,12 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
// Linkage helpers (inline). The default linkage is external.
//===--------------------------------------------------------------------===//

mlir::StringAttr createCommonLinkage() { return getStringAttr("common"); }
static mlir::StringAttr createCommonLinkage(mlir::MLIRContext *context) {
return mlir::StringAttr::get(context, "common");
}
mlir::StringAttr createCommonLinkage() {
return createCommonLinkage(getContext());
}

mlir::StringAttr createInternalLinkage() { return getStringAttr("internal"); }

Expand Down
13 changes: 11 additions & 2 deletions flang/lib/Optimizer/Analysis/TBAAForest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,21 @@

mlir::LLVM::TBAATagAttr
fir::TBAATree::SubtreeState::getTag(llvm::StringRef uniqueName) const {
std::string id = (parentId + "/" + uniqueName).str();
std::string id = (parentId + '/' + uniqueName).str();
mlir::LLVM::TBAATypeDescriptorAttr type =
mlir::LLVM::TBAATypeDescriptorAttr::get(
context, id, mlir::LLVM::TBAAMemberAttr::get(parent, 0));
return mlir::LLVM::TBAATagAttr::get(type, type, 0);
// return tag;
}

fir::TBAATree::SubtreeState &
fir::TBAATree::SubtreeState::getOrCreateNamedSubtree(mlir::StringAttr name) {
if (!namedSubtrees.contains(name))
namedSubtrees.insert(
{name, SubtreeState(context, parentId + '/' + name.str(), parent)});
auto it = namedSubtrees.find(name);
assert(it != namedSubtrees.end());
return it->second;
}

mlir::LLVM::TBAATagAttr fir::TBAATree::SubtreeState::getTag() const {
Expand Down
90 changes: 71 additions & 19 deletions flang/lib/Optimizer/Transforms/AddAliasTags.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "flang/Optimizer/Analysis/AliasAnalysis.h"
#include "flang/Optimizer/Analysis/TBAAForest.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/Dialect/FirAliasTagOpInterface.h"
#include "flang/Optimizer/Transforms/Passes.h"
Expand Down Expand Up @@ -61,8 +62,10 @@ namespace {
class PassState {
public:
PassState(mlir::DominanceInfo &domInfo,
std::optional<unsigned> localAllocsThreshold)
: domInfo(domInfo), localAllocsThreshold(localAllocsThreshold) {}
std::optional<unsigned> localAllocsThreshold,
const mlir::SymbolTable &symTab)
: domInfo(domInfo), localAllocsThreshold(localAllocsThreshold),
symTab(symTab) {}
/// memoised call to fir::AliasAnalysis::getSource
inline const fir::AliasAnalysis::Source &getSource(mlir::Value value) {
if (!analysisCache.contains(value))
Expand All @@ -72,13 +75,14 @@ class PassState {
}

/// get the per-function TBAATree for this function
inline const fir::TBAATree &getFuncTree(mlir::func::FuncOp func) {
return forrest[func];
inline fir::TBAATree &getMutableFuncTreeWithScope(mlir::func::FuncOp func,
fir::DummyScopeOp scope) {
auto &scopeMap = scopeNames.at(func);
return forrest.getMutableFuncTreeWithScope(func, scopeMap.lookup(scope));
}
inline const fir::TBAATree &getFuncTreeWithScope(mlir::func::FuncOp func,
fir::DummyScopeOp scope) {
auto &scopeMap = scopeNames.at(func);
return forrest.getFuncTreeWithScope(func, scopeMap.lookup(scope));
return getMutableFuncTreeWithScope(func, scope);
}

void processFunctionScopes(mlir::func::FuncOp func);
Expand All @@ -98,8 +102,14 @@ class PassState {
// attachment.
bool attachLocalAllocTag();

fir::GlobalOp getGlobalDefiningOp(mlir::StringAttr name) const {
return symTab.lookup<fir::GlobalOp>(name);
}

private:
mlir::DominanceInfo &domInfo;
std::optional<unsigned> localAllocsThreshold;
const mlir::SymbolTable &symTab;
fir::AliasAnalysis analysis;
llvm::DenseMap<mlir::Value, fir::AliasAnalysis::Source> analysisCache;
fir::TBAAForrest forrest;
Expand All @@ -117,8 +127,6 @@ class PassState {
// Local pass cache for derived types that contain descriptor
// member(s), to avoid the cost of isRecordWithDescriptorMember().
llvm::DenseSet<mlir::Type> typesContainingDescriptors;

std::optional<unsigned> localAllocsThreshold;
};

// Process fir.dummy_scope operations in the given func:
Expand Down Expand Up @@ -310,14 +318,55 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
source.kind == fir::AliasAnalysis::SourceKind::Global &&
!source.isBoxData()) {
mlir::SymbolRefAttr glbl = llvm::cast<mlir::SymbolRefAttr>(source.origin.u);
const char *name = glbl.getRootReference().data();
LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to global " << name
<< " at " << *op << "\n");
if (source.isPointer())
mlir::StringAttr name = glbl.getRootReference();
LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to global "
<< name.str() << " at " << *op << "\n");
if (source.isPointer()) {
tag = state.getFuncTreeWithScope(func, scopeOp).targetDataTree.getTag();
else
tag =
state.getFuncTreeWithScope(func, scopeOp).globalDataTree.getTag(name);
} else {
// In general, place the tags under the "global data" root.
fir::TBAATree::SubtreeState *subTree =
&state.getMutableFuncTreeWithScope(func, scopeOp).globalDataTree;

// The COMMON blocks have their own sub-tree root under the "global data"
Copy link
Contributor

@tblah tblah Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can non-common block global data alias with common blocks? Maybe it depends how confident we are that information about common blocks doesn't get lost during lowering.

If we are confident then we could express this by putting the common block tree one level up (as a sibling to the global data tree).

Copy link
Contributor Author

@vzakhari vzakhari Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the answer is no. There is a way to equivalence a non-common variable A with a common variable B, but then A effectively becomes a part of the B's common block, so we think of A as just another section of the common block.

I think we can put the common blocks roots as siblings of the global data tree. I think it should not matter now, because we may never create tags attached to the root of the global data tree, since the global variable names are always present. I would like to keep the common blocks sub-trees under the global data tree for the following (aritificial) reason: if we ever step on a memory access that we know accesses global memory, but we do not know exactly which one, we should assume this access aliases with any global variable (including the COMMON ones). In this case, the common sub-trees have to be under the global data root.

// root, which is named after the name of the COMMON block.
// If we can identify the name of the member variable, then
// we create a sub-tree under the root of the COMMON block
// and place the tag there. If we cannot identify the name
// of the member variable (e.g. for whatever reason there is no
// fir.declare for it), then we place the tag under the root
// of the COMMON block.
auto globalOp = state.getGlobalDefiningOp(name);
// TODO: this is a subtle identification of the fact that
// the variable belongs to a COMMON block.
// Should we have an attribute on [hl]fir.declare
// that specifies the name of the COMMON block the variable
// belongs to?
Copy link
Contributor

@tblah tblah Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree this would be better. In OpenMP we handle it all whilst we still have access to the symbol table, but I think with that gone it would be worth including that information here.

Copy link
Contributor Author

@vzakhari vzakhari Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured my changes do not work properly for equivalence cases where two common block variables may [partially ]overlap while having different names. In order to make this right, I will need information about the starting offset of a variable within the COMMON block (their sizes are always known in the compilation time). So I am thinking about an attribute on [hl]fir.declare that will provide a pair of the COMMON block symbol ref and the integer offset.

I am going to check how [partially ]overlapping variables may be represented with TBAA the best way. If I cannot communicate the exact offsets/sizes via TBAA, I will probably fall back to creating cliques of overlapping variables within a COMMON block and place them under the same root/tag.

if (globalOp &&
globalOp.getLinkName() ==
fir::FirOpBuilder::createCommonLinkage(globalOp->getContext())) {
// Get or create a sub-tree for the COMMON block.
subTree = &subTree->getOrCreateNamedSubtree(name);

auto declOp = mlir::dyn_cast_or_null<fir::DeclareOp>(
source.origin.instantiationPoint);
mlir::StringAttr varName;
if (declOp) {
// The tag for the variable will be placed under its own
// root in the COMMON sub-tree.
varName = declOp.getUniqName();
tag = subTree->getTag(varName.str());
} else {
tag = subTree->getTag();
}
LLVM_DEBUG(llvm::dbgs().indent(2)
<< "Variable named '"
<< (varName ? varName.str() : "<unknown>")
<< "' is from COMMON block '" << name.str() << "'\n");
} else {
tag = subTree->getTag(name.str());
}
}

// TBAA for global variables with descriptors
} else if (enableDirect &&
Expand Down Expand Up @@ -401,11 +450,14 @@ void AddAliasTagsPass::runOnOperation() {
// thinks the pass operates on), then the real work of the pass is done in
// runOnAliasInterface
auto &domInfo = getAnalysis<mlir::DominanceInfo>();
PassState state(domInfo, localAllocsThreshold.getPosition()
? std::optional<unsigned>(localAllocsThreshold)
: std::nullopt);

mlir::ModuleOp mod = getOperation();
mlir::SymbolTable symTab(mod);
PassState state(domInfo,
localAllocsThreshold.getPosition()
? std::optional<unsigned>(localAllocsThreshold)
: std::nullopt,
symTab);

mod.walk(
[&](fir::FirAliasTagOpInterface op) { runOnAliasInterface(op, state); });

Expand Down
Loading