[PATCH] D98316: [rs4gc] Simplify code by cloning existing instructions when inserting base chain [NFC]

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D98316#2627960 <https://reviews.llvm.org/D98316#2627960>, @skatkov wrote:

> 

< ...

> So the main difference I see is related to ShuffleVector but not phi.
>
> Not sure but original way seems a bit more protective than new one,
> What kind of "some spurious test changes" do you observe?

For the phi case, the new pattern is actually safer.  Consider the case where we miss an operand.  With the old code, we'd have a syntactically valid phi node with the wrong input.  With the new code, the verifier would reject the phi as it would be missing an operand.

You're right that (combined with the later change) the primary effect is reusing the second operand of the shuffle.  It turns out I'd screwed up the diffs I'd posted.  I'd posted that change *without* being rebased on this one.  The diffs I'd been concerned about was the use of undef operands (as opposed to the original operand).  What is very unclear to my now, is why I thought that was a bad thing.  :)

I ended up landing this with a change to explicitly use undef for the second operand of a broadcast.  That's entirely NFC, and frankly, looks cleaner to me now.  Now sure what I was thinking when I first posted this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98316