[PATCH] D105020: [SLP]Improve graph reordering.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2973
+/// Tracks the state we can represent the loads in the given sequence.
+enum class LoadsState { Gather, Vectorize, ScatterVectorize };
+} // anonymous namespace
----------------
RKSimon wrote:
> This is very similar to the EntryState enum - merge them?
I would not do this. Though the values look similar the meaning is completely different. This state handles only loads, while the entry state handles all possible entries kinds. In the future, we may have different values in these enums, which may lead to some unpredictable results. I would keep it as is.


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/addsub.ll:347
 ;  In the above code we can swap the 1st and 2nd operation as fadd is commutative
 ;  but not 2nd or 4th as fsub is not commutative.
 
----------------
RKSimon wrote:
> Update comment? I had to do something similar on D103925, although the wording here might be different.
Ok, will do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105020