[cfe-commits] Patch: support for new backend for Hexagon processor
eli.friedman at gmail.com
Mon Nov 14 16:25:39 CST 2011
On Mon, Nov 14, 2011 at 1:34 PM, Tony Linthicum <tlinth at codeaurora.org> wrote:
> Hi Eli,
> Thanks for the quick responses!
> On 11/14/2011 2:34 PM, Eli Friedman wrote:
>> +BUILTIN(__builtin_HEXAGON_C2_and, "bii", "")
>> +BUILTIN(__builtin_HEXAGON_C2_or, "bii", "")
>> +BUILTIN(__builtin_HEXAGON_C2_xor, "bii", "")
>> Do you really need builtins for these? Fewer builtins means that
>> standard LLVM optimizations generally work better. Not a requirement,
>> just something to think about.
> There are a couple of reasons why we have the large number of builtins that
> we have. First, they are auto-generated from an instruction database.
> Second, some of our customers who use builtins for instructions that the
> compiler cannot otherwise generate will write all of that piece of code
> using builtins. We can cull the list and are willing to do so if it is
> indeed a problem. It might cause us some headaches with backward
> compatibility to our gcc compiler that we hope to replace with LLVM, so we'd
> prefer not to do it if it isn't strictly necessary.
Okay, that makes sense.
>> --- 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
>> Please use a consistent naming convention.
> We will remove the naming convention backward compatibility option. Some of
> our customers use it, but we can maintain it locally if we need to carry it
What? I was just referring to the name of the option itself.
>> Please don't include trailing whitespace changes to unrelated code in
>> your patch. This applies across the whole patch.
> My apologies for that. I made lots of changes to get our code within the 80
> column limit and introduced trailing white spaces in the process. I had a
> lot of them to fix and assumed all of them were mine. I thought from the
> developer policy that removing them was a requirement. Is that not so?
You aren't supposed to add trailing whitespace, but given that we
don't have a commit hook to stop it, it sometimes slips in anyway.
> Others will be responding to some of your other points. A general question,
> though, since the patches are so large. When we address an issue and wish
> to resubmit the patch to the list, should we resubmit the whole thing or
> just a "patch to the patch" as it were.
No, please don't submit a "patch to a patch". You could probably
split this patch, though: one patch for the non-Driver changes without
the builtins, one patch for the builtins, and one patch for the
More information about the cfe-commits