[PATCH] D50999: [APFloat] Remove dead store in the move constructor of IEEEFloat

[Victor Gao via Phabricator via llvm-commits]

vgao created this revision.
vgao added reviewers: bkramer, sberg, timshen.
Herald added subscribers: llvm-commits, dexonsmith.

In the move constructor, field semantics is first set to &semBogus, which is a static constant.
It is then set to rhs.semantics, killing the first store. This dead store is not removed in https://reviews.llvm.org/owners/package/3/.

freeSignificand() is called between the two stores, and it does read from the field semantics. However with semantics set to &semBogus, this operation always evaluates to nothing.

This diff adds a helper function (moveFields) that copies the fields without calling freeSignificand().
By doing this we save a store (and potentially a call to freeSignicand()) in IEEEFloat(IEEEFloat &&), and do not lose any performance in operator =(IEEEFloat &&), since moveFields is inlined in both callsites.


Repository:
  rL LLVM

https://reviews.llvm.org/D50999

Files:
  include/llvm/ADT/APFloat.h
  lib/Support/APFloat.cpp


Index: lib/Support/APFloat.cpp
===================================================================
--- lib/Support/APFloat.cpp
+++ lib/Support/APFloat.cpp
@@ -744,9 +744,7 @@
   return *this;
 }
 
-IEEEFloat &IEEEFloat::operator=(IEEEFloat &&rhs) {
-  freeSignificand();
-
+void IEEEFloat::moveFields(IEEEFloat &&rhs) {
   semantics = rhs.semantics;
   significand = rhs.significand;
   exponent = rhs.exponent;
@@ -754,6 +752,11 @@
   sign = rhs.sign;
 
   rhs.semantics = &semBogus;
+}
+
+IEEEFloat &IEEEFloat::operator=(IEEEFloat &&rhs) {
+  freeSignificand();
+  moveFields(std::move(rhs));
   return *this;
 }
 
@@ -873,9 +876,7 @@
   assign(rhs);
 }
 
-IEEEFloat::IEEEFloat(IEEEFloat &&rhs) : semantics(&semBogus) {
-  *this = std::move(rhs);
-}
+IEEEFloat::IEEEFloat(IEEEFloat &&rhs) { moveFields(std::move(rhs)); }
 
 IEEEFloat::~IEEEFloat() { freeSignificand(); }
 
Index: include/llvm/ADT/APFloat.h
===================================================================
--- include/llvm/ADT/APFloat.h
+++ include/llvm/ADT/APFloat.h
@@ -537,6 +537,8 @@
   void copySignificand(const IEEEFloat &);
   void freeSignificand();
 
+  void moveFields(IEEEFloat &&);
+
   /// Note: this must be the first data member.
   /// The semantics that this value obeys.
   const fltSemantics *semantics;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50999.161569.patch
Type: text/x-patch
Size: 1295 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180820/153effd1/attachment.bin>