[RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Hans-Peter Nilsson]

As the comments say and as the example show, without the patch
at the end of the message, more than one field with the same
name can appear in union sem_fields in the generated
sim/x/cpu.h.  For example, apply the following nonsensical but
supposedly harmless patch to m32r.cpu.  It makes the div insn
access sr in QI mode (besides the existing access in its default
or natural mode) and reg h-gr 13 in both SI and QI mode.  If you
think using the same operand in different modes in an insn is a
semantic error, let's fix the parser so that it's identified as
such.

Index: m32r.cpu
===================================================================
RCS file: /cvs/src/src/cgen/cpu/m32r.cpu,v
retrieving revision 1.1
diff -c -p -u -p -r1.1 m32r.cpu
cvs server: conflicting specifications of output style
--- m32r.cpu	5 Jul 2001 12:45:47 -0000	1.1
+++ m32r.cpu	5 Dec 2002 19:06:16 -0000
@@ -1026,7 +1026,13 @@
      ()
      "div $dr,$sr"
      (+ OP1_9 OP2_0 dr sr (f-simm16 0))
-     (if (ne sr (const 0)) (set dr (div dr sr)))
+     (sequence
+       ((SI tmp))
+       (if (ne sr (const 0)) (set dr (div dr sr)))
+       (set tmp
+	    (add SI (ext SI (and QI (reg QI h-gr 13)
+				 (and QI sr 254)))
+		 (reg SI h-gr 13))))
      ((m32r/d (unit u-exec (cycles 37)))
       (m32rx (unit u-exec (cycles 37))))
 )

Now, build the simulator and notice the compilation failure due
to this change (copyright-year change snipped) in the generated
code:

Index: cpu.h
===================================================================
RCS file: /cvs/src/src/sim/m32r/cpu.h,v
retrieving revision 1.4
diff -c -p -r1.4 cpu.h
*** cpu.h	14 Nov 2001 19:51:40 -0000	1.4
--- cpu.h	5 Dec 2002 19:07:15 -0000
*************** union sem_fields {
*** 226,234 ****
      UINT f_r1;
      UINT f_r2;
      unsigned char in_dr;
      unsigned char in_sr;
      unsigned char out_dr;
!   } sfmt_add;
  #if WITH_SCACHE_PBB
    /* Writeback handler.  */
    struct {
--- 226,237 ----
      UINT f_r1;
      UINT f_r2;
      unsigned char in_dr;
+     unsigned char in_h_gr_QI_13;
+     unsigned char in_h_gr_SI_13;
+     unsigned char in_sr;
      unsigned char in_sr;
      unsigned char out_dr;
!   } sfmt_div;
  #if WITH_SCACHE_PBB
    /* Writeback handler.  */
    struct {

There are now two members called "in_sr".  Correspondingly, in
decode.c, "FLD (in_sr)" is (harmlessly) set twice.

Looking at the code (more-than-three times 8-) the mode of the
*use* of the operand *in the semantics* makes a difference to
how the operands of the insn are collected outside of the
semantic code.  That looks wrong: operands should be handled as
statically sized entities, only depending on the define-operand
(et al) data.  Whatever comes from compilation of the semantic
code should not spill over into the operand tables.

Maybe it'd be best to scrap the saved mode.  That'd seems best
handled by using another class than <operand>, which have uses
for the mode besides as a in-ops+out-ops container.  Then again
there may be use of the used size of the operand some time.
Also, that would add another class for that reason only, which
does not seem reasonable.

Instead I just changed to store the natural mode of the operand,
ignoring the used mode.  This should not matter for the
generated code; just that the narrowing of operands happen in
the operators rather than at the input.  I also did this to
reg-operands for consistency.

I built binutils and sim for m32r, xstormy16 and i960 (which BTW
requires a (intelligently applied s/index/o-index/g to build).
There were differences in the generated code.  The m32r sim
testsuite passed.  I'm interested in hearing what would be
considered a decent test-run for a change like this.

Ok to commit?  If another solution is suggested, I can probably
be lured into doing that instead.

	* semantics.scm (-build-operand!, -build-reg-operand!): Store the
	natural mode of the operand, not the use of it.

Index: semantics.scm
===================================================================
RCS file: /cvs/src/src/cgen/semantics.scm,v
retrieving revision 1.1.1.1
diff -c -p -r1.1.1.1 semantics.scm
*** semantics.scm	28 Jul 2000 04:11:52 -0000	1.1.1.1
--- semantics.scm	5 Dec 2002 16:18:22 -0000
***************
*** 404,412 ****
  
  (define (-build-operand! op-name op mode tstate ref-type op-list sem-attrs)
    ;(display (list op-name mode ref-type)) (newline) (force-output)
!   (let* ((mode (mode-real-name (if (eq? mode 'DFLT)
! 				   (op:mode op)
! 				   mode)))
           ; The first #f is a placeholder for the object.
  	 (try (list '-op- #f mode op-name #f))
  	 (existing-op (-rtx-find-op try op-list)))
--- 404,416 ----
  
  (define (-build-operand! op-name op mode tstate ref-type op-list sem-attrs)
    ;(display (list op-name mode ref-type)) (newline) (force-output)
! 
!   ; For now, always use the natural mode of the operand, not of the
!   ; mode of the use of it.  Storing the used mode causes multiple
!   ; occurences of the same operand, and applications such as
!   ; cgen-cpu.h get confused and e.g. output structures with multiple
!   ; fields having the same name.
!   (let* ((mode (mode-real-name (op:mode op)))
           ; The first #f is a placeholder for the object.
  	 (try (list '-op- #f mode op-name #f))
  	 (existing-op (-rtx-find-op try op-list)))
***************
*** 442,451 ****
  	 (hw (current-hw-sem-lookup-1 hw-name)))
  
      (if hw
! 	; If the mode is DFLT, use the object's natural mode.
! 	(let* ((mode (mode-real-name (if (eq? (rtx-mode expr) 'DFLT)
! 					 (obj:name (hw-mode hw))
! 					 (rtx-mode expr))))
  	       (indx-sel (rtx-reg-index-sel expr))
  	       ; #f is a place-holder for the object (filled in later)
  	       (try (list 'reg #f mode hw-name indx-sel))
--- 446,458 ----
  	 (hw (current-hw-sem-lookup-1 hw-name)))
  
      (if hw
! 
! 	; For now, always use the natural mode of the operand, not of
! 	; the mode of the use of it.  Storing the used mode causes
! 	; multiple occurences of the same operand, and applications
! 	; such as cgen-cpu.h get confused and e.g. output structures
! 	; with multiple fields having the same name.
! 	(let* ((mode (mode-real-name (obj:name (hw-mode hw))))
  	       (indx-sel (rtx-reg-index-sel expr))
  	       ; #f is a place-holder for the object (filled in later)
  	       (try (list 'reg #f mode hw-name indx-sel))

brgds, H-P

[RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Doug Evans]

Sorry for the delay.  Now that I'm back up to speed on the m32r response
times may actually improve (but don't hold your breath of course, sigh).

The short answer is, please don't commit this just yet.
The purpose of this message is to (try to) solicit input from folks.

Hans-Peter Nilsson writes:
 > As the comments say and as the example show, without the patch
 > at the end of the message, more than one field with the same
 > name can appear in union sem_fields in the generated
 > sim/x/cpu.h.  For example, apply the following nonsensical but
 > supposedly harmless patch to m32r.cpu.  It makes the div insn
 > access sr in QI mode (besides the existing access in its default
 > or natural mode) and reg h-gr 13 in both SI and QI mode.  If you
 > think using the same operand in different modes in an insn is a
 > semantic error, let's fix the parser so that it's identified as
 > such.
 > 
 > Index: m32r.cpu
 > ===================================================================
 > RCS file: /cvs/src/src/cgen/cpu/m32r.cpu,v
 > retrieving revision 1.1
 > diff -c -p -u -p -r1.1 m32r.cpu
 > cvs server: conflicting specifications of output style
 > --- m32r.cpu	5 Jul 2001 12:45:47 -0000	1.1
 > +++ m32r.cpu	5 Dec 2002 19:06:16 -0000
 > @@ -1026,7 +1026,13 @@
 >       ()
 >       "div $dr,$sr"
 >       (+ OP1_9 OP2_0 dr sr (f-simm16 0))
 > -     (if (ne sr (const 0)) (set dr (div dr sr)))
 > +     (sequence
 > +       ((SI tmp))
 > +       (if (ne sr (const 0)) (set dr (div dr sr)))
 > +       (set tmp
 > +	    (add SI (ext SI (and QI (reg QI h-gr 13)
 > +				 (and QI sr 254)))
 > +		 (reg SI h-gr 13))))
 >       ((m32r/d (unit u-exec (cycles 37)))
 >        (m32rx (unit u-exec (cycles 37))))
 >  )

For the sake of discussion, let's modify this patch to be this:

diff -u -p -r1.1 m32r.cpu
--- m32r.cpu	5 Jul 2001 12:45:47 -0000	1.1
+++ m32r.cpu	13 Dec 2002 09:16:08 -0000
@@ -1026,7 +1026,14 @@
      ()
      "div $dr,$sr"
      (+ OP1_9 OP2_0 dr sr (f-simm16 0))
-     (if (ne sr (const 0)) (set dr (div dr sr)))
+     ;(if (ne sr (const 0)) (set dr (div dr sr)))
+     (sequence 
+       ((SI tmp)) 
+       (if (ne sr (const 0)) (set dr (div dr sr))) 
+       (set tmp 
+	    (add SI (ext SI (and QI (reg QI h-gr 13)
+				 (or QI (and QI sr #xf0) (and QI sr #x0f))))
+		 (reg SI h-gr 13))))
      ((m32r/d (unit u-exec (cycles 37)))
       (m32rx (unit u-exec (cycles 37))))
 )

Note the two copies of uses of `sr' in QImode.

Here's the diff I get for sim/m32r/cpu.h (without your patch):

@@ -226,9 +226,12 @@ union sem_fields {
     UINT f_r1;
     UINT f_r2;
     unsigned char in_dr;
+    unsigned char in_h_gr_QI_13;
+    unsigned char in_h_gr_SI_13;
+    unsigned char in_sr;
     unsigned char in_sr;
     unsigned char out_dr;
-  } sfmt_add;
+  } sfmt_div;
 #if WITH_SCACHE_PBB
   /* Writeback handler.  */
   struct {

Note that although `sr' is referenced twice in QImode, there's still only
one additional copy of `in_sr' (as is, still a bug of course! :-).

Note also that while there are two copies of in_h_gr_<mumble>,
there's no problem with them as the mode distinguishes them.

So another way to look at the bug (no claim is made that this
isn't obvious) is that the two copies of `in_sr' are ok except
that one or both should include a mode so there's no compilation error.
Clearly that's what's already happening with in_h_gr_<mumble>.
No claim is also made that this is a better way to look at the bug.
Maybe it is, maybe it isn't.  It comes down to "what do we want?".

We should also look at the effect on opcodes/m32r-opinst.c.
The salient change here is:

@@ -202,6 +202,16 @@ static const CGEN_OPINST sfmt_cmpz_ops[]
 
 static const CGEN_OPINST sfmt_div_ops[] = {
   { INPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
+  { INPUT, "h_gr_QI_13", HW_H_GR, CGEN_MODE_QI, 0, 13, 0 },
+  { INPUT, "h_gr_SI_13", HW_H_GR, CGEN_MODE_SI, 0, 13, 0 },
+  { INPUT, "sr", HW_H_GR, CGEN_MODE_SI, OP_ENT (SR), 0, 0 },
+  { INPUT, "sr", HW_H_GR, CGEN_MODE_QI, OP_ENT (SR), 0, 0 },
+  { OUTPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
+  { END }
+};
+
+static const CGEN_OPINST sfmt_divu_ops[] = {
+  { INPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
   { INPUT, "sr", HW_H_GR, CGEN_MODE_SI, OP_ENT (SR), 0, 0 },
   { OUTPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
   { END }

And with your patch the change to opcodes/m32r-opinst.c is:

@@ -202,6 +202,14 @@ static const CGEN_OPINST sfmt_cmpz_ops[]
 
 static const CGEN_OPINST sfmt_div_ops[] = {
   { INPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
+  { INPUT, "h_gr_SI_13", HW_H_GR, CGEN_MODE_SI, 0, 13, 0 },
+  { INPUT, "sr", HW_H_GR, CGEN_MODE_SI, OP_ENT (SR), 0, 0 },
+  { OUTPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
+  { END }
+};
+
+static const CGEN_OPINST sfmt_divu_ops[] = {
+  { INPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
   { INPUT, "sr", HW_H_GR, CGEN_MODE_SI, OP_ENT (SR), 0, 0 },
   { OUTPUT, "dr", HW_H_GR, CGEN_MODE_SI, OP_ENT (DR), 0, COND_REF },
   { END }

Is the mode relevant here?  What do you think?

IIRC, way back when, I wanted the mode to be relevant,
it was (at the time) useful information.

Plus, for architectures where the set of a register in different modes
has different effects on the register (e.g. x86, m68k),
the mode is important.  The x86 also has to deal with ah,bh,etc.
where there's no current mode one can apply to eax,ebx,etc. and get the
intended meaning.  So do we outlaw such things and say
registers can only be set in their natural mode, and for x86,m68k,etc.
we create additional registers that alias the appropriate bits of the
"real" register?  Or do we attach meaning to setting a register in
a mode different than its natural mode?
[for x86 and ah,bh,etc. we can still go the latter way and either
create new modes (blech (*1)) or define aliases]

I've started to come up with a patch that allows us to retain the mode
of the use, rather than always using the natural mode, should we want to go
this route.  I think we do.
I have one thing left to add but I've been up for a fair while
and need a break.  I'm sending this because you've been very patient
but I don't want to let this linger.

(*1) though if we ever wanted to generalize modes to handle
any portion of an object this isn't so blech (within the context
of a non-blech design/implementation :-).

 > Now, build the simulator and notice the compilation failure due
 > to this change (copyright-year change snipped) in the generated
 > code:
 > 
 > Index: cpu.h
 > ===================================================================
 > RCS file: /cvs/src/src/sim/m32r/cpu.h,v
 > retrieving revision 1.4
 > diff -c -p -r1.4 cpu.h
 > *** cpu.h	14 Nov 2001 19:51:40 -0000	1.4
 > --- cpu.h	5 Dec 2002 19:07:15 -0000
 > *************** union sem_fields {
 > *** 226,234 ****
 >       UINT f_r1;
 >       UINT f_r2;
 >       unsigned char in_dr;
 >       unsigned char in_sr;
 >       unsigned char out_dr;
 > !   } sfmt_add;
 >   #if WITH_SCACHE_PBB
 >     /* Writeback handler.  */
 >     struct {
 > --- 226,237 ----
 >       UINT f_r1;
 >       UINT f_r2;
 >       unsigned char in_dr;
 > +     unsigned char in_h_gr_QI_13;
 > +     unsigned char in_h_gr_SI_13;
 > +     unsigned char in_sr;
 >       unsigned char in_sr;
 >       unsigned char out_dr;
 > !   } sfmt_div;
 >   #if WITH_SCACHE_PBB
 >     /* Writeback handler.  */
 >     struct {
 > 
 > There are now two members called "in_sr".  Correspondingly, in
 > decode.c, "FLD (in_sr)" is (harmlessly) set twice.
 > 
 > Looking at the code (more-than-three times 8-) the mode of the
 > *use* of the operand *in the semantics* makes a difference to
 > how the operands of the insn are collected outside of the
 > semantic code.  That looks wrong: operands should be handled as
 > statically sized entities, only depending on the define-operand
 > (et al) data.  Whatever comes from compilation of the semantic
 > code should not spill over into the operand tables.
 > 
 > Maybe it'd be best to scrap the saved mode.  That'd seems best
 > handled by using another class than <operand>, which have uses
 > for the mode besides as a in-ops+out-ops container.  Then again
 > there may be use of the used size of the operand some time.
 > Also, that would add another class for that reason only, which
 > does not seem reasonable.
 > 
 > Instead I just changed to store the natural mode of the operand,
 > ignoring the used mode.  This should not matter for the
 > generated code; just that the narrowing of operands happen in
 > the operators rather than at the input.  I also did this to
 > reg-operands for consistency.
 > 
 > I built binutils and sim for m32r, xstormy16 and i960 (which BTW
 > requires a (intelligently applied s/index/o-index/g to build).
 > There were differences in the generated code.  The m32r sim
 > testsuite passed.  I'm interested in hearing what would be
 > considered a decent test-run for a change like this.
 > 
 > Ok to commit?  If another solution is suggested, I can probably
 > be lured into doing that instead.
 > 
 > 	* semantics.scm (-build-operand!, -build-reg-operand!): Store the
 > 	natural mode of the operand, not the use of it.
 > 
 > Index: semantics.scm
 > ===================================================================
 > RCS file: /cvs/src/src/cgen/semantics.scm,v
 > retrieving revision 1.1.1.1
 > diff -c -p -r1.1.1.1 semantics.scm
 > *** semantics.scm	28 Jul 2000 04:11:52 -0000	1.1.1.1
 > --- semantics.scm	5 Dec 2002 16:18:22 -0000
 > ***************
 > *** 404,412 ****
 >   
 >   (define (-build-operand! op-name op mode tstate ref-type op-list sem-attrs)
 >     ;(display (list op-name mode ref-type)) (newline) (force-output)
 > !   (let* ((mode (mode-real-name (if (eq? mode 'DFLT)
 > ! 				   (op:mode op)
 > ! 				   mode)))
 >            ; The first #f is a placeholder for the object.
 >   	 (try (list '-op- #f mode op-name #f))
 >   	 (existing-op (-rtx-find-op try op-list)))
 > --- 404,416 ----
 >   
 >   (define (-build-operand! op-name op mode tstate ref-type op-list sem-attrs)
 >     ;(display (list op-name mode ref-type)) (newline) (force-output)
 > ! 
 > !   ; For now, always use the natural mode of the operand, not of the
 > !   ; mode of the use of it.  Storing the used mode causes multiple
 > !   ; occurences of the same operand, and applications such as
 > !   ; cgen-cpu.h get confused and e.g. output structures with multiple
 > !   ; fields having the same name.
 > !   (let* ((mode (mode-real-name (op:mode op)))
 >            ; The first #f is a placeholder for the object.
 >   	 (try (list '-op- #f mode op-name #f))
 >   	 (existing-op (-rtx-find-op try op-list)))
 > ***************
 > *** 442,451 ****
 >   	 (hw (current-hw-sem-lookup-1 hw-name)))
 >   
 >       (if hw
 > ! 	; If the mode is DFLT, use the object's natural mode.
 > ! 	(let* ((mode (mode-real-name (if (eq? (rtx-mode expr) 'DFLT)
 > ! 					 (obj:name (hw-mode hw))
 > ! 					 (rtx-mode expr))))
 >   	       (indx-sel (rtx-reg-index-sel expr))
 >   	       ; #f is a place-holder for the object (filled in later)
 >   	       (try (list 'reg #f mode hw-name indx-sel))
 > --- 446,458 ----
 >   	 (hw (current-hw-sem-lookup-1 hw-name)))
 >   
 >       (if hw
 > ! 
 > ! 	; For now, always use the natural mode of the operand, not of
 > ! 	; the mode of the use of it.  Storing the used mode causes
 > ! 	; multiple occurences of the same operand, and applications
 > ! 	; such as cgen-cpu.h get confused and e.g. output structures
 > ! 	; with multiple fields having the same name.
 > ! 	(let* ((mode (mode-real-name (obj:name (hw-mode hw))))
 >   	       (indx-sel (rtx-reg-index-sel expr))
 >   	       ; #f is a place-holder for the object (filled in later)
 >   	       (try (list 'reg #f mode hw-name indx-sel))
 > 
 > brgds, H-P
 > 

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Hans-Peter Nilsson]

> From: Doug Evans <dje@transmeta.com>
> Date: Fri, 13 Dec 2002 05:49:03 -0800 (PST)

> Sorry for the delay.  Now that I'm back up to speed on the m32r response
> times may actually improve (but don't hold your breath of course, sigh).

I've still not replied to your reply about the previous patch so
no need to be sorry.

> The short answer is, please don't commit this just yet.

I have no intention to.  (Except locally.  Muhahaha! :-)

> The purpose of this message is to (try to) solicit input from folks.

> Note also that while there are two copies of in_h_gr_<mumble>,
> there's no problem with them as the mode distinguishes them.

Sorry, I think you're missing the point: I want to get the
register accesses through to the model machinery.

> So another way to look at the bug (no claim is made that this
> isn't obvious) is that the two copies of `in_sr' are ok except
> that one or both should include a mode so there's no compilation error.
>
> Clearly that's what's already happening with in_h_gr_<mumble>.
> No claim is also made that this is a better way to look at the bug.
> Maybe it is, maybe it isn't.  It comes down to "what do we want?".

No no no.  Not a copy for each accessed mode, please.  Please
don't do that.  For one, how would you map that on the model
machinery?  How you you send this to the unit function in:

    (unit u-foo "Foo Unit" () 1 1 ()
          ((dr INT -1))
          () ()))

Consider the case where one insn accesses a register in multiple
modes, while another accesses it in one mode.

> We should also look at the effect on opcodes/m32r-opinst.c.
> The salient change here is:
> 
> @@ -202,6 +202,16 @@ static const CGEN_OPINST sfmt_cmpz_ops[]

> Is the mode relevant here?  What do you think?

I don't claim to know my way around the CGEN assembler/opcode
parts, except as I told previously, that the assembler parts
should obviously *not* be affected by anything in the semantics.
It should only look at the (+ OP1_9 OP2_0 dr sr (f-simm16 0))
(heh, cute, looks like a curse :-) format line.

> IIRC, way back when, I wanted the mode to be relevant,
> it was (at the time) useful information.
> 
> Plus, for architectures where the set of a register in different modes
> has different effects on the register (e.g. x86, m68k),
> the mode is important.  The x86 also has to deal with
> ah,bh,etc.

I'm using this in a related situation, actually.

> where there's no current mode one can apply to eax,ebx,etc. and get the
> intended meaning.  So do we outlaw such things and say
> registers can only be set in their natural mode, and for x86,m68k,etc.
> we create additional registers that alias the appropriate bits of the
> "real" register?

Does the patch have this "outlawing" effect?  I didn't intend
that.

>  Or do we attach meaning to setting a register in
> a mode different than its natural mode?

Like what? (Except for the obvious: affecting only the part of
the register corresponding to the mode.  I did not intend to
change this.)

> [for x86 and ah,bh,etc. we can still go the latter way and either
> create new modes (blech (*1)) or define aliases]

No new modes or aliases, please.

> I've started to come up with a patch that allows us to retain the mode
> of the use, rather than always using the natural mode, should we want to go
> this route.  I think we do.

And the model thingy?  That's the one thing that's important to
me.

brgds, H-P

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Doug Evans]

Hans-Peter Nilsson writes:
 > > Note also that while there are two copies of in_h_gr_<mumble>,
 > > there's no problem with them as the mode distinguishes them.
 > 
 > Sorry, I think you're missing the point: I want to get the
 > register accesses through to the model machinery.

Righto.
My point was solely focused on the compilation error.
Clearly we want to be able to feed explicitly specified
registers into the model machinery.

 > > Clearly that's what's already happening with in_h_gr_<mumble>.
 > > No claim is also made that this is a better way to look at the bug.
 > > Maybe it is, maybe it isn't.  It comes down to "what do we want?".
 > 
 > No no no.  Not a copy for each accessed mode, please.  Please
 > don't do that.  For one, how would you map that on the model
 > machinery?  How you you send this to the unit function in:
 > 
 >     (unit u-foo "Foo Unit" () 1 1 ()
 >           ((dr INT -1))
 >           () ()))
 > 
 > Consider the case where one insn accesses a register in multiple
 > modes, while another accesses it in one mode.

I recognize the issue.

But suppose each of those multiple-mode accesses feed into different
units and suppose they're conditionally referenced.  Collapsing
them all into one would be wrong.
The proposed new `rename' rtx would help here, each relevant reference
that needs to be fed into a model would be given a special name and
that name would be used to plug that operand into the model units.
No claim is made that it is the best solution for this particular case,
but it seems reasonable.

 > > We should also look at the effect on opcodes/m32r-opinst.c.
 > > The salient change here is:
 > > 
 > > @@ -202,6 +202,16 @@ static const CGEN_OPINST sfmt_cmpz_ops[]
 > 
 > > Is the mode relevant here?  What do you think?
 > 
 > I don't claim to know my way around the CGEN assembler/opcode
 > parts, except as I told previously, that the assembler parts
 > should obviously *not* be affected by anything in the semantics.
 > It should only look at the (+ OP1_9 OP2_0 dr sr (f-simm16 0))
 > (heh, cute, looks like a curse :-) format line.

I don't want opcodes/* to be treated as solely assembler/opcode parts.
For example, suppose one has a fairly big and complicated program
and verifying its correctness is critical.  One would wish to employ
lots of different kinds of verification tools.  One tool one could employ
would be a static program analysis tool.  Crack open each instruction in the
compiled form of the program, build a flow graph, and analyze away.
I'd like to bring opcodes/* to a point where it would form the basis
for such a tool (among many other things).

 > > where there's no current mode one can apply to eax,ebx,etc. and get the
 > > intended meaning.  So do we outlaw such things and say
 > > registers can only be set in their natural mode, and for x86,m68k,etc.
 > > we create additional registers that alias the appropriate bits of the
 > > "real" register?
 > 
 > Does the patch have this "outlawing" effect?  I didn't intend
 > that.

No, nothing outlawed.  Just trying to spur some creative juices.
Fermented grape juice, mmmm....

 > >  Or do we attach meaning to setting a register in
 > > a mode different than its natural mode?
 > 
 > Like what? (Except for the obvious: affecting only the part of
 > the register corresponding to the mode.  I did not intend to
 > change this.)

Just the obvious.   I was thinking out loud about the possibility
of applying your patch and seeing where it leads.  What if we make the
design choice of saying "mode is irrelevant", and all objects are _only_
accessed in their natural mode (except for memory of course),
treating reading and writing of objects equivalently.
It is an appealing simplification, but there are no free lunches.
gcc doesn't have different register numbers for %eax vs %ax vs %al
for good reasons.
[note: for memory, often byte enables are more useful than mode
anyways, so even for memory maybe we don't want modes.  Of course,
we could create a mode for each possible byte enable combination.
Muhahaha! :-)]

This would mean, for example, that one couldn't have "(set QI eax 42)"
mean "(set QI al 42)" in x86.  To accomplish this one would have to
create a set of register arrays for each of the ways eax,ebx,etc.
can be accessed.  I'm not sure we want to go there.

 > > [for x86 and ah,bh,etc. we can still go the latter way and either
 > > create new modes (blech (*1)) or define aliases]
 > 
 > No new modes or aliases, please.

Righto.  Here was just (trying to) state all the possibilities that
came to mind, regardless of their merit.

 > > I've started to come up with a patch that allows us to retain the mode
 > > of the use, rather than always using the natural mode, should we want to go
 > > this route.  I think we do.
 > 
 > And the model thingy?  That's the one thing that's important to
 > me.

Righto.

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Hans-Peter Nilsson]

> From: Doug Evans <dje@transmeta.com>
> Date: Fri, 13 Dec 2002 16:50:25 -0800 (PST)

> Hans-Peter Nilsson writes:
>  >     (unit u-foo "Foo Unit" () 1 1 ()
>  >           ((dr INT -1))
>  >           () ()))
>  > 
>  > Consider the case where one insn accesses a register in multiple
>  > modes, while another accesses it in one mode.
> 
> I recognize the issue.
> 
> But suppose each of those multiple-mode accesses feed into different
> units and suppose they're conditionally referenced.  Collapsing
> them all into one would be wrong.
> The proposed new `rename' rtx would help here,

Yep.  All my problems would be solved with the unit redirection
stuff (well, AFAICS).  I won't argue about the modes if I get
that.  Gimme! :-)

brgds, H-P

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Doug Evans]

Having thought about things a lot more, I think we should "remove"
the mode when computing the operands (for registers only, not sure
about memory).

So your patch it is!  With one alteration.

One reason why I was uncomfortable with removing the mode completely
is that while it may not have any effect on the generated code, per se,
mixed mode arithmetic is illegal.  To simplify writing description
files several shortcuts are allowed, but in general I think we want
to outlaw mode mismatches.

Therefore the result of -build-operand!,-build-reg-operand! still
has to have the requested mode.
So I propose this patch.  It records the natural mode of the operand
in the operand table, but if the requested mode is different, then
the operand's mode is explicitly converted before returning.

Ugh, I just realized that this patch doesn't handle floating point.
And I've left as a todo the checking of legal mode conversions.

Index: semantics.scm
===================================================================
RCS file: /cvs/src/src/cgen/semantics.scm,v
retrieving revision 1.2
diff -u -p -r1.2 semantics.scm
--- semantics.scm	20 Dec 2002 06:39:04 -0000	1.2
+++ semantics.scm	20 Dec 2002 10:39:04 -0000
@@ -407,8 +407,14 @@
   (let* ((mode (mode-real-name (if (eq? mode 'DFLT)
 				   (op:mode op)
 				   mode)))
+	 ; NUB-MODE is the mode to use to uniquify all the operands.
+	 ; For registers it is the natural mode of the register.
+	 ; For memory it is the mode of the use/set.
+	 (nub-mode (if (register? (op:type op))
+		       (op:mode op)
+		       mode))
          ; The first #f is a placeholder for the object.
-	 (try (list '-op- #f mode op-name #f))
+	 (try (list '-op- #f nub-mode op-name #f))
 	 (existing-op (-rtx-find-op try op-list)))
 
     (if (and (pc? op)
@@ -416,23 +422,32 @@
 	(append! sem-attrs
 		 (list (if (tstate-cond? tstate) 'COND-CTI 'UNCOND-CTI))))
 
-    ; If already present, return the object, otherwise add it.
-    (if existing-op
-
-	(cadr existing-op)
-
-	; We can't set the operand number yet 'cus we don't know it.
-	; However, when it's computed we'll need to set all associated
-	; operands.  This is done by creating shared rtx (a la gcc) - the
-	; operand number then need only be updated in one place.
-
-	(let ((xop (op:new-mode op mode)))
-	  (op:set-cond?! xop (tstate-cond? tstate))
-	  ; Set the object rtx in `try', now that we have it.
-	  (set-car! (cdr try) (rtx-make 'xop xop))
-	  ; Add the operand to in/out-ops.
-	  (append! op-list (list try))
-	  (cadr try))))
+    (let ((result-op
+	   ; If already recorded, use it, otherwise add it.
+	   (if existing-op
+
+	       (cadr existing-op)
+
+	       ; We can't set the operand number yet 'cus we don't know it.
+	       ; However, when it's computed we'll need to set all associated
+	       ; operands.  This is done by creating shared rtx (a la gcc) -
+	       ; the operand number then need only be updated in one place.
+
+	       (let ((xop (op:new-mode op nub-mode)))
+		 (op:set-cond?! xop (tstate-cond? tstate))
+		 ; Set the object rtx in `try', now that we have it.
+		 (set-car! (cdr try) (rtx-make 'xop xop))
+		 ; Add the operand to in/out-ops.
+		 (append! op-list (list try))
+		 (cadr try)))))
+
+      ; We can't return a mode different than requested.  Mixing
+      ; modes in things like and,add,etc. is illegal.
+      ; If MODE isn't the nub mode, wrap the operand in a mode change.
+      ; FIXME: error check, ensure smaller
+      (if (mode-compatible? 'samesize mode nub-mode)
+	  result-op
+	  (rtx-make 'trunc mode result-op))))
 )
 
 ; Subroutine of semantic-compile:process-expr!, to simplify it.
@@ -443,28 +458,40 @@
 
     (if hw
 	; If the mode is DFLT, use the object's natural mode.
-	(let* ((mode (mode-real-name (if (eq? (rtx-mode expr) 'DFLT)
-					 (obj:name (hw-mode hw))
+	(let* (; Always record the register in the operand table using its
+	       ; natural mode.
+	       (nub-mode (obj:name (hw-mode hw)))
+	       (mode (mode-real-name (if (eq? (rtx-mode expr) 'DFLT)
+					 nub-mode
 					 (rtx-mode expr))))
 	       (indx-sel (rtx-reg-index-sel expr))
 	       ; #f is a place-holder for the object (filled in later)
-	       (try (list 'reg #f mode hw-name indx-sel))
+	       (try (list 'reg #f nub-mode hw-name indx-sel))
 	       (existing-op (-rtx-find-op try op-list)))
 
-	  ; If already present, return the object, otherwise add it.
-	  (if existing-op
-
-	      (cadr existing-op)
-
-	      (let ((xop (apply reg (cons (tstate->estate tstate)
-					  (cons mode
-						(cons hw-name indx-sel))))))
-		(op:set-cond?! xop (tstate-cond? tstate))
-		; Set the object rtx in `try', now that we have it.
-		(set-car! (cdr try) (rtx-make 'xop xop))
-		; Add the operand to in/out-ops.
-		(append! op-list (list try))
-		(cadr try))))
+	  (let ((result-op
+		 ; If already recorded, use it, otherwise add it.
+		 (if existing-op
+
+		     (cadr existing-op)
+
+		     (let ((xop (apply reg (cons (tstate->estate tstate)
+						 (cons nub-mode
+						       (cons hw-name indx-sel))))))
+		       (op:set-cond?! xop (tstate-cond? tstate))
+		       ; Set the object rtx in `try', now that we have it.
+		       (set-car! (cdr try) (rtx-make 'xop xop))
+		       ; Add the operand to in/out-ops.
+		       (append! op-list (list try))
+		       (cadr try)))))
+
+	    ; We can't return a mode different than requested.  Mixing
+	    ; modes in things like and,add,etc. is illegal.
+	    ; If MODE isn't the nub mode, wrap the operand in a mode change.
+	    ; FIXME: error check, ensure smaller
+	    (if (mode-compatible? 'samesize mode nub-mode)
+		result-op
+		(rtx-make 'trunc mode result-op))))
 
 	(parse-error "FIXME" "unknown reg" expr)))
 )

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Doug Evans]

Doug Evans writes:
 > Ugh, I just realized that this patch doesn't handle floating point.
 > And I've left as a todo the checking of legal mode conversions.

And I don't properly handle sets.
I was going to, but it was 3 in the morning. :-)

I think what we want to do is require all sets of registers
to use the natural mode of the register.
For registers like x86's al,ah,ax,eax, we can either
- add set-partial, or some such
- use (set (subword ...) ...)
- require the description of additional registers that are
  smaller-mode aliases of the "real" register

I think I prefer either of the latter two.

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Hans-Peter Nilsson]

> From: Doug Evans <dje@transmeta.com>
> Date: Fri, 20 Dec 2002 09:17:10 -0800 (PST)

> I think what we want to do is require all sets of registers
> to use the natural mode of the register.
> For registers like x86's al,ah,ax,eax, we can either
> - add set-partial, or some such
> - use (set (subword ...) ...)
> - require the description of additional registers that are
>   smaller-mode aliases of the "real" register
> 
> I think I prefer either of the latter two.

My vote is for (set (subword ...) ...)

brgds, H-P

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Doug Evans]

When we last left our story, I had proposed this patch and then
pointed out that it didn't handle floating point or sets.

I'm not sure how to handle floating point other than to defer
a decision on how to handle it and flag an error if the size
changes for now.  So that's what this patch does now.

w.r.t. sets: there's various solutions but I think this patch
can still go in.

I propose checking this in.

2003-01-13  Doug Evans  <dje@sebabeach.org>

	* semantics.scm (-build-operand!): Record operand in its natural mode.
	(-build-reg-operand!): Ditto.

Index: semantics.scm
===================================================================
RCS file: /cvs/src/src/cgen/semantics.scm,v
retrieving revision 1.2
diff -u -p -r1.2 semantics.scm
--- semantics.scm	20 Dec 2002 06:39:04 -0000	1.2
+++ semantics.scm	14 Jan 2003 06:03:55 -0000
@@ -407,8 +407,14 @@
   (let* ((mode (mode-real-name (if (eq? mode 'DFLT)
 				   (op:mode op)
 				   mode)))
+	 ; NUB-MODE is the mode to use to uniquify all the operands.
+	 ; For registers it is the natural mode of the register.
+	 ; For memory it is the mode of the use/set.
+	 (nub-mode (if (register? (op:type op))
+		       (op:mode op)
+		       mode))
          ; The first #f is a placeholder for the object.
-	 (try (list '-op- #f mode op-name #f))
+	 (try (list '-op- #f nub-mode op-name #f))
 	 (existing-op (-rtx-find-op try op-list)))
 
     (if (and (pc? op)
@@ -416,23 +422,37 @@
 	(append! sem-attrs
 		 (list (if (tstate-cond? tstate) 'COND-CTI 'UNCOND-CTI))))
 
-    ; If already present, return the object, otherwise add it.
-    (if existing-op
-
-	(cadr existing-op)
-
-	; We can't set the operand number yet 'cus we don't know it.
-	; However, when it's computed we'll need to set all associated
-	; operands.  This is done by creating shared rtx (a la gcc) - the
-	; operand number then need only be updated in one place.
-
-	(let ((xop (op:new-mode op mode)))
-	  (op:set-cond?! xop (tstate-cond? tstate))
-	  ; Set the object rtx in `try', now that we have it.
-	  (set-car! (cdr try) (rtx-make 'xop xop))
-	  ; Add the operand to in/out-ops.
-	  (append! op-list (list try))
-	  (cadr try))))
+    (let ((result-op
+	   ; If already recorded, use it, otherwise add it.
+	   (if existing-op
+
+	       (cadr existing-op)
+
+	       ; We can't set the operand number yet 'cus we don't know it.
+	       ; However, when it's computed we'll need to set all associated
+	       ; operands.  This is done by creating shared rtx (a la gcc) -
+	       ; the operand number then need only be updated in one place.
+
+	       (let ((xop (op:new-mode op nub-mode)))
+		 (op:set-cond?! xop (tstate-cond? tstate))
+		 ; Set the object rtx in `try', now that we have it.
+		 (set-car! (cdr try) (rtx-make 'xop xop))
+		 ; Add the operand to in/out-ops.
+		 (append! op-list (list try))
+		 (cadr try)))))
+
+      ; We can't return a mode different than requested.  Mixing
+      ; modes in things like and,add,etc. is illegal.
+      ; If MODE isn't the nub mode, wrap the operand in a mode change.
+      ; Not sure how to handle floats so just punt for now and flag an
+      ; error if the size changes.
+      (if (mode-compatible? 'samesize mode nub-mode)
+	  result-op
+	  (if (mode-float? (mode:lookup mode))
+	      (error "floating point operand implicitly referenced in different mode")
+	      (if (mode-bigger? nub-mode mode)
+		  (rtx-make 'trunc mode result-op)
+		  (error "implicit reference of operand in bigger mode"))))))
 )
 
 ; Subroutine of semantic-compile:process-expr!, to simplify it.
@@ -443,28 +463,45 @@
 
     (if hw
 	; If the mode is DFLT, use the object's natural mode.
-	(let* ((mode (mode-real-name (if (eq? (rtx-mode expr) 'DFLT)
-					 (obj:name (hw-mode hw))
+	(let* (; Always record the register in the operand table using its
+	       ; natural mode.
+	       (nub-mode (obj:name (hw-mode hw)))
+	       (mode (mode-real-name (if (eq? (rtx-mode expr) 'DFLT)
+					 nub-mode
 					 (rtx-mode expr))))
 	       (indx-sel (rtx-reg-index-sel expr))
 	       ; #f is a place-holder for the object (filled in later)
-	       (try (list 'reg #f mode hw-name indx-sel))
+	       (try (list 'reg #f nub-mode hw-name indx-sel))
 	       (existing-op (-rtx-find-op try op-list)))
 
-	  ; If already present, return the object, otherwise add it.
-	  (if existing-op
-
-	      (cadr existing-op)
-
-	      (let ((xop (apply reg (cons (tstate->estate tstate)
-					  (cons mode
-						(cons hw-name indx-sel))))))
-		(op:set-cond?! xop (tstate-cond? tstate))
-		; Set the object rtx in `try', now that we have it.
-		(set-car! (cdr try) (rtx-make 'xop xop))
-		; Add the operand to in/out-ops.
-		(append! op-list (list try))
-		(cadr try))))
+	  (let ((result-op
+		 ; If already recorded, use it, otherwise add it.
+		 (if existing-op
+
+		     (cadr existing-op)
+
+		     (let ((xop (apply reg (cons (tstate->estate tstate)
+						 (cons nub-mode
+						       (cons hw-name indx-sel))))))
+		       (op:set-cond?! xop (tstate-cond? tstate))
+		       ; Set the object rtx in `try', now that we have it.
+		       (set-car! (cdr try) (rtx-make 'xop xop))
+		       ; Add the operand to in/out-ops.
+		       (append! op-list (list try))
+		       (cadr try)))))
+
+	    ; We can't return a mode different than requested.  Mixing
+	    ; modes in things like and,add,etc. is illegal.
+	    ; If MODE isn't the nub mode, wrap the operand in a mode change.
+	    ; Not sure how to handle floats so just punt for now and flag an
+	    ; error if the size changes.
+	    (if (mode-compatible? 'samesize mode nub-mode)
+		result-op
+		(if (mode-float? (mode:lookup mode))
+		    (error "floating point operand implicitly referenced in different mode")
+		    (if (mode-bigger? nub-mode mode)
+			(rtx-make 'trunc mode result-op)
+			(error "implicit reference of operand in bigger mode"))))))
 
 	(parse-error "FIXME" "unknown reg" expr)))
 )

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Doug Evans]

Oops.  Since this is basically your patch with some tweaks ...

2003-01-13  Hans-Peter Nilsson  <hans-peter.nilsson@axis.com>
            Doug Evans  <dje@sebabeach.org>

	* semantics.scm (-build-operand!): Record operand in its natural mode.
	(-build-reg-operand!): Ditto.

[...]

Sorry 'bout that.

Re: [RFA:] In -build-operand!, -build-reg-operand, collect the natural mode, not the used mode,

[Hans-Peter Nilsson]

> Date: Mon, 13 Jan 2003 22:11:51 -0800
> From: Doug Evans <dje@transmeta.com>

> Oops.  Since this is basically your patch with some tweaks ...

That's ok, I didn't really recognize it. :-)  Thanks for hanging
on to this.  BTW, I'll follow-up with that revised
RTL-simplification patch wishfully-soon.

brgds, H-P