[all-commits] [llvm/llvm-project] 0895b8: [SimplifyCFG] FoldBranchToCommonDest(): don't deal...

[Roman Lebedev via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 0895b836d74ed333468ddece2102140494eb33b6
      https://github.com/llvm/llvm-project/commit/0895b836d74ed333468ddece2102140494eb33b6
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    R llvm/test/Transforms/SimplifyCFG/X86/fold-branch-debuginvariant.ll
    M llvm/test/Transforms/SimplifyCFG/branch-fold.ll

  Log Message:
  -----------
  [SimplifyCFG] FoldBranchToCommonDest(): don't deal with unconditional branches

The case where BB ends with an unconditional branch,
and has a single predecessor w/ conditional branch
to BB and a single successor of BB is exactly the pattern
SpeculativelyExecuteBB() transform deals with.
(and in this case they both allow speculating only a single instruction)

Well, or FoldTwoEntryPHINode(), if the final block
has only those two predecessors.

Here, in FoldBranchToCommonDest(), only a weird subset of that
transform is supported, and it's glued on the side in a weird way.
  In particular, it took me a bit to understand that the Cond
isn't actually a branch condition in that case, but just the value
we allow to speculate (otherwise it reads as a miscompile to me).
  Additionally, this only supports for the speculated instruction
to be an ICmp.

So let's just unclutter FoldBranchToCommonDest(), and leave
this transform up to SpeculativelyExecuteBB(). As far as i can tell,
this shouldn't really impact optimization potential, but if it does,
improving SpeculativelyExecuteBB() will be more beneficial anyways.

Notably, this only affects a single test,
but EarlyCSE should have run beforehand in the pipeline,
and then FoldTwoEntryPHINode() would have caught it.

This reverts commit rL158392 / commit d33f4efbfdef6ffccf212ab3e40a7673589085fd.


  Commit: aabed3718ae25476c0f6b7e70c83ba4658f00e5c
      https://github.com/llvm/llvm-project/commit/aabed3718ae25476c0f6b7e70c83ba4658f00e5c
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp

  Log Message:
  -----------
  [NFCI-ish][SimplifyCFG] FoldBranchToCommonDest(): really don't deal with uncond branches

While we already ignore uncond branches, we could still potentially
end up with a conditional branches with identical destinations
due to the visitation order, or because we were called as an utility.
But if we have such a disguised uncond branch,
we still probably shouldn't deal with it here.


  Commit: 256a0357524b6cea3c705a77ec3d3c0122ede861
      https://github.com/llvm/llvm-project/commit/256a0357524b6cea3c705a77ec3d3c0122ede861
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp

  Log Message:
  -----------
  [NFC][SimplifyCFG] FoldBranchToCommonDest(): unclutter Cond/CondInPred handling

We don't need those variables, we can just get the final value directly.


  Commit: 7b89efb55e4e5d6078aa9571f40859cc9ea01bcc
      https://github.com/llvm/llvm-project/commit/7b89efb55e4e5d6078aa9571f40859cc9ea01bcc
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp

  Log Message:
  -----------
  [NFC][SimplifyCFG] FoldBranchToCommonDest(): somewhat better structure weight updating code

Hoist the successor updating out of the code that deals with branch
weight updating, and hoist the 'has weights' check from the latter,
making code more consistent and easier to follow.


  Commit: b482560a597697789d81e4b9b22fb14e1f2f3c9a
      https://github.com/llvm/llvm-project/commit/b482560a597697789d81e4b9b22fb14e1f2f3c9a
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp

  Log Message:
  -----------
  [NFC][SimplifyCFG] FoldBranchToCommonDest(): extract check for destination sharing into a helper function

As a follow-up, i'll extract the actual transform into a function,
and this helper will be called from both places,
so this avoids code duplication.


  Commit: efeb8caf8bd10f2ad794c6f434fbc4ba133cd7e3
      https://github.com/llvm/llvm-project/commit/efeb8caf8bd10f2ad794c6f434fbc4ba133cd7e3
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp

  Log Message:
  -----------
  [NFC][SimplifyCFG] FoldBranchToCommonDest(): extract the actual transform into helper function

I'm intentionally structuring it this way, so that the actual fold only
does the fold, and no legality/correctness checks, all of which must be
done by the caller. This allows for the fold code to be more compact
and more easily grokable.


  Commit: 4ed0d8f2f07d0e17942366d48a29c165384ace52
      https://github.com/llvm/llvm-project/commit/4ed0d8f2f07d0e17942366d48a29c165384ace52
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
    M llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
    M llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    M llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

  Log Message:
  -----------
  [NFC][InstCombine] Extract freelyInvertAllUsersOf() out of canonicalizeICmpPredicate()

I'd like to use it in an upcoming fold.


  Commit: 62604906b5b29c4a55f83226a60f0de9ff9f8df2
      https://github.com/llvm/llvm-project/commit/62604906b5b29c4a55f83226a60f0de9ff9f8df2
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    A llvm/test/Transforms/InstCombine/sink-not-into-another-hand-of-and.ll
    A llvm/test/Transforms/InstCombine/sink-not-into-another-hand-of-or.ll

  Log Message:
  -----------
  [NFC][InstCombine] Add tests for `(~x) &/| y` --> `~(x |/& (~y))` fold

Iff y is free to invert, and the users of the expression can be updated,
we can undo De-Morgan fold, and immediately get rid of the `not` op.


  Commit: 79b0d21ce92f1a5ff4c822d1a5c664196b338535
      https://github.com/llvm/llvm-project/commit/79b0d21ce92f1a5ff4c822d1a5c664196b338535
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    M llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    M llvm/test/Transforms/InstCombine/sink-not-into-another-hand-of-and.ll

  Log Message:
  -----------
  [InstCombine] Fold `(~x) & y` --> `~(x | (~y))` iff it is free to do so

Iff we know we can get rid of the inversions in the new pattern,
we can thus get rid of the inversion in the old pattern,
this decreasing instruction count.


  Commit: d1a6f92fd545726aab0784e2dcfb193ce185c418
      https://github.com/llvm/llvm-project/commit/d1a6f92fd545726aab0784e2dcfb193ce185c418
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2021-01-22 (Fri, 22 Jan 2021)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    M llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    M llvm/test/Transforms/InstCombine/sink-not-into-another-hand-of-or.ll
    M llvm/test/Transforms/SimplifyCFG/merge-cond-stores.ll

  Log Message:
  -----------
  [InstCombine] Fold `(~x) | y` --> `~(x & (~y))` iff it is free to do so

Iff we know we can get rid of the inversions in the new pattern,
we can thus get rid of the inversion in the old pattern,
this decreasing instruction count.

Note that we could position this transformation as just hoisting
of the `not` (still, iff y is freely negatible), but the test changes
show a number of regressions, so let's not do that.


Compare: https://github.com/llvm/llvm-project/compare/98a8344895a8...d1a6f92fd545