Message: 4
Date: Thu, 26 May 2011 13:13:10 +0100
From: “James Molloy” <james.molloy@arm.com>
Subject: [cfe-dev] [PATCH] Driver modifications for cross compilation
To: <cfe-dev@cs.uiuc.edu>
Message-ID: <000401cc1b9e$4796f540$d6c4dfc0$@molloy@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:
- 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.
- 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.
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.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@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-devEnd 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