[llvm] r206228 - [ARM64][MC] Set the default CPU string to generic.

[Quentin Colombet]

Hi Juergen,

On Apr 21, 2014, at 7:54 PM, Juergen Ributzka <juergen at apple.com> wrote:

> Hi Quentin,
> 
> this change “breaks" any JIT client that uses the C interface and generates code for ARM64.
> The JIT won’t complain about the generic CPU and still generate code, but for the generic target. 
> 
> The problem is that the C interface currently doesn’t allow you to set the CPU type at all.
> For example “CreateMCJITCompilerForModule” can pick up the target triple from the
> module, but the CPU will always default to “generic”.
> 
> To fix this we could:
> 1.) Extend the C interface to also specify the CPU type and warn JIT clients that don’t specify a CPU for ARM64
This looks like the right approach to me.
The clients of the backend should not rely on any default CPU.

What is the current behavior on X86?

> 2.) Default to “cyclone” for arm64-apple-* triples
I am not a huge fan of changing the CPU behind the hood, but I do not have a strong opinion either if you think you should pursue this way.

> 3.) Every JIT client has to add the target processor attribute to the IR
That sounds like a good safe belt, especially to reproduce failures (see http://llvm.org/bugs/show_bug.cgi?id=19483). That said, how this attribute is propagated?
In particular does ARM64 backend do the right thing if you set it.

> 4.) Revert this change
This change has been actively discussed on the mailing list (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140414/212869.html) and IRC. This seemed to be the best approach especially with the merge with aarch64.

> 
> If we keep the “generic” CPU, then we should definitely add a warning message (took me quite some time to track this down).
Sorry about that.
The warning/error was also abandoned because (to sum up) for most codegen test cases using generic is fine and you do not want to have to specify a cpu or have a warning.

Thanks,
-Quentin
> 
> Thanks
> 
> Cheers,
> Juergen
> 
> On Apr 14, 2014, at 5:28 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
>> Author: qcolombet
>> Date: Mon Apr 14 19:28:39 2014
>> New Revision: 206228
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=206228&view=rev
>> Log:
>> [ARM64][MC] Set the default CPU string to generic.
>> 
>> Modified:
>>   llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp
>> 
>> Modified: llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp?rev=206228&r1=206227&r2=206228&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp Mon Apr 14 19:28:39 2014
>> @@ -44,10 +44,8 @@ static MCSubtargetInfo *createARM64MCSub
>>                                                   StringRef FS) {
>>  MCSubtargetInfo *X = new MCSubtargetInfo();
>> 
>> -  // FIXME: Make this darwin-only.
>>  if (CPU.empty())
>> -    // We default to Cyclone for now, on Darwin.
>> -    CPU = "cyclone";
>> +    CPU = "generic";
>> 
>>  InitARM64MCSubtargetInfo(X, TT, CPU, FS);
>>  return X;
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>