[cfe-commits] Patch: support for new backend for Hexagon processor

[Tony Linthicum]

On 11/15/2011 6:19 PM, Eli Friedman wrote:
> On Tue, Nov 15, 2011 at 3:15 PM, Tony Linthicum<tlinth at codeaurora.org>  wrote:
>> On 11/14/2011 2:34 PM, Eli Friedman wrote:
>>
>>>
>>> --- a/include/clang/Basic/LangOptions.def
>>> +++ b/include/clang/Basic/LangOptions.def
>>> @@ -127,6 +127,7 @@ LANGOPT(SinglePrecisionConstants , 1, 0, "treating
>>> double-precision floating poi
>>>   LANGOPT(FastRelaxedMath , 1, 0, "OpenCL fast relaxed math")
>>>   LANGOPT(DefaultFPContract , 1, 0, "FP_CONTRACT")
>>>   LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
>>> +LANGOPT(hexagon_qdsp6_compat , 1, 0, "hexagon-qdsp6 backward
>>> compatibility")
>>>
>>> Please use a consistent naming convention.
>>>
>>
>> Sorry about the confusion in my previous response.  I thought you objected
>> to the flag conceptually.  I have addressed this in the attached patch.
>>   Please let me know if this is what you are looking for.
>
> Something more like HexagonQdsp6Compat would be better... but not a big deal.

I agree. :)  Done.


>>>
>>> Your changes in lib/Driver/ include a bunch of stuff which I don't
>>> really understand with little to no explanation.  What operating
>>> system are you trying to target?  Why are you using gcc to run the
>>> assembler and linker for a toolchain which you should have control
>>> over?
>>>
>>
>> The OS is a proprietary RTOS, and we also use a proprietary libc.  Are you
>> looking for more comments in the code to be added?
>
> Oh... I see.  You should be changing Driver::GetHostInfo rather than
> hijacking LinuxHostInfo for Hexagon; see how TCEHostInfo deals with it
> for an example.
>

Okay, we'll fix this.  Is this required to commit or can we fix this one 
afterward as well?


>
> No tabs, please.
>

Fixed.

> +//   if (Args.getLastArg(options::OPT_fprint_after_all)) {
> +//     CmdArgs.push_back("-print-after-all");
> + //  }
> +
>
> I assume you didn't mean to leave this in the patch?
>

Nope.  Removed.

>
>     // Pass the linker version in use.
> -  if (Arg *A = Args.getLastArg(options::OPT_mlinker_version_EQ)) {
> -    CmdArgs.push_back("-target-linker-version");
> -    CmdArgs.push_back(A->getValue(Args));
> +  if (getToolChain().getTriple().getArch() != llvm::Triple::hexagon){
> +    if (Arg *A = Args.getLastArg(options::OPT_mlinker_version_EQ)) {
> +      CmdArgs.push_back("-target-linker-version");
> +      CmdArgs.push_back(A->getValue(Args));
> +    }
>     }
>
> I don't understand the point of this change.

Neither do we. :)  Removed.

>
> +void Clang::AddHexagonTargetArgs(const ArgList&Args,
> +				 ArgStringList&CmdArgs) const {
> +  llvm::Triple Triple = getToolChain().getTriple();
> +
> +  // FIXME: In this -march=v2 -mv3 will still select hexagonv2
> +  // Set the CPU based on -march= and -mcpu=.
> +  //  FIXED !! (See getLastHexagonArchArg above)
>
> Is this comment necessary?
>

Nope.  Removed.

> [...]
> +  CmdArgs.push_back("-nobuiltininc");
>
> I'm not sure why you need this...
>

We have our own proprietary libc and include files in a cross 
compilation environment.  We are avoiding /usr/include.

> +  CmdArgs.push_back("-mqdsp6-compat");
>
> What is the point of this option if it's always on?
>

Fixed.

> +  CmdArgs.push_back("-Wreturn-type");
>
> What is this supposed to do?
>

This was a hack to get around a test failure.  Shouldn't have been 
included.  Removed.

> +  if (!Args.hasArg(options::OPT_fno_short_enums))
> +    CmdArgs.push_back("-fshort-enums");
> +  if (!Args.hasArg(options::OPT_fuse_cxa_atexit))
> +    CmdArgs.push_back("-fno-use-cxa-atexit");
>
> If you need defaults for these, please move them to where we parse the options.
>

Fixed.

> +  if (Args.getLastArg(options::OPT_fno_simplify_div)) {
> +    CmdArgs.push_back ("-mllvm");
> +    CmdArgs.push_back ("-disable_simplify_div");
> +  }
>
> This option, as far as I can tell, doesn't exist; what is it supposed to do?
>

It was taken from our gcc port.  It's purpose was to get around a broken 
benchmark.  The flag itself was brought over, but nothing else.  Oops. 
:)  Removed.

Tony


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.patch.bz2
Type: application/octet-stream
Size: 14204 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111117/9931bd99/attachment.obj>