[PATCH] D55642: [X86] Fix assert fails in pass X86AvoidSFBPass

[Lama via Phabricator via llvm-commits]

lsaba added a comment.

In D55642#1331489 <https://reviews.llvm.org/D55642#1331489>, @nikic wrote:

> LGTM, with a small nit.
>
> In general, I'm not sure whether the overall behavior here makes sense. Especially in the case where the blocking stores don't come from different BBs, it would make more sense to me to preserve the last (in program order) of the overlapping stores, rather than the smallest, as that's the store that will be forwarded. If the stores come from multiple predecessors (like here) it's less clear which choice is right.
>
> But in any case, that would require larger changes and shouldn't block this assertion fix.


The idea behind using the smallest blocking store was to not introduce a new SFB between the smallest store and the newly created loads which are still bigger than it (since they are the size of the latest blocking store), I agree that improvements can be made to this patch especially for identifying and handling the behavior of blocking stores that come from predecessing blocks.


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

https://reviews.llvm.org/D55642