[PATCH] D151204: [VPlan] Allow sinking of instructions with no defs

Mon Jul 3 05:32:59 PDT 2023

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

LGTM with inline comments, thanks! `first-order-recurrence-multiply-recurrences.ll` needs updating, but the change there is fine and expected. We will be able to vectorize this again once FOR splice doesn't need to be considered as having side-effects.

================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:668
+    if (SinkCandidate->mayHaveSideEffects())
+          return false;
+
----------------
nit: indentation seems off here, please make sure the patch is formatted properly before landing.

================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:680
            "only recipes with a single defined value expected");
+    // Skip instruction if it doesn't define a new value (meaning there are no
+    // more users).
----------------
For now, I think with the fix above the original assert should be kept and handling for recipes with zero defined values should not be needed. If not, at least the assert will help to surface a test case.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151204