[Simh] O2 optimization on Linux

[Peter Allan]

Jan,

The solution I have adopted is to define ML_PXBR_TEST as

#define ML_PXBR_TEST(r) { int32 r2 = (r);                                 \
                        if (((r2 & 0x80000000) == 0) ||
     \
                             (r2 & 0x00000003)) RSVD_OPND_FAULT; \
                        }

This works on CentOS 5.5.

There are 3 differences between my version and yours:

1) I left in an extra set of bracked around r2 & ......., just because of my
paranoia
2) I believe your version has a typo of one bracket missing just after the
"if"
3) I have defined the internal block variable (r2, in my case) to be int32.
I have done this because the variable t in op_ldpctx and val in op_mtpr are
defined as int32. I am not claiming that I am right and you are wrong, just
that that is my reason. uint32 does feel like it should be correct, but then
surely t and val should be defined as uint32 as well.

I think this level of complication in the macro is as far as it is sensible
to go because the original, simpler version should work. It is only (I
believe) a bug in the compiler that we are trying to work around and you can
never second guess what any future bug might throw up.

Anyway, I now have a working VAX-11/780 emulator that I am happy with.

Thanks for everyone's help with this.

Peter Allan


On 6 January 2011 12:22, Jan-Benedict Glaw <jbglaw at lug-owl.de> wrote:

> On Thu, 2011-01-06 02:48:30 -0700, Tim Riker <Tim at Rikers.org> wrote:
> > On 01/03/2011 12:57 PM, Jason Stevens wrote:
> > > wait, it should have been more like this
> > >
> > >
> > > > #define ML_SBR_TEST(r) if (((uint32)(r)) & 0xC000003u) != 0)
> RSVD_OPND_FAULT
> > > > #define ML_PXBR_TEST(r) if (((((uint32)(r)) & 0x80000000u) == 0) || \
> > > >                            (((uint32)(r)) & 0x40000003u) != 0))
> > > > RSVD_OPND_FAULT
> > >
> > > into this:
> > >
> > > #define ML_SBR_TEST(r)  if (((uint32)(r) & 0xC0000003u)!=0)
> RSVD_OPND_FAULT
> > >
> > > #define ML_PXBR_TEST(r) if (((((uint32)r) & 0x80000000u) == 0) || \
> > >                             (((uint32)r) & 0x40000003u)!=0)
> RSVD_OPND_FAULT
> > You want the typecast to apply after any math that might be happening in
> > the expression r.
> >
> > You should include r in parentheses in the expansion. ie: not (uint32)r
> > ever, but instead (uint32)(r) so that r is evaluated first before
> > typecast is applied.
>
> In terms of correctness, there are several things that could be made
> better:
>
> First of all, `r' is used twice here. The current uses seem sane, but
> what, if `r' contained side effects (pre/post inc/dec etc.)? That
> could probably be written as:
>
> #define ML_PXBR_TEST(r) {                                               \
>                                uint32 ____t = (r);                     \
>                                if ((____t & 0x80000000u) == 0) ||      \
>                                    ____t & 0x40000003u)                \
>                                        RSVD_OPND_FAULT;                \
>                        }
>
> That would ensure `r' being evaluated only once. And it would
> introduce a block around the if () ...;  construct. Think of:
>
>        if (foo)
>                ML_PXBR_TEST (x);
>        else
>                do_something ();
>
> Looks sane, but would work totally irrational with the original macro.
> To accept a `;' after the closing brace of the block, maybe a do/while
> should be added (as eg. the Linux kernel code does it):
>
> #define ML_PXBR_TEST(r) do {                                            \
>                                uint32 ____t = (r);                     \
>                                if ((____t & 0x80000000u) == 0) ||      \
>                                    ____t & 0x40000003u)                \
>                                        RSVD_OPND_FAULT;                \
>                        } while (0)
>
> Comments?
>
> MfG, JBG
>
> --
>      Jan-Benedict Glaw      jbglaw at lug-owl.de              +49-172-7608481
>  Signature of:                          Zensur im Internet? Nein danke!
>  the second  :
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAk0ltA4ACgkQHb1edYOZ4bsjSACgkLWEJqvZh1JPurUcvL9WgJnp
> wGMAniIIQYEMEQCgIkvh3jk1OGgijDj5
> =DG6c
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Simh mailing list
> Simh at trailing-edge.com
> http://mailman.trailing-edge.com/mailman/listinfo/simh
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.trailing-edge.com/pipermail/simh/attachments/20110116/8b16d729/attachment-0002.html>