Page MenuHomePhabricator

[RISCV] Implement canRealignStack
AbandonedPublic

Authored by lenary on Nov 25 2019, 5:52 AM.

Details

Summary

The implementation of RISCVRegisterInfo::canRealignStack was left out of the
implementation of Stack Realignment in D62007.

I think this is the root cause of the issues we've seen in D70401 around the
spilling of fp64 registers in the ILP32E ABI. Applying this patch seems to
solve those issues.

Unfortunately, I think that failure requires spilling a register with a spill
alignment that is larger than the current stack alignment. Until ILP32E has
landed, we have no registers which satisfy these constraints, so no testcases
have changed with this commit (but, we want to land this patche separately to
ILP32E support).

Event Timeline

lenary created this revision.Nov 25 2019, 5:52 AM

This might have to be backported to 9.0.1 as stack realignment has got into 9.0.1.

With the patch, double load/store instructions may unaligned-access with -mabi=ilp32e -mattr=+d flags. Could the load/store support unaligned-access?

lenary abandoned this revision.Nov 26 2019, 2:50 AM

With the patch, double load/store instructions may unaligned-access with -mabi=ilp32e -mattr=+d flags. Could the load/store support unaligned-access?

My understanding was that RISC-V mandated that misaligned loads/stores had to be supported, even if that was via emulation in a trap handler. Having re-read the specification, I realise I am wrong (the trap handler, if used, is not required to emulate the misaligned load/store using aligned loads and stores).

I think this solution "works" because needsStackRealignment only returns true if canRealignStack returns true. I shall go back to the solution involving detecting ILP32E and the D extension (and potentially the sizes of the used registers).