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:
macho-arch-trivial.patch
Description: Binary data
More comprehensive alternative patch:
macho-arch-broad.patch
Description: Binary data
On 23/07/2014, at 7:40 AM, Rafael Espíndola <[email protected]> wrote: > can you add a testcase to the patch? > > On 16 July 2014 04:29, Stephen Drake <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
