Flyspray:: M5 Bugs: Recently opened tasks http://flyspray.rocks.cc/ While using the checkpoint tester script, I noticed that X86_FS encounters differences in the checkpoint state. This problem exists for both atomic and timing mode, as well as classic and Ruby memory systems.

A short test with the checkpoint tester script, will identify the problem:

% util/checkpoint-tester.py -i 2000 -- build/ALPHA_FS_MOESI_hammer/m5.debug configs/example/fs.py --script test/halt.sh

Identified differences in the checkpoint:

--- checkpoint-test/m5out/cpt.10000/m5.cpt Wed Jan 12 14:59:28 2011
+++ checkpoint-test/test.4/cpt.10000/m5.cpt Wed Jan 12 15:00:42 2011
@@ -10,20 +10,20 @@
so_state=2
locked=false
_status=1
-instCnt=10
+instCnt=9

[system.cpu.xc.0]
_status=0
-funcExeInst=16
+funcExeInst=15
quiesceEndTick=0
iplLast=0
iplLastTick=0
floatRegs.i=0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0
-intRegs=549755813888 0 2097152 0 0 0 590336 0 0 0 0 0 0 0 0 0 0 2097208 380 0 0 0 0 2097189 0 0 0 0
0 0 0 0 133 0 0 0 0 0 0
+intRegs=549755813888 0 2097152 0 0 0 590336 0 0 0 0 0 0 0 0 0
+18446743523955834880 2097182 380 0 0
0 0 2097189 0 0 0 0 0 0 0 0 133 0 0 0 0 0 0
_pc=2097202
-_npc=2097208
-_upc=1
-_nupc=2
+_npc=2097210
+_upc=0
+_nupc=1
regVal=3758096401 0 0 458752 32 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 4294905840 1024 2 243392 0 1288 0
0 0 260 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1974748653749254 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1280 0 0 0 0 0 0 0 0 0 0 0 0 0 0 132609
0 0 0 0 67108864 0 0 0 0 0 16 8 16 16 16 16 0 0 0 0 0 24 0 0 0 0 0 0 0 0 0 483328 0 0 0 0 0 0 0 0 0
0 0 0 483328 0 0 0 0 983295 983295 983295 983295 983295 983295 65535 65535 23 65535 65535 983295 655
35 45768 43728 45768 45768 45768 45768 45952 0 45952 45952 45952 43976 45952 0 0 0 0 0 0 0 0 0 0 0 4
276095232 0

[system.cpu.tickEvent]

http://flyspray.gem5.org/task/337 The remote gdb code could use some better structuring and perhaps templating on the ISA. http://flyspray.gem5.org/task/334 A non-trivial amount of code is exactly the same between multiple ISAs or is the same but with minor modifications. A directory in arch called "generic" should be created for these common implementations which will be put in an namespace called GenericISA. The individual ISAs can wrap these versions of functions or pieces of functions, or import them directly into their own namespace with "using".




The directory has been created, and now more common code needs to be moved into it. http://flyspray.gem5.org/task/333 Running the following command:
util/checkpoint-tester.py -i 200000000000 -- build/ALPHA_FS/m5.opt configs/example/fs.py --script tests/halt.sh
reveals a few discrepancies.

First, the files that restore from a checkpoint always seem to have 24 more instructions in instCnt and funcExeInst than the original run.

Second, sometimes the first field of rtc.clock_data (the seconds field?) is a 1 in the checkpoints regenerated from earlier checkpoints, even though it's always a 0 in the original checkpoints.
http://flyspray.gem5.org/task/332 The 'output' and 'errout' flags of the Process object (which can be set using the options of the same name on se.py) allow users to redirect the output of the simulated program independently from simulator output. However, the files created using these flags show up in the current working directory, even if the user has specified an output directory using the global '-d' option. This behavior is wrong; all output should go to the directory specified by '-d' unless absolute pathnames are given.

The fundamental problem is that the 'open()' call in Process::openOutputFile() doesn't use the 'simout' directory in base/output.{cc,hh}. It's not a totally trivial fix since the existing code for opening output files deals in ostreams, and Process wants a bare file descriptor.
http://flyspray.gem5.org/task/331 There's some code I added to tests/SConscript that intentionally avoids creating the 'status' output file if m5 terminates with one of a certain set of signals such as SIGINT. (Grep for retry_signals in that file to see where.) Without this code, premature termination of m5 counts as a test failure (as is often appropriate, e.g., if you hit an assertion failure or SIGSEGV). The intended effect here is that if you ^C a regression test for some reason then restart it, it will re-run the test automatically, rather than reporting a failed test and requiring you to go delete the output files to force scons to re-run it.

This used to work (I know it did when I added this code in March 2009, see http://repo.m5sim.org/m5/rev/e0344c15e73b). It doesn't anymore; I just ^C'd a regression and got the old behavior. I suspect the interrupt return code is getting lost in the transition from C++ to Python, but a quick skim doesn't turn up any changes since then that would look suspicious, and I don't have time to dig into this right now. Anything come to mind, Nate?

http://flyspray.gem5.org/task/330 Steve Reinhardt:
I do see lots of repeated definitions of rn, rd, and rm. Also for other fields like lsb, msb, rotation, that are defined in multiple places (e.g., 2 out of 3 branches of an if/else if tree) I'd prefer to see them defined at the top in a common place; I don't think that avoiding extracting them in the one case where they're not needed is a real performance issue. If our decode cache isn't broken then actual from-scratch decode should not be a performance issue at all.

I had a couple of other ideas:
1. Maybe we should define bits() as a method on ExtMachInst, so we can say machInst.bits(X,Y). That's not a huge win by itself, but if we couple it with getting rid of "def bitfield" and have bitfields in the ISA language be attributes on the ExtMachInst, then you could even say "decode bits(5,3)" right in the ISA language.
2. We could extend bits() to take multiple pairs of bit indices, concatenating the indicated ranges, so expressions like:
timm = (bits(machInst, 19, 16) << 12) | bits(machInst, 11, 0);
could be rewritten as:
timm = machInst.bits(19, 16, 11, 0);

I think #2 would even be useful in converting some of your if/else if structures back to switches, which I find preferable because they make it clear(er) that you've covered all the possible cases.


Gabe Black wrote:
I've had thoughts similar to these. For instance, it would be nice to add a function to either the ExtMachInst (or the StaticInst) that would return an IntRegIndex from a set of bits. That would get rid of some of the stacks of casts that tend to happen around register indexes. It wouldn't be that bad (knock on wood) to add new types of bitfields to BitUnions that would do arbitrarily complex things when reading or writing bits. The problem is that they'd need to be set up first, and if they only show up once or twice the overhead makes them a lot more verbose than just calling bits directly.

I'd be fine with moving the common definitions to the tops of the blocks they're used in. The thing to watch out for is bitfields that are -almost- the same but that are changed around sometimes. There are instances of that and they have caused some bugs.

While these sorts of extensions are a good idea and would clean things up even outside of ARM in some places, they won't remove all instances of having to do things the hard way because there are plenty of places where the bits of interest shift for what seems like (or literaly is) every different instruction in a group, or where things are combined like bit a == bit b, or a == 1 and b != 010. It's sort of a mess.

Steve Reinhardt:
I'm still not very happy with all the redundant bitfield stuff, even
if it is caused by IntRegIndex casting issues. (It looks like the big
problem is that IntRegIndex is an enum rather than an integer type...
is there a big benefit from that? Clearly the cost is pretty high, in
my opinion. Why not replace it with a typedef to an integer type and
a bunch of constants, maybe in their own Arm::IntReg namespace if that
helps?)

I'd also like to see the extended bits() notation (or something
similar) that concatenates bitfields, e.g., replacing expressions
like:
(bits(data, 15) << 2) | bits(data,11,10)
with:
bits(data, 15, 15, 11, 10)
(or something similar... I'm open to ideas for better syntax). This
makes it more obvious that the bitfields are being concatenated and
avoids the magic '2' that really represents the width of bits(_, 11,
10) without being obvious about that. There are even cases where
another shift gets folded in that are more confusing; for example, I
think this:
(bits(machInst, 9) << 6) | (bits(machInst, 7, 3) << 1)
is much less clear than:
bits(machInst, 9, 9, 7, 3) << 1

http://flyspray.gem5.org/task/328 SPARC's FSR controls floating point operations and also tracks floating point condition codes. The register should probably be split into a control register and an integer register for the condition codes. That way the condition codes would be renameable like the integer ones, and instructions that set them won't be serializing. http://flyspray.gem5.org/task/327 The 20.parser benchmark tends to fail because glibc reads /proc/meminfo to determine how much memory is available in the system. qsort() uses that information to choose an algorithm to use thus depending on the host system, the guest can make a different choice. Ultimately, we need to provide a fake /proc/meminfo on open() syscalls that either has some generic info or better yet has the correct information for the simulated system. http://flyspray.gem5.org/task/326 We never had any statistics for the bus in the new memory system. It would be nice to know how busy each one is and the amount of data, number of requests, etc that is transmitted across each one.

Ali

http://flyspray.gem5.org/task/324