Skip to content

Conversation

martien-de-jong
Copy link
Collaborator

@martien-de-jong martien-de-jong commented Jun 10, 2025

This is a port of Krishnam's original PR #173 which I will close.
I'll be running QoR today-ish.

QoR results are varying. Also, memory bank annotations may not always be respected by the buffer allocation. Although it can be argued that intent of the programmer will usually be to have different objects be designated by pointers to non-overlapping banks, the gain we see doesn't make it worth to take the risk. I converted to draft.

@krishnamtibrewala
Copy link
Collaborator

Thank you @martien-de-jong for porting the PR.
Wanted to give heads up, last I remember(long time back) there were no QoR gain. Therefore Gaetan suggested to hold onto it.

@andcarminati
Copy link
Collaborator

andcarminati commented Jun 10, 2025

Can we extent to aie2p? I see that it covers tile memory for aie2. Maybe it is not important...

@@ -82,7 +86,12 @@ class AIEBaseAAWrapperPass : public ImmutablePass {
const AIEBaseAAResult &getResult() const { return *Result; }

bool doInitialization(Module &M) override {
Result.reset(new AIEBaseAAResult(M.getDataLayout()));
if (!M.getFunctionList().empty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we do not have a function list in the module, shouldn't we still reset previous AliasResults`?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Theoretically yes. Although no one is going to query it in a module without functions. It seems to need TTI, which is normally found in the function. I wouldn't know where to find it otherwise.

@martien-de-jong martien-de-jong marked this pull request as draft July 9, 2025 13:08
mgehre-amd added a commit that referenced this pull request Aug 21, 2025
Ensure 0 <= x mod N < N semantics
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.

4 participants