[patch] Assert that clang's datalayout strings are in sync with llvm's

[Rafael Espíndola]

> The idea is fantastic, but I found your implementation burdensome.
>
> Bringing a TargetMachine dependency into IRGen, even if it's a "soft"
> dependency is best avoided because the clear layering between IRGen and
> CodeGen is LLVM's stated foremost design goal.
>
> The extraction of CreateTargetMachine() into a long list of parameters in
> your patch also left something to be desired and we should be able to avoid
> it.
>
>
>>
>> It might be possible to still refactor things to avoid the
>> duplication, but for now this should at least make sure both projects
>> stay in sync.
>
>
> I've attached my take on your idea, implementing it in a net 8 LoC.
>
> In my patch, the verification is done at IRGen so there's no runtime
> penalty, and moreover no change in layering.

Interesting. I started coding with the idea of removing the string
from clang and using the ones in llvm at their current location. I got
there and then went back to just asserting, but never looked back if
there was a better spot to put the assert.

I really like your patch. Some some small comments.

* Making TM a member of EmitAssemblyHelper is a nice independent
improvement. Please just commit that right away :-)

+  if (!TM) TM.reset(CreateTargetMachine(UsesCodeGen));
Using two lines seems to be the more current stile (what clang-format uses).

+  assert(TDesc.empty() || !AsmHelper.TM ||

This is failing on test/Frontend/ir-support-codegen.ll. It seems like
the assert found a real bug. The TargetInfo is created once with

  // Create the target instance.
  setTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), &getTargetOpts()));

but then CodeGenAction will simply parse the module and use the triple in it.

Given what a corner case this is, it might be reasonable to just pass
an empty string in CodeGenAction.cpp:424 or maybe patch the Triple in
the module. After all, clang always creates a single TargetInfo based
on what is passed to -triple (or the system default), in cannot really
handle an IR file with a different one.

> Even with this verification done, we should continue to investigate options
> for sharing the layouts and getting them threaded through the frontend. Two
> copies are one too many.

Agreed. I just don't promise I will have the cycles to do it right
now. The assertion gives me enough confidence to add the features I
need to DataLayout.

Cheers,
Rafael