[PATCH] D140647: Handle simple diamond CFG hoisting in DivRemPairs.

[Owen Anderson via Phabricator via llvm-commits]

resistor added a comment.

In D140647#4016111 <https://reviews.llvm.org/D140647#4016111>, @lebedev.ri wrote:

> I'm guessing what other reviewers are trying to say,
> is that we might be missing something to to effect of:
>
>   if(DivBB != PredBB && RemBB != PredBB && !HasDivRemOp)
>     continue; // Don't hoist both into predecessor if we don't have divrem instruction.
>
> But it's not obvious to me if that it's actually worse,
> because instruction counting is not really the right way
> to gauge code performance.
>
> Div/Rem is usually *really* that bad. We move really really
> costly instruction from both branches to their predecessor
>
> 1. if branch is mispredicted, we won't have to recompute it
> 2. if the inputs are already avaliable, it can start executing earlier than it would have in the branches, thus we hide some of it's latency
> 3. in turn, instructions that depend on results of those instructions may get results earlier, and start executing earlier

Continuing with Sergei's example, the transformation *is* profitable from a code size and latency perspective on x86: https://godbolt.org/z/xo7P1MnGr
It seems like the simplest path forward here is to conditionalize the transformation as you suggest. I will look into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140647