Re: Unreviewed patches

[Michael Snyder <msnyder@redhat.com>]

Kazu Hirata wrote:
> 
> Hi Michael,
> 
> > > I understand your concern.  But then if GET_W_REG may give a
> > > byte-swapped value, would the following be endian unsafe?
> > >
> > >         case O (O_ADD, SW):
> > >           rd = GET_W_REG (code->dst.reg);
> > >           ea = fetch (&code->src);
> > >           res = rd + ea;
> > >           goto alu16;
> > >
> > > The addition is done in host-order.
> >
> > I don't know, other than to say that it evidently works.  However in
> > this case we are fetching a half-word as a half-word.  In the other
> > case, we are fetching a byte as a half-word, and assuming
> > (incorrectly, I think) that the byte we actually want will be in the
> > lower half of the half-word.
> 
> Well, not quite.  In *both* cases (O_ADD and O_EXTU), we assume that
> 
>   (GET_W_REG(0) & 0xff) == r0l
> 
> Otherwise, the instructions like addition, subtraction, and shift
> wouldn't work.  If you think of extu.w as a special case of and.w,
> where you clear off the upper half by anding with 0x00ff, my patch
> should be correct.
> 
> What about something like the attached patch?  If every wreg[i] is set
> to some useful value, that means that both of
> 
>   if (*q == 0x2233)
> 
> and
> 
>   if (*q == 0x0011)
> 
> triggered for every i.  0x2233 is exactly the lower half of
> 0x00112233.  Likewise, 0x0011 is exactly the upper half of 0x00112233.
> This means that wreg[i] can access either the lower or upper half of a
> 32-bit register without the byte swap.  In turn, this means that
> GET_W_REG is guaranteed to return a non-swapped value.  If wreg[i] is
> NULL, the simulator won't work, so just abort().
> 
> Kazu Hirata

All right.
 
> Index: compile.c
> ===================================================================
> RCS file: /cvs/src/src/sim/h8300/compile.c,v
> retrieving revision 1.21
> diff -u -6 -r1.21 compile.c
> --- compile.c   1 Feb 2003 03:00:14 -0000       1.21
> +++ compile.c   3 Feb 2003 23:17:16 -0000
> @@ -750,24 +750,27 @@
>               if (*p == 0x33)
>                 {
>                   breg[i + 8] = p;
>                 }
>               p++;
>             }
> +         wreg[i] = wreg[i + 8] = 0;
>           while (q < u)
>             {
>               if (*q == 0x2233)
>                 {
>                   wreg[i] = q;
>                 }
>               if (*q == 0x0011)
>                 {
>                   wreg[i + 8] = q;
>                 }
>               q++;
>             }
> +         if (wreg[i] == 0 || wreg[i + 8] == 0)
> +           abort ();
>           cpu.regs[i] = 0;
>           lreg[i] = &cpu.regs[i];
>         }
> 
>        lreg[8] = &cpu.regs[8];
> 
> @@ -1603,23 +1606,23 @@
>             else
>               nz = 0;
>             SET_L_REG (code->dst.reg, (rd & 0xffff) | (tmp << 16));
>             goto next;
>           }
>         case O (O_EXTS, SW):
> -         rd = GET_B_REG (code->src.reg + 8) & 0xff; /* Yes, src, not dst.  */
> +         rd = GET_W_REG (code->src.reg) & 0xff; /* Yes, src, not dst.  */
>           ea = rd & 0x80 ? -256 : 0;
>           res = rd + ea;
>           goto log16;
>         case O (O_EXTS, SL):
>           rd = GET_W_REG (code->src.reg) & 0xffff;
>           ea = rd & 0x8000 ? -65536 : 0;
>           res = rd + ea;
>           goto log32;
>         case O (O_EXTU, SW):
> -         rd = GET_B_REG (code->src.reg + 8) & 0xff;
> +         rd = GET_W_REG (code->src.reg) & 0xff;
>           ea = 0;
>           res = rd + ea;
>           goto log16;
>         case O (O_EXTU, SL):
>           rd = GET_W_REG (code->src.reg) & 0xffff;
>           ea = 0;