[PATCH] D54593: [X86] Fix PR39658: avoid duplicated successors in condibr merge

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev requested changes to this revision.
mkazantsev added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:199
+      // NewMBB. If there are already PHIs from MBB to NewMBB, we simply remove
+      // them. See PR39658.
+      if (NewSuccs)
----------------
It is just wrong. In PR39658, we are dealing with the following situation:
  A:
    sub x, y
    jg C
    jmp B
  B:
    sub x, y
    jmp C
  C:
    Phi [v1, A], [v2, B]

You are trying to merge `A` and `B` together, but they cannot be merged specifically for the reason that it is actually important from which block we came to `C`. The value of the Phi node depends on this. You cannot just say that we no longer come from `B` and therefore Phi never takes value `v2`. It does take this value if `x` was less or equal than `y`.

This patch is not helping the ogirinal miscompile. I think that the right solution is to prohibit merging blocks `A` and `B` if there is a `Phi` which has different incoming values for these blocks.




================
Comment at: test/CodeGen/X86/pr39658.ll:10
+; Function Attrs: uwtable
+define void @zot(i8* %arg) #0 gc "statepoint-example" {
+bb:
----------------
Please use FileCheck to assert that the contents of Phis in question is correct.


https://reviews.llvm.org/D54593