[PATCH] D97136: [VPlan] Support to widen select intructions in VPlan native path

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 2 01:39:10 PST 2021


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM with a few additional changes.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:20
 void VPlanTransforms::VPInstructionsToVPRecipes(
-    Loop *OrigLoop, VPlanPtr &Plan,
+    ScalarEvolution &SE, Loop *OrigLoop, VPlanPtr &Plan,
     LoopVectorizationLegality::InductionList &Inductions,
----------------
I think it is more common to pass generic analysis at the end in LLVM. Might be good to adjust.


================
Comment at: llvm/test/Transforms/LoopVectorize/vplan-widen-select-instruction.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -loop-vectorize -force-vector-width=4 -enable-vplan-native-path -S %s | FileCheck %s
----------------
Looks like the tests are not auto-generated. Probably best to drop this?


================
Comment at: llvm/test/Transforms/LoopVectorize/vplan-widen-select-instruction.ll:28
+  %select.b = select i1 %select, double %a, double %b
+  %a.reduction1 = fadd double %select.b, %a.reduction
+  %indvar21 = add nuw nsw i64 %indvar2, 1
----------------
the body could probably be a bit further simplified by getting rid of the reduction in the inner loop and instead storing the select result directly in the inner loop (instead of storing sum in the outer loop latch). Same for the other tests.

With the reduced loops, could you add checks ensuring the the select is also in the expected blocks and the operands and result of the select are the values we expect/used as expected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97136



More information about the llvm-commits mailing list