Is this redundant?

[Torbjorn Granlund]

Paul Zimmermann <Paul.Zimmermann at loria.fr> writes:

  >    if (UNLIKELY (res->_mp_alloc < res_size))
  >      {
  >        _mpz_realloc (res, res_size);
  >        op1_ptr = op1->_mp_d;
  >        op2_ptr = op2->_mp_d;
  >       //** nobody modified op1_ptr,op1->_mp_d,op2_ptr,op2->_mp_d. Why do we
  > need to this again?
  >        res_ptr = res->_mp_d;
  >      }
  > I think it is redundant.
  
  Remember that res might equal op1 and/or op2. In that case the call
  to _mpz_realloc will change the pointer op1->_mp_d and/or op2->_mp_d.
  
  However if res = op1 or res = op1, then I guess
  ALLOC(res) >= min(ALLOC(op1), ALLOC(op2)) is true, thus it seems to me that
  the case ALLOC(res) < res_size <= MIN (op1_size, op2_size) cannot happen.
  At least this case is not covered by "make check".
  
I might make the following change.  If somebody feels like analysing the
code mpz/{and,ior,xor}.c for more opportunities, I'd appreciate it.

diff -r 430d1206f0f2 mpz/and.c
--- a/mpz/and.c	Tue Nov 11 17:45:30 2008 +0100
+++ b/mpz/and.c	Sun Nov 16 21:09:26 2008 +0100
@@ -55,9 +55,10 @@
 	  if (UNLIKELY (res->_mp_alloc < res_size))
 	    {
 	      _mpz_realloc (res, res_size);
-	      op1_ptr = op1->_mp_d;
-	      op2_ptr = op2->_mp_d;
 	      res_ptr = res->_mp_d;
+	      /* Don't re-read OP1_PTR and OP2_PTR.  Since res_size <=
+		 MIN(op1_size, op2_size), we will not reach this code when op1
+		 is identical to res or op2 is identical to res.  */
 	    }
 
 	  res->_mp_size = res_size;
@@ -251,7 +252,8 @@
 	  {
 	    _mpz_realloc (res, res_size);
 	    res_ptr = res->_mp_d;
-	    op1_ptr = op1->_mp_d;
+	    /* Don't re-read OP1_PTR.  Since res_size <= op1_size, we will
+	       not reach this code when op1 is identical to res.  */
 	    /* Don't re-read OP2_PTR.  It points to temporary space--never
 	       to the space RES->_mp_d used to point to before reallocation.  */
 	  }
diff -r 430d1206f0f2 mpz/ior.c
--- a/mpz/ior.c	Tue Nov 11 17:45:30 2008 +0100
+++ b/mpz/ior.c	Sun Nov 16 21:09:26 2008 +0100
@@ -48,7 +48,7 @@
 	      if (res->_mp_alloc < op1_size)
 		{
 		  _mpz_realloc (res, op1_size);
-		  op1_ptr = op1->_mp_d;
+		  /* No overlapping possible: op1_ptr = op1->_mp_d; */
 		  op2_ptr = op2->_mp_d;
 		  res_ptr = res->_mp_d;
 		}
@@ -66,7 +66,7 @@
 		{
 		  _mpz_realloc (res, op2_size);
 		  op1_ptr = op1->_mp_d;
-		  op2_ptr = op2->_mp_d;
+		  /* No overlapping possible: op2_ptr = op2->_mp_d; */
 		  res_ptr = res->_mp_d;
 		}
 

-- 
Torbjörn