release/19.x: [AVR] Fix 16-bit LDDs with immediate overflows (#104923) #106993
Conversation
|
@benshi001 / @aykevl, is there something we can do to push this forward or it's waiting for someone else? |
|
I think it's up to the release managers now to merge this PR. |
|
Hi, since we are wrapping up LLVM 19.1.0 we are very strict with the fixes we pick at this point. Can you please respond to the following questions to help me understand if this has to be included in the final release or not. Is this PR a fix for a regression or a critical issue? What is the risk of accepting this into the release branch? What is the risk of NOT accepting this into the release branch? |
It's a fix a critical issue within the AVR codegen which causes it to generate invalid binaries and/or crash llvm (depending on which concrete code path is taken). It was spotted downstream in Rust, Rahix/avr-hal#573.
The fix has been tested both within LLVM (as proven by the new tests added here) and by recompiling rustc and making sure the downstream project works correctly as well, so the risk is small. What's more, the change is isolated to the AVR target - there's no way it could affect other targets.
Not huge, I think - rustc has its own fork of llvm-project where I could cherry-pick this commit anyway. Having backport is just the preferred way of the rustc team, but I think it's not a rule set in stone. |
|
This needs to be approved - can someone approve it and I'll merge it before final. |
|
cc @benshi001 / @aykevl 🙂 |
|
Unfortunately the tree was unstable right before release, so I had to direct my attention to reverting some patches instead of merging new ones. This will land in 19.1.1 instead. |
16-bit loads are expanded into a pair of 8-bit loads, so the maximum offset of such 16-bit loads must be 62, not 63. (cherry picked from commit c7a4efa)
|
@EugeneZelenko (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Backport c7a4efa
Requested by: @EugeneZelenko