[PATCH] D136544: [SLP] For vectorizing chains in basic block, decide order of PHI nodes based on their result use.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:12439
+      return true;
+    if (!(V1->user_empty()) && !(V2->user_empty())) {
+      const Instruction *FirstUserOfPhi1 =
----------------
Remove extra parens


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:12440
+    if (!(V1->user_empty()) && !(V2->user_empty())) {
+      const Instruction *FirstUserOfPhi1 =
+          dyn_cast<Instruction>(*V1->user_begin());
----------------
Can this be a non-instruction user? Rather doubt, do it should be a cast


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:12446
+          (FirstUserOfPhi1->getParent() == FirstUserOfPhi2->getParent())) {
+        if (FirstUserOfPhi1->comesBefore(FirstUserOfPhi2))
+          return true;
----------------
Just `return FirstUserOfPhi1->comesBefore(FirstUserOfPhi2); `


================
Comment at: llvm/test/Transforms/SLPVectorizer/AMDGPU/phi-result-use-order.ll:1
+; RUN: opt -passes=slp-vectorizer -S -mtriple=amdgcn-amd-amdhsa -mcpu=gfx906 < %s | FileCheck %s
+
----------------
Precommit this test in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136544