[PATCH] D74285: [EarlyCSE] avoid crashing when detecting min/max/abs patterns (PR41083)

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
Herald added subscribers: llvm-commits, hiraditya, mcrosier.
Herald added a project: LLVM.
spatel edited the summary of this revision.
spatel added reviewers: samparker, efriedma, JosephTremoulet, DaniilSuchkov.

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.

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.


https://reviews.llvm.org/D74285

Files:
  llvm/lib/Transforms/Scalar/EarlyCSE.cpp
  llvm/test/Transforms/EarlyCSE/commute.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74285.243387.patch
Type: text/x-patch
Size: 8205 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200208/558001d7/attachment.bin>