I actually have to reiterate that the fact that we do the endian
conversion in the get function and not in the actual read instruction
is broken in my opinion.  Stores should be doing a htog (because
register values are stored in the host form so we can do math on them)
and loads should be doing a gtoh (same reason).  We should then fix
the devices to all do letoh on reads and htole on writes (because
almost all PCI devices are little endian, except for those that are
biendian because of a configuration flag.)

We'll never get to supporting heterogeneous ISAs or at least a unified
build if we don't do this.

  Nate

On Sat, Nov 8, 2008 at 8:44 PM, Gabe Black <[EMAIL PROTECTED]> wrote:
> What happens on big endian systems? Do the low order bits get set to 0,
> or do you just shove everything down to the bottom? What happens if the
> value you want doesn't fit in a uint64_t?
>
> Also more generally in the current code, if the underlying data pointer
> in the packet isn't aligned properly to be cast and dereferenced as a
> "T" on systems that care (so not x86), then this code will blow up.
>
> nathan binkert wrote:
>> It would seem to me that get<uint64_t> should automatically do that
>> for you.  I.e., it should grab the size of memory that it's got and
>> then give you the size that you need.
>>
>>   Nate
>>
>> On Sat, Nov 8, 2008 at 6:02 PM, Steve Reinhardt <[EMAIL PROTECTED]> wrote:
>>
>>> Just looking at this code, can we put a more generic version of this
>>> somewhere, like as a Packet method?  I don't know how many other
>>> places already do this, but other than the dataSize value none of that
>>> code looks x86-specific, and I'm guessing there are other places that
>>> do this kind of thing.  It might be usable to clean up some of the
>>> device code.
>>>
>>> Steve
>>>
>>> On Sat, Nov 8, 2008 at 12:08 AM,  <[EMAIL PROTECTED]> wrote:
>>>
>>>> # HG changeset patch
>>>> # User Gabe Black <[EMAIL PROTECTED]>
>>>> # Date 1226042030 28800
>>>> # Node ID 2e61b60e6614e026ba055946a45c5f577d8d8ff8
>>>> # Parent  94ef4905a939b782105ce0ebb3c063451744fb3d
>>>> X86: Fix completeAcc get call.
>>>>
>>>> diff --git a/src/arch/x86/insts/microldstop.hh 
>>>> b/src/arch/x86/insts/microldstop.hh
>>>> --- a/src/arch/x86/insts/microldstop.hh
>>>> +++ b/src/arch/x86/insts/microldstop.hh
>>>> @@ -59,6 +59,7 @@
>>>>  #define __ARCH_X86_INSTS_MICROLDSTOP_HH__
>>>>
>>>>  #include "arch/x86/insts/microop.hh"
>>>> +#include "mem/packet.hh"
>>>>
>>>>  namespace X86ISA
>>>>  {
>>>> @@ -149,6 +150,25 @@
>>>>             }
>>>>             return fault;
>>>>         }
>>>> +
>>>> +        uint64_t
>>>> +        get(PacketPtr pkt) const
>>>> +        {
>>>> +            switch(dataSize)
>>>> +            {
>>>> +              case 1:
>>>> +                return pkt->get<uint8_t>();
>>>> +              case 2:
>>>> +                return pkt->get<uint16_t>();
>>>> +              case 4:
>>>> +                return pkt->get<uint32_t>();
>>>> +              case 8:
>>>> +                return pkt->get<uint64_t>();
>>>> +              default:
>>>> +                panic("Bad operand size %d for read at %#x.\n",
>>>> +                        dataSize, pkt->getAddr());
>>>> +            }
>>>> +        }
>>>>     };
>>>>  }
>>>>
>>>> diff --git a/src/arch/x86/isa/microops/ldstop.isa 
>>>> b/src/arch/x86/isa/microops/ldstop.isa
>>>> --- a/src/arch/x86/isa/microops/ldstop.isa
>>>> +++ b/src/arch/x86/isa/microops/ldstop.isa
>>>> @@ -194,7 +194,7 @@
>>>>         %(op_decl)s;
>>>>         %(op_rd)s;
>>>>
>>>> -        Mem = pkt->get<typeof(Mem)>();
>>>> +        Mem = get(pkt);
>>>>
>>>>         %(code)s;
>>>>
>>>> _______________________________________________
>>>> m5-dev mailing list
>>>> m5-dev@m5sim.org
>>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>>
>>>>
>>> _______________________________________________
>>> m5-dev mailing list
>>> m5-dev@m5sim.org
>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>
>>>
>>>
>> _______________________________________________
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
>
> _______________________________________________
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
>
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to