[all-commits] [llvm/llvm-project] 0f22e7: [InstCombine] Revert rL341831: relax one-use check...

Roman Lebedev via All-commits all-commits at lists.llvm.org
Mon Dec 2 07:06:42 PST 2019


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 0f22e783a038b6983f0fe161eef6cf2add3a4156
      https://github.com/llvm/llvm-project/commit/0f22e783a038b6983f0fe161eef6cf2add3a4156
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2019-12-02 (Mon, 02 Dec 2019)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
    M llvm/test/Transforms/InstCombine/icmp-add.ll
    M llvm/test/Transforms/LoopUnroll/runtime-loop-multiple-exits.ll
    M llvm/test/Transforms/LoopVectorize/if-conversion-nest.ll
    M llvm/test/Transforms/LoopVectorize/runtime-check.ll

  Log Message:
  -----------
  [InstCombine] Revert rL341831: relax one-use check in foldICmpAddConstant() (PR44100)

rL341831 moved one-use check higher up, restricting a few folds
that produced a single instruction from two instructions to the case
where the inner instruction would go away.

Original commit message:
> InstCombine: move hasOneUse check to the top of foldICmpAddConstant
>
> There were two combines not covered by the check before now,
> neither of which actually differed from normal in the benefit analysis.
>
> The most recent seems to be because it was just added at the top of the
> function (naturally). The older is from way back in 2008 (r46687)
> when we just didn't put those checks in so routinely, and has been
> diligently maintained since.

>From the commit message alone, there doesn't seem to be a
deeper motivation, deeper problem that was trying to solve,
other than 'fixing the wrong one-use check'.

As i have briefly discusses in IRC with Tim, the original motivation
can no longer be recovered, too much time has passed.

However i believe that the original fold was doing the right thing,
we should be performing such a transformation even if the inner `add`
will not go away - that will still unchain the comparison from `add`,
it will no longer need to wait for `add` to compute.

Doing so doesn't seem to break any particular idioms,
as least as far as i can see.

References https://bugs.llvm.org/show_bug.cgi?id=44100




More information about the All-commits mailing list