`shorten_branches' and length of `asm' without operands

Joern Rennecke amylaar@cygnus.co.uk
Fri Dec 31 23:54:00 GMT 1999


> While I'm playing with my port (Atmel AVR) I have founded a strange thing
> in final.c shorten_branches () 
> 
> shorten_branches don't calculate insn length if insn is a `asm'
> without operands.
> 
> Is this a bug ?

Yes, this is a bug.  I've reported it in April, but nobody seems to care.

Message 4668/15496 Joern Rennecke                Apr 28, 1999  9:54:30 pm +0100

Return-Path: <owner-gcc-local@cygnus.com>
Subject: Re: gcc/19621: two patch alternatives
To: wilson@cygnus.com
Date: Wed, 28 Apr 1999 21:54:30 +0100 (BST)
Cc: gcc-local@cygnus.com

The out-of-range branch in this PR is due to inconsistent handling of
instruction length for asm statements without operands.  Some places
check for ASM_INPUT as well as for asm_noperands, while others don't.

I see two alternatives for fixing this problem:
Either, make the handling of asms in final.c consistent.  That's a small
 patch, but the underlying weirdness that asms have to be treated differently
 than other insns remains, so that future recurrences of this bug are likely.
Or, we can change genattrtab to generate a correct insn_default_length
 attribute in the first place, and remove the now unnecessary ASM checking
 code from final.

I have implemented both alterantives, as appended below.
Each alternative starts with its ChangeLog entry
(you can search e.g. for cygnus).

Wed Apr 28 21:37:34 1999  J"orn Rennecke <amylaar@cygnus.co.uk>

	* final.c (shorten_branches): Always check for ASM_INPUT when
	calculating a default insn length.

*** final.c-nofix	Wed Apr 28 17:51:02 1999
--- final.c		Wed Apr 28 17:50:38 1999
*************** shorten_branches (first)
*** 1301,1307 ****
  				 * GET_MODE_SIZE (GET_MODE (body)));
  	  /* Alignment is handled by ADDR_VEC_ALIGN.  */
  	}
!       else if (asm_noperands (body) >= 0)
  	insn_lengths[uid] = asm_insn_count (body) * insn_default_length (insn);
        else if (GET_CODE (body) == SEQUENCE)
  	{
--- 1301,1307 ----
  				 * GET_MODE_SIZE (GET_MODE (body)));
  	  /* Alignment is handled by ADDR_VEC_ALIGN.  */
  	}
!       else if (GET_CODE (body) == ASM_INPUT || asm_noperands (body) >= 0)
  	insn_lengths[uid] = asm_insn_count (body) * insn_default_length (insn);
        else if (GET_CODE (body) == SEQUENCE)
  	{
*************** shorten_branches (first)
*** 1321,1327 ****
  	      int inner_uid = INSN_UID (inner_insn);
  	      int inner_length;
  
! 	      if (asm_noperands (PATTERN (XVECEXP (body, 0, i))) >= 0)
  		inner_length = (asm_insn_count (PATTERN (inner_insn))
  				* insn_default_length (inner_insn));
  	      else
--- 1321,1328 ----
  	      int inner_uid = INSN_UID (inner_insn);
  	      int inner_length;
  
! 	      if (GET_CODE (PATTERN (XVECEXP (body, 0, i))) == ASM_INPUT
! 		  || asm_noperands (PATTERN (XVECEXP (body, 0, i))) >= 0)
  		inner_length = (asm_insn_count (PATTERN (inner_insn))
  				* insn_default_length (inner_insn));
  	      else

Wed Apr 28 21:37:34 1999  J"orn Rennecke <amylaar@cygnus.co.uk>

	* genattrtab.c (make_length_attrs): Fix up default length of asm
	statements to take number of actual asm insns into account.
	(write_attr_set): Handle MULT if first arg is a SYMBOL_REF.
	* final.c (asm_insn_count): No longer static.
	(get_attr_length, shorten_branches): Remove special case handling
	of ASMs.

Index: genattrtab.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/genattrtab.c,v
retrieving revision 1.53
diff -p -r1.53 genattrtab.c
*** genattrtab.c	1999/01/15 07:57:36	1.53
--- genattrtab.c	1999/04/28 20:39:49
*************** make_length_attrs ()
*** 2395,2400 ****
--- 2395,2401 ----
    struct attr_desc *length_attr, *new_attr;
    struct attr_value *av, *new_av;
    struct insn_ent *ie, *new_ie;
+   rtx new_value;
  
    /* See if length attribute is defined.  If so, it must be numeric.  Make
       it special so we don't output anything for it.  */
*************** make_length_attrs ()
*** 2419,2428 ****
        for (av = length_attr->first_value; av; av = av->next)
  	for (ie = av->first_insn; ie; ie = ie->next)
  	  {
! 	    new_av = get_attr_value (substitute_address (av->value,
! 							 no_address_fn[i],
! 							 address_fn[i]),
! 				     new_attr, ie->insn_code);
  	    new_ie = (struct insn_ent *) oballoc (sizeof (struct insn_ent));
  	    new_ie->insn_code = ie->insn_code;
  	    new_ie->insn_index = ie->insn_index;
--- 2420,2453 ----
        for (av = length_attr->first_value; av; av = av->next)
  	for (ie = av->first_insn; ie; ie = ie->next)
  	  {
! 	    new_value = substitute_address (av->value,
! 					    no_address_fn[i], address_fn[i]);
! 	    /* insn_default_length for an asm template should take the number
! 	       of individual asm insns into account.  */
! 	    if (ie->insn_code == -1 && i == 0)
! 	      {
! 		rtx old_value = NULL_RTX;
! 
! 		/* Wrapping NEW_EXP in a MULT will prevent optimizations
! 		   (which might be essential to strip attribute tests),
! 		   so do them now.  */
! 		while (GET_CODE (new_value) == COND && old_value != new_value)
! 		  {
! 		    old_value = new_value;
! 		    new_value = simplify_cond (new_value, ie->insn_code,
! 					       ie->insn_index);
! 		  }
! 		new_value
! 		  = attr_rtx (MULT,
! 			      attr_rtx (SYMBOL_REF,
! 					"asm_insn_count (PATTERN (insn))"),
! 			      new_value);
! 		/* Mark this SYMBOL_REF as unchanging so that walk_attr_value
! 		   won't mark it spuriously as needing extraction and
! 		   constraining.  */
! 		RTX_UNCHANGING_P (XEXP (new_value, 0)) = 1;
! 	      }
! 	    new_av = get_attr_value (new_value, new_attr, ie->insn_code);
  	    new_ie = (struct insn_ent *) oballoc (sizeof (struct insn_ent));
  	    new_ie->insn_code = ie->insn_code;
  	    new_ie->insn_index = ie->insn_index;
*************** write_attr_set (attr, indent, value, pre
*** 5007,5012 ****
--- 5032,5055 ----
  	  write_indent (indent + 2);
  	  printf ("}\n");
  	}
+     }
+   else if (GET_CODE (value) == MULT && GET_CODE (XEXP (value, 0)) == SYMBOL_REF)
+     {
+       /* This is needed for the insn_code == -1 (assembler) value of
+ 	 insn_default_length.  */
+       char *new_prefix
+ 	= (char *) alloca (strlen (prefix)
+ 			   + strlen (XSTR (XEXP (value, 0), 0)) + 8);
+       char *new_suffix = (char *) alloca (strlen (suffix) + 2);
+ 
+       strcpy (new_prefix, prefix);
+       strcat (new_prefix, " (");
+       strcat (new_prefix, XSTR (XEXP (value, 0), 0));
+       strcat (new_prefix, ") * (");
+       strcpy (new_suffix, ")");
+       strcat (new_suffix, suffix);
+       write_attr_set (attr, indent, XEXP (value, 1), new_prefix, new_suffix,
+ 		      known_true, insn_code, insn_index);
      }
    else
      abort ();
Index: final.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/final.c,v
retrieving revision 1.167.2.1
diff -p -r1.167.2.1 final.c
*** final.c	1999/04/27 19:47:33	1.167.2.1
--- final.c	1999/04/28 20:39:50
*************** static struct bb_str **sbb_tail	= &sbb_h
*** 319,325 ****
  static int sbb_label_num	= 0;		/* Last label used */
  
  #ifdef HAVE_ATTR_length
! static int asm_insn_count	PROTO((rtx));
  #endif
  static void profile_function	PROTO((FILE *));
  static void profile_after_prologue PROTO((FILE *));
--- 319,325 ----
  static int sbb_label_num	= 0;		/* Last label used */
  
  #ifdef HAVE_ATTR_length
! int asm_insn_count	PROTO((rtx));
  #endif
  static void profile_function	PROTO((FILE *));
  static void profile_after_prologue PROTO((FILE *));
*************** get_attr_length (insn)
*** 764,771 ****
  	if (GET_CODE (body) == USE || GET_CODE (body) == CLOBBER)
  	  return 0;
  
- 	else if (GET_CODE (body) == ASM_INPUT || asm_noperands (body) >= 0)
- 	  length = asm_insn_count (body) * insn_default_length (insn);
  	else if (GET_CODE (body) == SEQUENCE)
  	  for (i = 0; i < XVECLEN (body, 0); i++)
  	    length += get_attr_length (XVECEXP (body, 0, i));
--- 764,769 ----
*************** shorten_branches (first)
*** 1301,1308 ****
  				 * GET_MODE_SIZE (GET_MODE (body)));
  	  /* Alignment is handled by ADDR_VEC_ALIGN.  */
  	}
-       else if (asm_noperands (body) >= 0)
- 	insn_lengths[uid] = asm_insn_count (body) * insn_default_length (insn);
        else if (GET_CODE (body) == SEQUENCE)
  	{
  	  int i;
--- 1299,1304 ----
*************** shorten_branches (first)
*** 1321,1331 ****
  	      int inner_uid = INSN_UID (inner_insn);
  	      int inner_length;
  
! 	      if (asm_noperands (PATTERN (XVECEXP (body, 0, i))) >= 0)
! 		inner_length = (asm_insn_count (PATTERN (inner_insn))
! 				* insn_default_length (inner_insn));
! 	      else
! 		inner_length = insn_default_length (inner_insn);
  	      
  	      insn_lengths[inner_uid] = inner_length;
  	      if (const_delay_slots)
--- 1317,1323 ----
  	      int inner_uid = INSN_UID (inner_insn);
  	      int inner_length;
  
! 	      inner_length = insn_default_length (inner_insn);
  	      
  	      insn_lengths[inner_uid] = inner_length;
  	      if (const_delay_slots)
*************** shorten_branches (first)
*** 1576,1582 ****
     the number of machine instructions likely to be generated for this insn.
     This is used to compute its length.  */
  
! static int
  asm_insn_count (body)
       rtx body;
  {
--- 1568,1574 ----
     the number of machine instructions likely to be generated for this insn.
     This is used to compute its length.  */
  
! int
  asm_insn_count (body)
       rtx body;
  {



More information about the Gcc mailing list