Re: [PATCH] Index expmed.c's shift_cost by machine mode

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

> 2004-06-12  Roger Sayle  <roger@eyesopen.com>
>
> 	* expmed.c (shift_cost, shiftadd_cost, shiftsub_cost): Additionally
> 	index by machine mode.
> 	(init_expmed): Initialize shift_cost, shiftadd_cost and shiftsub_cost
> 	tables inside the loop over machine modes.
> 	(synth_mult, expand_mult_highpart_optab, expand_mult_highpart,
> 	expand_divmod): Index shift*_cost by the appropriate machine mode.

Broke bootstrap on SPARC 64-bit: lshift_significand is miscompiled.

It's specifically these lines:

> ! 	n = MIN (MAX_BITS_PER_WORD, GET_MODE_BITSIZE (mode));
> 	for (m = 1; m < n; m++)

This means that, on 64-bit targets, shifts by 32 in SImode have zero cost.  
So they are chosen in synt_mult when multiplying by -1 because the clamp is 
on BITS_PER_WORD, not GET_MODE_BITSIZE (mode):

      if (t % d == 0 && t > d && m < BITS_PER_WORD)

Then expand_shift enters the game and returns its unmodified argument.  So we 
end up with this RTL in 09.loop at -O2:

(insn 176 165 177 (set (reg:SI 169 [ ofs ])
        (reg/v:SI 113 [ ofs ])) -1 (nil)
    (nil))

(insn 177 176 178 (set (reg:SI 170)
        (minus:SI (reg:SI 169 [ ofs ])
            (reg/v:SI 113 [ ofs ]))) -1 (nil)
    (expr_list:REG_EQUAL (mult:SI (reg/v:SI 113 [ ofs ])
            (const_int 4294967295 [0xffffffff]))
        (nil)))

(insn 178 177 179 (set (reg:SI 171)
        (plus:SI (reg:SI 170)
            (reg:SI 170))) -1 (nil)
    (expr_list:REG_EQUAL (mult:SI (reg/v:SI 113 [ ofs ])
            (const_int -1 [0xffffffffffffffff]))
        (nil)))

(insn 179 178 181 (set (reg:SI 168)
        (plus:SI (reg:SI 171)
            (const_int 1 [0x1]))) -1 (nil)
    (nil))

(insn 181 179 183 (set (reg:SI 172)
        (neg:SI (reg/v:SI 113 [ ofs ]))) -1 (nil)
    (nil))

(insn 183 181 184 (set (reg:SI 174 [ ofs ])
        (reg/v:SI 113 [ ofs ])) -1 (nil)
    (nil))

(insn 184 183 185 (set (reg:SI 175)
        (minus:SI (reg:SI 174 [ ofs ])
            (reg/v:SI 113 [ ofs ]))) -1 (nil)
    (expr_list:REG_EQUAL (mult:SI (reg/v:SI 113 [ ofs ])
            (const_int 4294967295 [0xffffffff]))
        (nil)))

(insn 185 184 186 (set (reg:SI 176)
        (plus:SI (reg:SI 175)
            (reg:SI 175))) -1 (nil)
    (expr_list:REG_EQUAL (mult:SI (reg/v:SI 113 [ ofs ])
            (const_int -1 [0xffffffffffffffff]))
        (nil)))

for

  (mult:SI (reg/v:SI 113 [ ofs ]) (const_int -1))


Testcase attached, miscompiled by cross to sparc64-sun-solaris2.9 at -O2.


Restoring the original line

	for (m = 1; m < MAX_BITS_PER_WORD; m++)

fixes the problem.


Do you want to try anything else or can I commit the fix after testing?

-- 
Eric Botcazou

#define HOST_BITS_PER_LONG  64

#define SIGNIFICAND_BITS	(128 + HOST_BITS_PER_LONG)
#define EXP_BITS		(32 - 5)
#define MAX_EXP			((1 << (EXP_BITS - 1)) - 1)
#define SIGSZ			(SIGNIFICAND_BITS / HOST_BITS_PER_LONG)
#define SIG_MSB			((unsigned long)1 << (HOST_BITS_PER_LONG - 1))

struct real_value
{
  /* Use the same underlying type for all bit-fields, so as to make
     sure they're packed together, otherwise REAL_VALUE_TYPE_SIZE will
     be miscomputed.  */
  unsigned int /* ENUM_BITFIELD (real_value_class) */ class : 2;
  unsigned int sign : 1;
  unsigned int signalling : 1;
  unsigned int canonical : 1;
  unsigned int uexp : EXP_BITS;
  unsigned long sig[SIGSZ];
};

/* Various headers condition prototypes on #ifdef REAL_VALUE_TYPE, so it
   needs to be a macro.  We do need to continue to have a structure tag
   so that other headers can forward declare it.  */
#define REAL_VALUE_TYPE struct real_value

/* Left-shift the significand of A by N bits; put the result in the
   significand of R.  */

void
lshift_significand (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE *a,
		    unsigned int n)
{
  unsigned int i, ofs = n / HOST_BITS_PER_LONG;

  n &= HOST_BITS_PER_LONG - 1;
  if (n == 0)
    {
      for (i = 0; ofs + i < SIGSZ; ++i)
	r->sig[SIGSZ-1-i] = a->sig[SIGSZ-1-i-ofs];
      for (; i < SIGSZ; ++i)
	r->sig[SIGSZ-1-i] = 0;
    }
  else
    for (i = 0; i < SIGSZ; ++i)
      {
	r->sig[SIGSZ-1-i]
	  = (((ofs + i >= SIGSZ ? 0 : a->sig[SIGSZ-1-i-ofs]) << n)
	     | ((ofs + i + 1 >= SIGSZ ? 0 : a->sig[SIGSZ-1-i-ofs-1])
		>> (HOST_BITS_PER_LONG - n)));
      }
}