[PATCH] D12886: Clean up: Refactoring the hardcoded value of 6 for FindAvailableLoadedValue()'s parameter MaxInstsToScan.

[Larisse Voufo via llvm-commits]

On Fri, Sep 18, 2015 at 9:48 AM, Nick Lewycky <nlewycky at google.com> wrote:

> On 17 September 2015 at 22:25, Larisse Voufo <lvoufo at gmail.com> wrote:
>
>> lvoufo added a comment.
>>
>>
>>
>> > -2-
>>
>> >  Does "opt -basicaa -gvn" clean up the redundant load? If so, is it
>> okay that instcombine doesn't?
>>
>>
>> Now it does, as of Diff 35065 of http://reviews.llvm.org/D11826.
>
>
> Is that the correct link? That appears to introduce a new LLVM IR feature
> that changes both clang and llvm.
>
It does introduce a new feature that would remove redundant loads.

Right now, neither -instcombine nor -gvn remove the redundant loads I am
working with.
With the new feature, -gvn definitely does, while -instcombine does
sometimes.


> If the transform is legal for instcombine to do, then it's also legal for
> gvn to do with no new IR features involved.
>
This relationship is preserved with the new feature.

>
> But it still does not make it okay imo since once might still expect
>> something like "-basicaa -instcombine -inline -early-cse -instcombine" to
>> remove redundant loads when
>> "-basicaa -instcombine -early-cse -instcombine" does.
>>
>
> Well, yeah. I think it's unreasonable to expect instcombine to hack on
> your loads, but it does and I'll just move right along. What would surprise
> me is if instcombine folded redundant loads that gvn couldn't. That would
> suggest a problem in gvn...
>

Yup. You should checkout r60148
<http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20081124/070428.html>.
>From the commit message, -instcombine (as it later on uses
FindAvailableLoadedValue()) should be expected to remove "locally
redundant" and "partially redundant" loads.



>
>> http://reviews.llvm.org/D12886
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150918/bfbcbc7e/attachment.html>