Skip to content

[AVR] Remove earlyclobber from LDDRdPtrQ #85277

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 1 commit into from
Mar 15, 2024
Merged

Conversation

Patryk27
Copy link
Contributor

@Patryk27 Patryk27 commented Mar 14, 2024

LDDRdPtrQ was marked as earlyclobber, which doesn't play well with GreedyRA (which can generate this instruction through loadRegFromStackSlot()).

This seems to be the same case as:

// An identical pseudo instruction to LDDWRdPtrQ, expect restricted to the Y

Closes #81911.

@Patryk27
Copy link
Contributor Author

cc @benshi001, when you have a moment 🙂

@benshi001
Copy link
Member

Thanks for your great work! It looks good to me.

Just remove Note that I'm not really sure the approach presented here is correct - intuitively, in your commit message. Leave it as

This instruction shouldn't rely on `@earlyclobber`, since it doesn't have any input registers except for the implied `Y`, and `Y` is always marked as reserved in `AVRRegisterInfo::getReservedRegs`.

@benshi001 benshi001 self-requested a review March 15, 2024 01:50
LDDRdPtrQ was marked as earlyclobber, which doesn't play well with
GreedyRA (which can generate this instruction through
`loadRegFromStackSlot()`).

This seems to be the same case as:

https://github.com/llvm/llvm-project/blob/a99b912c9b74f6ef91786b4dfbc25160c27d3b41/llvm/lib/Target/AVR/AVRInstrInfo.td#L1421

Closes #81911.
@Patryk27
Copy link
Contributor Author

Done! 🚀

I've also tested the changes by compiling rustc locally and simavr-ing a few firmwares, and everything seems to be working correctly 🙂

@llvmbot
Copy link
Member

llvmbot commented Mar 15, 2024

/cherry-pick bcd4c72

Error: Command failed due to missing milestone.

@Patryk27
Copy link
Contributor Author

Patryk27 commented Mar 15, 2024

Ah, by the way, sorry - I haven't noticed that you've proposed that description after Leave it as: [...] and so I've just cut the note that [...] part; gonna read the comments more carefully next time 😅

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 16, 2024
LDDRdPtrQ was marked as `earlyclobber`, which doesn't play well with
GreedyRA (which can generate this instruction through `loadRegFromStackSlot()`).

This seems to be the same case as:

https://github.com/llvm/llvm-project/blob/a99b912c9b74f6ef91786b4dfbc25160c27d3b41/llvm/lib/Target/AVR/AVRInstrInfo.td#L1421

Closes llvm#81911.

(cherry picked from commit 328cb9b)
Rahix added a commit to Rahix/avr-hal that referenced this pull request Mar 22, 2024
Bump the toolchain version to pull in a fix for a recently discovered
miscompilation issue [1].

[1]: llvm/llvm-project#85277
Rahix added a commit to Rahix/avr-hal that referenced this pull request Mar 22, 2024
Bump the toolchain version to pull in a fix for a recently discovered
miscompilation issue [1].

[1]: llvm/llvm-project#85277
tippfehlr pushed a commit to tippfehlr/avr-hal that referenced this pull request Mar 24, 2024
Bump the toolchain version to pull in a fix for a recently discovered
miscompilation issue [1].

[1]: llvm/llvm-project#85277
@pointhex pointhex mentioned this pull request May 7, 2024
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.

[AVR/RegAllocGreedy] Allocator clobbers a live register
3 participants