[cfe-dev] FW: [PATCH] Driver modifications for cross compilation

[James Molloy]

Hi all,

As part of this patch I've noticed a part of the driver that interacts badly
with these changes.

driver.cpp:ParseProgName looks for a prefix on the name of the clang
executable (arm-none-eabi-clang, for example), and sets the target triple
accordingly. It does this in what I would consider an incorrect way however,
by forging a "-ccc-host-triple" argument and prepending it to the argument
list.

My patch assumes that if -ccc-host-triple is specified, it is specified by
the user and as such overrides any -mcpu=/-march=/-mos= inferred choice. I'd
argue that the correct behaviour in ParseProgName should be to set
Driver::DefaultHostTriple.

This would function as a hint or a starting point for the architecture
detection code, and allow -march=/-mos=/-mcpu= to override it.

The triple selection could then be pictured as a hierarchy, with lower
levels overriding those above:

  * llvm::sys::getHostTriple()
  * Any inferred executable prefix (ParseProgName)
  * Any -march, -mcpu or -mos arguments.
  * -ccc-host-triple

Do you have any thoughts on this? functionally I can't see how it would have
any change from the current mechanism (which is the same as the above
hierarchy minus the third item).

Cheers,

James

> -----Original Message-----
> From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu]
> On Behalf Of James Molloy
> Sent: 27 May 2011 10:05
> To: Roman Divacky
> Cc: cfe-dev at cs.uiuc.edu
> Subject: Re: [cfe-dev] [PATCH] Driver modifications for cross
> compilation
> 
> Hi Roman,
> 
> This is certainly possibly just not added. It is a two line change
> (adding a multiclass) - I will add this to the patch before I send it
> to cfe-commits.
> 
> Cheers,
> 
> James
> 
> > -----Original Message-----
> > From: Roman Divacky [mailto:rdivacky at freebsd.org]
> > Sent: 27 May 2011 09:46
> > To: James Molloy
> > Cc: cfe-dev at cs.uiuc.edu
> > Subject: Re: [cfe-dev] [PATCH] Driver modifications for cross
> > compilation
> >
> > On Thu, May 26, 2011 at 01:13:10PM +0100, James Molloy wrote:
> > > Hi,
> > >
> > >
> > >
> > > I noticed that the Clang driver requires the user to set "-ccc-
> host-
> > triple"
> > > for cross-compilation (if not using the -arch behaviour on Darwin).
> I
> > > thought this was a little strange as the option is not documented
> in
> > the
> > > --help, is nonstandard and nonobvious.
> > >
> > >
> > >
> > > I delved a little deeper, and found a bunch of FIXMEs in the driver
> > wanting
> > > to tablegen lots of if/else statements selecting specific
> > architecture
> > > options (AddARMTargetArgs being the worst culprit). Adding to that,
> > some of
> > > these monolithic functions were duplicated with static linkage in
> > multiple
> > > files.
> > >
> > >
> > >
> > > I've factored most of these if/else blocks into one tablegen file,
> > which
> > > when #included forms an "architecture definition database" from
> which
> > one
> > > can query default CPUs for given architecture/OS, which
> architecture
> > a CPU
> > > belongs to, and any other target-specific feature of the CPU
> defined
> > in the
> > > .td file.
> > >
> > >
> > >
> > > The tablegen file is a list of known architecture variants and CPUs
> > for
> > > different platforms; we are happy to maintain the ARM part of this.
> > >
> > >
> > >
> > > The upshot of this is that several monolithic functions have
> > disappeared:
> > >
> > >   * Tool{s,Chain}.cpp:GetARMTargetCPU          (replaced with
> > > Arch->DefaultCpu)
> > >
> > >   * Tool{s,Chain}.cpp:getLLVMArchSuffixForARM  (replaced with
> > > Arch->Properties["LLVMArchSuffix"])
> > >
> > >
> > >
> > > And "Tools.cpp:Add{ARM,Sparc,MIPS,X86}TargetArgs" have been
> > simplified to
> > > contain fewer if/elses.
> > >
> > >
> > >
> > > The above was all just general tidyup; after this refactor, a 3
> line
> > change
> > > in Driver.cpp now causes autodetection of the host triple from any
> -
> > mcpu= or
> > > -march= options passed. For example:
> > >
> > >
> > >
> > >     clang -mcpu=cortex-a8 test.c -o test.o -c
> > >
> > >
> > >
> > > will set the HostTriple to "armv7--", which obviously then enables
> > > AddARMTargetArgs to be called instead of AddX86TargetArgs, and
> > correct ARM
> > > code is produced.
> > >
> > >
> > >
> > > I feel that this is a much more sane behaviour than mandating the
> use
> > of
> > > -ccc-host-triple (although the behaviour of that option has not
> been
> > altered
> > > and overrides any auto-detection); if a user specifies a CPU which
> is
> > > obviously part of an architecture, or a
> > > -march=something-that-isn't-the-host, the HostTriple should be
> > detected
> > > automatically.
> > >
> > >
> > >
> > > As a result of this, taking the line of thought that -ccc-host-
> triple
> > should
> > > be optional and should be able to be fully functionally replaced by
> > > -mcpu=/-march= and friends, I realised there was no way to dictate
> > the host
> > > OS without use of -ccc-host-triple. So I added a -mos= option,
> which
> > sets
> > > the OSAndEnvironment part of the llvm::Triple. This way, almost all
> > > functionality offered by overriding the triple (still wanted for
> > power
> > > users) is available via more recognisable command line options.
> > >
> > >
> > >
> > > The patch I have attached is the entire patch + testcases, so is
> > rather
> > > large. If accepted, I intend to send to the commit list in several
> > chunks:
> > >
> > >
> > >
> > >   1. Tablegen file, new files ArchDefs.h and ArchDefs.cpp   (+ new
> > tablegen
> > > backend to llvm-commits)
> > >
> > >        - This patch should not alter the behaviour of Clang at all,
> > hence no
> > > tests being added.
> > >
> > >   2. First usecase of the ArchDefs class: the ARM target. Removes
> > > GetARMTargetCPU and getLLVMArchSuffixForARM, and changes the
> default
> > command
> > > line behaviour to infer target triple from -mcpu/-march if present.
> > >
> > >        - As there is now a use case for the ArchDefs class,
> testcases
> > will
> > > be added to this patch.
> > >
> > >   3. Refactoring intel / mips target specifics to use ArchDefs.
> This
> > is the
> > > most likely patch to cause problems with other uses that I haven't
> > forseen.
> >
> > FreeBSD needs to be able to specify alias for the arch as we want to
> > accept
> > both "amd64" and "x86_64" in the target triple. Is this somehow
> > possible
> > with your patch? If not, can it be added?
> >
> > roman
> 
> 
> -- IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy
> the information in any medium.  Thank you.
> 
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev