Thanks for the quick and insightful responses Rafael!
I've attached a new patch taking into account your feedback.
The reason why I first added addClangTargetOptions to Generic_GCC
is that the driver always produces a Generic_GCC toolchain for
bare metal targets.
In the patch, I've adapted that so that the driver now produces a
Generic_ELF toolchain for bare metal targets with an environment/ABI
indicating an ELF-based bare metal target:
--- a/lib/Driver/Driver.cpp
+++ b/lib/Driver/Driver.cpp
@@ -1992,7 +1992,20 @@ const ToolChain &Driver::getToolChain(const ArgList
&Args,
TC = new toolchains::XCore(*this, Target, Args);
break;
}
- TC = new toolchains::Generic_GCC(*this, Target, Args);
+ switch (Target.getEnvironment()) {
+ case llvm::Triple::GNU:
+ case llvm::Triple::GNUEABI:
+ case llvm::Triple::GNUEABIHF:
+ case llvm::Triple::GNUX32:
+ case llvm::Triple::EABI:
+ case llvm::Triple::EABIHF:
+ case llvm::Triple::ELF:
+ TC = new toolchains::Generic_ELF(*this, Target, Args);
+ break;
+ default:
+ TC = new toolchains::Generic_GCC(*this, Target, Args);
+ break;
+ }
break;
}
}I think this is an improvement, but I'm not entirely sure if Generic_ELF should be created for GNU and GNUX32. Kristof > -----Original Message----- > From: Rafael Espíndola [mailto:[email protected]] > Sent: 09 January 2014 17:00 > To: Kristof Beyls > Cc: llvm cfe; Tim Northover > Subject: Re: [PATCH] generate .init_array instead of .ctors sections for > AArch64 ELF targets > > 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
init_array_clang2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
