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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7059
+          Range) &&
+          "Memory widening decisions should have been taken care of by now.");
+
----------------
Ayal wrote:
> 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.
Yeah, initially I was hoping there would be a general way to assert that we only call handleReplication with instructions we expect to scalarize, but it seems like there is no general way to write such an assertion. The assertion I added is not that helpful in that regard, I've removed it as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77972