Bug 49088 - Combine fails to properly handle zero-extension and signed int constant
Summary: Combine fails to properly handle zero-extension and signed int constant
Status: RESOLVED DUPLICATE of bug 49504
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-20 16:57 UTC by H.J. Lu
Modified: 2011-06-23 14:57 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-05-20 19:42:51


Attachments
A patch (750 bytes, patch)
2011-05-20 20:59 UTC, H.J. Lu
Details | Diff
A new testcase (636 bytes, patch)
2011-05-21 01:03 UTC, H.J. Lu
Details | Diff
An updated patch (814 bytes, patch)
2011-05-21 04:28 UTC, H.J. Lu
Details | Diff
A new patch (683 bytes, patch)
2011-05-21 13:25 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2011-05-20 16:57:06 UTC
We have

(insn 5 2 7 2 (parallel [
            (set (reg/f:DI 61)
                (plus:DI (reg/f:DI 20 frame)
                    (const_int -64 [0xffffffffffffffc0])))
            (clobber (reg:CC 17 flags))
        ]) x.i:12 252 {*adddi_1}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))
(insn 17 9 18 2 (parallel [
            (set (reg:SI 72)
                (plus:SI (subreg:SI (reg/f:DI 61) 0)
                    (const_int 6 [0x6])))
            (clobber (reg:CC 17 flags))
        ]) x.i:14 251 {*addsi_1}
     (expr_list:REG_DEAD (reg/f:DI 61)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

(insn 18 17 10 2 (set (reg:DI 73)
        (zero_extend:DI (reg:SI 72))) x.i:14 112 {*zero_extendsidi2_rex64}
     (expr_list:REG_DEAD (reg:SI 72)
        (nil)))

(insn 11 10 12 2 (set (reg:DI 68)
        (reg:DI 73)) x.i:14 62 {*movdi_internal_rex64}
     (expr_list:REG_DEAD (reg:DI 73)
        (nil)))

combine turns it into

(insn 18 17 10 2 (set (reg:DI 73) 
        (const_int 4294967238 [0xffffffc6])) x.i:14 62 {*movdi_internal_rex64}
     (nil))

(insn 11 10 12 2 (set (reg:DI 68)
        (plus:DI (reg/f:DI 20 frame)
            (reg:DI 73))) x.i:14 247 {*lea_1}
     (expr_list:REG_DEAD (reg:DI 73)
        (nil)))

"const_int -64" is signed, not unsigned.
Comment 1 H.J. Lu 2011-05-20 17:19:26 UTC
We have

#10 0x0000000001121405 in simplify_and_const_int_1 (mode=DImode, 
    varop=0x7ffff0acbdf8, constop=4294967294)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:9413
9413	  varop = force_to_mode (varop, mode, constop, 0);
(gdb) call debug_rtx (varop)
(subreg:DI (plus:SI (subreg:SI (reg/f:DI 20 frame) 0)
        (const_int -58 [0xffffffffffffffc6])) 0)
(gdb) p/x constop
$5 = 0xfffffffe
(gdb) p mode
$6 = DImode
(gdb) f 9
#9  0x000000000111db3a in force_to_mode (x=0x7ffff0acbdf8, mode=DImode, 
    mask=4294967294, just_select=0)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:8154
8154	    return force_to_mode (SUBREG_REG (x), mode, mask, next_select);
(gdb) call debug_rtx (x)
(subreg:DI (plus:SI (subreg:SI (reg/f:DI 20 frame) 0)
        (const_int -58 [0xffffffffffffffc6])) 0)
(gdb) 

I think we need to adjust mask by sign-extend it.
Comment 2 H.J. Lu 2011-05-20 18:27:51 UTC
(gdb) bt
#0  force_to_mode (x=0x7ffff0acbdf8, mode=DImode, mask=4294967294, 
    just_select=0) at /export/gnu/import/git/gcc-x32/gcc/combine.c:8154
#1  0x0000000001121405 in simplify_and_const_int_1 (mode=DImode, 
    varop=0x7ffff0acbdf8, constop=4294967294)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:9413
#2  0x00000000011216e4 in simplify_and_const_int (x=0x0, mode=DImode, 
    varop=0x7ffff0acbdf8, constop=4294967294)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:9511
#3  0x0000000001123c57 in simplify_shift_const_1 (code=LSHIFTRT, 
    result_mode=DImode, varop=0x7ffff0acbdf8, orig_count=32)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:10568
#4  0x0000000001123d45 in simplify_shift_const (x=0x0, code=LSHIFTRT, 
    result_mode=DImode, varop=0x7ffff0acbe10, count=32)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:10599
#5  0x000000000111a923 in expand_compound_operation (x=0x7ffff0c4ddb0)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:6916
(gdb) f 5
#5  0x000000000111a923 in expand_compound_operation (x=0x7ffff0c4ddb0)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:6916
6916	      tem = simplify_shift_const (NULL_RTX, unsignedp ? LSHIFTRT : ASHIFTRT,
(gdb) call debug_rtx (x)
(zero_extend:DI (plus:SI (subreg:SI (reg/f:DI 20 frame) 0)
        (const_int -58 [0xffffffffffffffc6])))
(gdb)
Comment 3 H.J. Lu 2011-05-20 19:01:14 UTC
This patch works:

diff --git a/gcc/combine.c b/gcc/combine.c
index 8af86f2..60c6e13 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10184,6 +10184,7 @@ simplify_shift_const_1 (enum rtx_code code, enum machine_mode result_mode,
 
         if (code == ASHIFTRT
        || (code == ROTATE && first_code == ASHIFTRT)
+       || (code == LSHIFTRT && first_code == ASHIFT)
        || GET_MODE_BITSIZE (mode) > HOST_BITS_PER_WIDE_INT
        || (GET_MODE (varop) != result_mode
            && (first_code == ASHIFTRT || first_code == LSHIFTRT

Does it make sense?
Comment 4 Eric Botcazou 2011-05-20 19:42:51 UTC
> combine turns it into
> 
> (insn 18 17 10 2 (set (reg:DI 73) 
>         (const_int 4294967238 [0xffffffc6])) x.i:14 62 {*movdi_internal_rex64}
>      (nil))
> 
> (insn 11 10 12 2 (set (reg:DI 68)
>         (plus:DI (reg/f:DI 20 frame)
>             (reg:DI 73))) x.i:14 247 {*lea_1}
>      (expr_list:REG_DEAD (reg:DI 73)
>         (nil)))
> 
> "const_int -64" is signed, not unsigned.

RTL constants are always sign-extended for their mode but RTL objects, including constants, have no specific signedness, except for a few exceptions like the SUBREG_PROMOTED_UNSIGNED_P flag.

That being said, the transformation looks indeed invalid.
Comment 5 H.J. Lu 2011-05-20 20:12:10 UTC
The problem is

Breakpoint 3, force_to_mode (x=0x7ffff0acbde0, mode=DImode, mask=4294967295, 
    just_select=0) at /export/gnu/import/git/gcc-x32/gcc/combine.c:8325
8325	      op1 = force_to_mode (XEXP (x, 1), mode, mask, next_select);
(gdb) call debug_rtx (x)
(plus:SI (subreg:SI (reg/f:DI 20 frame) 0)
    (const_int -58 [0xffffffffffffffc6]))
(gdb) 

      /* For most binary operations, just propagate into the operation and
         change the mode if we have an operation of that mode.  */

      op0 = force_to_mode (XEXP (x, 0), mode, mask, next_select);
      op1 = force_to_mode (XEXP (x, 1), mode, mask, next_select);

Shouldn't MASK be sign-extended to MODE here?
Comment 6 H.J. Lu 2011-05-20 20:59:43 UTC
Created attachment 24312 [details]
A patch

I am testing this patch.
Comment 7 H.J. Lu 2011-05-21 01:03:37 UTC
Created attachment 24314 [details]
A new testcase
Comment 8 H.J. Lu 2011-05-21 04:28:51 UTC
Created attachment 24315 [details]
An updated patch
Comment 9 H.J. Lu 2011-05-21 13:25:10 UTC
Created attachment 24318 [details]
A new patch

For binary operations, if X is narrower than MODE and we want all
the bits in X's mode, just use the operand when it is CONST_INT.
Comment 10 hjl@gcc.gnu.org 2011-05-27 13:38:45 UTC
Author: hjl
Date: Fri May 27 13:38:41 2011
New Revision: 174332

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174332
Log:
Properly handle CONST_INT operands.

2011-05-21  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/49088
	* combine.c (force_to_mode): If X is narrower than MODE and we
	want all the bits in X's mode, just use the operand when it
	is CONST_INT.

Modified:
    branches/x32/gcc/ChangeLog.x32
    branches/x32/gcc/combine.c
Comment 11 H.J. Lu 2011-06-23 14:52:40 UTC
Dup

*** This bug has been marked as a duplicate of bug 49504 ***
Comment 12 hjl@gcc.gnu.org 2011-06-23 14:57:50 UTC
Author: hjl
Date: Thu Jun 23 14:57:45 2011
New Revision: 175335

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175335
Log:
Revert the last change on force_to_mode.

2011-06-23  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/49088
	* combine.c (force_to_mode): Revert the last change.

Modified:
    branches/x32/gcc/ChangeLog.x32
    branches/x32/gcc/combine.c