[PATCH] D110024: [MergeICmps] Don't reorder unmerged comparisons

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: courbet, pcc.
Herald added subscribers: mgrang, hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

MergeICmps will currently sort (by offset) all comparisons in a chain, including those that do not get merged. This is problematic in two ways:

- We may end up moving the original first block into the middle of the chain, in which case the "extra work" instructions will also be in the middle of the chain, resulting in invalid IR.
- Reordering branches is generally not legal, because it may introduce branch on poison, which is UB (https://bugs.llvm.org/show_bug.cgi?id=51845). The merging done by MergeICmps is legal as long as we assume that memcmp() works on frozen memory, but the reordering of unmerged comparisons is definitely incorrect (without inserting freeze instructions), so we should avoid it.

There are easier ways to fix the first issue, but I figured it was worthwhile to do this properly to also fix the second one. What we now do is to restore the original relative order of (potentially merged) comparisons.

I took the liberty of dropping the MERGEICMPS_DOT_ON functionality, because it would be more awkward to implement now (as the before and after representation is different) and it doesn't seem terribly useful nowadays.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110024

Files:
  llvm/lib/Transforms/Scalar/MergeICmps.cpp
  llvm/test/Transforms/MergeICmps/X86/entry-block-shuffled-2.ll
  llvm/test/Transforms/MergeICmps/X86/entry-block-shuffled.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110024.373418.patch
Type: text/x-patch
Size: 16020 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210918/d170d842/attachment.bin>