On 9 January 2014 11:40, Kristof Beyls <[email protected]> wrote: > The .init_array should be used on any platform that follows the AArch64 > C++ ABI, which I think would be any platform using ELF as an object file > format. I think that the patch does implement that (but this is an area > of the code I'm not very familiar with). > > As for factoring out defaulting -fuse-init-array: the decision logic > for Linux::addClangTargetOptions and Generic_GCC:addClangTargetOptions > is different, so the only thing to factor out would be > + if (DriverArgs.hasFlag(options::OPT_fuse_init_array, > + options::OPT_fno_use_init_array, > + UseInitArrayDefault)) > + CC1Args.push_back("-fuse-init-array"); > The conditions on when to add this by default would remain different > between Generic_GCC and Linux, and therefore the 2 implementations of > addClangTargetOptions in the Generic_GCC and Linux subclasses need to > remain. > Generic_GCC: > bool UseInitArrayDefault = getTriple().getArch() == llvm::Triple::aarch64; > > Linux: > const Generic_GCC::GCCVersion &V = GCCInstallation.getVersion(); > bool UseInitArrayDefault = > !V.isOlderThan(4, 7, 0) || > getTriple().getArch() == llvm::Triple::aarch64 || > getTriple().getEnvironment() == llvm::Triple::Android; > > Or am I missing something obvious here? > Is it worthwhile to factor out only the 4 lines that add the -fuse-init-array > option into a separate function?
Yes. First, from your description you probably want to change Generic_ELF instead of Generic_GCC. Second, Linux inherits from Generic_ELF, so I think what you can do is: remove Linux::addClangTargetOptions add a Generic_ELF::addClangTargetOptions Combine the logic you added with the one that is currently in the linux toolchain. That is, UseInitArrayDefault would be Arch == llvm::Triple::aarch64 || (OS == Linux && (!V.isOlderThan(4, 7, 0) || getTriple().getEnvironment() == llvm::Triple::Android)) Cheers, Rafael _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
