[all-commits] [llvm/llvm-project] b8ebc1: [EarlyCSE] avoid crashing when detecting min/max/a...

RotateRight via All-commits all-commits at lists.llvm.org
Mon Feb 10 14:25:58 PST 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: b8ebc11f032032c7ca449f020a1fe40346e707c8
      https://github.com/llvm/llvm-project/commit/b8ebc11f032032c7ca449f020a1fe40346e707c8
  Author: Sanjay Patel <spatel at rotateright.com>
  Date:   2020-02-10 (Mon, 10 Feb 2020)

  Changed paths:
    M llvm/lib/Transforms/Scalar/EarlyCSE.cpp
    M llvm/test/Transforms/EarlyCSE/commute.ll
    M llvm/test/Transforms/PhaseOrdering/min-max-abs-cse.ll

  Log Message:
  -----------
  [EarlyCSE] avoid crashing when detecting min/max/abs patterns (PR41083)

As discussed in PR41083:
https://bugs.llvm.org/show_bug.cgi?id=41083
...we can assert/crash in EarlyCSE using the current hashing scheme and
instructions with flags.

ValueTracking's matchSelectPattern() may rely on overflow (nsw, etc) or
other flags when detecting patterns such as min/max/abs composed of
compare+select. But the value numbering / hashing mechanism used by
EarlyCSE intersects those flags to allow more CSE.

Several alternatives to solve this are discussed in the bug report.
This patch avoids the issue by doing simple matching of min/max/abs
patterns that never requires instruction flags. We give up some CSE
power because of that, but that is not expected to result in much
actual performance difference because InstCombine will canonicalize
these patterns when possible. It even has this comment for abs/nabs:

  /// Canonicalize all these variants to 1 pattern.
  /// This makes CSE more likely.

(And this patch adds PhaseOrdering tests to verify that the expected
transforms are still happening in the standard optimization pipelines.

I left this code to use ValueTracking's "flavor" enum values, so we
don't have to change the callers' code. If we decide to go back to
using the ValueTracking call (by changing the hashing algorithm
instead), it should be obvious how to replace this chunk.

Differential Revision: https://reviews.llvm.org/D74285




More information about the All-commits mailing list