Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

[Dominik Vogt <vogt at linux dot vnet dot ibm dot com>]

On Wed, Dec 21, 2016 at 01:58:18PM +0100, Georg-Johann Lay wrote:
> On 12.12.2016 17:54, Segher Boessenkool wrote:
> >On Mon, Dec 12, 2016 at 05:46:02PM +0100, Dominik Vogt wrote:
> >>Patch with these changes and a fix because of not handling
> >>VOIDmode attached.  Bootstrapped and regression tested on s390 and
> >>s390x.
> >
> >Okay for trunk.
> >
> >When did you see VOIDmode, btw?  It wasn't on a const_int I hope?
> >
> >
> >Segher
> >
> >
> >>	* combine.c (change_zero_ext): Handle mode expanding zero_extracts.
> 
> This was committes as r243578 and triggered (amongst other similar
> test suite ICE failures):
> 
> $ avr-gcc
> /gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c
> -S -O1 -mmcu=avr4 -S -v
> 
> /gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:
> In function 'yasm_lc3b__parse_insn':
> /gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:19:1:
> error: insn does not satisfy its constraints:
>  }
>  ^
> (jump_insn 58 98 59 9 (set (pc)
>         (if_then_else (eq (and:HI (reg:HI 31 r31)
>                     (const_int 1 [0x1]))
>                 (const_int 0 [0]))
>             (label_ref 70)
>             (pc))) "/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c":11
> 415 {*sbrx_and_branchhi}
>      (int_list:REG_BR_PROB 375 (nil))
>  -> 70)

Does that mean that, on avr, this is valid:

  (zero_extract:HI (reg:QI 31) ...)

but this is not:

  (subreg:HI (reg:QI 31))

?

> /gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:19:1:
> internal compiler error: in extract_constrain_insn, at recog.c:2213
> 0x9836a3 _fatal_insn(char const*, rtx_def const*, char const*, int,
> char const*)
> 	../../../gcc.gnu.org/trunk/gcc/rtl-error.c:108
> 0x9836cf _fatal_insn_not_found(rtx_def const*, char const*, int,
> char const*)
> 	../../../gcc.gnu.org/trunk/gcc/rtl-error.c:119
> 0x95abdd extract_constrain_insn(rtx_insn*)
> 	../../../gcc.gnu.org/trunk/gcc/recog.c:2213
> 0x939105 reload_cse_simplify_operands
> 	../../../gcc.gnu.org/trunk/gcc/postreload.c:391
> 0x939ce5 reload_cse_simplify
> 	../../../gcc.gnu.org/trunk/gcc/postreload.c:179
> 0x939ce5 reload_cse_regs_1
> 	../../../gcc.gnu.org/trunk/gcc/postreload.c:218
> 0x93b96b reload_cse_regs
> 	../../../gcc.gnu.org/trunk/gcc/postreload.c:64
> 0x93b96b execute
> 	../../../gcc.gnu.org/trunk/gcc/postreload.c:2342
> 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.
> 
> 
> Target: avr
> Configured with: ../../gcc.gnu.org/trunk/configure --target=avr
> --prefix=/local/gnu/install/gcc-7 --disable-shared --disable-nls
> --with-dwarf2 --enable-target-optspace=yes --with-gnu-as
> --with-gnu-ld --enable-checking=release --enable-languages=c,c++
> 
> 
> Combine comes up with the following insn:
> (jump_insn 58 57 59 7 (set (pc)
>         (if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75
> [ operands ]) [1 *operands_17(D)+0 S1 A8]) 0)
>                     (const_int 1 [0x1]))
>                 (const_int 0 [0]))
>             (label_ref 70)
>             (pc))) "/home/georg/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c":11
> 415 {*sbrx_and_branchhi}
>      (int_list:REG_BR_PROB 375 (nil))
>  -> 70)
> 
> which cannot be correct because avr.md::*sbrx_and_branchhi reads:
> 
> (define_insn "*sbrx_and_branch<mode>"
>   [(set (pc)
>         (if_then_else
>          (match_operator 0 "eqne_operator"
>                          [(and:QISI
>                            (match_operand:QISI 1 "register_operand" "r")
>                            (match_operand:QISI 2 "single_one_operand" "n"))
>                           (const_int 0)])
>          (label_ref (match_operand 3 "" ""))
>          (pc)))]
>   "" { ... } ...)
> 
> Hence we have a memory operand (subreg of mem)) where only a
> register is allowed.  Reg alloc then reloads the mem:QI into R31,
> but R31 is the
> last hard reg, i.e. R31 cannot hold HImode.
> 
> Johann
> 
> 
> 


Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany