[PATCH] D65740: [ARM][ParallelDSP] Replace SExt uses

[Eli Friedman via Phabricator via llvm-commits]

efriedma added reviewers: SjoerdMeijer, dmgreen.
efriedma added a comment.

How did we end up with this testing gap?  I haven't been closely watching what tests were committed with previous patches.

Could you have caught this earlier by using AssertingVH in more places?

If you're confident this is the right patch, we can continue discussing post-commit; I'd like to unbreak the Android bot.



================
Comment at: lib/Target/ARM/ARMParallelDSP.cpp:700
+  Value *NewBaseSExt = IRB.CreateSExt(Bottom, BaseSExt->getType());
+  BaseSExt->replaceAllUsesWith(NewBaseSExt);
 
----------------
I don't understand how this makes a difference; the new SExt has to have the same operand and result type as the old one, I think, so making a new SExt followed by RAUW should be the same as changing the operand of the original.  Is the old instruction in the wrong position?  Or is there some sort of invalidation issue?


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

https://reviews.llvm.org/D65740