[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

[Dale Johannesen]

On Dec 7, 2008, at 10:52 PMPST, Evan Cheng wrote:
>>
>>>>>
>>>>> 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.
>>
>> Hmm, maybe.  I find it much easier to understand this if I think of
>> RequiresRegister as "RequiresExtraLoad plus some extra cases" and  
>> have
>> the implementation follow that.  Think of it as factoring.
>
> Any plan to change the code? It doesn't makes sense to perform the
> expensive checks first.

It makes even less sense to make the code less comprehensible, and  
possibly break it (as you have seen), for a microoptimization.   No, I  
think this is a good use of factoring, and I don't plan to change it.

> What about the first part of my comment? If PIC && !DirectCall, is PIC
> base ever unnecessary? From what I have see it's needed even if the
> linkage is weak, common, etc.

Those cases are picked up by RequiresExtraLoad.