[llvm] r243682 - Add a TargetMachine hook that verifies DataLayout compatibility

[Mehdi Amini]

Now that we redirected all the CodeGen to the DataLayout attached to the Module, the impact of not having the correct one is unpredictable, and I feel this is even more unfriendly. It’s not like there are no change in behavior without the assert.

As a client I would prefer an assert (that would not impact my production) that I will notice during testing instead of weird codegen bug at runtime.

— 
Mehdi


> On Jul 30, 2015, at 4:46 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
> I guess? I mean, it seems pretty unfriendly at the moment, but you might be right and have them remember to upgrade their DataLayout instead if they put one in the module.
> 
> -eric
> 
> On Thu, Jul 30, 2015 at 4:44 PM Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> It is very late to do that (override the DL).
> My impression is that it is the responsibility of the tool that will read older bitcode to handle that before feeding it to any LLVM pass.
> 
> — 
> Mehdi
> 
>> On Jul 30, 2015, at 4:41 PM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>> 
>> So, I know I approved this, but I just thought (and by "I just thought" I mean someone said "hey, what about") about older bitcode that has a target that changed their DataLayout representation? Now they'll get an assert. Perhaps a better thing would be to do a warning in -debug mode and set the module DL to the TargetMachine DL in this case?
>> 
>> -eric
>> 
>> On Thu, Jul 30, 2015 at 1:43 PM Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
>> Author: mehdi_amini
>> Date: Thu Jul 30 15:33:18 2015
>> New Revision: 243682
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=243682&view=rev <http://llvm.org/viewvc/llvm-project?rev=243682&view=rev>
>> Log:
>> Add a TargetMachine hook that verifies DataLayout compatibility
>> 
>> Summary: Also provide the associated assertion when CodeGen starts.
>> 
>> Reviewers: echristo
>> 
>> Subscribers: llvm-commits
>> 
>> Differential Revision: http://reviews.llvm.org/D11654 <http://reviews.llvm.org/D11654>
>> 
>> From: Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>>
>> 
>> Modified:
>>     llvm/trunk/include/llvm/Target/TargetMachine.h
>>     llvm/trunk/lib/CodeGen/MachineFunction.cpp
>> 
>> Modified: llvm/trunk/include/llvm/Target/TargetMachine.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetMachine.h?rev=243682&r1=243681&r2=243682&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetMachine.h?rev=243682&r1=243681&r2=243682&view=diff>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Target/TargetMachine.h (original)
>> +++ llvm/trunk/include/llvm/Target/TargetMachine.h Thu Jul 30 15:33:18 2015
>> @@ -133,6 +133,15 @@ public:
>>    /// Create a DataLayout.
>>    const DataLayout createDataLayout() const { return DL; }
>> 
>> +  /// Test if a DataLayout if compatible with the CodeGen for this target.
>> +  ///
>> +  /// The LLVM Module owns a DataLayout that is used for the target independent
>> +  /// optimizations and code generation. This hook provides a target specific
>> +  /// check on the validity of this DataLayout.
>> +  bool isCompatibleDataLayout(const DataLayout &Candidate) const {
>> +    return DL == Candidate;
>> +  }
>> +
>>    /// Get the pointer size for this target.
>>    ///
>>    /// This is the only time the DataLayout in the TargetMachine is used.
>> 
>> Modified: llvm/trunk/lib/CodeGen/MachineFunction.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineFunction.cpp?rev=243682&r1=243681&r2=243682&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineFunction.cpp?rev=243682&r1=243681&r2=243682&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineFunction.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineFunction.cpp Thu Jul 30 15:33:18 2015
>> @@ -85,6 +85,10 @@ MachineFunction::MachineFunction(const F
>> 
>>    FunctionNumber = FunctionNum;
>>    JumpTableInfo = nullptr;
>> +
>> +  assert(TM.isCompatibleDataLayout(getDataLayout()) &&
>> +         "Can't create a MachineFunction using a Module with a "
>> +         "Target-incompatible DataLayout attached\n");
>>  }
>> 
>>  MachineFunction::~MachineFunction() {
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 

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