[PATCH] D142885: [VPlan] Allow building a VPlan to may fail.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8823
+    if (auto Plan = buildVPlanWithVPRecipes(SubRange, DeadInstructions))
+      VPlans.push_back(std::move(*Plan));
     VF = SubRange.End;
----------------
tschuett wrote:
> Ayal wrote:
> > fhahn wrote:
> > > Ayal wrote:
> > > > nit: why is `std::move()` needed here now, because `buildVPlanWithVPRecipes()` now returns an `std::optional<VPlanPtr>` instead of a `VPlanPtr`? Could `nullptr` be returned instead of `optional`?
> > > IIUC it's needed because `std::unique_ptr` doesn't allow copying.
> > Ok, but can an empty unique_ptr initialized to null be returned, instead of optional and move?
> std::nullopt is the modern and saver version of nullptr. It is more obvious that std::nullopt denotes failure.
We cannot avoid the move I think, regardless of whether std::optional is used or not. Without the move, std::unique_ptr's copy constructor would be called which is deleted.

I could change it to not use std::optional while retaining the move if you have a strong preference to avoid `std::optional`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142885