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

[Hans-Peter Nilsson <hans-peter.nilsson@axis.com>]

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