[all-commits] [llvm/llvm-project] f2fa6a: [MergeICmps] Don't reorder unmerged comparisons
Nikita Popov via All-commits
all-commits at lists.llvm.org
Tue Sep 21 12:22:30 PDT 2021
Branch: refs/heads/main
Home: https://github.com/llvm/llvm-project
Commit: f2fa6ad0476b318fdba46f09a2d59187228431ee
https://github.com/llvm/llvm-project/commit/f2fa6ad0476b318fdba46f09a2d59187228431ee
Author: Nikita Popov <nikita.ppv at gmail.com>
Date: 2021-09-21 (Tue, 21 Sep 2021)
Changed paths:
M llvm/lib/Transforms/Scalar/MergeICmps.cpp
A llvm/test/Transforms/MergeICmps/X86/entry-block-shuffled-2.ll
M llvm/test/Transforms/MergeICmps/X86/entry-block-shuffled.ll
Log Message:
-----------
[MergeICmps] Don't reorder unmerged comparisons
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
(reported in https://reviews.llvm.org/D108782#3005583).
* Reordering branches is generally not legal, because it may
introduce branch on poison, which is UB (PR51845). 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.
Differential Revision: https://reviews.llvm.org/D110024
More information about the All-commits
mailing list