[PATCH] D89149: [SelectionDAG] Fix alias checking with potential later stack reuse

[Nirav via llvm-commits]

I've checked that I've permission with my current employer to
contribute code, so now I just need to find time to resurrect a
working repo on a reasonable machine which will probably need to wait until
the end of this month.

If you're inclined to look at it deeper in the meanwhile, I did look at the
cases to consider and I think the fix should just be on line 22262 where
we're removing the dependence between two memory operations if the
locations don't alias (which now needs to check they're referring
potentially aliasable frames).

  case ISD::LIFETIME_START
<https://llvm.org/doxygen/namespacellvm_1_1ISD.html#a22ea9cec080dd5f4f47ba234c2f59110aada0b926e7ebc2bc55d158d4c37bdcca>
:
22259  case ISD::LIFETIME_END
<https://llvm.org/doxygen/namespacellvm_1_1ISD.html#a22ea9cec080dd5f4f47ba234c2f59110ab49301b20458a7f312a0919e3c8abdb2>:
{
22260  // We can forward past any lifetime start/end that can be proven not
to
22261  // alias the memory access.
22262  if (!isAlias(N, C
<https://llvm.org/doxygen/NVVMIntrRange_8cpp.html#a2ae53ee71b2ccbb52b883c0b4f59f8c2>.getNode()))
{
22263  // Look further up the chain.
22264  C
<https://llvm.org/doxygen/NVVMIntrRange_8cpp.html#a2ae53ee71b2ccbb52b883c0b4f59f8c2>
= C
<https://llvm.org/doxygen/NVVMIntrRange_8cpp.html#a2ae53ee71b2ccbb52b883c0b4f59f8c2>
.getOperand
<https://llvm.org/doxygen/classllvm_1_1User.html#abe1de1520a21f77ac57cc210bf0fb0b4>
(0);
22265  return true;
22266  }


On Tue, Dec 1, 2020 at 2:38 AM Yonghong Song via Phabricator <
reviews at reviews.llvm.org> wrote:

> yonghong-song added a comment.
>
> > As an alternative, we could update ImproveChain (and visitLIFETIME_END)
> to limit the aliasing
> > around lifetime_start / end to disallow improving the chain dependence
> of a mem op node
> >  from a different lifetime node that may alias. That should prohibit
> access from two aliasable
> > frame indices from being concurrent while still allowing us to leverage
> that disjoint allocs should be disjoint.
>
> @niravd Any update on this?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D89149/new/
>
> https://reviews.llvm.org/D89149
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201201/d557df53/attachment.html>