[PATCH] Let llc and opt override "-target-cpu" and "-target-features" using command line options "-mcpu" and "-mattr"

[Akira Hatanaka]

On Wed, May 6, 2015 at 1:55 PM, Eric Christopher <echristo at gmail.com> wrote:

>
>
> On Wed, May 6, 2015 at 1:51 PM Akira Hatanaka <ahatanak at gmail.com> wrote:
>
>> In http://reviews.llvm.org/D9537#167157, @echristo wrote:
>>
>> > I agree, no TargetOptions for this. I don't necessarily agree with
>> rewriting, but it seems to be a small enough change and will work for what
>> the overrides are meant to be for. Some discipline in reviewing testcases
>> to make sure that a lot of overrides aren't there for incoming testcases is
>> going to be necessary for clean tests. One inline comment which basically
>> sums up to "why not two functions"? As long as you have a reasonable answer
>> go ahead.
>>
>>
>> I'm not sure if I understood your comment about testcases. Do you mean
>> people shouldn't duplicate functions in test cases just to test different
>> target-cpu and target-feature strings but instead should use -mcpu and
>> -mattrs (which I think makes sense)? Should the one inline comment go into
>> the test cases I added?
>>
>>
>>
> My comment about testcases is that someone shouldn't have:
>
> ; RUN: llc -march=foo
>
> as the only run line with a function with:
>
> target-cpu="notfoo"
>
> when they could just change the arch on the function to say "foo" rather
> than "notfoo".
>
>

OK, I agree. In that case, people should be advised to remove the command
line option and change the "target-cpu" attribute or just remove the
"target-cpu" attribute.

I'll add comments to the test cases in this patch to make it clear what
they are testing.

================
>> Comment at: include/llvm/CodeGen/CommandFlags.h:268
>> @@ -262,1 +267,3 @@
>>
>> +static inline std::pair<std::string, std::string> getCPUFeaturesStr() {
>> +  std::string CPUStr;
>> ----------------
>> echristo wrote:
>> > Why a pair?
>> Is it better to split this into two functions getCPUStr and
>> getFeaturesStr? Or you meant the two strings (cpu and features strings)
>> should be passed by reference?
>>
>>
> I meant split into two functions, any particular reason?
>
>
No particular reason. I'll split the function into two.


> -eric
>
>
>> http://reviews.llvm.org/D9537
>>
>> EMAIL PREFERENCES
>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150506/ecdc9333/attachment.html>