Re: doloop insn generated incorrectly (wrong mode)

2018-10-11 Thread Paul Koning



> On Oct 11, 2018, at 3:11 AM, Segher Boessenkool  
> wrote:
> 
> Hi!
> 
> On Wed, Oct 10, 2018 at 08:55:12PM -0400, Paul Koning wrote:
> 
> [ snip ]
> 
>> ...
>> Why is this happening, and how can I fix it (short of removing the 
>> doloop_end pattern)?  I see a comment in loop-doloop.c about handling a FAIL 
>> of the pattern -- am I going to have to check that the wrong mode is being 
>> passed in and FAIL if so?
> 
> That is exactly what other targets do.  Maybe you can improve doloop so
> this isn't necessary anymore?  :-)

Maybe.  For now I'll take the expand and check approach.  And propose a doc 
patch to explain that this is needed, because it's an unusual situation. 

paul

Re: doloop insn generated incorrectly (wrong mode)

2018-10-11 Thread Segher Boessenkool
Hi!

On Wed, Oct 10, 2018 at 08:55:12PM -0400, Paul Koning wrote:

[ snip ]

> Note that this isn't permitted by the .md file -- the mode is wrong (QI not 
> HI).

Other targets use an expander and check the mode explicitly in there.  See
rs6000 or sh for example.

> It's not obvious to me how that relates to the wrong insn generated by the 
> compiler, but clearly it can't be good to have something that won't match 
> when it's time to generate the output code.  It seems to me that the doloop 
> convertor should be checking the doloop_end pattern to make sure the mode 
> matches, and not apply it unless it does.  
> 
> Why is this happening, and how can I fix it (short of removing the doloop_end 
> pattern)?  I see a comment in loop-doloop.c about handling a FAIL of the 
> pattern -- am I going to have to check that the wrong mode is being passed in 
> and FAIL if so?

That is exactly what other targets do.  Maybe you can improve doloop so
this isn't necessary anymore?  :-)


Segher


doloop insn generated incorrectly (wrong mode)

2018-10-10 Thread Paul Koning
I have a doloop_end pattern in pdp11.md which looks like this:

(define_insn_and_split "doloop_end"
  [(set (pc)
(if_then_else
 (ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
 (const_int 1))
 (label_ref (match_operand 1 "" ""))
 (pc)))
   (set (match_dup 0)
(plus:HI (match_dup 0)
 (const_int -1)))]
  "TARGET_40_PLUS"
  "#"
...

And the loop2 dump file shows this insn sequence:

(insn 1417 1415 1418 282 (set (reg:HI 565)
(plus:HI (subreg:HI (reg/v:QI 269 [ infcount ]) 0)
(const_int -1 [0x]))) 
"../../../../../combined/newlib/libc/stdio/vfscanf.c":1474:18 62 {addhi3}
 (nil))
(insn 1418 1417 1419 282 (set (reg/v:QI 292 [ infcount ])
(subreg:QI (reg:HI 565) 0)) 
"../../../../../combined/newlib/libc/stdio/vfscanf.c":1474:18 20 {movqi}
 (expr_list:REG_DEAD (reg:HI 565)
(nil)))
(jump_insn 1419 1418 1423 282 (set (pc)
(if_then_else (ne (reg/v:QI 269 [ infcount ])
(const_int 3 [0x3]))
(label_ref 1430)
(pc))) "../../../../../combined/newlib/libc/stdio/vfscanf.c":1474:9 
12 {cbranchqi4}
 (int_list:REG_BR_PROB 955630228 (nil))
 -> 1430)

which is turned into this (in loop2_doloop):

and ends up like this:

(jump_insn 2158 1447 2111 285 (parallel [
(set (pc)
(if_then_else (ne (reg:QI 647)
(const_int 1 [0x1]))
(label_ref 2111)
(pc)))
(set (reg:QI 647)
(plus:HI (reg:QI 647)
(const_int -1 [0x])))
]) "../../../../../combined/newlib/libc/stdio/vfscanf.c":1474:9 -1
 (int_list:REG_BR_PROB 955630228 (nil))
 -> 2111)

Note that this isn't permitted by the .md file -- the mode is wrong (QI not 
HI).  The result is an ICE in cfgrtl.c line 1275:

  /* If the substitution doesn't succeed, die.  This can happen
 if the back end emitted unrecognizable instructions or if
 target is exit block on some arches.  */
  if (!redirect_jump (as_a  (insn),
  block_label (new_bb), 0))
{
  gcc_assert (new_bb == EXIT_BLOCK_PTR_FOR_FN (cfun));
  return false;
}

It's not obvious to me how that relates to the wrong insn generated by the 
compiler, but clearly it can't be good to have something that won't match when 
it's time to generate the output code.  It seems to me that the doloop 
convertor should be checking the doloop_end pattern to make sure the mode 
matches, and not apply it unless it does.  

Why is this happening, and how can I fix it (short of removing the doloop_end 
pattern)?  I see a comment in loop-doloop.c about handling a FAIL of the 
pattern -- am I going to have to check that the wrong mode is being passed in 
and FAIL if so?

paul