search

harvard-acc / gem5-aladdin

End-to-end SoC simulation: integrating the gem5 system simulator with the Aladdin accelerator simulator.
BSD 3-Clause "New" or "Revised" License
212 stars 59 forks source link

Checking ISA Word Length #1

Closed aroelke closed 7 years ago

aroelke commented 7 years ago

On line 622 of syscall_emul.cc, it looks like you're trying to get the ISA's word size? It might be better to just say size_t word_size = sizeof(Addr); instead of explicitly checking for ISA, since the size of an address should be the size of a word (actually you can probably entirely replace word_size with sizeof(Addr)). I think this will make gem5-Aladdin entirely ISA-independent, since I only had to change that to make it compatible with RISC-V.

xyzsam commented 7 years ago

Hi Alec, thanks for the suggestion. Based on src/base/types.hh, line 152, it seems to me Addr is defined as uint64_t, which would be defined based on the platform the simulator is being run on, not the executable the simulator is running.

aroelke commented 7 years ago

You're right, I assumed it would be based on the ISA, but it appears to not be. I don't think data widths are defined by the binary being loaded, though; I think they're defined by the target ISA being compiled. If you try to compile some code for gem5-Aladdin using a 32-bit x86 compiler, I suspect it will fail.

You could use sizeof(TheISA::IntReg), which would track the size of the ISA's word assuming that it is designed to hold one word in an integer register. Alternatively, since all three ISAs that have continued support for gem5 (x86, ARM, and RISC-V) are 64-bit, you could just assume a 64-bit word and then have special cases for 32-bit ISAs rather than the other way around as you have it.

xyzsam commented 7 years ago

Yeah, that's an idea. This part of the code is pretty fragile, now that I think about it, particularly since the struct being deserialized holds a const char, a void, an unsigned, and a size_t. The code right now probably works only because the two pointers and size_t are 32-bit or 64-bit based on the architecture, and so padding gets added after the 32-bit unsigned member in the case of a 64-bit ISA. I should revisit this part of the code.