[PATCH] Fix latent problem in expand_mult_highpart_optab

Roger Sayle roger@eyesopen.com
Fri Jan 28 04:03:00 GMT 2005


The following patch fixes a latent(ish) problem in GCC's RTL expanders
that was recently discovered by RTH as a bootstrap failure on alpha
triggered by one of my recent clean-ups to expmed.c.  The bootstrap
failure was later resolved by another of my clean-ups to expmed.c, but
I promised to find and fix the latent bug that had temporarily been
exposed.


The problem is that one of the implementation strategies in expmed.c's
expand_mult_highpart_optab attempts to implement highpart multiplication
by simply performing the multiplication in a wider mode.  Hence to
obtain the highpart of a SImode * SImode -> DImode multiplication,
the plan is to extend the operands to DImode and perform the non-widening
DImode multiplication then extract the highpart.  Unfortunately, this
code contained one critical oversight: it didn't actually extend the
operands and instead passed SImode registers and SImode constants to
expand_binop but with the mode argument set to DImode.  Impressively,
this (mostly) seems to work but don't ask me how :)


The reason my tweaks tripped this concerns the RTL representation for
integers.  Conventionally, although CONST_INT doesn't have a mode,
the RTL expanders and optimizers try to keep values canonical for
their intended/implicit mode, but always suitably sign-extending them
to HOST_WIDE_INT.  Unfortunately, expand_mult and friends predate this
guideline and instead currently always zero-extend SImode constants on
DImode hosts, even with the high-bit set.  This is tolerated as many
of the routines that operate on CONST_INTs ignore bits outside of the
implicit mode.


My original clean-up technically corrected this representation problem
by representing co-efficients in their sign-extended form by using
gen_int_mode.  "unsigned" division by 10, is done by multiplication by
0xcccccccd, which we represent on a DImode host as 0xffffffffcccccccd
instead of the 0x00000000cccccccd that choose_multiplier has used
traditionally.  Without the bug-fix to call to convert_modes in the
patch below, this "innocent" change accidentally results in using a
completely different constant in the DImode multiplication.  Now,
converting op1=0xffffffffcccccccd from SImode to DImode with unsignedp
true, gives 0x00000000cccccccd and all is happy again.


The following patch has been tested on both i686-pc-linux-gnu and
alphaev67-dec-osf5.1 with a full "make bootstrap", all default
languages, and regression tested with a top-level "make -k check"
with no new failures.  I've also confirmed on alphaev67-dec-osf5.1
that this patch fixes the bootstrap failure if expand_divmod is
tweaked to again sign-extend its coefficients.


Ok for mainline?



2005-01-27  Roger Sayle  <roger@eyesopen.com>

	* expmed.c (expand_mult_highpart_optab): When attempting to use
	a non-widening multiplication in a wider mode, the operands need
	to be converted (zero or sign extended) to that mode.


Index: expmed.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expmed.c,v
retrieving revision 1.218
diff -c -3 -p -r1.218 expmed.c
*** expmed.c	25 Jan 2005 14:28:55 -0000	1.218
--- expmed.c	27 Jan 2005 14:58:30 -0000
*************** expand_mult_highpart_optab (enum machine
*** 3332,3346 ****
      }

    /* Try widening the mode and perform a non-widening multiplication.  */
-   moptab = smul_optab;
    if (smul_optab->handlers[wider_mode].insn_code != CODE_FOR_nothing
        && size - 1 < BITS_PER_WORD
        && mul_cost[wider_mode] + shift_cost[mode][size-1] < max_cost)
      {
!       tem = expand_binop (wider_mode, moptab, op0, op1, 0,
  			  unsignedp, OPTAB_WIDEN);
        if (tem)
! 	return extract_high_half (mode, tem);
      }

    /* Try widening multiplication of opposite signedness, and adjust.  */
--- 3332,3360 ----
      }

    /* Try widening the mode and perform a non-widening multiplication.  */
    if (smul_optab->handlers[wider_mode].insn_code != CODE_FOR_nothing
        && size - 1 < BITS_PER_WORD
        && mul_cost[wider_mode] + shift_cost[mode][size-1] < max_cost)
      {
!       rtx insns, wop0, wop1;
!
!       /* We need to widen the operands, for example to ensure the
! 	 constant multiplier is correctly sign or zero extended.
! 	 Use a sequence to clean-up any instructions emitted by
! 	 the conversions if things don't work out.  */
!       start_sequence ();
!       wop0 = convert_modes (wider_mode, mode, op0, unsignedp);
!       wop1 = convert_modes (wider_mode, mode, op1, unsignedp);
!       tem = expand_binop (wider_mode, smul_optab, wop0, wop1, 0,
  			  unsignedp, OPTAB_WIDEN);
+       insns = get_insns ();
+       end_sequence ();
+
        if (tem)
! 	{
! 	  emit_insn (insns);
! 	  return extract_high_half (mode, tem);
! 	}
      }

    /* Try widening multiplication of opposite signedness, and adjust.  */

Roger
--



More information about the Gcc-patches mailing list