[PATCH] D78383: [LVI] Use Optional instead of out parameter (NFC)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: lib/Analysis/LazyValueInfo.cpp:839
+    return None;
+  ValueLatticeElement TrueVal = *OptTrueVal;
 
----------------
fhahn wrote:
> nikic wrote:
> > nikic wrote:
> > > My C++ foo is weak... would the proper way to write this without copying the lattice value be `TrueValue = *std::move(OptTrueVal)`?
> > Hm, it looks like moving an Optional moves the value (rather than making the Optional empty). As ValueLatticeElement does not specify a move constructor, I guess that means it would end up being copied, so that probably doesn't make sense.
> Can this just be `ValueLatticeElement &TrueVal`?
Right, I went with that variant now. The use of Optional will still make for a small but measurable regression (~0.1%), but that seems acceptable to me.

I'll also submit a followup to add a move constructor to ValueLatticeElement. It makes no difference for performance in practice, because we usually deal with APInts <= 64 bits, so move and copy constructor do about the same work. It would make a difference for large APInts.


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

https://reviews.llvm.org/D78383