Restrict ARM assembly header flags to the header and fix its behaviour for architecture attributes #21131
Comments
|
assigned to @rengolin |
|
Ignore item #4, as this is not what GAS does and I agree we should not do. Flags like vfp2 shall cancel NEON and CPU flags shall not change FP flags. |
|
*** Bug llvm/llvm-bugzilla-archive#21319 has been marked as a duplicate of this bug. *** |
1 similar comment
|
*** Bug llvm/llvm-bugzilla-archive#21319 has been marked as a duplicate of this bug. *** |
|
Just to be clear, moving the flags to the header won't fix the lack of attribute setting behaviour. This bug is about fixing that behaviour without introducing unexpected behaviour, like setting file-level attributes on .fpu/.cpu/.arch flags across the file. |
|
We've discussed this in the US dev meeting and the consensus is that using push/pop like MIPS for directives is the best way forward. We hope that other compilers follow suit, but we'll still support the old syntax for a long time, maybe with a warning that this is bad behaviour. As of r223147, cpu and fpu directives set the attribute bits, changing the properties of the file, failing to warn on real miscompilation errors, like: function HardDIV: function MiscompiledSoftDIV: A better approach would be: function HardDIV: Or something similar, so that MiscompiledSoftDiv can correctly error out. This also allows any inline assembly code to use whatever cpu/fpu/isa they want and make sure that the context will be restored afterwards. Implementing the old syntax will be an alias to .push without a pop. An alternative syntax created from that will be: function HardDIV: Which is weird, but acceptable. We could error out, warn or even ignore. Or maybe even make this the default syntax. We need to discuss this further when time comes. |
|
After long discussions, I think even if we do implement push/pop, the acceptance outside of Clang/LLVM will be zero, and that will create a problem for developers, that will end up ifdef'ing to go around LLVM vs. other compilers. I think a compromise is to produce a warning when those flags are used after the header in assembly files or in inline assembly. |
|
So, this seems to have little importance to everyone else, so I'll just abandon this revision. If anyone feels like it, they can continue from where I stopped. |
|
mentioned in issue #21161 |
|
mentioned in issue llvm/llvm-bugzilla-archive#21319 |
1 similar comment
|
mentioned in issue llvm/llvm-bugzilla-archive#21319 |
Quuxplusone
added
the
wontfix
Extended Description
As extensively discussed in this thread:
https://sourceware.org/ml/binutils/2014-08/msg00119.html
Some architecture flags (fpu/cpu/arch/arch_extension/eabi_attribute) do not belong in the middle of assembly files. This includes hand-coded, inline and included assembly blocks which should not change the behaviour of the compile unit mid-way through the compilation process.
GNU folks are (understandably) reluctant to restrict this unintentional, and possibly destructive, functionality, even though they agree that this is not how it should be.
The technical consensus seems to be that those flags should be either restricted to the header (ie. before any code/text), or on a first come, first served basis, though the latter creates problems if the original file didn't have any flags and some include/inline asm added the wrong one. Current behaviour is last come, last served, which is plain wrong, if in text, but ok if in the header.
We can start by adding a warning that could be turned into an error (via -Werror), and see how it goes from there. If at least we warn, the user can't complain that further steps (linking, execution) fail without warning.
My proposal is the following:
Header directives shall warn when used in text and only change the behaviour for instruction selection purposes, not for module-wide properties (like build attributes).
If header directives are not present in the header, module-wide properties will be inferred from the command line, architectural descriptions, etc. even if they are present in the text.
The semantics of directives in text will be that they will change only the specific behaviour they're related to and they will be valid until another similar directive is issues or the end of file (compile unit) is reached.
Related flags shall compliment each other. So, .cpu cortex-a8 shall set VFP3 and NEON, while .fpu vfp2 shall only unset VFP3, not NEON and not move the CPU back to where VFP2 was the default.
Duplicate header directives in the header generates a different warning, but are accepted on a last come, last served basis.
Both warnings should become errors when in -Werror mode.
cheers,
--renato
The text was updated successfully, but these errors were encountered: