[llvm-commits] [llvm] r60608 - in /llvm/trunk: lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86Subtarget.cpp lib/Transforms/Scalar/LoopStrengthReduce.cpp test/CodeGen/X86/loop-strength-reduce-2.ll test/CodeGen/X86/loop-strength-reduce-3.ll test/CodeGen/X86/loop-strength-reduce.ll

[Evan Cheng]

On Dec 5, 2008, at 3:58 PM, Dale Johannesen wrote:

>
> On Dec 5, 2008, at 3:52 PMPST, Evan Cheng wrote:
>>> On Dec 5, 2008, at 2:01 PMPST, Anton Korobeynikov wrote:
>>>
>>>> Hi, Dale
>>>>
>>>>>
>>>>> +/// True if accessing the GV requires a register.  This is a
>>>>> superset of the
>>>>> +/// cases where GVRequiresExtraLoad is true.  Some variations of
>>>>> PIC require
>>>>> +/// a register, but not an extra load.
>>>>> +bool X86Subtarget::GVRequiresRegister(const GlobalValue *GV,
>>>>> +                                       const TargetMachine& TM,
>>>>> +                                       bool isDirectCall) const
>>>>> +{
>>>>> +  if (GVRequiresExtraLoad(GV, TM, isDirectCall))
>>>>> +    return true;
>>>>> +  // Code below here need only consider cases where
>>>>> GVRequiresExtraLoad
>>>>> +  // returns false.
>>>>> +  if (TM.getRelocationModel() == Reloc::PIC_)
>>>>> +    return !isDirectCall &&
>>>>> +      (GV->hasInternalLinkage() || GV->hasExternalLinkage());
>>>> I don't understand this. Why only internal and external linkage?
>>>
>>> All the others are already handled by GVRequiresExtraLoad.  (At  
>>> least
>>> for Darwin.)
>>
>> That doesn't sound right. For Darwin x86 32-bit PIC, all data load
>> requires the pic base register. This has nothing to do with whether a
>> stub is needed.
>
> What is it that doesn't sound right?  You're quite right this has
> nothing to do with stubs, why do you think it does?
>
> There is a difference between requiring the pic register and requiring
> an extra
> load; the former was not being modelled.  Externals are referenced via
> 	leal	L_x5$non_lazy_ptr-"L00000000001$pb"(%ebx), %eax
> 	movl	(%eax), %eax         << loading address
> 	movl	(%eax), %eax         << loading value
> This case returns true from GVRequiresExtraLoad.
>
> Statics and globals are referenced via
> 	leal	_x0-"L00000000001$pb"(%ebx), %eax
> 	movl	(%eax), %eax         << loading value
> This case returns false from GVRequiresExtraLoad but true from
> GVRequiresRegister.

I don't think GVRequiresRegister needs to call GVRequiresExtraLoad  
though. Even if it returns false, GVRequiresRegister should always  
return true for PIC && !DirectCall. It needs PIC base even if linkage  
is weak, common, etc.

Plus, you should check relocation model first which is cheaper than  
GVRequiresExtraLoad.

Evan

>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits