[PATCH] D54885: Assigning to a local object in a return statement prevents copy elision. NFC.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/Support/BranchProbability.h:131-133
     BranchProbability Prob(*this);
-    return Prob += RHS;
+    Prob += RHS;
+    return Prob;
----------------
Would an alternative fix for this be to write ref-qualified overloads of opX=, then use them like:

  return BranchProbability(*this) += RHS;

? (or, I suppose, "BranchProbability Prob(*this); return std::move(Prob) += RHS;")


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:7845
   if (ShouldInvert)
-    return Cmp = DAG.getNOT(dl, Cmp, Cmp.getValueType());
+    Cmp = DAG.getNOT(dl, Cmp, Cmp.getValueType());
 
----------------
This looks like a good change regardless - the original phrasing is pretty weird, given that Cmp is going out of scope anyway, it could've been written as "return DAG.getNOT(...);" & skipped the assignment, but your solution here's more consistent with the surrounding code, I think


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54885