[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]

Obviously you and I don't think the same way about this problem.  I  
think it is fine the way it is.  If you disagree, you can change it  
(although I'd rather you didn't, I can probably figure it out.)

On Dec 8, 2008, at 2:35 PMPST, Evan Cheng wrote:
>>>>>
>>>>> 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?
>>
>> There has been no mention of "constraints" so far, what do you mean?
>
> Fast and clear (and correct).
>
>>> 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?
>>
>> You're losing the separation between "things handled by
>> RequiresExtraLoad"
>> and "things not handled that way".  There is overlap the way you've
>> got it.
>
> Why does that matter? The purpose of of the function is  not testing
> "requires an extra load".  Why insisting on testing that part first?

Because that is the way that makes sense to me.

> Is it not true in PIC mode accessing GV always require a register? Is
> it not true when isDirectCall is true this should return false?
>
> Looking at your code:
>
>   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());
>   return false;
>
> If you don't look into GVRequiresExtraLoad, then you wouldn't see that
> weak, common, externalweak, and linkonce linkages were handled by
> GVRequiresExtraLoad. That's why the checks for internal and external
> linkages look strange.
>
> EVan
>
>>
>>
>>> {
>>> 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
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits