Skip to content

Conversation

@F-Stuckmann
Copy link
Collaborator

This implements a global combiner for determining post increments or offset Memory Instructions.
The following is still work in progress:

  • There are 3 CombineResult classes with similarities that should be combined to one. This also includes adding the new CombineResult Class to the AIECombiners.
  • The gain function of the combiners should be more general and moved to AIEGlobalCombiner. Depending on the QoR results, there may be some finetuning in the cost calculation.
  • Matching rules should be provided by .td, instead of being implemented as classes

That being said, the structure in AIEPtrModPass , AIEGlobalCombiner and BaseInstrInfo should be stable and close to the final version.

@F-Stuckmann F-Stuckmann force-pushed the stuckmann.combiner branch 2 times, most recently from 2e49f47 to df06e92 Compare March 26, 2025 14:13
@F-Stuckmann F-Stuckmann force-pushed the stuckmann.combiner branch 2 times, most recently from 11c7b5e to fefb44a Compare June 10, 2025 15:26
@andcarminati
Copy link
Collaborator

I see this warning:

 warning: definition of implicit copy assignment operator for 'CombinerSolution' is deprecated because it has a user-declared copy constructor [-Wdeprecated-copy]
  CombinerSolution(const CombinerSolution &Other) = default;
  ^
../llvm/lib/Target/AIE/AIEGlobalCombiner.cpp:278:22: note: in implicit copy assignment operator for 'llvm::AIE::CombinerSolution' first required here
        BestSolution = Current;
                     ^
1 warning generated.

@F-Stuckmann
Copy link
Collaborator Author

Once #460 is merged, the placeholder commit can be removed

@F-Stuckmann
Copy link
Collaborator Author

I removed the placeholder PR commit

Copy link
Collaborator

@martien-de-jong martien-de-jong left a comment

Choose a reason for hiding this comment

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

To stop it from haunting you.
It looks good to me.

@F-Stuckmann F-Stuckmann enabled auto-merge (rebase) June 18, 2025 15:55
@F-Stuckmann F-Stuckmann merged commit 03f2a63 into aie-public Jun 18, 2025
6 checks passed
@andcarminati
Copy link
Collaborator

I see this warning:

 warning: definition of implicit copy assignment operator for 'CombinerSolution' is deprecated because it has a user-declared copy constructor [-Wdeprecated-copy]
  CombinerSolution(const CombinerSolution &Other) = default;
  ^
../llvm/lib/Target/AIE/AIEGlobalCombiner.cpp:278:22: note: in implicit copy assignment operator for 'llvm::AIE::CombinerSolution' first required here
        BestSolution = Current;
                     ^
1 warning generated.

Hi @F-Stuckmann , this comment was not addressed, could you please address this in a follow-up PR? Thank you!

@andcarminati andcarminati deleted the stuckmann.combiner branch June 23, 2025 07:31
return true;
}

void llvm::foundPattern(MachineInstr &MemI) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be unused!

mgehre-amd added a commit that referenced this pull request Aug 21, 2025
ArithToEmitC: Support using opaque types for floating point
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