Skip to content

[llvm][PowerPC] Correct handling of spill slots for SPE when EXPENSIVE_CHECKS is enabled #73940

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

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

DavidSpickett
Copy link
Collaborator

This was modifying a container as it iterated it, which tripped a check in libstdc++'s debug checks.

Instead, just assign to the item via the reference we already have.

This fixes the following expensive checks failures on my machine:
LLVM :: CodeGen/PowerPC/fp-strict.ll
LLVM :: CodeGen/PowerPC/pr55463.ll
LLVM :: CodeGen/PowerPC/register-pressure.ll
LLVM :: CodeGen/PowerPC/spe.ll

Which are some of the tests noted by #68594.

…E_CHECKS is enabled

This was modifying a container as it iterated it, which tripped a
check in libstdc++'s debug checks.

Instead, just assign to the item via the reference we already have.

This fixes the following expensive checks failures on my machine:
  LLVM :: CodeGen/PowerPC/fp-strict.ll
  LLVM :: CodeGen/PowerPC/pr55463.ll
  LLVM :: CodeGen/PowerPC/register-pressure.ll
  LLVM :: CodeGen/PowerPC/spe.ll

Which are some of the tests noted by llvm#68594.
@DavidSpickett
Copy link
Collaborator Author

@arsenm On https://reviews.llvm.org/D152437 you said:
"Don't use reference to MCPhysReg"

I can address that too if you want. Looks like MCPhysReg is passed around by copy is that right?

Copy link
Collaborator

@kosarev kosarev left a comment

Choose a reason for hiding this comment

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

Thanks!

@arsenm
Copy link
Contributor

arsenm commented Dec 1, 2023

@arsenm On https://reviews.llvm.org/D152437 you said: "Don't use reference to MCPhysReg"

I can address that too if you want. Looks like MCPhysReg is passed around by copy is that right?

Yes, MCPhysReg is just an int16_t

@DavidSpickett DavidSpickett merged commit da1aff2 into llvm:main Dec 1, 2023
@DavidSpickett DavidSpickett deleted the fix-ppc branch December 1, 2023 10:52
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.

3 participants