[PATCH] D77245: [AMDGPU] Fix crash in SILoadStoreOptimizer

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 2 08:39:09 PDT 2020


rampitec added a comment.

In D77245#1957290 <https://reviews.llvm.org/D77245#1957290>, @dstuttard wrote:

> My fix has been hanging around for a little while now - so my memory of it is a little hazy.
>
> However, the issue I saw (and attempted to fix) was similar to yours. Agreed that I think this is a rare occurrence, so perhaps your approach makes more sense (and is simpler/cheaper).
>  One question - the reason I went with the solution I did was that a previous iteration of the loop doing the optimisation was invalidating the iterator stored in a later list - which caused a crash - is your fix working around this by avoiding creating those potentially dependent lists in the first place?? (I wasn't 100% sure about the logic).
>
> I guess one potential advantage with my approach is that you still get the potential optimisations at the expense of having to re-compute the lists for dependent instructions.


It sounds like related but not exactly the same issue. In my case iterators are valid, but the order is swapped.

In general the idea to collect all merge lists in advance seems problematic. We may do a better job collecting one list at a time. It is also simpler, although slower.


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

https://reviews.llvm.org/D77245





More information about the llvm-commits mailing list