[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