[PATCH] Call powerpc-darwin external tools with -arch ppc

[Stephen Drake]

Hi Rafael,

Thanks for having a look at this.  I've updated my patch to add some tests to Driver/darwin-arch-default.c, which I hope is an appropriate place for them.

After posting the initial patch, I had a go at resolving the FIXME code instead of just copy-pasting it.  I'm also attaching the resulting alternative patch that uses getDefaultUniversalArchName() instead of getArchName() as the starting point for MachO::getMachOArchName().  Having checked the history, I don't think that I'm subverting the intent of either function, but since I'm both an LLVM and C++ noob, it could use some fairly strenuous scrutiny.  In particular, I changed the return type of getDefaultUniversalArchName() from std::string to StringRef, and I'm not sure if that's allowed.

Cheers,
Steve

Original patch updated to add tests:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: macho-arch-trivial.patch
Type: application/octet-stream
Size: 2727 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140725/bd735d0e/attachment.obj>
-------------- next part --------------


More comprehensive alternative patch:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: macho-arch-broad.patch
Type: application/octet-stream
Size: 4610 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140725/bd735d0e/attachment-0001.obj>
-------------- next part --------------



On 23/07/2014, at 7:40 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

> can you add a testcase to the patch?
> 
> On 16 July 2014 04:29, Stephen Drake <steve at synergyconsultingnz.com> wrote:
>> Hi,
>> 
>> I'm attaching a patch to call external tools for powerpc-darwin with "-arch ppc" instead of "-arch powerpc", so as to be compatible with the cctools assembler and ld64 linker.
>> 
>> This bug report describing the case has been closed, but I see the same behaviour now:
>> http://llvm.org/bugs/show_bug.cgi?id=3830
>> 
>> The bug report also contains a patch, which along with similar code further up at lib/Driver/Tools.cpp:4721, forms the basis for this patch.  The existing code is marked "FIXME", which I have carried over - if someone can explain the problem and/or suggest a better way to implement this I'll be happy to give it a shot.  Perhaps ToolChain::getDefaultUniversalArchName() is the correct way to find the arch name here?
>> 
>> Cheers,
>> Steve
>> 
>> 
>> Index: lib/Driver/Tools.cpp
>> ===================================================================
>> --- lib/Driver/Tools.cpp        (revision 213105)
>> +++ lib/Driver/Tools.cpp        (working copy)
>> @@ -5313,12 +5313,22 @@
>> 
>> void darwin::MachOTool::AddMachOArch(const ArgList &Args,
>>                                      ArgStringList &CmdArgs) const {
>> +  llvm::Triple::ArchType Arch = getToolChain().getArch();
>>   StringRef ArchName = getMachOToolChain().getMachOArchName(Args);
>> 
>>   // Derived from darwin_arch spec.
>>   CmdArgs.push_back("-arch");
>> -  CmdArgs.push_back(Args.MakeArgString(ArchName));
>> 
>> +  // FIXME: Remove these special cases.
>> +  if (Arch == llvm::Triple::ppc)
>> +    CmdArgs.push_back("ppc");
>> +  else if (Arch == llvm::Triple::ppc64)
>> +    CmdArgs.push_back("ppc64");
>> +  else if (Arch == llvm::Triple::ppc64le)
>> +    CmdArgs.push_back("ppc64le");
>> +  else
>> +    CmdArgs.push_back(Args.MakeArgString(ArchName));
>> +
>>   // FIXME: Is this needed anymore?
>>   if (ArchName == "arm")
>>     CmdArgs.push_back("-force_cpusubtype_ALL");
>> 
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>