> 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-feature-code-duplicated.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
