[cfe-dev] Cleaning up ARM parts of the Clang driver

[Eli Friedman]

On Tue, Oct 4, 2011 at 4:09 AM, James Molloy <james.molloy at arm.com> wrote:
> Does anyone have any opinions or answers to the following? Thanks!
>
> 1:
>
>   // Select the ABI to use.
>   //
>   // FIXME: Support -meabi.
>   const char *ABIName = 0;
>   if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ)) {
>     ABIName = A->getValue(Args);
>   } else {
>     // Select the default based on the platform.
>     switch(Triple.getEnvironment()) {
>     case llvm::Triple::GNUEABI:
>       ABIName = "aapcs-linux";
>       break;
>     case llvm::Triple::EABI:
>       ABIName = "aapcs";
>       break;
>     default:
>       ABIName = "apcs-gnu";
>     }
>   }
>   CmdArgs.push_back("-target-abi");
>   CmdArgs.push_back(ABIName);
>
> I don’t really think the target ABI needs to be set if it is part of the
> triple (the else clause) – that should be inferred by all other tools by the
> triple.

You mean only pass down -target-abi when it's specified on the
command-line?  Sure.

> 2: The driver currently tries to pick a default CPU for the architecture if
> none is specified. I think this is wrong – if no CPU is specified then the
> compiler should *not* tune for a specific CPU. The user would expect if
> specifying –arch armv7, for the compiler to give an output that has good
> blended performance on all armv7 cores, not cortex-a8 specifically for
> example.

We don't actually have any "blended" tuning at the moment...cortex-a8
tuning is probably at least as good as anything else we have at the
moment.

> 4: These are hacks:
>
>   // Use software floating point operations?
>   if (FloatABI == "soft") {
>     CmdArgs.push_back("-target-feature");
>     CmdArgs.push_back("+soft-float");
>   }
>
>   // Use software floating point argument passing?
>   if (FloatABI != "hard") {
>     CmdArgs.push_back("-target-feature");
>     CmdArgs.push_back("+soft-float-abi");
>   }
>
> They’re referenced from lib/Basic/Targets.cpp – would it not be better for
> Targets.cpp to infer these from the other passed parameters? That’s actually
> a self-answering question, so my question really is: “Is there any reason
> why I can’t make the trivial change to make this so?”

Sounds fine.

> 7:
>   // Setting -mno-global-merge disables the codegen global merge pass.
> Setting
>   // -mglobal-merge has no effect as the pass is enabled by default.
>   if (Arg *A = Args.getLastArg(options::OPT_mglobal_merge,
>                                options::OPT_mno_global_merge)) {
>     if (A->getOption().matches(options::OPT_mno_global_merge))
>       CmdArgs.push_back("-mno-global-merge");
>   }
>
> It seems like this should be better handled by the sys::cl code in llc.

I'm not following.  sys::cl command-line options aren't relevant to
clang command-line processing.


I'm not sure about 3, 5, and 6; probably Daniel or Eric knows.

-Eli