This is the mail archive of the
`gcc-patches@gcc.gnu.org`
mailing list for the GCC project.

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |

Other format: | [Raw text] |

*From*: Roger Sayle <roger at eyesopen dot com>*To*: gcc-patches at gcc dot gnu dot org*Cc*: Richard Henderson <rth at redhat dot com>*Date*: Thu, 27 Jan 2005 19:18:59 -0700 (MST)*Subject*: [PATCH] Fix latent problem in expand_mult_highpart_optab

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

**Follow-Ups**:**Re: [PATCH] Fix latent problem in expand_mult_highpart_optab***From:*Richard Henderson

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |