| Summary: | Combine fails to properly handle zero-extension and signed int constant | ||
|---|---|---|---|
| Product: | gcc | Reporter: | H.J. Lu <hjl.tools> |
| Component: | rtl-optimization | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | normal | CC: | ebotcazou |
| Priority: | P3 | ||
| Version: | 4.7.0 | ||
| Target Milestone: | 4.7.0 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2011-05-20 19:42:51 | |
| Attachments: |
A patch
A new testcase An updated patch A new patch |
||
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.
(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)
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?
> 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.
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?
Created attachment 24312 [details]
A patch
I am testing this patch.
Created attachment 24314 [details]
A new testcase
Created attachment 24315 [details]
An updated patch
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.
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 Dup *** This bug has been marked as a duplicate of bug 49504 *** 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 |
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.