[llvm-commits] [llvm] r114856 - in /llvm/trunk/lib/Target/ARM: ARMELFWriterInfo.cpp ARMELFWriterInfo.h ARMSubtarget.h ARMTargetMachine.cpp ARMTargetMachine.h CMakeLists.txt

[Jim Grosbach]

On Sep 28, 2010, at 8:42 AM, Rafael Espíndola wrote:

>>> An easy way to refactor this is to have a is64Bit method instead of a
>>> field, but I am not sure about the runtime cost of it. Any other idea?
>> 
>> The TargetELFWriter wants to know is64Bit and endian-ness in the constructor, both of which it calls into the target data for. It seems to me those are both things that the  caller will already know explicitly (indeed is64Bit is already a parameter for the X86TargetMachine constructor, for example) and so can be changed to explicit parameters of the TargetELFWriter constructor. What do you think?
> 
> I should be possible. The attached patch is probably 1/3rd of the way
> there. The problem is that the same issue happens in other areas. For
> example, with the attached patch we will get a failure in
> TargetLowering::TargetLowering calling TD(TM.getTargetData()) with a
> TM provided with "TLInfo(*this)" from the X86TargetMachine
> constructor.

Since the TargetLowering wants information based on the TargetData, the reference to it (TLInfo) should be a member of the X86_64/X86_32TargetMachine subclasses, not the X86TargetMachine base class. There's already a virtual method to retrieve it (getTargetLowering), so that should be a very simple adjustment and will address this ordering issue. TSInfo will need a similar adjustment. See the ARM target implementation of this for reference.

That is, just revert the portions of the patch which changed the ARM target and change the X86 target to implement these bits in the same way the ARM target does them.

> Maybe it is better to go the other way and try to make getTargetData
> non virtual? The necessary information to create it can then be passed
> down by the constructors.