[PATCH] D28907: [SLP] Fix for PR30787: Failure to beneficially vectorize 'copyable' elements in integer binary ops.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1324
 
+    /// Reorder bundles from PseudoScheduleData data after scheduling,
+    /// if an Instruction is present in PseudoScheduleData that means this
----------------
dtemirbulatov wrote:
> dtemirbulatov wrote:
> > ABataev wrote:
> > > dtemirbulatov wrote:
> > > > ABataev wrote:
> > > > > If you implement `InstructionOrPseudoOp` correctly, this reorder stuff should not be required, I think
> > > > well, There should be several(>=2) independent scheduling events(one for real instruction and other for pseudos) and there is just one real instruction, in the end, I don't see how it could be done without reordering or tracking the last scheduled instance for the same instruction. We could introduce something like IsLastScheduled field in ScheduleData struct, but it would be quite similar to reordering.
> > > If you add the real instruction instead of this pseudoinstruction, will you need all these scheduling events? No. Will you need to do some extra reordering etc.? No. Why you cannot simulate it with the new class/structure?
> > do you mean that the last one scheduling becomes the real one and we just ignore any pseudos?
> do you mean that the last one scheduled becomes the real one and we just ignore any pseudos?
No. I mean, that if you insert the real instructions instead of those pseudo-instructions, you won't need all that reordering/new scheduling etc. Why can't you mimic this behavior with the pseudo-instruction?


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

https://reviews.llvm.org/D28907