[PATCH] D11103: Remove access to the DataLayout in the TargetMachine

[Eric Christopher]

echristo added inline comments.

================
Comment at: lib/LTO/LTOCodeGenerator.cpp:524
@@ -523,3 +523,3 @@
   // Add an appropriate DataLayout instance for this module...
-  mergedModule->setDataLayout(*TargetMach->getDataLayout());
+  mergedModule->setDataLayout(TargetMach->createDataLayout());
 
----------------
lhames wrote:
> echristo wrote:
> > lhames wrote:
> > > joker.eph wrote:
> > > > rafael wrote:
> > > > > I think this is from the days when we supported the module not having a DL. We should probably try to remove it in another patch.
> > > > I'm not very familiar with the JIT use case, but how is the client creating the module? And how can he makes sure that the DL on the module that he will give to the JIT is the one the TargetMachine expects? 
> > > > Is he supposed to keep a reference to the TargetMachine when creating the JIT?
> > > This is LTO code, and the LTO and JIT cases may be different.
> > > 
> > > Historically the JIT hasn't required a data layout. That seems like a reasonable restriction for us to add, but we'd have to give clients a heads-up first. For now, the JIT code should still set the data layout if there isn't one present.
> > Well, you have, you're just relied on the TargetMachine for this. Also, I imagine there's a DataLayout in the module since it's required now anyhow right? :)
> Sorry: To clarify, what I meant was that we haven't historically required JIT clients to supply Modules with a DataLayout attached. In fact, until fairly recently MCJIT would happily consume & codegen IR without a DataLayout.
> 
> When I wrote Orc I added the data layout requirement to ensure that symbol naming was consistent between JIT'd and static code. To avoid changing the contract on clients, MCJIT and the in-tree Orc stacks (OrcMCJITReplacement, OrcLazy) take a TargetMachine and use it to attach a DataLayout if there isn't already one present.
> 
> I'd be open to changing the contract on MCJIT and Orc to require a DataLayout on the modules if that would be more consistent with LTO.
That makes sense. After Mehdi's earlier patches DataLayout is required on the Module, but we're probably not enforcing it. I'm working on how to help fix consumers of the TargetMachine DataLayout construction routines to construct a DataLayout ahead of time.


http://reviews.llvm.org/D11103