[llvm-commits] CVS: llvm/lib/Target/X86/X86AsmPrinter.cpp X86TargetAsmInfo.cpp

[Chris Lattner]

On Mar 7, 2007, at 11:22 PM, Evan Cheng wrote:

>
> On Mar 7, 2007, at 9:15 PM, Chris Lattner wrote:
>
>>> +          // Read-only data.
>>> +          bool isIntFPLiteral = Type->isInteger()  || Type- 
>>> >isFloatingPoint();
>>> +          if (C->ContainsRelocations() && Subtarget- 
>>> >isTargetDarwin() &&
>>> +              TM.getRelocationModel() != Reloc::Static)
>>
>> Please rearrange this to:
>> if (isdarwin && not static && containsrelocations)
>>
>> putting the expensive check last.
>>
>>> +            SwitchToDataSection("\t.const_data\n");
>>> +          else if (isIntFPLiteral && Size == 4 &&
>>> +                   TAI->getFourByteConstantSection())
>>> +            SwitchToDataSection(TAI->getFourByteConstantSection 
>>> (), I);
>>> +          else if (isIntFPLiteral && Size == 8 &&
>>> +                   TAI->getEightByteConstantSection())
>>> +            SwitchToDataSection(TAI->getEightByteConstantSection 
>>> (), I);
>>> +          else if (isIntFPLiteral && Size == 16 &&
>>> +                   TAI->getSixteenByteConstantSection())
>>> +            SwitchToDataSection(TAI- 
>>> >getSixteenByteConstantSection(), I);
>>
>> Do we really need to check isIntFPLiteral here?  Why can't  
>> something like {short,short} go in literal4 ?
>
> We don't. But ContainsRelocations has to be checked first. We can't  
> use FourByteConstantSection, etc. if the constant has relocations.

Ah, ok, so instead of "isIntFPLiteral", you really mean "!contains  
relocations"

-Chris