[PATCH] D75011: [InstCombine] Remove trivially empty ranges from end

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 2 inline comments as done.
nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1488-1489
+        continue;
+      if (IsStart(*I)) {
+        if (haveSameOperands(EndI, *I, EndI.getNumArgOperands())) {
+          IC.eraseInstFromFunction(*I);
----------------
spatel wrote:
> Is there a logic change from 'if &&' to 'if, if ...continue'? If so, that deserves a code comment. 
> 
> If it's just a matter of taste, it would be better to make that + a partial function signature change as a preliminary NFC commit. (Trying to limit patch risk here because I'm not very familiar with these transforms.)
There is a logic change: This skips start intrinsics that don't pair with the end intrinsic (I've added a comment to that effect below).

This doesn't affect the final behavior (due to the existing skipping for the end intrinsic), but avoids doing extra iterations, for example:

```
start(0) <- erase
start(1) <- skip
end(0) <- I
end(1)
```

Without this change, we'd not do anything for `end(0)`, but then

```
start(0)
start(1) <- erase
end(0) <- skip
end(1) <- I
```

and would collect `end(0)` on the next InstCombine iteration.


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

https://reviews.llvm.org/D75011