<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Message: 4<br>
Date: Thu, 26 May 2011 13:13:10 +0100<br>
From: "James Molloy" <<a href="mailto:james.molloy@arm.com">james.molloy@arm.com</a>><br>
Subject: [cfe-dev] [PATCH] Driver modifications for cross compilation<br>
To: <<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a>><br>
Message-ID: <000401cc1b9e$4796f540$d6c4dfc0$@<a href="mailto:molloy@arm.com">molloy@arm.com</a>><br>
Content-Type: text/plain; charset="windows-1252"<br>
<br>
Hi,<br>
<br>
<br>
<br>
I noticed that the Clang driver requires the user to set "-ccc-host-triple"<br>
for cross-compilation (if not using the -arch behaviour on Darwin). I<br>
thought this was a little strange as the option is not documented in the<br>
--help, is nonstandard and nonobvious.<br>
<br>
<br>
<br>
I delved a little deeper, and found a bunch of FIXMEs in the driver wanting<br>
to tablegen lots of if/else statements selecting specific architecture<br>
options (AddARMTargetArgs being the worst culprit). Adding to that, some of<br>
these monolithic functions were duplicated with static linkage in multiple<br>
files.<br>
<br>
<br>
<br>
I've factored most of these if/else blocks into one tablegen file, which<br>
when #included forms an "architecture definition database" from which one<br>
can query default CPUs for given architecture/OS, which architecture a CPU<br>
belongs to, and any other target-specific feature of the CPU defined in the<br>
.td file.<br>
<br>
<br>
<br>
The tablegen file is a list of known architecture variants and CPUs for<br>
different platforms; we are happy to maintain the ARM part of this.<br>
<br>
<br>
<br>
The upshot of this is that several monolithic functions have disappeared:<br>
<br>
* Tool{s,Chain}.cpp:GetARMTargetCPU (replaced with<br>
Arch->DefaultCpu)<br>
<br>
* Tool{s,Chain}.cpp:getLLVMArchSuffixForARM (replaced with<br>
Arch->Properties["LLVMArchSuffix"])<br>
<br>
<br>
<br>
And "Tools.cpp:Add{ARM,Sparc,MIPS,X86}TargetArgs" have been simplified to<br>
contain fewer if/elses.<br>
<br>
<br>
<br>
The above was all just general tidyup; after this refactor, a 3 line change<br>
in Driver.cpp now causes autodetection of the host triple from any -mcpu= or<br>
-march= options passed. For example:<br>
<br>
<br>
<br>
clang -mcpu=cortex-a8 test.c -o test.o -c<br>
<br>
<br>
<br>
will set the HostTriple to "armv7--", which obviously then enables<br>
AddARMTargetArgs to be called instead of AddX86TargetArgs, and correct ARM<br>
code is produced.<br>
<br>
<br>
<br>
I feel that this is a much more sane behaviour than mandating the use of<br>
-ccc-host-triple (although the behaviour of that option has not been altered<br>
and overrides any auto-detection); if a user specifies a CPU which is<br>
obviously part of an architecture, or a<br>
-march=something-that-isn't-the-host, the HostTriple should be detected<br>
automatically.<br>
<br>
<br>
<br>
As a result of this, taking the line of thought that -ccc-host-triple should<br>
be optional and should be able to be fully functionally replaced by<br>
-mcpu=/-march= and friends, I realised there was no way to dictate the host<br>
OS without use of -ccc-host-triple. So I added a -mos= option, which sets<br>
the OSAndEnvironment part of the llvm::Triple. This way, almost all<br>
functionality offered by overriding the triple (still wanted for power<br>
users) is available via more recognisable command line options.<br>
<br>
<br>
<br>
The patch I have attached is the entire patch + testcases, so is rather<br>
large. If accepted, I intend to send to the commit list in several chunks:<br>
<br>
<br>
<br>
1. Tablegen file, new files ArchDefs.h and ArchDefs.cpp (+ new tablegen<br>
backend to llvm-commits)<br>
<br>
- This patch should not alter the behaviour of Clang at all, hence no<br>
tests being added.<br>
<br>
2. First usecase of the ArchDefs class: the ARM target. Removes<br>
GetARMTargetCPU and getLLVMArchSuffixForARM, and changes the default command<br>
line behaviour to infer target triple from -mcpu/-march if present.<br>
<br>
- As there is now a use case for the ArchDefs class, testcases will<br>
be added to this patch.<br>
<br>
3. Refactoring intel / mips target specifics to use ArchDefs. This is the<br>
most likely patch to cause problems with other uses that I haven't forseen.<br>
<br>
4. Adding -mos= and making it have an effect on the host triple.<br>
<br>
<br>
<br>
Patch 2. also contains several bugfixes to the ARM driver, namely:<br>
<br>
* ARM target driver would not detect -mabi=eabi/gnueabi correctly.<br>
<br>
* ARM target driver when setting -mabi= would not update the Triple and so<br>
the float-abi check later would be incorrect.<br>
<br>
* ARM target driver did not have knowledge of several ARM cores:<br>
Cortex-A{8,9,15}, Cortex-R{4,5,7}, Cortex-M{0,3,4}, ARM11MPCore, ARM1176.<br>
<br>
<br>
<br>
Review of both the patch and the intended functional change would be<br>
excellent.<br>
<br>
<br>
<br>
Cheers,<br>
<br>
<br>
<br>
James<br>
-------------- next part --------------<br>
An HTML attachment was scrubbed...<br>
URL: <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment.html</a><br>
-------------- next part --------------<br>
A non-text attachment was scrubbed...<br>
Name: clang-all.patch<br>
Type: application/octet-stream<br>
Size: 38709 bytes<br>
Desc: not available<br>
Url : <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment.obj" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment.obj</a><br>
-------------- next part --------------<br>
A non-text attachment was scrubbed...<br>
Name: tablegen-clang-gen-arch-defs.patch<br>
Type: application/octet-stream<br>
Size: 7289 bytes<br>
Desc: not available<br>
Url : <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment-0001.obj" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/attachments/20110526/e2d1ac2d/attachment-0001.obj</a><br>
<br>
------------------------------<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br>
<br>
End of cfe-dev Digest, Vol 47, Issue 83<br>
***************************************<br>
</blockquote></div><br>Hello,<br><br>Sorry to barge in, but I found the subject interesting so I delved into the patch, and I have a couple of comments.<br><br>If I may:<br><pre style="font-family: arial,helvetica,sans-serif; margin-left: 40px;">
<font size="2">+ typedef std::map<llvm::StringRef, ArchDef*> StringArchMap;
+ typedef std::map<llvm::StringRef, CpuDef*> StringCpuMap;</font></pre>Why use pointers here ? Plain values would avoid leaks.<br><br><br>Continuing:<br><br><div style="margin-left: 40px;"><font size="2">+ if(!Archs[id]) Archs[id] = new ArchDef;<br>
+ Archs[id]->ArchID = id;<br>+ Archs[id]->OS = os;<br>+ Archs[id]->Name = name;<br>+ Archs[id]->DefaultCpuName = defaultcpuname;</font><br></div><pre style="font-family: arial,helvetica,sans-serif;">
<font size="2">This involves a lot of lookups (which in turns involve a lot of calls to strcmp), the equivalent could be achieved with:</font><br></pre><div style="margin-left: 40px;"><font size="2">ArchDef *& A = Archs[id]; // default constructed value is 0<br>
if (!A) { A = new ArchDef; }<br></font><font size="2">A->ArchiID = id;</font><br><font size="2">A->OS = os;</font><br><font size="2">A->Name = name;</font><br><font size="2">A->DefaultCpuName = defaultcpuname;</font><br>
<br></div>If overwriting is not a feature, then the whole lot should be wrapped into the block of the if <br><br><br>Cheers,<br>-- Matthieu<br>