[llvm-dev] [RFC] Stop giving a default CPU to the LTO plugin?

[Friedman, Eli via llvm-dev]

On 3/15/2018 9:43 AM, Peter Smith via llvm-dev wrote:
> Hello everyone, this is most likely Arm specific, but could affect
> other targets where there is a somewhat complex relationship between
> the triple and mcpu option.
>
> At present when clang is used as a linker driver for the gold-plugin
> and when using and an explicit -mcpu is not given to clang, then clang
> will always generate a -Wl,-plugin-opt=mcpu=<default CPU> where the
> default CPU is based on the triple.
>
> I think that this is causing more problems than it is worth and I'd
> like clang to not pass -Wl,-plugin-opt=mcpu=<CPU> unless the
> -mcpu=<CPU> is explicitly on the clang command line.
>
> I'd like to know if I'm missing something important, and if there
> would be any support for a patch to not pass an implicit mcpu via
> plugin? There may also be a way of handling an inappropriate
> cpu/triple combination more gracefully.
>
> The full story of a problem with the Arm target is available at
> https://bugs.llvm.org/show_bug.cgi?id=36542 . A short description of
> the problem is:
> - Source files are compiled with -mthumb
> --target=arm-linux-androideabi -mcpu=cortex-a7,
> this records a triple of thumbv7a-linux-androideabi in the bitcode file.
> - The command line for the link step only passes --target=
> arm-linux-androideabi.
> - The default cpu passed to LTO for a triple arm-linux-androideabi is
> the very old arm7tdmi.
> - When the ARMAsmBackend is created from the combination of triple
> thumbv7a-linux-androideabi and cpu arm7tdmi the resultant target
> features only include Thumb1. Amongst other things this prevents MC
> from widening branch instructions to the Thumb2 wide branch.
> - Code generation prior to MC seems to prefer the attributes in the
> bitcode file (triple thumbv7a-linux-androideabi -mcpu=generic), this
> selects a narrow branch instruction for a tail call with the
> expectation it will be widened by MC. End result is a relocation out
> of range error.
>
> In summary the combination of -mcpu=arm7tdmi and a triple from the
> bitcode file of thumbv7a-linux-androideabi does not make sense, but
> when passed to LTO seems to:
> - Escape all the existing warnings about nonsensical triples [*].
> - On Arm targets at least; having a final link step without a cpu is
> likely to be quite common so I don't think that this will be a corner
> case.
> - The Build Attributes of the output ELF file get the triple ARMv4t
> from -mcpu=arm7tdmi which is lying to the linker as the file contains
> ARMv7a instructions.

Having ARMv7a instructions in an ARMv4t file shouldn't be a problem: a 
function should be allowed to override the CPU attributes to generate 
code for a newer target.  This is generally done using the "target" 
function attribute.  If this doesn't work correctly, we should fix it.  
It looks like it's currently broken; testcase:

void g();
__attribute__((target("thumb,arch=cortex-a53")))
void f() { g(); }

>
> My personal preference is that we shouldn't pass an implicit CPU
> derived from the triple, at least for Arm targets. If that isn't
> possible I think that we should make sure we detect inappropriate
> cpu/triple combinations and either warn or error. Does anyone have any
> strong opinions on which way to go? I'm happy to work on a patch to
> improve this.
>
> I know that this can be worked around by explicitly passing a cpu or
> more specific triple to the link line, however this is often not done
> in existing builds.
>
> Thanks in advance for any suggestions
It doesn't really make sense to override the target specified in the 
bitcode; I agree we shouldn't do that by default.  But I don't think 
changing that actually fixes the underlying bug.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project