Fix lowering of must_preserve_tags memset without capabilities #734
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Copying from a zero-constant can be optimized to a memset() and should therefore not trigger the underaligned capability warnings. This is also a baseline test for a crash found in the Morello backend where this memcpy->memset optimization triggers an assertion in diagnoseInefficientCheriMemOp().
Noticed while writing tests for copying capabilities from constants.
When a must_preserve_tags memcpy is converted to a memset, we may end up using a non-capability MVT as the copy type. This change allow us to use inline integer memset lowering for copies from zero constants that are not necessarily sufficiently aligned. This also fixes an assertion found while compiling ICU4C for Morello where a copy from a large zero constant was lowered using NEON registers instead of capability ones.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Noticed this while merging Morello LLVM to 15. The assertion does not trigger for RISC-V, but does for Morello since it tries to use NEON registers.
Should be uncontroversial since it just adds a
&& !Op.isMemset(). PR for CI.Also pull in a diff from Morello to handle ISD::PTRADD in isMemSrcFromConstant() with a newly written test (not tested there).