Re: Improve code generated by cmpstrsi patterns

[Jan Hubicka <jh at suse dot cz>]

> On Sun, Jul 16, 2000 at 01:17:45AM +0200, Jan Hubicka wrote:
> 
> I am not sure I understand your description, but I can see the problem
> in Jakub's test case.  We've got a triad of insns like so:
OK, looks like you got it from the testcase anyway.
> but there's no such insn.  Later, reload blows up trying to set up
> zero_extract of a constant.  But this is not the entire story, because
> adding
> 
> (define_insn "*pc_nop_move"
>   [(set (pc) (pc))]
>   ""
>   "* abort ();"
>   [(set_attr "type" "other")])
> 
> to i386.md does no good at all.  Part of the problem is a bug in
Interesting, this worked for me at time I tried it.
> simplify_ternary_operation, such that zero_extract operations are
> never simplified.

Yes, there are three unrelated problems, first in simplify-rtx, second
in combine and third in the reload. Thanks for fixing the first one :)
The reload problem is probably most serious and last time I checked this
testcase I decided to ask Bernd about the solution, decided to do that
"later when I will be online" and forgot about this issue entirely.

> 
> ===================================================================
> Index: simplify-rtx.c
> --- simplify-rtx.c	2000/05/24 20:26:54	1.19
> +++ simplify-rtx.c	2000/07/16 00:25:25
> @@ -1912,8 +1930,7 @@ simplify_ternary_operation (code, mode, 
>        if (GET_CODE (op0) == CONST_INT
>  	  && GET_CODE (op1) == CONST_INT
>  	  && GET_CODE (op2) == CONST_INT
> -	  && ((unsigned) INTVAL (op1) + (unsigned) INTVAL (op2)
> -	      <= GET_MODE_BITSIZE (op0_mode))
> +	  && ((unsigned) INTVAL (op1) + (unsigned) INTVAL (op2) <= width)
>  	  && width <= (unsigned) HOST_BITS_PER_WIDE_INT)
>  	{
>  	  /* Extracting a bit-field from a constant */
> 
> If op0 is a CONST_INT, then op0_mode will always be VOIDmode, so its
> bitsize will always be 0, and INTVAL (op1) + INTVAL (op2) will always
> be greater than that (unless both are zero).

I have equivalent patch at home, but probably somehow I can't find it in
the archives.
> 
> The other part of the problem is that combine never considers merging
> insn 99 with insn 16, so we never get as far as that.  It does
> consider merging insns 99 and 100, but only with a register inside the
> zero_extract.  I don't know why that is, but my guess would be:
> before combine begins, we have
> 
> (insn/i 98 97 99 (parallel[ 
>             (set (reg:QI 58)
>                 (and:QI (reg/v:QI 44)
>                     (const_int 4 [0x4])))
>             (clobber (reg:CC 17 flags))
>         ] ) 167 {*andqi_1} (nil)
>     (expr_list:REG_UNUSED (reg:CC 17 flags)
>         (nil)))
> (gdb) call debug_rtx (i3)
> 
> (insn/i 99 98 100 (set (reg:CCZ 17 flags)
>         (compare:CCZ (reg:QI 58)
>             (const_int 0 [0x0]))) 5 {cmpqi_ccz_1} (insn_list 98 (nil))
>     (expr_list:REG_DEAD (reg:QI 58)
>         (nil)))
> 
> Combine does manage to merge those two insns.  But notice that there
> is no LOG_LINK on insn 98 pointing to insn 16.  So combine has no idea
> that (reg/v:QI 44) has a known value.  The link should've been put
> there by life analysis, but it didn't - I wonder if this is because
> insn 72 got it.
Interesting. I didn't noticed this. I believe flow ought to put the log
link there, I will try to check this out.
> 
> I need to go eat dinner now, so I will not chase this any further.
> But I wouldn't worry about the no-op moves until we are actually
> trying to generate them :)
> 
> I do think it's a good idea for combine to be able to delete no-op
> moves.  When I tried to do it, however, I ran into one problem after
> another.  The cleanup block simply isn't expecting I3 to disappear.

Exactly, thats the problem I run into.  OK... it is my turn now :)

Thanks for investigating this.
Honza
> 
> zw