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

Fix compiler crash and profitability test in memset optimisation #22

Open
wants to merge 3 commits into
base: nanomips
Choose a base branch
from

Conversation

cme
Copy link

@cme cme commented Jul 24, 2023

The legacy getDestAlignment member used to get alignment for existing memset intrinsics can use a value of 0 to represent undefined alignment, which will cause the alignment offset calculation to fail as it uses this as the denominator of a modulo operation.

Colin McEwan added 2 commits July 31, 2023 23:36
Targets with unaligned accesses can make a better job of inlining
memsets without knowing exact alignment.
@cme cme changed the title Fix compiler crash in memset optimisation Fix compiler crash and profitability test in memset optimisation Aug 1, 2023
@@ -297,7 +297,7 @@ void MemsetRanges::addRange(int64_t Start, int64_t Size, Value *Ptr,
I->Start = Start;
I->StartPtr = Ptr;
I->Alignment = Alignment;
if (I->MaxAlignmentOffset != 0)
if (I->MaxAlignment != 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. This now makes sense.

@djtodoro
Copy link
Collaborator

djtodoro commented Aug 3, 2023

This makes sense to me.
This PR fixes:

  LLVM :: Transforms/MemCpyOpt/merge-into-memset.ll
  LLVM :: Transforms/MemCpyOpt/profitable-memset.ll

But, the new heuristics [0] introduced couple of other failures as well that still fail:

  Clang :: CodeGenCXX/auto-var-init.cpp
  LLVM :: Transforms/MemCpyOpt/form-memset.ll

[0] efeed9e

@djtodoro
Copy link
Collaborator

djtodoro commented Jun 4, 2024

@cme what is the status of this?

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