This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: A patch for gcc.c-torture/execute/980505-1.c


> 
> 
>   In message <m0ymjZV-00026AC@ocean.lucon.org>you write:
>   > 	* cse.c (delete_trivially_dead_insns): Fix the insn with the
>   > 	REG_RETVAL note when deleting an insn.
> First, style issue, we do not used mixed case for variable names.  Please
> use all lower case.
> 
> I would recommend "retval_insn" and "libcall_note".

I can fix that.

> 
> 
>   >        if (! live_insn)
>   >  	{
>   > + 	  rtx set = NULL_RTX;
>   > +	  rtx dest;
>   > +	  rtx src = NULL_RTX;
>   > +	  rtx next;
>   > +
>   > +	  /* Check if the whole LIBCALL block is deleted. */
>   > +	  if (insn == insn_RETVAL)
>   > +	    insn_RETVAL = NULL_RTX;
>   > +	  else
>   > +	    set = single_set (insn);
> I do not believe it is possible for an insn with a RETVAL note
> to ever be considered dead.  Thus the condition "insn == insn_RETVAL" 
> should never be true.  I believe this test is unnecessary.

If that is true, why is this in delete_trivially_dead_insns:

      /* Don't delete any insns that are part of a libcall block unless
         we can delete the whole libcall block.

         Flow or loop might get confused if we did that.  Remember
         that we are scanning backwards.  */
      if (find_reg_note (insn, REG_RETVAL, NULL_RTX))
        {
          in_libcall = 1;
          live_insn = 1;
          dead_libcall = 0;

          /* See if there's a REG_EQUAL note on this insn and try to
             replace the source with the REG_EQUAL expression.

             We assume that insns with REG_RETVALs can only be reg->reg
             copies at this point.  */
          note = find_reg_note (insn, REG_EQUAL, NULL_RTX);
          if (note)
            {
              rtx set = single_set (insn);
              if (set
                  && validate_change (insn, &SET_SRC (set), XEXP (note, 0), 0))
                {
                  remove_note (insn,
                               find_reg_note (insn, REG_RETVAL, NULL_RTX));
                  dead_libcall = 1;
                }
            }
        }

> 
>   > +	  if (set)
> Change this to (set && in_libcall) since we do not need to make any
> of these transformations unless we are in a libcall block.
> 

That is not true. Please see the enclosed RTL dump. When we delete

(insn 9 6 11 (set (reg/v:SI 23)
        (const_int 1)) -1 (nil)
    (nil))

and

(insn 22 19 24 (set (reg/v:SI 23)
        (const_int 2)) -1 (nil)
    (nil))

We have to check XEXP (note_LIBCALL, 0).

> 
>   > +	  else if (insn_RETVAL)
> This should check && in_libcall too.
> 
> While checking "insn_RETVAL" may be equivalent, "in_libcall" is clearer
> IMHO.

Ok.

> 
>   > +	  if (src)
> Add (&& in_libcall)
> 

No. See above.

>   > +	      if (insn_RETVAL && REG_NOTES (insn_RETVAL))
>   > +		/* Fix the insn with the REG_RETVAL note within the
>   > +		   LIBCALL block. */
>   > +		fix = insn_RETVAL;
>   > +	      else if (note_LIBCALL
>   > +		       && REG_NOTES (XEXP (note_LIBCALL, 0)))
>   > +		/* Fix the insn with the REG_RETVAL note in the next
>   > +		   LIBCALL block. */
>   > +		fix = XEXP (note_LIBCALL, 0);
> Why it is necessary to fix insn with the REG_RETVAL note in the next
> libcall?  It seems to me we should only fix the REG_RETVAL for the
> current libcall block.

See above.

> 
> If we have to fix other libcall blocks, then this scheme is totally
> broken and we'll have to start over with a different scheme because
> if cse-skip-blocks, cse-follow-jumps and friends.
> 

Maybe. Please check out 980505-1.c.jump and 980505-1.c.addressof.
You will see where the problem is.

>   > +		    {
>   > +		      set = single_set (next);
>   > +		      if (set && rtx_equal_p (SET_DEST (set), dest))
>   > +			break;
>   > +		    }
> It also seems to me that we might want to abort if set is null, or try
> to handle that case better (ie by using note_stores to determine what
> things are set by an insn).
> 

I don't think so since we have to check XEXP (note_LIBCALL, 0).


-- 
H.J. Lu (hjl@gnu.org)
----980505-1.c.jump-----

;; Function main

(note 2 0 3 "" NOTE_INSN_DELETED)

(note 3 2 4 "" NOTE_INSN_FUNCTION_BEG)

(note 4 3 6 "" NOTE_INSN_DELETED)

(note 6 4 9 0 NOTE_INSN_BLOCK_BEG)

(insn 9 6 11 (set (reg/v:SI 23)
        (const_int 1)) -1 (nil)
    (nil))

(insn 11 9 13 (set (mem:SI (pre_dec:SI (reg:SI 7 %esp)))
        (reg/v:SI 23)) -1 (nil)
    (insn_list:REG_LIBCALL 17 (nil)))

(call_insn/u 13 11 15 (set (reg:SI 0 %eax)
        (call (mem:QI (symbol_ref:SI ("f")))
            (const_int 4))) -1 (nil)
    (nil)
    (nil))

(insn 15 13 17 (set (reg:SI 7 %esp)
        (plus:SI (reg:SI 7 %esp)
            (const_int 4))) -1 (nil)
    (nil))

(insn 17 15 19 (set (reg:SI 24)
        (reg:SI 0 %eax)) -1 (nil)
    (insn_list:REG_RETVAL 11 (expr_list:REG_EQUAL (expr_list (symbol_ref:SI ("f"))
                (expr_list (reg/v:SI 23)
                    (nil)))
            (nil))))

(insn 19 17 22 (set (reg/v:SI 21)
        (reg:SI 24)) -1 (nil)
    (nil))

(insn 22 19 24 (set (reg/v:SI 23)
        (const_int 2)) -1 (nil)
    (nil))

(insn 24 22 26 (set (mem:SI (pre_dec:SI (reg:SI 7 %esp)))
        (reg/v:SI 23)) -1 (nil)
    (insn_list:REG_LIBCALL 30 (nil)))

(call_insn/u 26 24 28 (set (reg:SI 0 %eax)
        (call (mem:QI (symbol_ref:SI ("f")))
            (const_int 4))) -1 (nil)
    (nil)
    (nil))

(insn 28 26 30 (set (reg:SI 7 %esp)
        (plus:SI (reg:SI 7 %esp)
            (const_int 4))) -1 (nil)
    (nil))

(insn 30 28 32 (set (reg:SI 25)
        (reg:SI 0 %eax)) -1 (nil)
    (insn_list:REG_RETVAL 24 (expr_list:REG_EQUAL (expr_list (symbol_ref:SI ("f"))
                (expr_list (reg/v:SI 23)
                    (nil)))
            (nil))))

(insn 32 30 34 (set (reg/v:SI 22)
        (reg:SI 25)) -1 (nil)
    (nil))

(insn 34 32 35 (set (cc0)
        (compare (reg/v:SI 21)
            (const_int 1))) -1 (nil)
    (nil))

(jump_insn 35 34 36 (set (pc)
        (if_then_else (ne (cc0)
                (const_int 0))
            (label_ref 40)
            (pc))) -1 (nil)
    (nil))

(insn 36 35 37 (set (cc0)
        (compare (reg/v:SI 22)
            (const_int 2))) -1 (nil)
    (nil))

(jump_insn 37 36 40 (set (pc)
        (if_then_else (eq (cc0)
                (const_int 0))
            (label_ref 45)
            (pc))) 279 {beq+1} (nil)
    (nil))

(code_label 40 37 43 3 "")

(call_insn 43 40 44 (parallel[ 
            (set (reg:SI 0 %eax)
                (call (mem:QI (symbol_ref:SI ("abort")))
                    (const_int 0)))
            (set (reg:SI 7 %esp)
                (plus:SI (reg:SI 7 %esp)
                    (const_int 0)))
        ] ) -1 (nil)
    (nil)
    (nil))

(barrier 44 43 45)

(code_label 45 44 48 2 "")

(insn 48 45 50 (set (mem:SI (pre_dec:SI (reg:SI 7 %esp)))
        (const_int 0)) -1 (nil)
    (nil))

(call_insn 50 48 51 (set (reg:SI 0 %eax)
        (call (mem:QI (symbol_ref:SI ("exit")))
            (const_int 4))) -1 (nil)
    (nil)
    (nil))

(barrier 51 50 53)

(note 53 51 0 0 NOTE_INSN_BLOCK_END)

;; Function f

(note 2 0 4 "" NOTE_INSN_DELETED)

(insn 4 2 5 (set (reg/v:SI 21)
        (mem:SI (reg:SI 16 %argp))) -1 (nil)
    (expr_list:REG_EQUIV (mem:SI (reg:SI 16 %argp))
        (nil)))

(note 5 4 6 "" NOTE_INSN_FUNCTION_BEG)

(note 6 5 8 "" NOTE_INSN_DELETED)

(note 8 6 10 "" NOTE_INSN_DELETED)

(insn 10 8 11 (set (reg/i:SI 0 %eax)
        (reg/v:SI 21)) -1 (nil)
    (nil))

(insn 11 10 0 (use (reg/i:SI 0 %eax)) -1 (nil)
    (nil))


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]