| Message ID | 20160212163421.GT3017@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
> So do you prefer e.g. following? Bootstrapped/regtested on x86_64-linux and > i686-linux. > > - mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > + mode1 = (GET_MODE (xop1) != VOIDmode || canonicalize_op1) > + ? GET_MODE (xop1) : mode; Placement of parentheses is wrong for formatting, but otherwise I think this patch is good. Bernd
On Fri, Feb 12, 2016 at 05:34:21PM +0100, Jakub Jelinek wrote: > On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: > > >>- mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > > >>+ mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1; > > >> if (xmode1 != VOIDmode && xmode1 != mode1) > > >> { > > > > Here, things aren't so clear, and the fact that the mode1 calculation now > > differs from the mode0 one may be overlooked by someone in the future. > > > > Rather than use codes like "mode variable is VOIDmode", I'd prefer to use > > booleans with descriptive names, like "op1_may_need_conversion". > > So do you prefer e.g. following? Bootstrapped/regtested on x86_64-linux and > i686-linux. > > 2016-02-12 Jakub Jelinek <jakub@redhat.com> > > PR rtl-optimization/69764 > PR rtl-optimization/69771 > * optabs.c (expand_binop_directly): For shift_optab_p, force > convert_modes with VOIDmode if xop1 has VOIDmode. > > * c-c++-common/pr69764.c: New test. > * gcc.dg/torture/pr69771.c: New testcase. > These two new tests are failing for me on AArch64 as so: .../gcc/testsuite/c-c++-common/pr69764.c:7:12: internal compiler error: in decompose, at rtl.h:2107 0x7d30be wi::int_traits<std::pair<rtx_def*, machine_mode> >::decompose(long*, unsigned int, std::pair<rtx_def*, machine_mode> const&) .../gcc/rtl.h:2105 0x7d30be wide_int_ref_storage<false>::wide_int_ref_storage<std::pair<rtx_def*, machine_mode> >(std::pair<rtx_def*, machine_mode> const&) .../gcc/wide-int.h:936 0x7d30be generic_wide_int<wide_int_ref_storage<false> >::generic_wide_int<std::pair<rtx_def*, machine_mode> >(std::pair<rtx_def*, machine_mode> const&) .../gcc/wide-int.h:714 0x7d30be convert_modes(machine_mode, machine_mode, rtx_def*, int) .../gcc/expr.c:697 0x9ec7c6 widen_operand .../gcc/optabs.c:208 0x9f1e79 expand_binop(machine_mode, optab_tag, rtx_def*, rtx_def*, rtx_def*, int, optab_methods) .../gcc/optabs.c:1280 0x7b7a95 expand_shift_1 .../gcc/expmed.c:2458 0x7bca49 expand_variable_shift(tree_code, machine_mode, rtx_def*, tree_node*, rtx_def*, int) .../gcc/expmed.c:2517 0x7e1d43 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) .../gcc/expr.c:9029 0x6d9ed9 expand_gimple_stmt_1 .../gcc/cfgexpand.c:3642 0x6d9ed9 expand_gimple_stmt .../gcc/cfgexpand.c:3702 0x6dc369 expand_gimple_basic_block .../gcc/cfgexpand.c:5708 0x6dfcdc execute .../gcc/cfgexpand.c:6323 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <http://gcc.gnu.org/bugs.html> for instructions. I can't dig deeper today, but I will look closer on Monday if the fix is not obvious. Thanks, James
On Sat, 2016-02-13 at 07:50 +0000, James Greenhalgh wrote: > > So do you prefer e.g. following? Bootstrapped/regtested on x86_64 > > -linux and > > i686-linux. > > > > 2016-02-12 Jakub Jelinek <jakub@redhat.com> > > > > PR rtl-optimization/69764 > > PR rtl-optimization/69771 > > * optabs.c (expand_binop_directly): For shift_optab_p, force > > convert_modes with VOIDmode if xop1 has VOIDmode. > > > > * c-c++-common/pr69764.c: New test. > > * gcc.dg/torture/pr69771.c: New testcase. > > > > These two new tests are failing for me on AArch64 as so: > I see the same failures on SH, too. Cheers, Oleg
--- gcc/optabs.c.jj 2016-02-12 00:50:30.410240366 +0100 +++ gcc/optabs.c 2016-02-12 10:33:32.743492532 +0100 @@ -993,6 +993,7 @@ expand_binop_directly (machine_mode mode bool commutative_p; rtx_insn *pat; rtx xop0 = op0, xop1 = op1; + bool canonicalize_op1 = false; /* If it is a commutative operator and the modes would match if we would swap the operands, we can save the conversions. */ @@ -1006,6 +1007,11 @@ expand_binop_directly (machine_mode mode xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp); if (!shift_optab_p (binoptab)) xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp); + else + /* Shifts and rotates often use a different mode for op1 from op0; + for VOIDmode constants we don't know the mode, so force it + to be canonicalized using convert_modes. */ + canonicalize_op1 = true; /* In case the insn wants input operands in modes different from those of the actual operands, convert the operands. It would @@ -1020,7 +1026,8 @@ expand_binop_directly (machine_mode mode mode0 = xmode0; } - mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; + mode1 = (GET_MODE (xop1) != VOIDmode || canonicalize_op1) + ? GET_MODE (xop1) : mode; if (xmode1 != VOIDmode && xmode1 != mode1) { xop1 = convert_modes (xmode1, mode1, xop1, unsignedp); --- gcc/testsuite/c-c++-common/pr69764.c.jj 2016-02-12 10:27:34.522587995 +0100 +++ gcc/testsuite/c-c++-common/pr69764.c 2016-02-12 10:27:34.522587995 +0100 @@ -0,0 +1,38 @@ +/* PR rtl-optimization/69764 */ +/* { dg-do compile { target int32plus } } */ + +unsigned char +fn1 (unsigned char a) +{ + return a >> ~6; /* { dg-warning "right shift count is negative" } */ +} + +unsigned short +fn2 (unsigned short a) +{ + return a >> ~6; /* { dg-warning "right shift count is negative" } */ +} + +unsigned int +fn3 (unsigned int a) +{ + return a >> ~6; /* { dg-warning "right shift count is negative" } */ +} + +unsigned char +fn4 (unsigned char a) +{ + return a >> 0xff03; /* { dg-warning "right shift count >= width of type" } */ +} + +unsigned short +fn5 (unsigned short a) +{ + return a >> 0xff03; /* { dg-warning "right shift count >= width of type" } */ +} + +unsigned int +fn6 (unsigned int a) +{ + return a >> 0xff03; /* { dg-warning "right shift count >= width of type" } */ +} --- gcc/testsuite/gcc.dg/torture/pr69771.c.jj 2016-02-12 10:27:34.522587995 +0100 +++ gcc/testsuite/gcc.dg/torture/pr69771.c 2016-02-12 10:27:34.522587995 +0100 @@ -0,0 +1,12 @@ +/* PR rtl-optimization/69771 */ +/* { dg-do compile } */ + +unsigned char a = 5, c; +unsigned short b = 0; +unsigned d = 0x76543210; + +void +foo (void) +{ + c = d >> ~(a || ~b); /* { dg-warning "shift count is negative" } */ +}