[PATCH] D134726: [AMDGPU][SetWavePriority] Fix dealing with MBBInfo records.

[Ivan Kosarev via Phabricator via llvm-commits]

kosarev marked 2 inline comments as done.
kosarev added a comment.

In D134726#3826821 <https://reviews.llvm.org/D134726#3826821>, @arsenm wrote:

> LGTM. I really hate insert by index operator.

I still think the initial implementation with non-relocatable `MBBInfo`s would work better here.

Thanks for reviewing.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSetWavePriority.cpp:150-152
       SuccsMayReachVMEMLoad |= MBBInfos[Succ].MayReachVMEMLoad;
       NumFollowingVALUInsts =
           std::max(NumFollowingVALUInsts, MBBInfos[Succ].NumVALUInstsAtStart);
----------------
arsenm wrote:
> kosarev wrote:
> > arsenm wrote:
> > > kosarev wrote:
> > > > These potentially invalidate `Info`.
> > > Doesn't this imply it's reading uncomputed values for the successors? Don't those need to be computed for this to do something sensible?
> > I understand where the edge to a successor is a back edge, we will examine its `MBBInfo` here before the main `post_order(&MF)` loop had a chance to compute it. Logic-wise that itself is not a problem because we ignore loops, but since such successors are new to `MBBInfos`, this invalidates `Info`.
> Should this not ignore loops?
That's something to be tried and seen. We don't know it yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134726/new/

https://reviews.llvm.org/D134726