<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Mar 16, 2018 at 4:09 AM Alex Bradbury via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 16 March 2018 at 10:38, Peter Smith via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> On 15 March 2018 at 19:12, Friedman, Eli <<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>> wrote:<br>
>> Having ARMv7a instructions in an ARMv4t file shouldn't be a problem: a<br>
>> function should be allowed to override the CPU attributes to generate code<br>
>> for a newer target.  This is generally done using the "target" function<br>
>> attribute.  If this doesn't work correctly, we should fix it.  It looks like<br>
>> it's currently broken; testcase:<br>
>><br>
>> void g();<br>
>> __attribute__((target("thumb,arch=cortex-a53")))<br>
>> void f() { g(); }<br>
>><br>
><br>
> Hmmm, allowing that makes life much more complicated. For example I<br>
> can also write:<br>
> void g();<br>
> __attribute__((target("thumb,arch=cortex-m0")))<br>
> void f() { g(); }<br>
><br>
> void i();<br>
> __attribute__((target("arm,arch=cortex-a53")))<br>
> void h() { i(); }<br>
><br>
> With -mcpu=cortex-m0 and get ARM code within an object claiming to be<br>
> Thumb only with no errors or warnings, with no chance of a linker<br>
> detecting a mismatch either.<br>
<br>
I think we can all agree that there should be no real problem with<br>
instruction selection when adding these sorts of target attributes. As<br>
you point out below, the problems start occurring when it gets to the<br>
MC layer and object emission.<br>
<br>
As the author of the well-intentioned cleanup patch that unmasked this<br>
issue, I'd like to thank you for putting in the time to delve into<br>
things. The patches in question were:<br>
* <a href="https://reviews.llvm.org/rL321707" rel="noreferrer" target="_blank">https://reviews.llvm.org/rL321707</a><br>
* <a href="https://reviews.llvm.org/rL321692" rel="noreferrer" target="_blank">https://reviews.llvm.org/rL321692</a><br>
<br>
> I think that part of this is the same problem that is observed in<br>
> PR36542 the ARMAsmBackend that is responsible for widening the tail<br>
> call to a Thumb2 branch is created with ARMv4T which doesn't support<br>
> Thumb1. There has been a recent change that threads through the<br>
> existing SubtargetInfo instead of recreating it from the triple alone.<br>
> It is worth mentioning that the object level BuildAttributes do not<br>
> include Thumbv7a which is misleading to a linker as it will be<br>
> expecting no ARMv7A in the object.<br>
><br>
> Has there already been a discussion about what per function<br>
> code-generation with BuildAttributes higher than the base object<br>
> should mean in the context of capabilities of the ARMAsmBackend and<br>
> BuildAttributes? My thoughts right now are that if ARMAsmBackend is to<br>
> operate at an object level, rather than a per-function level then it<br>
> has to use the capabilities of the highest architecture in the file.<br>
> This also means giving the object BuildAttributes of the highest<br>
> architecture in the file, and giving an error if they contradict, for<br>
> example mixing Thumb Cortex-M0 and ARM Cortex-A53. If the<br>
> ARMAsmBackend could be made to work on a per-function level then there<br>
> is a chance that we could only widen the tail call to g() in f(), but<br>
> not elsewhere. To honestly describe this in the BuildAttributes we<br>
> would need to use per Section or per Function attributes though.<br>
><br>
> My suggestion to move forward here is:<br>
> - Recreate the SubtargetInfo based on the merge of all the Targets and<br>
> CPU information that we have seen, or warn/error if they are<br>
> incompatible.<br>
> - Ouput the Tag_CPU_arch BuildAttributes based on the merge of all the<br>
> Targets and CPU information that we have seen.<br>
><br>
> It is probably worth moving any discussion of this particular part to<br>
> PR36542 since it is somewhat Arm specific. I'll add this comment to<br>
> there.<br>
<br>
I'm not so sure this is ARM specific, as other targets might well<br>
encounter similar issues (even if there is no direct equivalent to<br>
build attributes, there are cases where information is encoded into<br>
ELF flags on a per-object basis).<br><br></blockquote><div><br></div><div>FWIW I've followed up in the bug with both a high level description of how these things work (and should work) as well as what to do for things that are encoded on a per-object basis.</div><div><br></div><div>-eric </div></div></div>