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

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MergeICmps.cpp:406
   PHINode &Phi_;
-  std::vector<BCECmpBlock> Comparisons_;
+  std::vector<std::vector<BCECmpBlock>> MergedBlocks_;
   // The original entry block (before sorting);
----------------
Can you please add a comment for the reader to say what these represent ? Something like "The list of all blocks in the chain, grouped by contiguity."

I think it would also help readability to introduce an alias:

```
using ContiguousBlocks = std::vector<BCECmpBlock>;

std::vector<ContiguousBlocks> MergedBlocks;
```



================
Comment at: llvm/lib/Transforms/Scalar/MergeICmps.cpp:427
+static std::vector<std::vector<BCECmpBlock>>
+mergeBlocks(std::vector<BCECmpBlock> &&Blocks) {
+  std::vector<std::vector<BCECmpBlock>> MergedBlocks;
----------------
Add a comment to state what this does, e.g.

```
// Given a  chain of comparison blocks, groups the blocks into contiguous ranges that can be merged together in a single comparison.
```


================
Comment at: llvm/test/Transforms/MergeICmps/X86/entry-block-shuffled-2.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -mergeicmps < %s | FileCheck %s
----------------
can you submit the test as a preliminary step so that we can see the diff here ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110024/new/

https://reviews.llvm.org/D110024