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

[Alex Bradbury via llvm-dev]

On 16 March 2018 at 10:38, Peter Smith via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> On 15 March 2018 at 19:12, Friedman, Eli <efriedma at codeaurora.org> wrote:
>> 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(); }
>>
>
> Hmmm, allowing that makes life much more complicated. For example I
> can also write:
> void g();
> __attribute__((target("thumb,arch=cortex-m0")))
> void f() { g(); }
>
> void i();
> __attribute__((target("arm,arch=cortex-a53")))
> void h() { i(); }
>
> With -mcpu=cortex-m0 and get ARM code within an object claiming to be
> Thumb only with no errors or warnings, with no chance of a linker
> detecting a mismatch either.

I think we can all agree that there should be no real problem with
instruction selection when adding these sorts of target attributes. As
you point out below, the problems start occurring when it gets to the
MC layer and object emission.

As the author of the well-intentioned cleanup patch that unmasked this
issue, I'd like to thank you for putting in the time to delve into
things. The patches in question were:
* https://reviews.llvm.org/rL321707
* https://reviews.llvm.org/rL321692

> I think that part of this is the same problem that is observed in
> PR36542 the ARMAsmBackend that is responsible for widening the tail
> call to a Thumb2 branch is created with ARMv4T which doesn't support
> Thumb1. There has been a recent change that threads through the
> existing SubtargetInfo instead of recreating it from the triple alone.
> It is worth mentioning that the object level BuildAttributes do not
> include Thumbv7a which is misleading to a linker as it will be
> expecting no ARMv7A in the object.
>
> Has there already been a discussion about what per function
> code-generation with BuildAttributes higher than the base object
> should mean in the context of capabilities of the ARMAsmBackend and
> BuildAttributes? My thoughts right now are that if ARMAsmBackend is to
> operate at an object level, rather than a per-function level then it
> has to use the capabilities of the highest architecture in the file.
> This also means giving the object BuildAttributes of the highest
> architecture in the file, and giving an error if they contradict, for
> example mixing Thumb Cortex-M0 and ARM Cortex-A53. If the
> ARMAsmBackend could be made to work on a per-function level then there
> is a chance that we could only widen the tail call to g() in f(), but
> not elsewhere. To honestly describe this in the BuildAttributes we
> would need to use per Section or per Function attributes though.
>
> My suggestion to move forward here is:
> - Recreate the SubtargetInfo based on the merge of all the Targets and
> CPU information that we have seen, or warn/error if they are
> incompatible.
> - Ouput the Tag_CPU_arch BuildAttributes based on the merge of all the
> Targets and CPU information that we have seen.
>
> It is probably worth moving any discussion of this particular part to
> PR36542 since it is somewhat Arm specific. I'll add this comment to
> there.

I'm not so sure this is ARM specific, as other targets might well
encounter similar issues (even if there is no direct equivalent to
build attributes, there are cases where information is encoded into
ELF flags on a per-object basis).

Best,

Alex