public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* asm clobbers, !SMALL_REGISTER_CLASSES patch.
@ 1998-02-12 19:36 Geoffrey KEATING
  1998-02-12 20:24 ` David Edelsohn
  0 siblings, 1 reply; 14+ messages in thread
From: Geoffrey KEATING @ 1998-02-12 19:36 UTC (permalink / raw)
  To: egcs; +Cc: David Edelsohn

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

From the earlier discussion about how to avoid problems with asm
clobbers, on machines with small register classes on which
SMALL_REGISTER_CLASSES is not defined, I have produced this patch.

The patch affects all ports.  I believe that it works and is good on
PowerPC/RS6000.  I believe that it has no effect on machines that
don't have any register classes containing only one register, like
sparc.

I have absolutely no idea what effect it will have on machines like
x86.  I'm willing to believe anything from 'stops egcs crashing in
reload and causes better code generation' to the reverse.  Thus, it
needs some testing, preferably on some pieces of code that use lots of
complex asm() statements (c-torture doesn't).

--
Geoff Keating <Geoff.Keating@anu.edu.au>


[-- Attachment #2: egcs-asm-2.diff --]
[-- Type: text/x-diff, Size: 5751 bytes --]

Fri Feb 13 13:48:25 1998  Geoffrey KEATING  <geoffk@ozemail.com.au>

	* stmt.c (expand_asm_operands): When an asm statement
	clobbers a register that is in a class of its own, and there
	is space remaining for an extra output, turn the clobber into
	an output to a dummy pseudo.  At present, don't do this if
	there are constraints that have alternatives.

--- stmt.c.orig-2	Sat Feb  7 18:07:55 1998
+++ stmt.c	Sat Feb  7 19:43:18 1998
@@ -1444,6 +1444,7 @@ expand_asm_operands (string, outputs, in
   int noutputs = list_length (outputs);
   int ninout = 0;
   int nclobbers;
+  int n_promoted_clobbers;
   tree tail;
   register int i;
   /* Vector of RTX's of evaluated output operands.  */
@@ -1453,6 +1454,10 @@ expand_asm_operands (string, outputs, in
     = (enum machine_mode *) alloca (noutputs * sizeof (enum machine_mode));
   /* The insn we have emitted.  */
   rtx insn;
+  /* The class letters for each promoted clobber, or '\0' if the clobber
+     won't be promoted.  */
+  char *clobber_class;
+  int constraint_alternatives = 0;
 
   /* An ASM with no outputs needs to be treated as volatile.  */
   if (noutputs == 0)
@@ -1527,12 +1532,16 @@ expand_asm_operands (string, outputs, in
 	    found_equal = 1;
 	    break;
 
+	  case ',':
+	    constraint_alternatives = 1;
+	    break;
+
 	  case '?':  case '!':  case '*':  case '%':  case '&':
 	  case 'V':  case 'm':  case 'o':  case '<':  case '>':
 	  case 'E':  case 'F':  case 'G':  case 'H':  case 'X':
 	  case 's':  case 'i':  case 'n':
 	  case 'I':  case 'J':  case 'K':  case 'L':  case 'M':
-	  case 'N':  case 'O':  case 'P':  case ',':
+	  case 'N':  case 'O':  case 'P':
 #ifdef EXTRA_CONSTRAINT
 	  case 'Q':  case 'R':  case 'S':  case 'T':  case 'U':
 #endif
@@ -1609,6 +1618,47 @@ expand_asm_operands (string, outputs, in
       return;
     }
 
+  n_promoted_clobbers = 0;
+  clobber_class = (char *) alloca (nclobbers * sizeof (char));
+  for (i = -1, tail = clobbers; tail; tail = TREE_CHAIN (tail))
+    {
+      char *regname = TREE_STRING_POINTER (TREE_VALUE (tail));
+      int regno = decode_reg_name (regname);
+
+      if (regno <= 0 && regno != -4)
+	continue;
+
+      clobber_class[++i] = '\0';
+
+      /* We can't promote clobbers to output operands if that produces
+	 too many output operands.  Perhaps we should print a warning
+	 in this case?
+
+	 Also, at present we don't try if the constraints contain commas.
+	 */
+      if (ninputs + noutputs + n_promoted_clobbers >= MAX_RECOG_OPERANDS
+	  || constraint_alternatives)
+	continue;
+
+      if (reg_class_size[REGNO_REG_CLASS(regno)] == 1)
+	{
+	  int class;
+	  if (REGNO_REG_CLASS(regno) == GENERAL_REGS)
+	    {
+	      ++n_promoted_clobbers;
+	      clobber_class[i] = 'r';
+	      continue;
+	    }
+	  for (class = 'a'; class <= 'z'; class++)
+	    if (REG_CLASS_FROM_LETTER(class) == REGNO_REG_CLASS(regno))
+	      {
+		++n_promoted_clobbers;
+		clobber_class[i] = class;
+		break;
+	      }
+	}
+    }
+
   /* Make vectors for the expression-rtx and constraint strings.  */
 
   argvec = rtvec_alloc (ninputs);
@@ -1763,10 +1813,12 @@ expand_asm_operands (string, outputs, in
     }
   else
     {
+      int clobb_counter;
       rtx obody = body;
-      int num = noutputs;
+      int num = noutputs + n_promoted_clobbers;
       if (num == 0) num = 1;
-      body = gen_rtx (PARALLEL, VOIDmode, rtvec_alloc (num + nclobbers));
+      body = gen_rtx (PARALLEL, VOIDmode,
+		      rtvec_alloc (num + nclobbers - n_promoted_clobbers));
 
       /* For each output operand, store a SET.  */
 
@@ -1783,14 +1835,49 @@ expand_asm_operands (string, outputs, in
 	  MEM_VOLATILE_P (SET_SRC (XVECEXP (body, 0, i))) = vol;
 	}
 
-      /* If there are no outputs (but there are some clobbers)
-	 store the bare ASM_OPERANDS into the PARALLEL.  */
+      /* Store a SET for each promoted clobber.  */
+      
+      clobb_counter = -1;
+      for (tail = clobbers; tail; tail = TREE_CHAIN (tail))
+	{
+	  char *regname = TREE_STRING_POINTER (TREE_VALUE (tail));
+	  int regno = decode_reg_name (regname);
+
+	  if (regno <= 0 && regno != -4)
+	    continue;
+
+	  if (clobber_class[++clobb_counter] != '\0')
+	    {
+	      extern struct obstack *rtl_obstack;
+	      char *constraint;
+	      
+	      constraint = obstack_alloc (rtl_obstack, 4);
+	      constraint[0] = '=';
+	      constraint[1] = '&';
+	      constraint[2] = clobber_class[clobb_counter];
+	      constraint[3] = '\0';
+	      XVECEXP (body, 0, i)
+		= gen_rtx (SET, VOIDmode,
+			   gen_reg_rtx(reg_raw_mode[regno]),
+			   gen_rtx (ASM_OPERANDS, VOIDmode,
+				    TREE_STRING_POINTER (string),
+				    constraint,
+				    i, argvec, constraints,
+				    filename, line));
+	      MEM_VOLATILE_P (SET_SRC (XVECEXP (body, 0, i))) = vol;
+	      i++;
+	    }
+	}
+
+      /* If there are no outputs or promoted clobbers store the bare
+	 ASM_OPERANDS into the PARALLEL.  */
 
       if (i == 0)
 	XVECEXP (body, 0, i++) = obody;
 
-      /* Store (clobber REG) for each clobbered register specified.  */
+      /* Store (clobber REG) for each non-promoted clobbered register.  */
 
+      clobb_counter = 0;
       for (tail = clobbers; tail; tail = TREE_CHAIN (tail))
 	{
 	  char *regname = TREE_STRING_POINTER (TREE_VALUE (tail));
@@ -1807,12 +1894,16 @@ expand_asm_operands (string, outputs, in
 		    = gen_rtx (CLOBBER, VOIDmode,
 			       gen_rtx (MEM, BLKmode,
 					gen_rtx (SCRATCH, VOIDmode, 0)));
+		  clobb_counter++;
 		  continue;
 		}
 
 	      /* Ignore unknown register, error already signalled.  */
 	      continue;
 	    }
+
+	  if (clobber_class[clobb_counter++] != '\0')
+	      continue;
 
 	  /* Use QImode since that's guaranteed to clobber just one reg.  */
 	  XVECEXP (body, 0, i++)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-12 20:24 ` David Edelsohn
@ 1998-02-12 20:24   ` Geoffrey KEATING
  1998-02-13  2:28   ` Richard Henderson
  1998-02-13  2:28   ` Jeffrey A Law
  2 siblings, 0 replies; 14+ messages in thread
From: Geoffrey KEATING @ 1998-02-12 20:24 UTC (permalink / raw)
  To: David Edelsohn; +Cc: egcs

> Cc: egcs@cygnus.com
> Date: Thu, 12 Feb 1998 23:10:32 -0500
> From: David Edelsohn <dje@watson.ibm.com>
> 
>         * stmt.c (expand_asm_operands): When an asm statement
>         clobbers a register that is in a class of its own, and there
>         is space remaining for an extra output, turn the clobber into
>         an output to a dummy pseudo.  At present, don't do this if
>         there are constraints that have alternatives.
> 
> 	This is a good effort and I am glad that you are continuing to
> pursue this needed feature, but I am curious why you are choosing to
> implement it this way instead of as a match_scratch of the appropriate
> register class?  I think that the correct solution is to turn the clobber
> of a named register into the equivalent that GCC uses internally for
> machine description files: match_scratch.  This patch probably works to
> some extent, but it simply lies to GCC by transforming the clobber -- and
> lying to GCC always causes problems in the long run.

I looked into this, and it seems that match_scratch in a MD file gets
turned into (i) a dummy pseudo and (ii) a constraint on the insn.  The
only way I can see to specify a constraint on an asm statement is like
this.

Of course, there might be someone out there who knows how this should
_really_ be done, in which case suggestions are welcomed.

--
Geoff Keating <Geoff.Keating@anu.edu.au>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-12 19:36 asm clobbers, !SMALL_REGISTER_CLASSES patch Geoffrey KEATING
@ 1998-02-12 20:24 ` David Edelsohn
  1998-02-12 20:24   ` Geoffrey KEATING
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: David Edelsohn @ 1998-02-12 20:24 UTC (permalink / raw)
  To: Geoffrey KEATING; +Cc: egcs

        * stmt.c (expand_asm_operands): When an asm statement
        clobbers a register that is in a class of its own, and there
        is space remaining for an extra output, turn the clobber into
        an output to a dummy pseudo.  At present, don't do this if
        there are constraints that have alternatives.

	This is a good effort and I am glad that you are continuing to
pursue this needed feature, but I am curious why you are choosing to
implement it this way instead of as a match_scratch of the appropriate
register class?  I think that the correct solution is to turn the clobber
of a named register into the equivalent that GCC uses internally for
machine description files: match_scratch.  This patch probably works to
some extent, but it simply lies to GCC by transforming the clobber -- and
lying to GCC always causes problems in the long run.

David


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-12 20:24 ` David Edelsohn
  1998-02-12 20:24   ` Geoffrey KEATING
  1998-02-13  2:28   ` Richard Henderson
@ 1998-02-13  2:28   ` Jeffrey A Law
  1998-02-13  2:28     ` Geoffrey KEATING
  2 siblings, 1 reply; 14+ messages in thread
From: Jeffrey A Law @ 1998-02-13  2:28 UTC (permalink / raw)
  To: Geoffrey KEATING; +Cc: egcs

  In message < 9802130410.AA27074@rios1.watson.ibm.com > David Edelsohn wrote:
  >         * stmt.c (expand_asm_operands): When an asm statement
  >         clobbers a register that is in a class of its own, and there
  >         is space remaining for an extra output, turn the clobber into
  >         an output to a dummy pseudo.  At present, don't do this if
  >         there are constraints that have alternatives.
  > 
  > 	This is a good effort and I am glad that you are continuing to
  > pursue this needed feature, but I am curious why you are choosing to
  > implement it this way instead of as a match_scratch of the appropriate
  > register class?  I think that the correct solution is to turn the clobber
  > of a named register into the equivalent that GCC uses internally for
  > machine description files: match_scratch.  This patch probably works to
  > some extent, but it simply lies to GCC by transforming the clobber -- and
  > lying to GCC always causes problems in the long run.
David's approach sounds cleaner, but I haven't thought too much about
where to "hook in" and do this.

Also, with your patch you could end up introducing new aborts.

Basically, if you had two asms which clobbered "r1" (assume that it is
in a class by itself) and you were only able to "fix" one of them
with your trick, then you can lose.

Basically you end up with r1 appearing explicitly in the unfixed asm, and
appearing via a pseudo which must be allocated to r1 in the fixed asm.
At some point this will cause reload to abort -- it might take a million
lines of code to trigger it, but it will eventually bite you.

If you fix one, you need to fix them all.

Note the same applies to David's approach.  In fact, we may have a fundamental
problem -- if the register appeared as an explicit inout/output to one
asm, but was turned into a max_scratch for another asm because it was a 
clobber, then we run into the same problem.  So it seems that we have to
fix the inputs, outputs, and clobbers.  Yuck.  Sorry I didn't think of this
earlier.

jeff


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-13  2:28   ` Jeffrey A Law
@ 1998-02-13  2:28     ` Geoffrey KEATING
  1998-02-14  0:32       ` Jeffrey A Law
  0 siblings, 1 reply; 14+ messages in thread
From: Geoffrey KEATING @ 1998-02-13  2:28 UTC (permalink / raw)
  To: law; +Cc: Geoffrey KEATING, egcs

> From: Jeffrey A Law <law@hurl.cygnus.com>
...
> [David:]
>   > I think that the correct solution is to turn the clobber
>   > of a named register into the equivalent that GCC uses internally for
>   > machine description files: match_scratch.  ... 
>   > lying to GCC always causes problems in the long run.

> David's approach sounds cleaner, but I haven't thought too much about
> where to "hook in" and do this.

I agree with this.

> Also, with your patch you could end up introducing new aborts.
> 
> Basically, if you had two asms which clobbered "r1" (assume that it is
> in a class by itself) and you were only able to "fix" one of them
> with your trick, then you can lose.

On PPC, of course, this is not a problem; generally, if you can't fix
a clobber of a register in a class of its own, eventually gcc will
abort anyway :-(.  The only small register classes contain cr0, lr, ctr,
and mq, and they all have this same problem (although I can't find an
example for ctr, because I can't get gcc to use it at all... this is
probably a bug).

Also, fortunately, the cases it can't fix are rare (they relate to
relatively complex 'asm' statements).  It would be easy to extend it
to cope with multiple alternatives, but the (inputs + outputs < 10)
problem is a harder limit.

One big advantage of David's solution is that, if I understand
correctly, it would just come out as (clobber some_pseudo), so there
would be no problem with having too many inputs/outputs.  You would
need to teach register allocation that whatever pseudo some_pseudo is
can only be put in one class, and I'm not sure how to do this.

Hmmm.  I'd like to write asm ("bar" : : : "cc0") as

(parallel [
     (asm_operands "bar" "x" 0 [] []))
     (set something-2 (asm_operands "" "x" 0 [] []))
     ;; aka '(clobber cc0)'
  ])

but I imagine gcc will say `something-2 isn't used anywhere, so we can
just eliminate the second asm statement'.

> Note the same applies to David's approach.  In fact, we may have a fundamental
> problem -- if the register appeared as an explicit inout/output to one
> asm, but was turned into a max_scratch for another asm because it was a 
> clobber, then we run into the same problem.  So it seems that we have to
> fix the inputs, outputs, and clobbers.  Yuck.  Sorry I didn't think of this
> earlier.

I don't understand this.  Does this mean that

int something, dummy2;
asm ("foo" : "+l"(something) );
asm ("bar" : "=&l"(dummy2) );

(on ppc, say) should crash, because "l" is a class with only one
register?

This is equivalent, with my patch, to:

int something;
asm ("foo" : "+l"(something) );
asm ("bar" : : : "lr");

-- 
Geoff Keating <Geoff.Keating@anu.edu.au>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-12 20:24 ` David Edelsohn
  1998-02-12 20:24   ` Geoffrey KEATING
@ 1998-02-13  2:28   ` Richard Henderson
  1998-02-13 10:34     ` David Edelsohn
  1998-02-13  2:28   ` Jeffrey A Law
  2 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 1998-02-13  2:28 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Geoffrey KEATING, egcs

On Thu, Feb 12, 1998 at 11:10:32PM -0500, David Edelsohn wrote:
> 	This is a good effort and I am glad that you are continuing to
> pursue this needed feature, but I am curious why you are choosing to
> implement it this way instead of as a match_scratch of the appropriate
> register class?

A SCRATCH only has a mode, not a register class.  There is nowhere to
set a register class except in the output constraint of an asm_output.
So the obvious (clobber (scratch:XX )) doesn't do the trick.

Of course, why 

+                          gen_reg_rtx(reg_raw_mode[regno]),

instead of

+                          gen_rtx_SCRATCH (reg_raw_mode[regno]),

I don't know.

And in the PowerPC example, isn't the problem that you're clobbering
CC, which wants an input of CCmode?  It is still technically possible
to get these cases with

  (clobber (match_scratch:XX))

in which GET_MODE_CLASS(XX) is MODE_CC or MODE_RANDOM.  Whether it is
worth the additional hastle while solving the larger problem I don't
know.


r~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-13  2:28   ` Richard Henderson
@ 1998-02-13 10:34     ` David Edelsohn
  1998-02-13 10:58       ` Richard Henderson
  0 siblings, 1 reply; 14+ messages in thread
From: David Edelsohn @ 1998-02-13 10:34 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Geoffrey KEATING, egcs

>>>>> Richard Henderson writes:

On Thu, Feb 12, 1998 at 11:10:32PM -0500, David Edelsohn wrote:
>> This is a good effort and I am glad that you are continuing to
>> pursue this needed feature, but I am curious why you are choosing to
>> implement it this way instead of as a match_scratch of the appropriate
>> register class?

Richard> A SCRATCH only has a mode, not a register class.  There is nowhere to
Richard> set a register class except in the output constraint of an asm_output.
Richard> So the obvious (clobber (scratch:XX )) doesn't do the trick.

	SCRATCH only has a mode, but RTL match_scratch includes both a
mode and a register class constraint.  For instance some of the AIX
common-mode calls effectively are inlined assembly and are defined with

	(clobber (match_scratch:SI 0 "=l"))

and
	(clobber (match_scratch:CC 1 "=x"))

i.e., clobbering both the link register and the default fields of the CC
register.  I believe that these types of clobbers are what those writing
GCC inlined assembly would like to describe.

David


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-13 10:58       ` Richard Henderson
@ 1998-02-13 10:58         ` David Edelsohn
  1998-02-14 20:23         ` Geoffrey KEATING
  1 sibling, 0 replies; 14+ messages in thread
From: David Edelsohn @ 1998-02-13 10:58 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Geoffrey KEATING, egcs

>>>>> Richard Henderson writes:

Richard> What if we provide a way to allocate scratches explicitly?  My
Richard> thought here is to put something special in the () part of the
Richard> output section (whether we turn it into a clobber is immaterial;
Richard> we need a parameter number for the asm string).  I considered
Richard> using something like __scratch__, but it'd be nice to avoid a
Richard> new keyword.  Using auto instead appeals to me.

	I agree that it would be nice to express regular scratch register
needs to GCC extended asm when no high-level language variable can
correspond to the class.  This seems somewhat tangential to the clobber
field which breaks the symmetry of the other fields by mentioning
registers by name instead of using register classes.  We still need to
extend register classes into the clobber domain; I would not want to have
to list artificial operands simply to clobber them.

Richard> although this is a bad example cause you could do the cmp+branch
Richard> thing in C around the blah.  And doesn't ppc have separate fp and
Richard> int cc regs (4 each)?  They don't seem to be given different
Richard> register classes...

	Kenner did not distinguish the FP and FX CC registers, he just
considered the entire 32-bit collection of 4-bit fields killed.  There are
some nice optimizations that GCC could do if it could perform full
register allocation on the individual 4-bit fields, but that has not
percolated up to the most important problem with the port.

David


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-13 10:34     ` David Edelsohn
@ 1998-02-13 10:58       ` Richard Henderson
  1998-02-13 10:58         ` David Edelsohn
  1998-02-14 20:23         ` Geoffrey KEATING
  0 siblings, 2 replies; 14+ messages in thread
From: Richard Henderson @ 1998-02-13 10:58 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Richard Henderson, Geoffrey KEATING, egcs

On Fri, Feb 13, 1998 at 12:47:00PM -0500, David Edelsohn wrote:
> 	SCRATCH only has a mode, but RTL match_scratch includes both a
> mode and a register class constraint.

Yes, but match_scratch at present can only be used in patterns
fed to genrecog and the like.  But I see no reason why we
couldn't support something like that.  Certainly it would match
what we're trying to describe much better.  It should only 
involve changes to the asm expander and reload, no?

I was thinking along the same lines last night about scratches
and allocating temps from classes that are not describable from 
C, like CCmode registers and whatnot.

PowerPC, Sparc64, and others have multiple CC registers.  It
comes to pass that while writing some inline assembly, you need
one.  At present you can only just pick one at random and 
clobber it.

What if we provide a way to allocate scratches explicitly?  My
thought here is to put something special in the () part of the
output section (whether we turn it into a clobber is immaterial;
we need a parameter number for the asm string).  I considered
using something like __scratch__, but it'd be nice to avoid a
new keyword.  Using auto instead appeals to me.

So you could then do

	asm ("cmpwi %0,%1,123\n\t"
	     "beq %0,1f\n\t"
	     "blah\n"
	     "1:"
	  : "=y"(auto) : "r"(foo));

which might expand to

	cmpwi  cr3,r4,123
	beq   cr3,1f
	blah
1:

although this is a bad example cause you could do the cmp+branch
thing in C around the blah.  And doesn't ppc have separate fp and
int cc regs (4 each)?  They don't seem to be given different
register classes...


r~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-13  2:28     ` Geoffrey KEATING
@ 1998-02-14  0:32       ` Jeffrey A Law
  1998-02-14 21:14         ` Geoffrey KEATING
  0 siblings, 1 reply; 14+ messages in thread
From: Jeffrey A Law @ 1998-02-14  0:32 UTC (permalink / raw)
  To: Geoffrey KEATING; +Cc: egcs

  In message < 199802130931.UAA18941@discus.anu.edu.au >you write:
  > On PPC, of course, this is not a problem; generally, if you can't fix
  > a clobber of a register in a class of its own, eventually gcc will
  > abort anyway :-(.  The only small register classes contain cr0, lr, ctr,
  > and mq, and they all have this same problem (although I can't find an
  > example for ctr, because I can't get gcc to use it at all... this is
  > probably a bug).
Yes, but your patch can/will end up breaking other ports, possibly
more so than they currently break.

And consider a port where a general register happens to be in a class
by itself -- for various reasons this happens on other ports.

  > > problem -- if the register appeared as an explicit inout/output to one
  > > asm, but was turned into a max_scratch for another asm because it was a 
  > > clobber, then we run into the same problem.  So it seems that we have to
  > > fix the inputs, outputs, and clobbers.  Yuck.  Sorry I didn't think of th
  > is
  > > earlier.
  > 
  > I don't understand this.  Does this mean that
  > 
  > int something, dummy2;
  > asm ("foo" : "+l"(something) );
  > asm ("bar" : "=&l"(dummy2) );
I don't think so, but I'm not familiar with the ppc to know what
"l" is (a register?  A register letter?"



The problem is you can't have registers which are in a class by
themselves appear explicitly in RTL and via a a reference to their
register class.

So, for example, say we have class R1_REGS, which contains register
r1, and that we use letter 'a' in constraints to specify R1_REGS.

Now, assume we have this pattern:

(define_insn "pic_load_label"
  [(set (match_operand:SI 0 "register_operand" "=a")
        (match_operand:SI 1 "pic_label_operand" ""))]

Note that is specifies R1_REGS for operand 0 via the "=a" constraint.

If we write an asm which explicitly uses r1 in a function which also
has uses pattern, then the compiler can/will abort during reload
if it has to spill the register.


jeff


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-13 10:58       ` Richard Henderson
  1998-02-13 10:58         ` David Edelsohn
@ 1998-02-14 20:23         ` Geoffrey KEATING
  1 sibling, 0 replies; 14+ messages in thread
From: Geoffrey KEATING @ 1998-02-14 20:23 UTC (permalink / raw)
  To: Richard Henderson
  Cc: David Edelsohn, Richard Henderson, Geoffrey KEATING, egcs

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

> Date: Fri, 13 Feb 1998 10:29:56 -0800
> From: Richard Henderson <rth@dot.cygnus.com>

> PowerPC, Sparc64, and others have multiple CC registers.  It
> comes to pass that while writing some inline assembly, you need
> one.  At present you can only just pick one at random and 
> clobber it.
> 
> What if we provide a way to allocate scratches explicitly?  My
> thought here is to put something special in the () part of the
> output section (whether we turn it into a clobber is immaterial;
> we need a parameter number for the asm string).  I considered
> using something like __scratch__, but it'd be nice to avoid a
> new keyword.  Using auto instead appeals to me.
> 
> So you could then do
> 
> 	asm ("cmpwi %0,%1,123\n\t"
> 	     "beq %0,1f\n\t"
> 	     "blah\n"
> 	     "1:"
> 	  : "=y"(auto) : "r"(foo));

Funny you should mention that.  I have such a patch attached; I did it
as a warmup to see how I should do the rewriting patch.  It uses the syntax

	: "=y" : "r"(foo));

That is, you just omit the expression.

My problem is that I can't find a compelling use for it.  Usually, you
only have one register that is best; on PPC, typically this is cr0
(for loops and suchlike) or cr7 (because it occupies the least
significant bits when you put the ccr in an integer register).

Thus I'm not terribly interested in it, unless someone else can find
an actual use for it.  I would rather spend the effort on important
things, like making sure that the released GNU C library doesn't crash
the released GNU C compiler :-).


[-- Attachment #2: egcs-asm-1.diff --]
[-- Type: text/x-diff, Size: 7012 bytes --]

--- gcc/c-parse.in.orig	Sat Feb  7 14:11:46 1998
+++ gcc/c-parse.in	Sat Feb  7 14:04:25 1998
@@ -2204,6 +2204,8 @@ nonnull_asm_operands:
 asm_operand:
 	  STRING '(' expr ')'
 		{ $$ = build_tree_list ($1, $3); }
+	| STRING
+		{ $$ = build_tree_list ($1, NULL_TREE); }
 	;
 
 asm_clobbers:
--- gcc/c-typeck.c.orig	Sat Sep 27 13:46:33 1997
+++ gcc/c-typeck.c	Fri Feb  6 23:50:00 1998
@@ -6519,7 +6519,7 @@ c_expand_asm_operands (string, outputs, 
 	}
       /* Detect modification of read-only values.
 	 (Otherwise done by build_modify_expr.)  */
-      else
+      else if (o[i] != NULL_TREE)
 	{
 	  tree type = TREE_TYPE (o[i]);
 	  if (TREE_READONLY (o[i])
--- gcc/stmt.c.orig	Fri Feb  6 21:25:08 1998
+++ gcc/stmt.c	Sat Feb  7 14:03:09 1998
@@ -47,6 +47,7 @@ Boston, MA 02111-1307, USA.  */
 #include "insn-config.h"
 #include "insn-codes.h"
 #include "expr.h"
+#include "regs.h"
 #include "hard-reg-set.h"
 #include "obstack.h"
 #include "loop.h"
@@ -1383,6 +1384,38 @@ expand_asm (body)
   last_expr_type = 0;
 }
 
+/* Choose a mode that might be suitable for a constraint.
+   If the constraint is too tricky, this can get it wrong.  */
+static enum machine_mode
+choose_constraint_mode(constraint)
+     char *constraint;
+{
+  for (;;)
+    switch (*constraint++)
+      {
+      case '0': case ',':
+	/* Wild guess.  This is why you shouldn't use this facility
+	   to (for instance) provide a memory location.  */
+	return byte_mode;
+      case 'r': case 'g':
+	/* Assume that general registers can at least hold a byte.  */
+	return byte_mode;
+      default:
+	{
+	  enum reg_class class = REG_CLASS_FROM_LETTER (constraint[-1]);
+	  int regno;
+
+	  if (class == NO_REGS)
+	    continue;
+
+	  for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+	    if (TEST_HARD_REG_BIT (reg_class_contents[(int) class], regno))
+	      return reg_raw_mode[regno];
+	  continue;
+	}
+      }
+}
+
 /* 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.
@@ -1455,15 +1488,18 @@ expand_asm_operands (string, outputs, in
   for (i = 0, tail = outputs; tail; tail = TREE_CHAIN (tail), i++)
     {
       tree val = TREE_VALUE (tail);
-      tree type = TREE_TYPE (val);
+      tree type;
       tree val1;
       int j;
       int found_equal = 0;
       int found_plus = 0;
       int allows_reg = 0;
 
+      if (val != NULL_TREE)
+	type = TREE_TYPE (val);
+
       /* If there's an erroneous arg, emit no insn.  */
-      if (TREE_TYPE (val) == error_mark_node)
+      if (val != NULL_TREE && type == error_mark_node)
 	return;
 
       /* Make sure constraint has `=' and does not have `+'.  Also, see
@@ -1519,17 +1555,29 @@ expand_asm_operands (string, outputs, in
 	  return;
 	}
 
+      if (found_plus && val == NULL_TREE)
+	{
+	  error ("inout operand to `asm' should have value");
+	  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.
 	 Make the asm insn write into that, then our caller will copy it to
-	 the real output operand.  Likewise for promoted variables.  */
+	 the real output operand.  Likewise for promoted variables,
+	 and for when we ignore the output (but then we don't want to have
+	 the caller copy it anywhere).  */
 
-      if (TREE_CODE (val) == INDIRECT_REF
-	  || (TREE_CODE_CLASS (TREE_CODE (val)) == 'd'
-	      && ! (GET_CODE (DECL_RTL (val)) == REG
-		    && GET_MODE (DECL_RTL (val)) != TYPE_MODE (type)))
-	  || ! allows_reg
-	  || found_plus)
+      if (val == NULL_TREE)
+	{
+	  char *constraint = TREE_STRING_POINTER (TREE_PURPOSE (tail));
+	  output_rtx[i] = gen_reg_rtx (choose_constraint_mode(constraint));
+	}
+      else if (TREE_CODE (val) == INDIRECT_REF
+	       || (TREE_CODE_CLASS (TREE_CODE (val)) == 'd'
+		   && ! (GET_CODE (DECL_RTL (val)) == REG
+			 && GET_MODE (DECL_RTL (val)) != TYPE_MODE (type)))
+	       || ! allows_reg
+	       || found_plus)
 	{
 	  if (! allows_reg)
 	    mark_addressable (TREE_VALUE (tail));
@@ -1579,6 +1627,13 @@ expand_asm_operands (string, outputs, in
     {
       int j;
       int allows_reg = 0;
+
+      /* Null inputs make little sense.  */
+      if (TREE_VALUE (tail) == NULL_TREE)
+	{
+	  error ("input operand to `asm' should have value");
+	  return;
+	}
 
       /* If there's an erroneous arg, emit no insn,
 	 because the ASM_INPUT would get VOIDmode
--- gcc/cp/parse.y.orig	Sat Feb  7 14:12:50 1998
+++ gcc/cp/parse.y	Sat Feb  7 14:12:33 1998
@@ -4065,6 +4065,8 @@ nonnull_asm_operands:
 asm_operand:
 	  STRING '(' expr ')'
 		{ $$ = build_tree_list ($$, $3); }
+	| STRING
+		{ $$ = build_tree_list ($1, NULL_TREE); }
 	;
 
 asm_clobbers:
--- gcc/ChangeLog~	Sun Jan 25 00:35:52 1998
+++ gcc/ChangeLog	Sat Feb  7 14:21:05 1998
@@ -1,3 +1,12 @@
+Fri Feb  6 22:15:24 1998  Geoff Keating  (geoffk@ozemail.com.au)
+
+	* c-parse.in (asm_operand): Add new `ignore output' syntax.
+	* c-typeck.c (c_expand_asm_operands): If we are ignoring the
+	output, don't copy it anywhere.
+	* stmt.c (choose_constraint_mode): New function.
+	(expand_asm_operands): Handle `ignore output' syntax by creating a
+	dummy pseudo to output to.
+
 Fri Jan  2 23:40:09 1998  Jim Wilson  (wilson@cygnus.com)
 			  Jeffrey A Law  (law@cygnus.com)
 
--- gcc/cp/ChangeLog~	Sun Jan 25 00:36:04 1998
+++ gcc/cp/ChangeLog	Sat Feb  7 14:21:35 1998
@@ -1,3 +1,7 @@
+Fri Feb  6 22:21:27 1998  Geoff Keating  <geoffk@ozemail.com.au>
+
+	* parse.y (asm_operand): Add new `ignore output' syntax.
+
 Sat Dec 20 13:00:30 1997  Jason Merrill  <jason@yorick.cygnus.com>
 
 	* pt.c (instantiate_decl): Defer all templates but inline functions.
--- gcc/extend.texi.orig	Sat Feb  7 14:50:39 1998
+++ gcc/extend.texi	Sat Feb  7 15:33:02 1998
@@ -2430,6 +2430,28 @@
 and therefore they cannot take account of them when deciding how to
 optimize.
 
+In some of these more complex @code{asm} statements, you need temporary
+registers.  You could just choose a register at random (and specify it
+as clobbered), but it is usually better to instead make the temporary an
+`output' of the @code{asm}; that way the register allocator can pick the
+most convenient temporary.  There is a special syntax to simplify using
+such temporaries: if you omit the C expression specifying the output
+destination, the output will be ignored.  For instance, the following
+example implements test-and-set on RS6000, using a temporary
+condition code register:
+
+@example
+asm volatile("0:  lwarx %0,0,%3;"
+             "    cmpwi %1,%0,0;"
+             "    bne %1,1f;"
+             "    stwcx. %4,0,%3;"
+             "    bne- 0b;"
+             "1: "
+     : "=&r"(oldvalue), "=y", "=x"
+     : "r"(address), "r"(1)
+     : "memory");
+@end example
+
 @cindex macros containing @code{asm}
 Usually the most convenient way to use these @code{asm} instructions is to
 encapsulate them in macros that look like functions.  For example,

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-14  0:32       ` Jeffrey A Law
@ 1998-02-14 21:14         ` Geoffrey KEATING
  1998-02-15 11:33           ` Jeffrey A Law
  0 siblings, 1 reply; 14+ messages in thread
From: Geoffrey KEATING @ 1998-02-14 21:14 UTC (permalink / raw)
  To: law; +Cc: egcs

> Date: Sat, 14 Feb 1998 01:28:20 -0700
> From: Jeffrey A Law <law@hurl.cygnus.com>

>   In message < 199802130931.UAA18941@discus.anu.edu.au >you write:
>   > On PPC, of course, this is not a problem; generally, if you can't fix
>   > a clobber of a register in a class of its own, eventually gcc will
>   > abort anyway :-(.  The only small register classes contain cr0, lr, ctr,
>   > and mq, and they all have this same problem (although I can't find an
>   > example for ctr, because I can't get gcc to use it at all... this is
>   > probably a bug).
> Yes, but your patch can/will end up breaking other ports, possibly
> more so than they currently break.

I said that in my first message.  Isn't it great we now have EGCS, so
we can just find out what, if anything, breaks, rather than trying to
guess?

The reason this patch is `safe' is that it doesn't do anything that
the user couldn't have done (except for the cases involving MODE_CC,
which I'm pretty sure only affect ppc).  It may (I haven't seen an
example yet and am not sure that such a thing exists) cause gcc to
crash, but if you can show me an input which crashes gcc with this
patch I can show you an input which causes gcc to crash _without_ this
patch (excepting mistakes in the patch, of course).

Any volunteers to rewrite gcc's reload pass? :-)

--
Geoff Keating <Geoff.Keating@anu.edu.au>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-14 21:14         ` Geoffrey KEATING
@ 1998-02-15 11:33           ` Jeffrey A Law
  1998-02-15 18:47             ` David Edelsohn
  0 siblings, 1 reply; 14+ messages in thread
From: Jeffrey A Law @ 1998-02-15 11:33 UTC (permalink / raw)
  To: Geoffrey KEATING; +Cc: egcs

  In message < 199802150511.QAA00712@discus.anu.edu.au >you write:
  > > Yes, but your patch can/will end up breaking other ports, possibly
  > > more so than they currently break.
  > 
  > I said that in my first message.  Isn't it great we now have EGCS, so
  > we can just find out what, if anything, breaks, rather than trying to
  > guess?
It's not a question of "if" something breaks, but "when".

Or to put it another way, your patch makes an already bad problem
worse instead of better by introducing more places where we can
lose.

It is plain wrong to have a register which is in a class by itself
mentioned explicitly in RTL *and* in a pattern via its register
class.

I wonder if it makes more sense to have asm clobbers specify a
register class instead of explicit registers as an option.  That
would at least give us the option of doing the clobber both ways
depending on how the target port handled the register in question.

  > The reason this patch is `safe' is that it doesn't do anything that
  > the user couldn't have done (except for the cases involving MODE_CC,
  > which I'm pretty sure only affect ppc).  It may (I haven't seen an
  > example yet and am not sure that such a thing exists) cause gcc to
  > crash, but if you can show me an input which crashes gcc with this
  > patch I can show you an input which causes gcc to crash _without_ this
  > patch (excepting mistakes in the patch, of course).
Right.  I don't disagree that we can cause crashes with or without
your patch.  I'm saying your patch makes it easier to introduce
crashes and violates a basic assumption made by the compiler.

asms are dangerous and have to be written very carefully -- it's
trivial to write an asm that will crash any port.

What we need to do is look for a better solution for asms that need
to clobber a register that is in a class by itself.

jeff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: asm clobbers, !SMALL_REGISTER_CLASSES patch.
  1998-02-15 11:33           ` Jeffrey A Law
@ 1998-02-15 18:47             ` David Edelsohn
  0 siblings, 0 replies; 14+ messages in thread
From: David Edelsohn @ 1998-02-15 18:47 UTC (permalink / raw)
  To: law; +Cc: Geoffrey KEATING, egcs

>>>>> Jeffrey A Law writes:

Jeff> I wonder if it makes more sense to have asm clobbers specify a
Jeff> register class instead of explicit registers as an option.  That
Jeff> would at least give us the option of doing the clobber both ways
Jeff> depending on how the target port handled the register in question.

	Exactly!  This is the problem most people are encountering and the
solution that I have been trying to recommend all along.  If the register
is alone in a class, then the class is preferred -- assuming that the port
did not need to specify SMALL_REGISTER_CLASSES for other reasons.

	Once one can specify a register class for a clobber (essentially
creating a clobber match-scratch, it should not be difficult for GCC to
recognize this and convert a register specified by name as a clobber into
the equivalent class containing only one register.

David

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~1998-02-15 18:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1998-02-12 19:36 asm clobbers, !SMALL_REGISTER_CLASSES patch Geoffrey KEATING
1998-02-12 20:24 ` David Edelsohn
1998-02-12 20:24   ` Geoffrey KEATING
1998-02-13  2:28   ` Richard Henderson
1998-02-13 10:34     ` David Edelsohn
1998-02-13 10:58       ` Richard Henderson
1998-02-13 10:58         ` David Edelsohn
1998-02-14 20:23         ` Geoffrey KEATING
1998-02-13  2:28   ` Jeffrey A Law
1998-02-13  2:28     ` Geoffrey KEATING
1998-02-14  0:32       ` Jeffrey A Law
1998-02-14 21:14         ` Geoffrey KEATING
1998-02-15 11:33           ` Jeffrey A Law
1998-02-15 18:47             ` David Edelsohn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).