Skip to content
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.

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

[mlir][mlir-link] Initial skeleton for an MLIR-linker #10

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

hbrodin
Copy link
Collaborator

@hbrodin hbrodin commented Jan 31, 2025

This version can link two files. Probably not any two files/functions, but at least two specific test functions with one decl.

This version can link two files. Probably not any two files/functions,
but at least two specific test functions with one decl.
@hbrodin hbrodin requested a review from xlauko January 31, 2025 15:59
return WalkResult::skip();
});

if (!anythingToLink)
// TODO: Consider if there are combinations of Function/GlobalVariable where
Copy link
Member

Choose a reason for hiding this comment

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

yes no global values will cause the function to proceed even if we can terminate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this good or badf?

DenseSet<Operation *> valuesToLink;
std::vector<Operation *> worklist;
// Replace-all-uses-with worklist
std::vector<std::pair<Operation *, Operation *>> rauwWorklist;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::vector<std::pair<Operation *, Operation *>> rauwWorklist;
std::vector<std::pair<Operation *, Value>> rauwWorklist;

}

if (dst) {
if (auto dstGVL = dyn_cast<GlobalValueLinkageOpInterface>(dst))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (auto dstGVL = dyn_cast<GlobalValueLinkageOpInterface>(dst))
if (auto dgv = dyn_cast<GlobalValueLinkageOpInterface>(dst))


if (dst) {
if (auto dstGVL = dyn_cast<GlobalValueLinkageOpInterface>(dst))
if (!dstGVL.isDeclarationForLinkage())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!dstGVL.isDeclarationForLinkage())
if (!(dgv.isDeclaration() || dgv.hasAvailableExternallyLinkage())

Copy link
Member

Choose a reason for hiding this comment

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

isDeclarationForLinkage is not isDeclarationForLinker

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you please clarify? I don't follow.

return LazilyAdded;
}

Operation *MLIRLinker::copyGlobalVariableProto(Operation *src,
Copy link
Member

Choose a reason for hiding this comment

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

I think we will do this as dialect interface. Where dialect will specify how to copy globals function etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In that case, I think we'll have to have two separate phases (at least that is how it is done in llvm-linker), one for creating a prototype/declaration and one for copying the body.

@xlauko xlauko merged commit 7ab4013 into mlir-link Feb 4, 2025
3 checks passed
@xlauko xlauko deleted the mlir-link-wip branch February 7, 2025 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants