[PATCH] generate .init_array instead of .ctors sections for AArch64 ELF targets

[Kristof Beyls]

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:rafael.espindola at gmail.com]
> 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 <kristof.beyls at arm.com> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: init_array_clang2.patch
Type: application/octet-stream
Size: 4572 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140109/da619f73/attachment.obj>