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.
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
