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

[Filip Pizlo]

On April 22, 2014 at 1:06:58 PM, Eric Christopher (echristo at gmail.com) wrote:

On Tue, Apr 22, 2014 at 11:22 AM, Filip Pizlo <fpizlo at apple.com> wrote: 
> 
> 
> On April 22, 2014 at 11:09:40 AM, Tim Northover (t.p.northover at gmail.com) 
> wrote: 
> 
>> No, I'm saying that the default CPU in the JIT should always be the CPU 
>> we're actually running on. That's not insane, that's how any sensible JIT 
>> would work. 
> 
> Possibly (though the lldb developers may quibble about their JIT not 
> being sensible). 
> 
> Sure, but lldb already manually sets the CPU. 
> 
> The question here is what behavior to give to those who don't manually set 
> the CPU. I'm arguing that the API previously guaranteed that the JIT would 
> target the CPU flavor that the process was running on, and that we should 
> arrange for the JIT to continue to obey this in the case that the JIT's 
> client doesn't manually set the CPU. 
> 

This is patently false except for the single cpu that changed. 
I think it would be useful if you were more specific about what is false about the above statement.  I think this is worth clearing up.

The MCJIT doesn't require the module to have a target set, and it doesn't require the caller to set the target of the ExecutionEngine.  It also doesn't require the client to set Arch or CPU.  This is true both in the C and C++ APIs.  EngineBuilder will set these things for you to sensible defaults on all platforms *except* ARM64, where things get murky.  For example on X86, even if you don't explicitly set the CPU the MCJIT will figure out what CPU you're running on along with all of its relevant features (whether it has AVX, etc) and it will generate code accordingly.  Also, the C API currently only lets you set the target on the module, not the ExecutionEngine, and so this prevents you from setting the Arch+CPU.

The MCJIT has explicit code to set the target/CPU/etc to a sensible default that is appropriate for JITing code into the process in which the JIT itself runs.

On ARM64 things were murky before r206228 and they are still murky now; the only thing that changed is that one flavor of ARM64 was broken: it's now the only target on which you cannot reliably create a module and an MCJIT without setting the target and expect code to run correctly.

And yes, LLDB does explicitly set the target/cpu/arch by using one of the C++ EngineBuilder methods that allows you to do it.  There may be other clients that do this.  But that doesn't mean that others should *have* to also do this.

It would be useful if you were more specific about what you think is false about any of this.

Anyway, I would be against any change to the C API that would require JIT clients to set the CPU, Arch, or target triple as a prerequisite to using the MCJIT.  This would be a breaking change to stable API and the MCJIT should continue to do as it always did: target the host by default.



> In the future, if someone does break one platform to fix another - 
> especially when it involves stable API, and where it's clear what the right 
> fix is - then we should revert to avoid a situation where clients have to 
> blacklist a series of LLVM revisions. 
> 

It's only stable between releases. Anything else is anything goes. As 
I don't believe it was in 3.4... 
MCJIT C API was in 3.4.

-Filip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140425/d7f48767/attachment.html>