[PATCH] D12596: Fix for bootstrap bug introduced in r244921

[hfinkel@anl.gov via llvm-commits]

hfinkel requested changes to this revision.
hfinkel added a comment.
This revision now requires changes to proceed.

In http://reviews.llvm.org/D12596#248753, @nemanjai wrote:

> Hi Hal, sorry I was on vacation so I'm only getting to this now.
>  I agree that this code sequence in isolation is not better than what we had in the particular case. However, the direct move sequence is applicable in every situation whereas the old simpler sequence only occurred in the specific sequence you mentioned. Furthermore, if the loaded value is used for non-vector computations as well, it is still in the register we moved it from - although I have no idea if this actually happens or if LLVM infrastructure knows this.
>  Please advise whether I should:
>
> 1. Check this in as-is and move on
> 2. Add some info in the README about improving this sequence
> 3. Re-implement SCALAR_TO_VECTOR of v2i64 as custom lowering, detect this sequence and do not emit the direct move
> 4. Do something different - i.e. a peephole optimization or something along those lines
>
>   I am of the opinion that going with options 1 or 2 is fine as the new sequence is only marginally worse than what we had and it is a lot of work to get this small win.


Something more like (3), although you might be able to do that in TableGen too (we have patterns that use callbacks to match shuffle patterns already).

Before that, however, I still don't understand the problem. Why does it matter how SCALAR_TO_VECTOR is lowered? We're selecting the shuffle node to be the LXVDSX, so it becomes the result of that shuffle (which does represent a splat). The SCALAR_TO_VECTOR is then dead regardless of how it is lowered, if it is lowered at all.


================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:2785
@@ +2784,3 @@
+         those are available. In that case, we can't just put in a VSX load
+         and splat here.
+      */
----------------
You'd need to explain *why* in the comment.


Repository:
  rL LLVM

http://reviews.llvm.org/D12596