llvm dev - Sep 2019 - MergeLoadStoreMotion and GVNHoist

Hello

I'm investigating a regression while bumping our target from llvm-4 to
llvm7. We see that common loads are no longer being hoisted by the
MergedLoadStoreMotion pass. We've tracked this issue down to [1]. My
understanding of [1] is that load hoisting is performed by GVNHoist and is
therefore no longer required to be duplicated by MLSM, but as far as I
can tell, GVNHoist is not enabled by default at any standard
optimization level [2][3][4] (it's hidden behind a flag). However the MLSM
code is enabled at any (level > 1).

Is this an oversight?

If this _is_ an oversight, does it make sense to now enable GVNHoist in the same
cases as MLSM i.e. at all optlevels > 1 ?

If not, what is the reason for not enabling this?

All the Best

Luke

[1]
[0fbee48d](https://github.com/llvm-mirror/llvm/commit/0fbee48dc4899e045537b9118a65664f1706596c#diff-a1aa52c6b72ac5fb6e39994c431e24c2L537)
[2]
https://github.com/llvm-mirror/llvm/blob/3a58154/lib/Transforms/IPO/PassManagerBuilder.cpp#L388-L392
[3]
https://github.com/llvm-mirror/llvm/blob/3a58154/lib/Transforms/IPO/PassManagerBuilder.cpp#L325-L326
[4]
https://github.com/llvm-mirror/llvm/blob/3a58154/lib/Transforms/IPO/PassManagerBuilder.cpp#L117-L119
-- 
Codeplay Software Ltd.
Company registered in England and Wales, number: 04567874
Registered office: Regent House, 316 Beulah Hill, London, United Kingdom, SE19
3HF

I don't have an answer, but a small self-contained test that
demonstrates the issue may be helpful.
i

On Mon, Sep 2, 2019 at 6:10 AM Luke Drummond via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Hello
>
> I'm investigating a regression while bumping our target from llvm-4 to
> llvm7. We see that common loads are no longer being hoisted by the
> MergedLoadStoreMotion pass. We've tracked this issue down to [1]. My
> understanding of [1] is that load hoisting is performed by GVNHoist and is
> therefore no longer required to be duplicated by MLSM, but as far as I
> can tell, GVNHoist is not enabled by default at any standard
> optimization level [2][3][4] (it's hidden behind a flag). However the
MLSM
> code is enabled at any (level > 1).
>
> Is this an oversight?
>
> If this _is_ an oversight, does it make sense to now enable GVNHoist in the
same
> cases as MLSM i.e. at all optlevels > 1 ?
>
> If not, what is the reason for not enabling this?
>
> All the Best
>
> Luke
>
> [1]
[0fbee48d](https://github.com/llvm-mirror/llvm/commit/0fbee48dc4899e045537b9118a65664f1706596c#diff-a1aa52c6b72ac5fb6e39994c431e24c2L537)
> [2]
https://github.com/llvm-mirror/llvm/blob/3a58154/lib/Transforms/IPO/PassManagerBuilder.cpp#L388-L392
> [3]
https://github.com/llvm-mirror/llvm/blob/3a58154/lib/Transforms/IPO/PassManagerBuilder.cpp#L325-L326
> [4]
https://github.com/llvm-mirror/llvm/blob/3a58154/lib/Transforms/IPO/PassManagerBuilder.cpp#L117-L119
> --
> Codeplay Software Ltd.
> Company registered in England and Wales, number: 04567874
> Registered office: Regent House, 316 Beulah Hill, London, United Kingdom,
SE19 3HF
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Hi,
> On Sep 2, 2019, at 14:10, Luke Drummond via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hello
> 
> I'm investigating a regression while bumping our target from llvm-4 to
> llvm7. We see that common loads are no longer being hoisted by the
> MergedLoadStoreMotion pass. We've tracked this issue down to [1]. My
> understanding of [1] is that load hoisting is performed by GVNHoist and is
> therefore no longer required to be duplicated by MLSM, but as far as I
> can tell, GVNHoist is not enabled by default at any standard
> optimization level [2][3][4] (it's hidden behind a flag). However the
MLSM
> code is enabled at any (level > 1).
> 
> Is this an oversight?
> 
> If this _is_ an oversight, does it make sense to now enable GVNHoist in the
same
> cases as MLSM i.e. at all optlevels > 1 ?
> 
> If not, what is the reason for not enabling this?

I guess one reason why GVNHoist is still disabled is that there are a few known
issues:

https://bugs.llvm.org/buglist.cgi?quicksearch=gvn-hoist&list_id=169717
<https://bugs.llvm.org/buglist.cgi?quicksearch=gvn-hoist&list_id=169717>

Cheers,
Florian
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190909/e2710d74/attachment.html>