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

[Matthieu Monrocq]

> Message: 4
> Date: Thu, 26 May 2011 13:13:10 +0100
> From: "James Molloy" <james.molloy at arm.com>
> Subject: [cfe-dev] [PATCH] Driver modifications for cross compilation
> To: <cfe-dev at cs.uiuc.edu>
> Message-ID: <000401cc1b9e$4796f540$d6c4dfc0$@molloy at arm.com>
> Content-Type: text/plain; charset="windows-1252"
>
> 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.
>
>  4. Adding -mos= and making it have an effect on the host triple.
>
>
>
> Patch 2. also contains several bugfixes to the ARM driver, namely:
>
>  * ARM target driver would not detect -mabi=eabi/gnueabi correctly.
>
>  * ARM target driver when setting -mabi= would not update the Triple and so
> the float-abi check later would be incorrect.
>
>  * ARM target driver did not have knowledge of several ARM cores:
> Cortex-A{8,9,15}, Cortex-R{4,5,7}, Cortex-M{0,3,4}, ARM11MPCore, ARM1176.
>
>
>
> Review of both the patch and the intended functional change would be
> excellent.
>
>
>
> Cheers,
>
>
>
> James
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment.html
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: clang-all.patch
> Type: application/octet-stream
> Size: 38709 bytes
> Desc: not available
> Url :
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment.obj
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: tablegen-clang-gen-arch-defs.patch
> Type: application/octet-stream
> Size: 7289 bytes
> Desc: not available
> Url :
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment-0001.obj
>
> ------------------------------
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
> End of cfe-dev Digest, Vol 47, Issue 83
> ***************************************
>

Hello,

Sorry to barge in, but I found the subject interesting so I delved into the
patch, and I have a couple of comments.

If I may:

+        typedef std::map<llvm::StringRef, ArchDef*> StringArchMap;
+        typedef std::map<llvm::StringRef, CpuDef*>  StringCpuMap;

Why use pointers here ? Plain values would avoid leaks.


Continuing:

+ if(!Archs[id]) Archs[id] = new ArchDef;
+ Archs[id]->ArchID = id;
+ Archs[id]->OS = os;
+ Archs[id]->Name = name;
+ Archs[id]->DefaultCpuName = defaultcpuname;

This involves a lot of lookups (which in turns involve a lot of calls
to strcmp), the equivalent could be achieved with:

ArchDef *& A = Archs[id]; // default constructed value is 0
if (!A) { A = new ArchDef; }
A->ArchiID = id;
A->OS = os;
A->Name = name;
A->DefaultCpuName = defaultcpuname;

If overwriting is not a feature, then the whole lot should be wrapped into
the block of the  if


Cheers,
-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110526/b77a55a1/attachment.html>