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]

C++ PATCH: Fix PR2733



PR2733 (and four duplicates thereof) regards a case where the C++
front-end mishandled the extended asm syntax.  This lead to a failure
buliding KDE, which is why it got reported a lot. 

The problem was that expand_asm_operands calls mark_addressable for
output operands that need to be addressable.  The way this used to
work was that mark_addressable set TREE_ADDRESSABLE and then call
put_var_into_stack.  In other words, during RTL generation, we were
calling back into tree-land to set bits on trees, and then calling
back into RTL land to put the variable into the stack.  This is what
we technically call "a complete and utter tangle".

The C++ front-end now wants to have TREE_ADDRESSABLE set correctly
before any RTL generation.  (This is the only approach that will work
if we are going to do optimizations at the tree level.)  It then just
puts the variable on the stack directly when creating RTL, saving the
expense of the later put_var_into_stack call, as well as being much
simpler: first, you generate trees, then you generate RTL.

In the long run, all calls to mark_addressable during RTL-generation
should go away.

Anyhow, we have to have a (conservative) understanding of what
expand_asm_operands is going to want to treat as addressable when
we're in the front-end in order to get this right.

Tested on i686-pc-linux-gnu, applied on the mainline and the branch.
(Although the patch for the mainline was very slightly different, due
to other changes in the C++ front-end.)

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

2001-07-11  Mark Mitchell  <mark@codesourcery.com>

	* stmt.c (parse_output_constraint): New function, split out 
	from ...
	(expand_asm_operands): ... here.  Use parse_output_constraint.
	* tree.h (parse_output_constraint): Declare it.

2001-07-11  Mark Mitchell  <mark@codesourcery.com>

	* semantics.c (finish_asm_stmt): Mark the output operands
	to an asm addressable, if necessary.

Index: stmt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/stmt.c,v
retrieving revision 1.184.2.6
diff -c -p -r1.184.2.6 stmt.c
*** stmt.c	2001/05/17 11:37:30	1.184.2.6
--- stmt.c	2001/07/11 16:44:45
*************** expand_asm (body)
*** 1292,1297 ****
--- 1292,1455 ----
    last_expr_type = 0;
  }
  
+ /* Parse the output constraint pointed to by *CONSTRAINT_P.  It is the
+    OPERAND_NUMth output operand, indexed from zero.  There are NINPUTS
+    inputs and NOUTPUTS outputs to this extended-asm.  Upon return,
+    *ALLOWS_MEM will be TRUE iff the constraint allows the use of a
+    memory operand.  Similarly, *ALLOWS_REG will be TRUE iff the
+    constraint allows the use of a register operand.  And, *IS_INOUT
+    will be true if the operand is read-write, i.e., if it is used as
+    an input as well as an output.  If *CONSTRAINT_P is not in
+    canonical form, it will be made canonical.  (Note that `+' will be
+    rpelaced with `=' as part of this process.)
+ 
+    Returns TRUE if all went well; FALSE if an error occurred.  */
+ 
+ bool
+ parse_output_constraint (constraint_p, 
+ 			 operand_num,
+ 			 ninputs,
+ 			 noutputs,
+ 			 allows_mem, 
+ 			 allows_reg, 
+ 			 is_inout)
+      const char **constraint_p;
+      int operand_num;
+      int ninputs;
+      int noutputs;
+      bool *allows_mem;
+      bool *allows_reg;
+      bool *is_inout;
+ {
+   const char *constraint = *constraint_p;
+   const char *p;
+ 
+   /* Assume the constraint doesn't allow the use of either a register
+      or memory.  */
+   *allows_mem = false;
+   *allows_reg = false;
+ 
+   /* Allow the `=' or `+' to not be at the beginning of the string,
+      since it wasn't explicitly documented that way, and there is a
+      large body of code that puts it last.  Swap the character to
+      the front, so as not to uglify any place else.  */
+   p = strchr (constraint, '=');
+   if (!p)
+     p = strchr (constraint, '+');
+ 
+   /* If the string doesn't contain an `=', issue an error
+      message.  */
+   if (!p)
+     {
+       error ("output operand constraint lacks `='");
+       return false;
+     }
+ 
+   /* If the constraint begins with `+', then the operand is both read
+      from and written to.  */
+   *is_inout = (*p == '+');
+ 
+   /* Make sure we can specify the matching operand.  */
+   if (*is_inout && operand_num > 9)
+     {
+       error ("output operand constraint %d contains `+'", 
+ 	     operand_num);
+       return false;
+     }
+ 
+   /* Canonicalize the output constraint so that it begins with `='.  */
+   if (p != constraint || is_inout)
+     {
+       char *buf;
+       size_t c_len = strlen (constraint);
+ 
+       if (p != constraint)
+ 	warning ("output constraint `%c' for operand %d is not at the beginning",
+ 		 *p, operand_num);
+ 
+       /* Make a copy of the constraint.  */
+       buf = alloca (c_len + 1);
+       strcpy (buf, constraint);
+       /* Swap the first character and the `=' or `+'.  */
+       buf[p - constraint] = buf[0];
+       /* Make sure the first character is an `='.  (Until we do this,
+ 	 it might be a `+'.)  */
+       buf[0] = '=';
+       /* Replace the constraint with the canonicalized string.  */
+       *constraint_p = ggc_alloc_string (buf, c_len);
+       constraint = *constraint_p;
+     }
+ 
+   /* Loop through the constraint string.  */
+   for (p = constraint + 1; *p; ++p)
+     switch (*p)
+       {
+       case '+':
+       case '=':
+ 	error ("operand constraint contains '+' or '=' at illegal position.");
+ 	return false;
+ 	
+       case '%':
+ 	if (operand_num + 1 == ninputs + noutputs)
+ 	  {
+ 	    error ("`%%' constraint used with last operand");
+ 	    return false;
+ 	  }
+ 	break;
+ 
+       case 'V':  case 'm':  case 'o':
+ 	*allows_mem = true;
+ 	break;
+ 
+       case '?':  case '!':  case '*':  case '&':  case '#':
+       case 'E':  case 'F':  case 'G':  case 'H':
+       case 's':  case 'i':  case 'n':
+       case 'I':  case 'J':  case 'K':  case 'L':  case 'M':
+       case 'N':  case 'O':  case 'P':  case ',':
+ 	break;
+ 
+       case '0':  case '1':  case '2':  case '3':  case '4':
+       case '5':  case '6':  case '7':  case '8':  case '9':
+ 	error ("matching constraint not valid in output operand");
+ 	return false;
+ 
+       case '<':  case '>':
+ 	/* ??? Before flow, auto inc/dec insns are not supposed to exist,
+ 	   excepting those that expand_call created.  So match memory
+ 	   and hope.  */
+ 	*allows_mem = true;
+ 	break;
+ 
+       case 'g':  case 'X':
+ 	*allows_reg = true;
+ 	*allows_mem = true;
+ 	break;
+ 	
+       case 'p': case 'r':
+ 	*allows_reg = true;
+ 	break;
+ 
+       default:
+ 	if (!ISALPHA (*p))
+ 	  break;
+ 	if (REG_CLASS_FROM_LETTER (*p) != NO_REGS)
+ 	  *allows_reg = true;
+ #ifdef EXTRA_CONSTRAINT
+ 	else
+ 	  {
+ 	    /* Otherwise we can't assume anything about the nature of
+ 	       the constraint except that it isn't purely registers.
+ 	       Treat it like "g" and hope for the best.  */
+ 	    *allows_reg = true;
+ 	    *allows_mem = true;
+ 	  }
+ #endif
+ 	break;
+       }
+ 
+   return true;
+ }
+ 
  /* Generate RTL for an asm statement with arguments.
     STRING is the instruction template.
     OUTPUTS is a list of output arguments (lvalues); INPUTS a list of inputs.
*************** expand_asm_operands (string, outputs, in
*** 1411,1425 ****
        tree val = TREE_VALUE (tail);
        tree type = TREE_TYPE (val);
        const char *constraint;
!       char *p;
!       int c_len;
!       int j;
!       int is_inout = 0;
!       int allows_reg = 0;
!       int allows_mem = 0;
  
        /* If there's an erroneous arg, emit no insn.  */
!       if (TREE_TYPE (val) == error_mark_node)
  	return;
  
        /* Make sure constraint has `=' and does not have `+'.  Also, see
--- 1569,1580 ----
        tree val = TREE_VALUE (tail);
        tree type = TREE_TYPE (val);
        const char *constraint;
!       bool is_inout;
!       bool allows_reg;
!       bool allows_mem;
  
        /* If there's an erroneous arg, emit no insn.  */
!       if (type == error_mark_node)
  	return;
  
        /* Make sure constraint has `=' and does not have `+'.  Also, see
*************** expand_asm_operands (string, outputs, in
*** 1429,1547 ****
  
        constraint = TREE_STRING_POINTER (TREE_PURPOSE (tail));
        output_constraints[i] = constraint;
-       c_len = strlen (constraint);
- 
-       /* Allow the `=' or `+' to not be at the beginning of the string,
- 	 since it wasn't explicitly documented that way, and there is a
- 	 large body of code that puts it last.  Swap the character to
- 	 the front, so as not to uglify any place else.  */
-       switch (c_len)
- 	{
- 	default:
- 	  if ((p = strchr (constraint, '=')) != NULL)
- 	    break;
- 	  if ((p = strchr (constraint, '+')) != NULL)
- 	    break;
- 	case 0:
- 	  error ("output operand constraint lacks `='");
- 	  return;
- 	}
-       j = p - constraint;
-       is_inout = *p == '+';
- 
-       if (j || is_inout)
- 	{
- 	  /* Have to throw away this constraint string and get a new one.  */
- 	  char *buf = alloca (c_len + 1);
- 	  buf[0] = '=';
- 	  if (j)
- 	    memcpy (buf + 1, constraint, j);
- 	  memcpy (buf + 1 + j, p + 1, c_len - j);  /* not -j-1 - copy null */
- 	  constraint = ggc_alloc_string (buf, c_len);
- 	  output_constraints[i] = constraint;
- 
- 	  if (j)
- 	    warning (
- 		"output constraint `%c' for operand %d is not at the beginning",
- 		*p, i);
- 	}
  
!       /* Make sure we can specify the matching operand.  */
!       if (is_inout && i > 9)
! 	{
! 	  error ("output operand constraint %d contains `+'", i);
! 	  return;
! 	}
! 
!       for (j = 1; j < c_len; j++)
! 	switch (constraint[j])
! 	  {
! 	  case '+':
! 	  case '=':
! 	    error ("operand constraint contains '+' or '=' at illegal position.");
! 	    return;
! 
! 	  case '%':
! 	    if (i + 1 == ninputs + noutputs)
! 	      {
! 		error ("`%%' constraint used with last operand");
! 		return;
! 	      }
! 	    break;
! 
! 	  case '?':  case '!':  case '*':  case '&':  case '#':
! 	  case 'E':  case 'F':  case 'G':  case 'H':
! 	  case 's':  case 'i':  case 'n':
! 	  case 'I':  case 'J':  case 'K':  case 'L':  case 'M':
! 	  case 'N':  case 'O':  case 'P':  case ',':
! 	    break;
! 
! 	  case '0':  case '1':  case '2':  case '3':  case '4':
! 	  case '5':  case '6':  case '7':  case '8':  case '9':
! 	    error ("matching constraint not valid in output operand");
! 	    break;
! 
! 	  case 'V':  case 'm':  case 'o':
! 	    allows_mem = 1;
! 	    break;
! 
! 	  case '<':  case '>':
!           /* ??? Before flow, auto inc/dec insns are not supposed to exist,
!              excepting those that expand_call created.  So match memory
! 	     and hope.  */
! 	    allows_mem = 1;
! 	    break;
! 
! 	  case 'g':  case 'X':
! 	    allows_reg = 1;
! 	    allows_mem = 1;
! 	    break;
! 
! 	  case 'p': case 'r':
! 	    allows_reg = 1;
! 	    break;
! 
! 	  default:
! 	    if (! ISALPHA (constraint[j]))
! 	      {
! 		error ("invalid punctuation `%c' in constraint",
! 		       constraint[j]);
! 		return;
! 	      }
! 	    if (REG_CLASS_FROM_LETTER (constraint[j]) != NO_REGS)
! 	      allows_reg = 1;
! #ifdef EXTRA_CONSTRAINT
! 	    else
! 	      {
! 		/* Otherwise we can't assume anything about the nature of
! 		   the constraint except that it isn't purely registers.
! 		   Treat it like "g" and hope for the best.  */
! 		allows_reg = 1;
! 		allows_mem = 1;
! 	      }
! #endif
! 	    break;
! 	  }
  
        /* If an output operand is not a decl or indirect ref and our constraint
  	 allows a register, make a temporary to act as an intermediate.
--- 1584,1600 ----
  
        constraint = TREE_STRING_POINTER (TREE_PURPOSE (tail));
        output_constraints[i] = constraint;
  
!       /* Try to parse the output constraint.  If that fails, there's
! 	 no point in going further.  */
!       if (!parse_output_constraint (&output_constraints[i],
! 				    i,
! 				    ninputs,
! 				    noutputs,
! 				    &allows_mem,
! 				    &allows_reg,
! 				    &is_inout))
! 	return;
  
        /* If an output operand is not a decl or indirect ref and our constraint
  	 allows a register, make a temporary to act as an intermediate.
Index: tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree.h,v
retrieving revision 1.223.2.12
diff -c -p -r1.223.2.12 tree.h
*** tree.h	2001/06/08 02:03:09	1.223.2.12
--- tree.h	2001/07/11 16:44:46
*************** extern int div_and_round_double		PARAMS 
*** 2861,2866 ****
--- 2861,2869 ----
  extern void emit_nop			PARAMS ((void));
  extern void expand_computed_goto	PARAMS ((tree));
  extern struct rtx_def *label_rtx	PARAMS ((tree));
+ extern bool parse_output_constraint     PARAMS ((const char **,
+ 						 int, int, int,
+ 						 bool *, bool *, bool *));
  extern void expand_asm_operands		PARAMS ((tree, tree, tree, tree, int,
  						 const char *, int));
  extern int any_pending_cleanups		PARAMS ((int));
Index: cp/semantics.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.189.2.15
diff -c -p -r1.189.2.15 semantics.c
*** semantics.c	2001/06/06 21:51:11	1.189.2.15
--- semantics.c	2001/07/11 16:44:53
*************** finish_asm_stmt (cv_qualifier, string, o
*** 922,929 ****
      }
  
    if (!processing_template_decl)
!     for (t = input_operands; t; t = TREE_CHAIN (t))
!       TREE_VALUE (t) = decay_conversion (TREE_VALUE (t));
  
    r = build_stmt (ASM_STMT, cv_qualifier, string,
  		  output_operands, input_operands,
--- 922,971 ----
      }
  
    if (!processing_template_decl)
!     {
!       int i;
!       int ninputs;
!       int noutputs;
! 
!       for (t = input_operands; t; t = TREE_CHAIN (t))
! 	TREE_VALUE (t) = decay_conversion (TREE_VALUE (t));
! 
!       ninputs = list_length (input_operands);
!       noutputs = list_length (output_operands);
! 
!       for (i = 0, t = output_operands; t; t = TREE_CHAIN (t), ++i)
! 	{
! 	  bool allows_mem;
! 	  bool allows_reg;
! 	  bool is_inout;
! 	  const char *constraint;
! 	  tree operand;
! 
! 	  constraint = TREE_STRING_POINTER (TREE_PURPOSE (t));
! 	  operand = TREE_VALUE (output_operands);
! 
! 	  if (!parse_output_constraint (&constraint,
! 					i, ninputs, noutputs,
! 					&allows_mem,
! 					&allows_reg,
! 					&is_inout))
! 	    {
! 	      /* By marking the type as erroneous, we will not try to
! 		 process this operand again in expand_asm_operands.  */
! 	      TREE_TYPE (operand) = error_mark_node;
! 	      continue;
! 	    }
! 
! 	  /* If the operand is a DECL that is going to end up in
! 	     memory, assume it is addressable.  This is a bit more
! 	     conservative than it would ideally be; the exact test is
! 	     buried deep in expand_asm_operands and depends on the
! 	     DECL_RTL for the OPERAND -- which we don't have at this
! 	     point.  */
! 	  if (!allows_reg && DECL_P (operand))
! 	    mark_addressable (operand);
! 	}
!     }
  
    r = build_stmt (ASM_STMT, cv_qualifier, string,
  		  output_operands, input_operands,
Index: testsuite/g++.old-deja/g++.other/asm2.C
===================================================================
RCS file: asm2.C
diff -N asm2.C
*** /dev/null	Tue May  5 13:32:27 1998
--- asm2.C	Wed Jul 11 09:44:53 2001
***************
*** 0 ****
--- 1,11 ----
+ // Build don't link:
+ // Skip if not target: i?86-*-*
+ // Special g++ Options: -O2
+ 
+ typedef unsigned long long uint64;
+ uint64 fstps(void)
+ {
+   uint64 ret;
+   asm volatile("fstps %0" : "=m" (ret));
+   return ret;
+ }


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