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

[Roger Sayle]

Hi Eric,

On Sun, 13 Jun 2004, Eric Botcazou wrote:
> > 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)

Doh!  Obviously it makes no sense to shift a register by more than it's
mode's width.  The correct fix is to change each of these tests in
expmed.c to test against GET_MODE_BITSIZE (mode) instead of against
BITS_PER_WORD.  Or more safely, the lower of the two.

> 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?


I think this change is reasonable if you wish to restore bootstrap as
quickly as possible.  However my preference would be to add

  int maxm = MIN (MAX_BITS_PER_WORD, GET_MODE_BITSIZE (mode));

to the top of synth_mult, and then replace all but the last use of
MAX_BITS_PER_WORD in synth_mult with maxm.


Note that SImode multiplication by 0xffffffff should generate the same
sequence of shifts/adds as -1, even on host's where these aren't the
same value!

It's impressive that the backend's have never encountered problems
being asked to estimate the costs of clearly invalid shifts.  There
may even be a few platforms the benefit as synth_mult is no longer
misguided by cost estimates based on operand value etc...


If you'd prefer to commit your fix, whilst I bootstrap and regression
test the proposed solution above, let me know.  I'd appreciate it if
you could confirm that it also restores sparc64 bootstrap.

Sorry for the breakage.

Roger
--