[PATCH] D78728: [ARM] Convert floating point splats to integer

[Dave Green via Phabricator via llvm-commits]

dmgreen added a subscriber: spatel.
dmgreen added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:6436
 /// codegen can spot all lanes are identical.
-bool CodeGenPrepare::optimizeShuffleVectorInst(ShuffleVectorInst *SVI) {
+bool CodeGenPrepare::sinkShuffleVectorToShift(ShuffleVectorInst *SVI) {
   BasicBlock *DefBB = SVI->getParent();
----------------
efriedma wrote:
> I guess this is only loosely related to your patch, but should this transform also be handled by tryToSinkFreeOperands?
Yeah, I think it pre-dates the tryToSinkFreeOperands version but only handles the shifts that X86 cares about. I can have a look at changing it over to see if it can use SinkFreeOperands, but @spatel is updating it in D79718. I'll at least wait until that has been done.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:6525
+    if (auto *Op = dyn_cast<Instruction>(BCI->getOperand(0)))
+      if (BCI->getParent() != Op->getParent() && !isa<PHINode>(Op))
+        BCI->moveAfter(Op);
----------------
efriedma wrote:
> Checking whether an insertion point is legal is more complicated than this.  In particular, you can't insert code after an invoke/callbr, and you can't insert code into a block terminated by a catchswitch.
> 
> I'm not sure if there's a helper for this check anywhere.
Ah right. I've added Terminators and EHPads (mostly because there is likely little to be gained from moving the bitcast, even if it is safe). That with the phi's should exclude catchswitch too I believe.

There is ScalarEvolutionExpander::findInsertPointAfter which is trying to find an insertion point after Op. Here we don't care as much about giving up, and some of the semantics of following invokes will be different. I didn't spot much else.

I can try and make a function if you think that's better.


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

https://reviews.llvm.org/D78728