src/arch/alpha/isa/mem.isa (Diff revision 1)

I have a couple of issues:
1) Where did the byte swapping come from?  I don't see it elsewhere.

2) We've never used typeof before.  Should we start?  In the future (c++0x), we'll have auto, but typeof is nonstandard.

Would it be bad to do the following?

Mem = htog(Mem)
and then just use Mem as the parameter to that function?  Updating in place could of course be sketchy if the value is not ephemeral.

If it is, could we not simply use:
uint%(mem_acc_size)d_t gMem = htog(Mem);

There are other options involving templates too.

1. Gabe Black June 21, 2011, 12:43 p.m. (June 21, 2011, 12:43 p.m.)

   1) The byte swapping used to be done internal to the read/write functions since they knew what the type was and how to swap it. The readBytes and writeBytes functions don't, so that has to move up into the instructions.
   
   2) I have no strong opinion about typeof. The reason I used that instead of modifying Mem in place was that I didn't know for sure whether Mem would be used after that as the code is now or in the future. I tried to update Mem in place in templates where there wouldn't be anything after it. A major motivation for this change is to make my previous change (which would be applied after this one) that simplifies operand types able to deal with signed and unsigned types in a (subjectively) less hacky way. I think those changes get rid of the mem_acc_size field all together since the parser wouldn't really know what the size is any more. I'd have to look at them again to be sure.

2. Nathan Binkert June 22, 2011, 2:51 a.m. (June 22, 2011, 2:51 a.m.)

   1) I saw it (I think in your other diff)
   
   2) What is the type of Mem?  I assume that the parser would have to know what the type of Mem was, no?

3. Steve Reinhardt June 22, 2011, 3:23 a.m. (June 22, 2011, 3:23 a.m.)

   Can we keep the byte swapping in the read/write functions by passing in a flag (like the ByteOrderDiffers flag in sim/byteswap.hh) that says whether it's needed, then adding a switch on the data size to call the right swap_byteNN function?  Normally I might not suggest this, but it solves the trace data problem, and if we're going to push the info in for that then we might as well do all the byte swapping in one place.  Also I expect that most of the time the flag will be a compile-time constant false, so the performance won't matter anyway.

4. Nathan Binkert June 22, 2011, 4:03 a.m. (June 22, 2011, 4:03 a.m.)

   Without a template, you have no type information, so you'd have to add a switch statement to get the correct swap.  I still don't understand why the read/write functions in base_dyn_inst need to be removed.  They handle everything correctly as far as I can tell.  The base_dyn_inst knows its Impl and as a result should know its own endianness.
   
   I also don't quite understand this issue with signed/unsigned types.  Is the type of Mem not necessarily the correct size?  If so, then the byte swapping can't work.  If it is, then I don't understand why we should be getting rid of the template versions of these functions which handle the byte swapping and tracing for us.

Having a new, separate header for each ISA that does nothing but import the helpers from GenericISA into the specific ISA namespace seems overly complicated... do they really need to be in the ISA namespace to be used in the ISA definitions?  Would it be so bad to just have these outside of a namespace in a header somewhere that just gets included directly where needed?

Other than that admittedly nitpicky complaint, this looks great to me.