[PATCH] D77972: [VPlan] Move Load/Store checks out of tryToWiden (NFC).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Thanks for this cleanup!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6993
     case Instruction::Or:
     case Instruction::PHI:
     case Instruction::PtrToInt:
----------------
Another cleanup: remove PHI from this list and replace it with an assert(!isa<PHINode>(I)) at the outset, as PHIs are all handled by other, earlier recipes. This will further simplify willWiden() below, which can also absorb IsPredicated above: for non IsVectorizableOpcodes, handleReplication() will clamp the range according to IsPredicated anyhow.

(That will in turn lead to identical willWiden() across tryToWiden, tryToWidenSelect, and the inlined tryToWidenGEP ("Scalarize"); which suggests folding/further cleanup.)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7059
+          Range) &&
+          "Memory widening decisions should have been taken care of by now.");
+
----------------
tryToWidenMemoryInstruction() is expected to have clamped the Range already; if not, Range should either be asserted to be clamped (w/o clamping) or it should be clamped (redundantly) but not under assert. Note that checking VF==1, Load or Store should be done alongside IsPredicated, outside getDecisionAndClampRange(), as they're independent of Range.

This assert, moved here from tryToWiden(), is practically the complementary of tryToWidenMemoryInstruction()'s willWiden(). Due to its complexity and redundancy, it should probably simply be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77972