[PATCH] D11815: Pass subtarget feature "force-align-stack"

[Vasileios Kalintiris via cfe-commits]

vkalintiris added a subscriber: vkalintiris.
vkalintiris added a comment.

In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:

> The cc1 option "-mstackrealign" now means "force stack realignment" rather than "allow stack realignment" and causes function attribute "stackrealign" to be attached to the functions in the IR.


Please, correct me if I'm wrong but I believe that the -force-align-stack option. which was removed in http://reviews.llvm.org/D11814, was x86-specific (in the sense that it was only really tested in X86) and almost always accompanied by a -stack-alignment value that was equal or greater than the requirements of the ABI.

With this change the -mstackrealign option will attach the "stackrealign" attribute on every function definition (and declaration) without specifying a valid -stack-alignment value.

I suspect that this option will be broken for every Target that relies only on the maximum alignment provided by MachineFrameInfo (see X86FrameLowering::calculateMaxStackAlign() for the correct way to handle this).

Is this the intended behaviour here? I'm a little bit cautious because this option would be exposed from the Clang frontend and our users would generate bad code if they were to try this option.

For example, on a Mips target, where the O32 ABI requires either way an 8-byte alignment, we would generate redundant code for realigning the stack to a 4-byte alignment if a function contains objects with maximum alignment of 4-bytes (see attached files to get an idea).

F803573: main.s <http://reviews.llvm.org/F803573>

F803574: main.c <http://reviews.llvm.org/F803574>

F803575: main.ll <http://reviews.llvm.org/F803575>


http://reviews.llvm.org/D11815