I think a constant per System (as in the simobject) might work, but x86 doesn't have a fixed pointer size. In long mode it's 64 bits, and in the other modes that would realistically get used it's 32 bits. This is ignoring user mode where they could be either 32 bit or 64 bit in 64 bit depending on if you're in the compatibility or 64 bit submode (basically 32 bit or 64 bit application). This is the only place the *Bytes constants are used, and they're only defined in arm, alpha and mips probably because they were all copy pasted from alpha.
Maybe instead of making the generic version pick a size based on the intended target, we could just leave out the size altogether as a signal the ISA specific function needs to pick? Or make it some sentinel value like 0, or use a different function, or whatever. Maybe the version that has a size would be getSizedArgument. In the long term like I mentioned we'd probably also want to make it a function of the system object which knows at least more about what size a pointer should be. Gabe Ali Saidi wrote: > Well, we need to have some constant for all the different systems which is > the native size of a pointer. That is what I was trying to accomplish with > MachineBytes. > > > Ali > > On Oct 10, 2010, at 10:36 PM, Gabe Black wrote: > > >> I think this change broke X86_FS compilation. I actually have a fix >> built into my pc object change, but I think I originally meant for it to >> be a temporary work around. Basically the problem is that x86 doesn't >> define the MachineBytes constant which isn't really a constant for x86. >> My semi-hack fix is to change MachineBytes to sizeof(IntReg). >> >> Gabe >> >> Ali Saidi wrote: >> >>> changeset f1db1000d957 in /z/repo/m5 >>> details: http://repo.m5sim.org/m5?cmd=changeset;node=f1db1000d957 >>> description: >>> Debug: Implement getArgument() and function skipping for ARM. >>> >>> In the process make add skipFuction() to handle isa specific function >>> skipping >>> instead of ifdefs and other ugliness. For almost all ABIs, 64 bit >>> arguments can >>> only start in even registers. Size is now passed to getArgument() so >>> that 32 >>> bit systems can make decisions about register selection for 64 bit >>> arguments. >>> The number argument is now passed by reference because getArgument() >>> will need >>> to change it based on the size of the argument and the current argument >>> number. >>> >>> For ARM, if the argument number is odd and a 64-bit register is >>> requested the >>> number must first be incremented to because all 64 bit arguments are >>> passed >>> in an even argument register. Then the number will be incremented again >>> to >>> access both halves of the argument. >>> >>> diffstat: >>> >>> src/arch/alpha/utility.cc | 11 +++++++- >>> src/arch/alpha/utility.hh | 3 +- >>> src/arch/arm/system.hh | 5 +++- >>> src/arch/arm/utility.cc | 60 >>> +++++++++++++++++++++++++++++++++++++++------- >>> src/arch/arm/utility.hh | 4 ++- >>> src/arch/mips/utility.cc | 11 +++++++- >>> src/arch/mips/utility.hh | 4 ++- >>> src/arch/power/utility.cc | 7 +++++ >>> src/arch/power/utility.hh | 2 + >>> src/arch/sparc/utility.cc | 12 ++++++++- >>> src/arch/sparc/utility.hh | 4 ++- >>> src/arch/x86/utility.cc | 9 ++++++- >>> src/arch/x86/utility.hh | 4 ++- >>> src/kern/system_events.cc | 14 +++------- >>> src/sim/arguments.cc | 4 +- >>> src/sim/arguments.hh | 10 +++--- >>> 16 files changed, 129 insertions(+), 35 deletions(-) >>> >>> diffs (truncated from 424 to 300 lines): >>> >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/alpha/utility.cc >>> --- a/src/arch/alpha/utility.cc Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/alpha/utility.cc Fri Oct 01 16:02:46 2010 -0500 >>> @@ -40,7 +40,7 @@ >>> namespace AlphaISA { >>> >>> uint64_t >>> -getArgument(ThreadContext *tc, int number, bool fp) >>> +getArgument(ThreadContext *tc, int &number, uint8_t size, bool fp) >>> { >>> #if FULL_SYSTEM >>> const int NumArgumentRegs = 6; >>> @@ -96,5 +96,14 @@ >>> copyIprs(src, dest); >>> } >>> >>> +void >>> +skipFunction(ThreadContext *tc) >>> +{ >>> + Addr newpc = tc->readIntReg(ReturnAddressReg); >>> + tc->setPC(newpc); >>> + tc->setNextPC(tc->readPC() + sizeof(TheISA::MachInst)); >>> +} >>> + >>> + >>> } // namespace AlphaISA >>> >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/alpha/utility.hh >>> --- a/src/arch/alpha/utility.hh Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/alpha/utility.hh Fri Oct 01 16:02:46 2010 -0500 >>> @@ -41,7 +41,7 @@ >>> >>> namespace AlphaISA { >>> >>> -uint64_t getArgument(ThreadContext *tc, int number, bool fp); >>> +uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, bool >>> fp); >>> >>> inline bool >>> inUserMode(ThreadContext *tc) >>> @@ -94,6 +94,7 @@ >>> >>> void copyMiscRegs(ThreadContext *src, ThreadContext *dest); >>> >>> +void skipFunction(ThreadContext *tc); >>> } // namespace AlphaISA >>> >>> #endif // __ARCH_ALPHA_UTILITY_HH__ >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/arm/system.hh >>> --- a/src/arch/arm/system.hh Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/arm/system.hh Fri Oct 01 16:02:46 2010 -0500 >>> @@ -67,7 +67,10 @@ >>> >>> virtual Addr fixFuncEventAddr(Addr addr) >>> { >>> - //XXX This may eventually have to do something useful. >>> + // Remove the low bit that thumb symbols have set >>> + // but that aren't actually odd aligned >>> + if (addr & 0x1) >>> + return (addr & ~1) | PcTBit; >>> return addr; >>> } >>> }; >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/arm/utility.cc >>> --- a/src/arch/arm/utility.cc Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/arm/utility.cc Fri Oct 01 16:02:46 2010 -0500 >>> @@ -42,6 +42,10 @@ >>> #include "arch/arm/utility.hh" >>> #include "cpu/thread_context.hh" >>> >>> +#if FULL_SYSTEM >>> +#include "arch/arm/vtophys.hh" >>> +#include "mem/vport.hh" >>> +#endif >>> >>> namespace ArmISA { >>> >>> @@ -57,17 +61,43 @@ >>> reset->invoke(tc); >>> } >>> >>> -uint64_t getArgument(ThreadContext *tc, int number, bool fp) { >>> +uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, bool >>> fp) { >>> #if FULL_SYSTEM >>> + if (fp) >>> + panic("getArgument(): Floating point arguments not implemented\n"); >>> + >>> if (number < NumArgumentRegs) { >>> - if (fp) >>> - panic("getArgument(): Floating point arguments not >>> implemented\n"); >>> - else >>> - return tc->readIntReg(number); >>> - } >>> - else { >>> - panic("getArgument(): Argument index %d beyond max supported >>> (%d).\n", >>> - number, NumArgumentRegs - 1); >>> + // If the argument is 64 bits, it must be in an even regiser number >>> + // Increment the number here if it isn't even >>> + if (size == sizeof(uint64_t)) { >>> + if ((number % 2) != 0) >>> + number++; >>> + // Read the two halves of the data >>> + // number is inc here to get the second half of the 64 bit reg >>> + uint64_t tmp; >>> + tmp = tc->readIntReg(number++); >>> + tmp |= tc->readIntReg(number) << 32; >>> + return tmp; >>> + } else { >>> + return tc->readIntReg(number); >>> + } >>> + } else { >>> + Addr sp = tc->readIntReg(StackPointerReg); >>> + VirtualPort *vp = tc->getVirtPort(); >>> + uint64_t arg; >>> + if (size == sizeof(uint64_t)) { >>> + // If the argument is even it must be aligned >>> + if ((number % 2) != 0) >>> + number++; >>> + arg = vp->read<uint64_t>(sp + >>> + (number-NumArgumentRegs) * sizeof(uint32_t)); >>> + // since two 32 bit args == 1 64 bit arg, increment number >>> + number++; >>> + } else { >>> + arg = vp->read<uint32_t>(sp + >>> + (number-NumArgumentRegs) * sizeof(uint32_t)); >>> + } >>> + return arg; >>> } >>> #else >>> panic("getArgument() only implemented for FULL_SYSTEM\n"); >>> @@ -90,5 +120,17 @@ >>> >>> } >>> >>> +void >>> +skipFunction(ThreadContext *tc) >>> +{ >>> + Addr newpc = tc->readIntReg(ReturnAddressReg); >>> + newpc &= ~ULL(1); >>> + if (isThumb(tc->readPC())) >>> + tc->setPC(newpc | PcTBit); >>> + else >>> + tc->setPC(newpc); >>> + tc->setNextPC(tc->readPC() + sizeof(TheISA::MachInst)); >>> +} >>> + >>> >>> } >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/arm/utility.hh >>> --- a/src/arch/arm/utility.hh Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/arm/utility.hh Fri Oct 01 16:02:46 2010 -0500 >>> @@ -156,11 +156,13 @@ >>> return !cpacr.asedis && vfpEnabled(cpacr, cpsr, fpexc); >>> } >>> >>> -uint64_t getArgument(ThreadContext *tc, int number, bool fp); >>> +uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, bool >>> fp); >>> >>> Fault setCp15Register(uint32_t &Rd, int CRn, int opc1, int CRm, int opc2); >>> Fault readCp15Register(uint32_t &Rd, int CRn, int opc1, int CRm, int opc2); >>> >>> +void skipFunction(ThreadContext *tc); >>> + >>> }; >>> >>> >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/mips/utility.cc >>> --- a/src/arch/mips/utility.cc Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/mips/utility.cc Fri Oct 01 16:02:46 2010 -0500 >>> @@ -52,7 +52,7 @@ >>> namespace MipsISA { >>> >>> uint64_t >>> -getArgument(ThreadContext *tc, int number, bool fp) >>> +getArgument(ThreadContext *tc, int &number, uint8_t size, bool fp) >>> { >>> #if FULL_SYSTEM >>> if (number < 4) { >>> @@ -264,5 +264,14 @@ >>> { >>> panic("Copy Misc. Regs Not Implemented Yet\n"); >>> } >>> +void >>> +skipFunction(ThreadContext *tc) >>> +{ >>> + Addr newpc = tc->readIntReg(ReturnAddressReg); >>> + tc->setPC(newpc); >>> + tc->setNextPC(tc->readPC() + sizeof(TheISA::MachInst)); >>> + tc->setNextPC(tc->readNextPC() + sizeof(TheISA::MachInst)); >>> +} >>> + >>> >>> } // namespace MipsISA >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/mips/utility.hh >>> --- a/src/arch/mips/utility.hh Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/mips/utility.hh Fri Oct 01 16:02:46 2010 -0500 >>> @@ -45,7 +45,7 @@ >>> >>> namespace MipsISA { >>> >>> -uint64_t getArgument(ThreadContext *tc, int number, bool fp); >>> +uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, bool >>> fp); >>> >>> //////////////////////////////////////////////////////////////////////// >>> // >>> @@ -103,6 +103,8 @@ >>> void copyRegs(ThreadContext *src, ThreadContext *dest); >>> void copyMiscRegs(ThreadContext *src, ThreadContext *dest); >>> >>> +void skipFunction(ThreadContext *tc); >>> + >>> }; >>> >>> >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/power/utility.cc >>> --- a/src/arch/power/utility.cc Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/power/utility.cc Fri Oct 01 16:02:46 2010 -0500 >>> @@ -55,4 +55,11 @@ >>> dest->setNextPC(src->readNextPC()); >>> } >>> >>> +void >>> +skipFunction(ThreadContext *tc) >>> +{ >>> + panic("Not Implemented for POWER"); >>> +} >>> + >>> + >>> } // PowerISA namespace >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/power/utility.hh >>> --- a/src/arch/power/utility.hh Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/power/utility.hh Fri Oct 01 16:02:46 2010 -0500 >>> @@ -61,6 +61,8 @@ >>> { >>> } >>> >>> +void skipFunction(ThreadContext *tc); >>> + >>> } // PowerISA namespace >>> >>> #endif // __ARCH_POWER_UTILITY_HH__ >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/sparc/utility.cc >>> --- a/src/arch/sparc/utility.cc Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/sparc/utility.cc Fri Oct 01 16:02:46 2010 -0500 >>> @@ -45,7 +45,7 @@ >>> //the sixth are passed on the stack past the 16 word window save area, >>> //space for the struct/union return pointer, and space reserved for the >>> //first 6 arguments which the caller may use but doesn't have to. >>> -uint64_t getArgument(ThreadContext *tc, int number, bool fp) { >>> +uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, bool >>> fp) { >>> #if FULL_SYSTEM >>> const int NumArgumentRegs = 6; >>> if (number < NumArgumentRegs) { >>> @@ -219,6 +219,16 @@ >>> } >>> >>> void >>> +skipFunction(ThreadContext *tc) >>> +{ >>> + Addr newpc = tc->readIntReg(ReturnAddressReg); >>> + tc->setPC(newpc); >>> + tc->setNextPC(tc->readPC() + sizeof(TheISA::MachInst)); >>> + tc->setNextPC(tc->readNextPC() + sizeof(TheISA::MachInst)); >>> +} >>> + >>> + >>> +void >>> initCPU(ThreadContext *tc, int cpuId) >>> { >>> static Fault por = new PowerOnReset(); >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/sparc/utility.hh >>> --- a/src/arch/sparc/utility.hh Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/sparc/utility.hh Fri Oct 01 16:02:46 2010 -0500 >>> @@ -41,7 +41,7 @@ >>> >>> namespace SparcISA >>> { >>> - uint64_t getArgument(ThreadContext *tc, int number, bool fp); >>> + uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, >>> bool fp); >>> >>> static inline bool >>> inUserMode(ThreadContext *tc) >>> @@ -75,6 +75,8 @@ >>> >>> void copyMiscRegs(ThreadContext *src, ThreadContext *dest); >>> >>> + void skipFunction(ThreadContext *tc); >>> + >>> } // namespace SparcISA >>> >>> #endif >>> diff -r 8173327c9c65 -r f1db1000d957 src/arch/x86/utility.cc >>> --- a/src/arch/x86/utility.cc Fri Oct 01 16:02:45 2010 -0500 >>> +++ b/src/arch/x86/utility.cc Fri Oct 01 16:02:46 2010 -0500 >>> @@ -52,7 +52,7 @@ >>> >>> namespace X86ISA { >>> >>> -uint64_t getArgument(ThreadContext *tc, int number, bool fp) { >>> +uint64_t getArgument(ThreadContext *tc, int &number, uint8_t size, bool >>> fp) { >>> #if FULL_SYSTEM >>> _______________________________________________ >>> m5-dev mailing list >>> [email protected] >>> http://m5sim.org/mailman/listinfo/m5-dev >>> >>> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> >> > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
