Re: remembering comparison modes

[Michael Meissner <meissner at cygnus dot com>]

On Tue, Jan 23, 2001 at 11:53:18PM -0500, DJ Delorie wrote:
> 
> Back in July, this patch was applied, which fixed 990324-1 but caused
> 960608-1 to fail, at least in my particular build:
> 
> Sun Jul 30 20:08:37 MET DST 2000  Jan Hubicka  <jh@suse.cz>
> 
> 	* simplify-rtx.c (simplify_relational_operation): Verify that mode ==
> 	VOIDmode implies both operands to be VOIDmode.
> 	(simplify_ternary_operation): Compute properly the mode of comparison.
> 	* combine.c (combine_simplify_rtx): Likewise.
> 
> In the affected functions, the code takes pains to keep track of the
> mode the comparisons should be done in.  With that patch, that mode is
> ignored.  In the case where both the modes are VOIDmode, shouldn't we
> give the original mode a chance to control the mode of the comparison,
> as in the patch below?  This one lets both testcases pass.
> 
> The code is doing this when it breaks:
> 
> (insn 30 28 76 (set (reg:QI 58)
>         (const_int -50 [0xffffffffffffffce])) 42 {*movqi_1} (nil)
>     (nil))
> 
> (insn/i 81 80 82 (set (reg:CC 17 flags)
>         (compare:CC (reg:QI 58)
>             (const_int 206 [0xce]))) 7 {*cmpqi_1} (insn_list 30 (nil))
>     (expr_list:REG_DEAD (reg:QI 58)
>         (nil)))
> 
> (insn/i 82 81 88 (set (strict_low_part (subreg:QI (reg:SI 64) 0))
>         (ne:QI (reg:CC 17 flags)
>             (const_int 0 [0x0]))) 250 {*setcc_2} (insn_list 80 (insn_list 81 (nil)))
>     (expr_list:REG_DEAD (reg:CC 17 flags)
>         (nil)))
> 
> dj: simplify code ne mode QI (op0 QI, op1 VOID)
> 
> (reg:QI 58)
> 
> (const_int 206 [0xce])
> 
> dj: simplify code ne mode QI (op0 VOID, op1 VOID)
> 
> (const_int -50 [0xffffffffffffffce])
> 
> (const_int 206 [0xce])
> 
> In the last step (above), if the comparison is done in VOIDmode, the
> result is wrong, but if it's done in QImode as op0_mode says, the
> results are correct.
> 
> Index: combine.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/combine.c,v
> retrieving revision 1.178
> diff -p -2 -r1.178 combine.c
> *** combine.c	2001/01/17 18:27:37	1.178
> --- combine.c	2001/01/24 04:19:47
> *************** combine_simplify_rtx (x, op0_mode, last,
> *** 3656,3659 ****
> --- 3656,3661 ----
>   	if (cmp_mode == VOIDmode)
>   	  cmp_mode = GET_MODE (XEXP (x, 1));
> + 	if (cmp_mode == VOIDmode)
> + 	  cmp_mode = op0_mode;
>   	temp = simplify_relational_operation (code, cmp_mode,
>   					      XEXP (x, 0), XEXP (x, 1));

I would probably write the above as:

	if (cmp_mode == VOIDmode)
	  {
	    cmp_mode = GET_MODE (XEXP (x, 1));
	    if (cmp_mode == VOIDmode)
	      cmp_mode = op0_mode;
	  }

as a microoptimization.

> Index: simplify-rtx.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
> retrieving revision 1.38
> diff -p -2 -r1.38 simplify-rtx.c
> *** simplify-rtx.c	2001/01/08 15:16:40	1.38
> --- simplify-rtx.c	2001/01/24 04:19:50
> *************** simplify_ternary_operation (code, mode, 
> *** 2058,2061 ****
> --- 2058,2063 ----
>   					? GET_MODE (XEXP (op0, 1))
>   					: GET_MODE (XEXP (op0, 0)));
> + 	  if (cmp_mode == VOIDmode)
> + 	    cmp_mode = op0_mode;
>   	  rtx temp
>   	     = simplify_relational_operation (GET_CODE (op0), cmp_mode,

Other than that, the patch looks ok.  I would argue the the x86 machine
description should use an appropriate mode on strict_low_part, ie:

	(set (strict_low_part:QI (subreg:QI (reg:SI ..)))
	     ..)

-- 
Michael Meissner, Red Hat, Inc.  (GCC group)
PMB 198, 174 Littleton Road #3, Westford, Massachusetts 01886, USA
Work:	  meissner@redhat.com		phone: +1 978-486-9304
Non-work: meissner@spectacle-pond.org	fax:   +1 978-692-4482