On Fri, Aug 15, 2014 at 2:03 PM, Reid Kleckner <[email protected]> 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 <[email protected]> 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 <[email protected]> >> wrote: >> >>> On 14 August 2014 19:16, Nico Weber <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
