[PATCH] D70800: Fix AArch64 AAPCS frame record chain

Owen Anderson via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 1 12:24:55 PDT 2020



> On Sep 1, 2020, at 11:59 AM, Paul Walker <Paul.Walker at arm.com> wrote:
> 
> Fair enough, thanks for the information Owen.
> 
> Clearly my preference would be to not fix one AArch64 bug by introducing another.  You sound more knowledgeable in this area than myself so perhaps could create your suggested SVE test cases to ensure this isn't the case.  That said, if you prefer to just reapply the patch as is, at least we've recorded the issue for others working on SVE.

I’m going to go ahead and re-apply the patch.

I want to be careful about conflating two different discussions here:

* Strengthening the assertion check.  I’m happy to continue discussing this and see if we can come up with a better way to expression the constraint that the stack pointer must always be 16-byte aligned, but it seems like a significant refactoring would be required.  The weakened version of the assertion present in the patch is the strongest assertion that can be made with the current structure of the code.  The weakened assertion does not introduce new bugs, and fixes an existing one.

* Fixing whatever is causing spec2k/eon to break.  I believe that this patch is exposing a latent bug in the SVE frame layout code, likely because it is not handling an unaligned frame pointer properly in some way.  I don’t have access to the eon sources, an SVE machine, or any knowledge of SVE AAPCS.  Since this is not broken on a publicly accessible build-bot (AFAIK?), this shouldn’t block landing the patch.  That said, I’m happy to work with someone who has the knowledge and resources for SVE work to help resolve the latent bug that is being exposed.

—Owen


More information about the llvm-commits mailing list