[PATCH] D107262: [CodeGenPrepare] The instruction to be sunk should be inserted before its user in a block

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

This happens because out of the chain of `insert; shuffle` only the `insert` needs to be sunk?
It may be better to remove the `UI->getParent() == TargetBB` from `if (UI->getParent() == TargetBB || isa<PHINode>(UI))` and update `InsertPoint` in the loop as we go. It won't need to clone the instruction, but it can update InsertPoint and continue.



================
Comment at: llvm/test/Transforms/CodeGenPrepare/AArch64/sink-free-instructions-1.ll:4-5
+
+; ModuleID = '<stdin>'
+source_filename = "<stdin>"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
----------------
This can be removed.


================
Comment at: llvm/test/Transforms/CodeGenPrepare/AArch64/sink-free-instructions-1.ll:25
+  %broadcast.splatinsert143 = insertelement <4 x i32> poison, i32 %conv25, i32 0
+  br i1 undef, label %for.cond4.preheader.us.preheader, label %for.cond4.preheader.preheader
+
----------------
Bugpoint has a habit of over-reducing test cases, introducing undef where they only make things worse and less maintainable in the longrun. Can you make the test not use any undef?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107262