Page MenuHomePhabricator

Fix ABI breakage with noimplicitfloat and varargs functions
Needs ReviewPublic

Authored by asl on May 29 2019, 7:01 PM.

Details

Summary

Currently we have a very nasty ABI breakage, because we will pass arguments in XMM registers, however the callee is expecting them on stack. Note that it is perfectly fine to spill XMM registers on stack even for functions with noimplicitfloat attribute: the spill is guarded by additional check and only happens if some XMM registers are live-ins with arguments.

Diff Detail

Event Timeline

asl created this revision.May 29 2019, 7:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2019, 7:01 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
RKSimon added a subscriber: llvm-commits.

This makes sense, I think. Can we update LangRef and clang's command-line reference so it's clear this is actually the expected behavior?

asl added a comment.May 30 2019, 9:29 PM

@efriedma The LangRef states that "This attributes disables implicit floating-point instructions". I believe that the definition of "implicit" is very vague here. And clang cmdline documentation does not explain "-mno-implicit-float" at all. What you'd suggest? :)

asl edited the summary of this revision. (Show Details)May 30 2019, 9:30 PM

The way the definition is written is a little backwards. Really, it means "The compiler will not generate any floating-point instructions unless there is an explicit floating-point operation."

Then maybe worth adding a quick explanation of what constructs count as an explicit floating-point operation. This falls into two categories. One, expressions where a value has floating-point type, vector type, or a struct type that contains a floating-point or vector type. Two, anywhere the platform's ABI for a call would require the use of float registers, possibly including varargs function definitions which don't have any explicit float arguments. Maybe also mention that this doesn't control the synthesis of calls to certain runtime functions, like memcpy or compiler-rt builtins.

The definition of a "floating-point instruction" is target-specific, and probably obvious to anyone who cares, so I think we can continue to use that term without defining it.

Note that on AArch64, there is no equivalent to x86's use of "al" for varargs calls.

asl added a comment.May 31 2019, 6:22 PM

Note that on AArch64, there is no equivalent to x86's use of "al" for varargs calls.

Correct. Everywhere else we're using the same ABI regardless of this flag.

salim.nasser added a subscriber: salim.nasser.EditedJun 10 2019, 1:21 PM

Note that this change will expose bug 42219 ([X86_64] Variadic functions unconditionally spill %XMM registers). In that bug report I wrote:

"Specifically this behavior may become a problem for OS kernel code compiled with -no-implicit-float when bug 36507 is fixed (https://reviews.llvm.org/D62639).

Previously, variadic functions compiled with -no-implicit-float could safely be used even with the SSE unit disabled. If we fix 36507 without fixing the present bug, such code will lead to a crash at runtime due to reading XMM registers.

I understand that it is arguable whether or not that would be a bug. For example AArch64 variadic functions always unconditionally access the vector registers. But that is because the AArch64 ABI does not provide a way to avoid this. Whereas the X86_64 ABI *does*."