[PATCH] D68941: [ScopBuilder]Fix bug 38358 by preserve correct order of ScopStmts

[bin via Phabricator via llvm-commits]

bin.narwal added a comment.

In D68941#1708693 <https://reviews.llvm.org/D68941#1708693>, @Meinersbur wrote:

> I remember I cared about the epilogue being the last, which is the reason for the reversed list. It ensures that the terminator is always visited first, hence added first into the list, hence being the last statement. (I am a bit surprised that no test fails). An example could be:
>
>   %b = load %B  // merged with terminator by joinOperandTree
>   store 42 // independent statement
>   br %b // terminator, epilogue
>
>
> Seeing this example, it is clear that `load %B` (and therefore the epilogue) cannot be put after `store 42`, as it might store to `%A`. That is, the epilogue is not necessarily always the last instruction. It should not matter since the PHI-writes have their own memory locations that do not alias with other memory.
>  [^^ just my thoughts about whether everything is sound; I could not find an issue]


That's because the code doesn't model branch instruction (which is terminator) at all right now.  If in case terminator instruction must be handled, we would need to handle it as an "ordered" instruction, just like memory access ones.

> Looks correct, Thanks for the patch!
>  Do you want me to commit this for you?

Yes, please.

Thanks,
bin


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

https://reviews.llvm.org/D68941