rfc and [patch]: Unify -mfpu and .fpu handling, let .fpu toggle features

[Nico Weber]

On Fri, Aug 15, 2014 at 2:03 PM, Reid Kleckner <rnk at google.com> wrote:

> What happens if I don't build the ARM backend? I use
> -DLLVM_TARGETS_TO_BUILD=X86 in cmake, for example. I believe we have a
> number of out-of-tree users of clang that do things like build Clang
> without building any targets.
>

That's a good point. I haven't tried it, but it probably won't work. A (to
me) reasonable fix might be to not translate -mfpu into frontend flags if
the arm backend isn't enabled?


> I think we have to suffer the duplication, or do some library splitting,
> unfortunately.
>
> This is actually very similar to a lot of problems we have. There are many
> layers where we need to know something about a target (TargetTransformInfo,
> anyone?) and we either work around it with late-binding-stuff (see target
> registry) or duplication (see the Target*.cpp files in Clang).
>
>
> On Fri, Aug 15, 2014 at 12:58 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> > What is the size impact in libclang? It links with lib/Driver, no?
>>
>> libclang.3.5.dylib grows by about 1.3 kB, so it's not much. (This make
>> sense, since Driver only needs one symbol from a single .o file
>> in libLLVMARMCodeGen.a, and that .o file doesn't need any symbols from
>> other cpp files, only some enums from a .inc file it includes. So the
>> linker only needs to load that single .o file.)
>>
>> > Clang and external tools depend on LLVM's APIs, containers and
>> > algorithms, but they're all (or should be) in a highly accessible
>> > area, and most certainly *not* in a source area.
>>
>> The new header included by clang is in
>> include/llvm/Target/ARMSubtargetCommon.h , in an include directory. If
>> that's not what you mean: What header are you referring to?
>>
>> On Fri, Aug 15, 2014 at 1:22 AM, Renato Golin <renato.golin at linaro.org>
>> wrote:
>>
>>> On 14 August 2014 19:16, Nico Weber <thakis at chromium.org> wrote:
>>> > Do folks think that this is ok as an incremental improvement than
>>> what's
>>> > currently in the tree? On the bug the suggestion was made to just
>>> duplicate
>>> > the fpu->feature mapping in two places, which would make this patch
>>> much
>>> > smaller (and which would undo the .fpu name fixes), but that seems
>>> like a
>>> > worse alternative to me.
>>>
>>> Nico,
>>>
>>> You're trying to solve two problems in one patch. The first problem is
>>> an .fpu directive parsing issue, just has to set the bit flag, like
>>> .arch, and the second is sharing the parser and bit flags with clang.
>>>
>>
>> Well, right now the feature flags are only needed in one place. I require
>> them in a second place, so I'm looking into ways to share them.
>>
>>
>>> Sometimes, solving one problem not in the best way, and then solving
>>> the other problem later is better from a VCS point of view. Also, they
>>> allow people to discuss the best implementation of the second problem
>>> *after* the original problem was fixed.
>>>
>>> I don't think anyone will agree that this is the *right* fix, but as
>>> an interim state, while we discuss a better patter for the second
>>> problem, it's acceptable, with a big FIXME, so that we clean that up
>>> once we agreed on the second problem, that is much greater than the
>>> first.
>>>
>>> Of course, if you're ok with the first problem waiting until we have a
>>> better solution for the second problem, than let's not rush ourselves
>>> and wait for a proper fix of both problems.
>>>
>>> But my argument so far has never been not to fix the second problem,
>>> just that the fix you propose is not good enough and will create other
>>> problems.
>>
>>
>> My perspective is that my patch is better than duplicating the code,
>> better than what's currently in the tree, and it doesn't make a future
>> grand refactoring harder if someone wants to do that. So I much prefer my
>> patch over just having a duplicate fpu name -> feature flags list in the
>> arm asm parser. (The current inconsistencies between just the fpu name
>> parsing between -mfpu and .fpu are a good example of this.)
>>
>> In my experience, "add a big FIXME and wait for a grand refactoring"
>> leads to a codebase full of FIXMEs and many pending refactorings, some of
>> which might never happen. I'm a bit surprised by you opposing this patch so
>> much, as it fixes a problem and doesn't make any of the things you propose
>> harder.
>>
>> (For comparison, here's how things would look if I just duplicate the
>> mapping. The patch is way shorter, but it's very easy to update one of the
>> two places when adding fpus. And with this patch, .fpu doesn't support a
>> bunch of fpus that -mfpu does support – because fpu names are listed in two
>> places and have fallen out of sync due to that.)
>>
>> Nico
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140815/f337b81c/attachment.html>