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

Michael Liao via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 2 08:39:07 PDT 2020


hliao 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.


The current implementation always *sink* instructions no matter whether it's a load or store. That makes the instruction movement inevitable. But, if we *lift* loads and sink stores, no instructions (or very few address calculation) need moving as uses of loads are still preceded by their loads.


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

https://reviews.llvm.org/D77245





More information about the llvm-commits mailing list