Issue during combine.

Issue during combine.

[Henri Cloetens]

Hello all,

I am building a gcc 9.2.0 custom compiler, and I am running in an issue 
during step 263, combine.

Before combine:

/(insn 2354 2352 1743 175 (set (reg/v:SI 197 [ dig ])//
//        (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
//                (const_int 1 [0x1]))//
//            (reg:SI 708)//
//            (reg/v:SI 197 [ dig ]))) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":771:6 35 
{select_internal3}//
//     (expr_list:REG_DEAD (reg:SI 708)//
//        (expr_list:REG_DEAD (reg:SI 632)//
//            (nil))))//
//(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0 
*s_304+0 S1 A8])//
//        (subreg:QI (reg/v:SI 197 [ dig ]) 0)) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6 
{movqi_internal}//
//     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
//        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
//            (nil))))/

This is during the combine-step transformed into:

/(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0 
*s_304+0 S1 A8])//
//        (subreg:QI (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
//                    (const_int 1 [0x1]))//
//                (reg:SI 708)//
//                (reg/v:SI 197 [ dig ])) 0)) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6 
{movqi_internal}//
//     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
//        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
//            (nil))))/

This, of course, is not correct any more. Now, how should I prevent this 
?, the pattern is defined in the .md-file as:

/(define_insn "movqi_internal" //
//[(set (match_operand:QI 0 "movqi_operand_0" 
"=r,r,r,r,r,r,u,u,W,t,r,c,*c*l,*h,*h,y")//
//(match_operand:QI 1 "movqi_operand_1" "O,k,i,r,W,t,W,t,r,r,*h,u, r,   
r, 0,y"))]/

/.../
and /movqi_operand_1/ is defined as:

/(define_predicate "movqi_operand_1"//
//  (ior (match_operand 0 "gpc_reg_operand")//
//       (ior (match_operand 0 
"quarterword_offset21_memref_operand_or_indirect")//
//            (match_operand 0 "immediate_operand"))))/


and /gpc_reg_operand/ as:

/(define_predicate "gpc_reg_operand"//
//   (and (match_operand 0 "register_operand")//
//        (match_test "(GET_CODE (op) != REG && GET_CODE(op) != SUBREG//
//                      || (REGNO (op) >= ARG_POINTER_REGNUM//
//                          && !CA_REGNO_P (REGNO (op)))//
//                      || REGNO (op) == SFP_REGNO//
//                      || REGNO (op) == ARG_POINTER_REGNUM//
//                      || REGNO (op) <= MAX_REGFILE_REGNO)")))/

Any of you any idea why the combine succeeds ?. I mean, it should fail 
!. After combine, it does not match /movqi_internal/ pattern
any more, but the compiler seems to think otherwise !.

Best Regards,

Henri.

Re: Issue during combine.

[Henri Cloetens]

Hello all,

This seems to be a bug in the non-port code of the compiler.
It crashes in
/rtx simplify_subreg(machine_mode outer_mode, rtx op, machine_mode 
inner_mode, poly_uint64 byte)/

The crash is, because this function is called with inner_mode = 
Voidmode. (And seems illegal.)

It seems to me this "illegal" mode is the result of the /combine/ step, 
trying to "improve" by doing combinations of
instructions "at random". Here, it tries a "combination" that does not 
make sense, but it causes a crash in
the abovementioned routine. When I change the code, so that this routine 
returns 0 (=fail the operation), it passes
the '263 step without errors. The meaningless combination is refused 
elsewhere. It is not done.

Best Regards,

Henri.


On 10/19/2020 02:53 PM, Henri Cloetens wrote:
> Hello all,
>
> I am building a gcc 9.2.0 custom compiler, and I am running in an 
> issue during step 263, combine.
>
> Before combine:
>
> /(insn 2354 2352 1743 175 (set (reg/v:SI 197 [ dig ])//
> //        (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
> //                (const_int 1 [0x1]))//
> //            (reg:SI 708)//
> //            (reg/v:SI 197 [ dig ]))) 
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":771:6 35 
> {select_internal3}//
> //     (expr_list:REG_DEAD (reg:SI 708)//
> //        (expr_list:REG_DEAD (reg:SI 632)//
> //            (nil))))//
> //(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0 
> *s_304+0 S1 A8])//
> //        (subreg:QI (reg/v:SI 197 [ dig ]) 0)) 
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6 
> {movqi_internal}//
> //     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
> //        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
> //            (nil))))/
>
> This is during the combine-step transformed into:
>
> /(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0 
> *s_304+0 S1 A8])//
> //        (subreg:QI (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
> //                    (const_int 1 [0x1]))//
> //                (reg:SI 708)//
> //                (reg/v:SI 197 [ dig ])) 0)) 
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6 
> {movqi_internal}//
> //     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
> //        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
> //            (nil))))/
>
> This, of course, is not correct any more. Now, how should I prevent 
> this ?, the pattern is defined in the .md-file as:
>
> /(define_insn "movqi_internal" //
> //[(set (match_operand:QI 0 "movqi_operand_0" 
> "=r,r,r,r,r,r,u,u,W,t,r,c,*c*l,*h,*h,y")//
> //(match_operand:QI 1 "movqi_operand_1" "O,k,i,r,W,t,W,t,r,r,*h,u, 
> r,   r, 0,y"))]/
>
> /.../
> and /movqi_operand_1/ is defined as:
>
> /(define_predicate "movqi_operand_1"//
> //  (ior (match_operand 0 "gpc_reg_operand")//
> //       (ior (match_operand 0 
> "quarterword_offset21_memref_operand_or_indirect")//
> //            (match_operand 0 "immediate_operand"))))/
>
>
> and /gpc_reg_operand/ as:
>
> /(define_predicate "gpc_reg_operand"//
> //   (and (match_operand 0 "register_operand")//
> //        (match_test "(GET_CODE (op) != REG && GET_CODE(op) != SUBREG//
> //                      || (REGNO (op) >= ARG_POINTER_REGNUM//
> //                          && !CA_REGNO_P (REGNO (op)))//
> //                      || REGNO (op) == SFP_REGNO//
> //                      || REGNO (op) == ARG_POINTER_REGNUM//
> //                      || REGNO (op) <= MAX_REGFILE_REGNO)")))/
>
> Any of you any idea why the combine succeeds ?. I mean, it should fail 
> !. After combine, it does not match /movqi_internal/ pattern
> any more, but the compiler seems to think otherwise !.
>
> Best Regards,
>
> Henri.
>
>
>

Re: Issue during combine.

[Segher Boessenkool]

[ Please don't top-post ]

Hi Henri,

On Mon, Oct 19, 2020 at 05:55:55PM +0200, Henri Cloetens wrote:
> On 10/19/2020 02:53 PM, Henri Cloetens wrote:
[ snip ]

> >This, of course, is not correct any more. Now, how should I prevent 
> >this ?, the pattern is defined in the .md-file as:

The combiner only ever does transformations that result in recognised
instructions, see this code (and many lines before):

  /* If it still isn't recognized, fail and change things back the way they
     were.  */
  if ((insn_code_number < 0
       /* Is the result a reasonable ASM_OPERANDS?  */
       && (! check_asm_operands (newpat) || added_sets_1 || added_sets_2)))
    {
      undo_all ();
      return 0;
    }

> >/(define_insn "movqi_internal" //
> >//[(set (match_operand:QI 0 "movqi_operand_0" 
> >"=r,r,r,r,r,r,u,u,W,t,r,c,*c*l,*h,*h,y")//
> >//(match_operand:QI 1 "movqi_operand_1" "O,k,i,r,W,t,W,t,r,r,*h,u, 
> >r,   r, 0,y"))]/
> >
> >/.../
> >and /movqi_operand_1/ is defined as:
> >
> >/(define_predicate "movqi_operand_1"//
> >//  (ior (match_operand 0 "gpc_reg_operand")//
> >//       (ior (match_operand 0 
> >"quarterword_offset21_memref_operand_or_indirect")//
> >//            (match_operand 0 "immediate_operand"))))/

What is quarterword_offset21_memref_operand_or_indirect defined as?

> >Any of you any idea why the combine succeeds ?. I mean, it should fail 
> >!. After combine, it does not match /movqi_internal/ pattern
> >any more, but the compiler seems to think otherwise !.

Maybe it matches some other pattern?  Use -fdump-rtl-combine-all and
then look at the RTL dump file: at the end of each function it shows the
resulting RTL stream, and each insn like

(insn 7 18 8 2 (set (reg:DI 3 3)
        (plus:DI (reg/f:DI 111 sfp)
            (const_int 112 [0x70]))) "vlf.c":18:10 69 {*adddi3}
     (nil))

shows the name of the pattern that matched this (*adddi3 here).

> This seems to be a bug in the non-port code of the compiler.
> It crashes in
> /rtx simplify_subreg(machine_mode outer_mode, rtx op, machine_mode 
> inner_mode, poly_uint64 byte)/
> 
> The crash is, because this function is called with inner_mode = 
> Voidmode. (And seems illegal.)

Before you said that bad code was generated, and now you say the
compiler crashes.  Which is it?

> It seems to me this "illegal" mode is the result of the /combine/ step, 
> trying to "improve" by doing combinations of
> instructions "at random".

Combine combines instructions that have a (register) dependency.  What
it comes up with as the combined insn is often not something that the
target in fact supports, but that is fine: then that combination is not
made.

> Here, it tries a "combination" that does not 
> make sense, but it causes a crash in
> the abovementioned routine.

Please open a bug report (see <https://gcc.gnu.org/bugs.html> for
instructions).  Thanks in advance!

> When I change the code, so that this routine 
> returns 0 (=fail the operation), it passes
> the '263 step without errors. The meaningless combination is refused 
> elsewhere. It is not done.

Btw, those numbers vary over time, and they are different for different
targets (combine currently is 265 for x86_64, and 270 for rs6000).  Just
use the name please :-)


Segher

Re: Issue during combine.

[Henri Cloetens]

Hello Segher, Dan, all,

The issue is following:
(as far as I understand it.)

- There are 2 instructions, they are combined by the combiner.
- As correctly stated by you, the combiner tries these, to see if it 
makes sense.
- Of course, it should not make sense, and should be rejected.
- However, during the "checking", the combiner calls "simplify_subreg" 
in simplify-rtx.c.
- There, it crashes on "/gcc_assert(innermode != VOIDmode)/.
- Please look at the rtx, and how this is combined. This 'mistake' is 
created by the combiner. Combining the RTX (see below),
    it changed the expression
/(set (...) (subreg:qi(reg: si 197,0)))
/into
/(set (...) )subreg:qi(if_then_else ...
/Apparently, the /if_then_else/ - operator not being subreggable, this 
is recognized as VOIDmode, causing simplify_subreg to crash.
- I fixed it by causing the subroutine to fail (= tell "this expression 
cannot be simplified"), instead of crash by adding a statement.
- If you want me to submit it, well, simplify the test case that is 
still possible, but ... how should I submit the custom back-end ?.
   (I am willing to do it, but does it make sense ?.)

Best Regards,

Henri.
*BELOW/:/*/routine simplify_subreg/ and the relevant rtx statements 
before and after combine.
//
/rtx//
//simplify_subreg (machine_mode outermode, rtx op,//
//                 machine_mode innermode, poly_uint64 byte)//
//{/*
/if(innermode == VOIDmode) return(0) ; // MY BUG//FIX/
*/gcc_assert(innermode != VOIDmode)/

Before combine:

/(insn 2354 2352 1743 175 (set (reg/v:SI 197 [ dig ])//
//        (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
//                (const_int 1 [0x1]))//
//            (reg:SI 708)//
//            (reg/v:SI 197 [ dig ]))) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":771:6 35 
{select_internal3}//
//     (expr_list:REG_DEAD (reg:SI 708)//
//        (expr_list:REG_DEAD (reg:SI 632)//
//            (nil))))//
//(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0 
*s_304+0 S1 A8])//
//        (subreg:QI (reg/v:SI 197 [ dig ]) 0)) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6 
{movqi_internal}//
//     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
//        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
//            (nil))))/

This is during the combine-step transformed into:

/(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0 
*s_304+0 S1 A8])//
//        (subreg:QI (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
//                    (const_int 1 [0x1]))//
//                (reg:SI 708)//
//                (reg/v:SI 197 [ dig ])) 0)) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6 
{movqi_internal}//
//     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
//        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
//            (nil))))/

Re: Issue during combine.

[Segher Boessenkool]

On Mon, Oct 19, 2020 at 09:56:04PM +0200, Henri Cloetens wrote:
> - However, during the "checking", the combiner calls "simplify_subreg" 
> in simplify-rtx.c.
> - There, it crashes on "/gcc_assert(innermode != VOIDmode)/.

Because that is invalid RTL.

> /(insn 2354 2352 1743 175 (set (reg/v:SI 197 [ dig ])//
> //        (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
> //                (const_int 1 [0x1]))//
> //            (reg:SI 708)//
> //            (reg/v:SI 197 [ dig ]))) 

This is incorrect already (should be if_then_else:SI).  It could have
crashed earlier...  You can configure with "--enable-checking=yes,rtl"
to find many such problems (default is just "yes", see the manual for
other things you can ask to be checked ("tree" is useful; some others
are very very slow)).

HtH,


Segher

Re: Issue during combine.

[Richard Earnshaw]

On 19/10/2020 13:53, Henri Cloetens wrote:
> Hello all,
> 
> I am building a gcc 9.2.0 custom compiler, and I am running in an issue
> during step 263, combine.
> 
> Before combine:
> 
> /(insn 2354 2352 1743 175 (set (reg/v:SI 197 [ dig ])//
> //        (if_then_else (eq (subreg:QI (reg:SI 632) 1)//

Your problem is likely here, on the input.  The SET_SRC (the
if_then_else) of the pattern should have the same mode as the SET_DEST,
unless the SET_SRC is a CONST_INT.


R.

> //                (const_int 1 [0x1]))//
> //            (reg:SI 708)//
> //            (reg/v:SI 197 [ dig ])))
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":771:6 35
> {select_internal3}//
> //     (expr_list:REG_DEAD (reg:SI 708)//
> //        (expr_list:REG_DEAD (reg:SI 632)//
> //            (nil))))//
> //(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0
> *s_304+0 S1 A8])//
> //        (subreg:QI (reg/v:SI 197 [ dig ]) 0))
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6
> {movqi_internal}//
> //     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
> //        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
> //            (nil))))/
> 
> This is during the combine-step transformed into:
> 
> /(insn 1743 2354 124 175 (set (mem:QI (reg:SI 316 [ ivtmp.118 ]) [0
> *s_304+0 S1 A8])//
> //        (subreg:QI (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
> //                    (const_int 1 [0x1]))//
> //                (reg:SI 708)//
> //                (reg/v:SI 197 [ dig ])) 0))
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/dtoa.c":772:13 6
> {movqi_internal}//
> //     (expr_list:REG_DEAD (reg:SI 316 [ ivtmp.118 ])//
> //        (expr_list:REG_DEAD (reg/v:SI 197 [ dig ])//
> //            (nil))))/
> 
> This, of course, is not correct any more. Now, how should I prevent this
> ?, the pattern is defined in the .md-file as:
> 
> /(define_insn "movqi_internal" //
> //[(set (match_operand:QI 0 "movqi_operand_0"
> "=r,r,r,r,r,r,u,u,W,t,r,c,*c*l,*h,*h,y")//
> //(match_operand:QI 1 "movqi_operand_1" "O,k,i,r,W,t,W,t,r,r,*h,u, r,  
> r, 0,y"))]/
> 
> /.../
> and /movqi_operand_1/ is defined as:
> 
> /(define_predicate "movqi_operand_1"//
> //  (ior (match_operand 0 "gpc_reg_operand")//
> //       (ior (match_operand 0
> "quarterword_offset21_memref_operand_or_indirect")//
> //            (match_operand 0 "immediate_operand"))))/
> 
> 
> and /gpc_reg_operand/ as:
> 
> /(define_predicate "gpc_reg_operand"//
> //   (and (match_operand 0 "register_operand")//
> //        (match_test "(GET_CODE (op) != REG && GET_CODE(op) != SUBREG//
> //                      || (REGNO (op) >= ARG_POINTER_REGNUM//
> //                          && !CA_REGNO_P (REGNO (op)))//
> //                      || REGNO (op) == SFP_REGNO//
> //                      || REGNO (op) == ARG_POINTER_REGNUM//
> //                      || REGNO (op) <= MAX_REGFILE_REGNO)")))/
> 
> Any of you any idea why the combine succeeds ?. I mean, it should fail
> !. After combine, it does not match /movqi_internal/ pattern
> any more, but the compiler seems to think otherwise !.
> 
> Best Regards,
> 
> Henri.
> 
> 

Re: Issue during combine.

[Henri Cloetens]

Hello Richard, all,

Well, I guess there is the confusion.
- For me, this is a constant int.
- For gcc, apparently not.
Anyway, I changed the mode, and now it works.

Many thanks, Best Regards,

Henri.


On 10/20/2020 03:19 PM, Richard Earnshaw wrote:
> On 19/10/2020 13:53, Henri Cloetens wrote:
>> Hello all,
>>
>> I am building a gcc 9.2.0 custom compiler, and I am running in an issue
>> during step 263, combine.
>>
>> Before combine:
>>
>> /(insn 2354 2352 1743 175 (set (reg/v:SI 197 [ dig ])//
>> //        (if_then_else (eq (subreg:QI (reg:SI 632) 1)//
> Your problem is likely here, on the input.  The SET_SRC (the
> if_then_else) of the pattern should have the same mode as the SET_DEST,
> unless the SET_SRC is a CONST_INT.
>
>

Re: Issue during combine.

[Segher Boessenkool]

On Tue, Oct 20, 2020 at 05:01:24PM +0200, Henri Cloetens wrote:
> Well, I guess there is the confusion.
> - For me, this is a constant int.

A "constant int" (which this is not, fwiw) is not the same as a
CONST_INT.  A CONST_INT is something specific in RTL (which always has
mode VOIDmode, and that is correct there, but not in most other
contexts).


Segher