[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 27, 2010, at 12:18 PM, Rafael Espíndola wrote:

>> Sorry about it. I understood that you preferred to have both ARM and
>> X86 consistent. I will write a patch changing both.
> 
> I think I found why X86 does it the way it does. The problem is that
> X86TargetMachine contains and constructs the ELFWriterInfo with
> 
> ELFWriterInfo(*this)
> 
> This eventually gets to TargetELFWriterInfo::TargetELFWriterInfo that does
> 
>  is64Bit = TM.getTargetData()->getPointerSizeInBits() == 64;
> 
> Unfortunately at this point the TargetMachine is still being
> constructed, so this calls the getTargetData in TargetMachine.h that
> just returns 0.
> 

Ick. That's some ugly ordering dependency.

> 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?

-Jim