[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 8, 2008, at 9:54 AM, Dale Johannesen wrote:

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

I don't understand your argument. Why can't we satisfy both  
constraints? We know when isDirectCall this is always false, we also  
know it's always true in PIC mode (at least for Darwin). How is this  
less clear?

{
   if (isDirectCall)
     return false;
   if (TM.getRelocationModel() == Reloc::PIC_) {
     if (isTargetDarwin())
       return true;
     else if (isTargetElf())
       ...
     else ...
       ...
   }
   return GVRequiresExtraLoad(GV, TM, false);
}

Evan

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