[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