[cfe-commits] [PATCH] Support MIPS n32 ABI in the Clang driver

[Simon Atanasyan]

On Thu, Oct 11, 2012 at 7:23 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> On 11 October 2012 10:43, Simon Atanasyan <satanasyan at mips.com> wrote:
>> Please review the patch.
>
> +      StringRef MultiarchSuffix = ::getMultiarchSuffix(TargetArch, Args);
>
> Why the ::?

There is GCCInstallationDetector.getMultiarchSuffix() and I tried to
avoid an ambiguity. In the new attached patch I just rename static
getMultiarchSuffix to getTargetMultiarchSuffix.

> +// FIXME: There is the same routine in the Tools.cpp.
> +static bool hasMipsN32ABIArg(const ArgList &Args) {
> +  Arg *A = Args.getLastArg(options::OPT_mabi_EQ);
> +  return A && (A->getValue(Args) == StringRef("n32"));
> +}
>
> Would making it a method of Generic_GCC fix this?

I'm not sure it is better than keeping a tiny code duplication. It
definitely removes the code duplication. But it will be the first and
the only so specialized function in Generic_GCC. It is not clear why
we do not factor out other checking expressions like
"Args.hasArg(options::OPT_static)" to the separate functions in the
Generic_GCC class.

> Why have you moved getMultilibDir? It makes the patch harder to read.

I want to group all Mips related helper function in one place. But you
are right - that pollutes the patch and should be done in separate
commit.

Please review new patch.

--
Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mips-n32.patch
Type: application/octet-stream
Size: 10427 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121011/95d5d8ef/attachment.obj>