[gem5-dev] changeset in gem5: LibElf: Build the error management code in li...
changeset 931ef19535e0 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=931ef19535e0
description:
LibElf: Build the error management code in libelf.
This change makes some minor changes to get the error management code in
libelf to build on Linux and to build it into the library.
diffstat:
ext/libelf/SConscript | 2 ++
ext/libelf/_libelf.h| 1 +
ext/libelf/elf_errmsg.c | 6 +++---
3 files changed, 6 insertions(+), 3 deletions(-)
diffs (40 lines):
diff -r 7a32aa3acd72 -r 931ef19535e0 ext/libelf/SConscript
--- a/ext/libelf/SConscript Sun Jun 12 21:35:03 2011 -0400
+++ b/ext/libelf/SConscript Sun Jun 12 23:51:59 2011 -0700
@@ -40,6 +40,8 @@
ElfFile('elf_cntl.c')
ElfFile('elf_data.c')
ElfFile('elf_end.c')
+ElfFile('elf_errmsg.c')
+ElfFile('elf_errno.c')
ElfFile('elf_fill.c')
ElfFile('elf_flag.c')
ElfFile('elf_getarhdr.c')
diff -r 7a32aa3acd72 -r 931ef19535e0 ext/libelf/_libelf.h
--- a/ext/libelf/_libelf.h Sun Jun 12 21:35:03 2011 -0400
+++ b/ext/libelf/_libelf.h Sun Jun 12 23:51:59 2011 -0700
@@ -30,6 +30,7 @@
#define__LIBELF_H_
#include elf_queue.h
+#include libelf.h
#ifndefNULL
#define NULL ((void *) 0)
diff -r 7a32aa3acd72 -r 931ef19535e0 ext/libelf/elf_errmsg.c
--- a/ext/libelf/elf_errmsg.c Sun Jun 12 21:35:03 2011 -0400
+++ b/ext/libelf/elf_errmsg.c Sun Jun 12 23:51:59 2011 -0700
@@ -71,10 +71,10 @@
if (error 0 || error = ELF_E_NUM)
return _libelf_errors[ELF_E_NUM];
if (oserr) {
-strlcpy(LIBELF_PRIVATE(msg), _libelf_errors[error],
+strncpy(LIBELF_PRIVATE(msg), _libelf_errors[error],
sizeof(LIBELF_PRIVATE(msg)));
-strlcat(LIBELF_PRIVATE(msg), : ,
sizeof(LIBELF_PRIVATE(msg)));
-strlcat(LIBELF_PRIVATE(msg), strerror(oserr),
+strncat(LIBELF_PRIVATE(msg), : ,
sizeof(LIBELF_PRIVATE(msg)));
+strncat(LIBELF_PRIVATE(msg), strerror(oserr),
sizeof(LIBELF_PRIVATE(msg)));
return (const char *)LIBELF_PRIVATE(msg);
}
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: Loader: Handle bad section names when loading...
changeset 9fb150de362e in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=9fb150de362e
description:
Loader: Handle bad section names when loading an ELF file.
If there's a problem when reading the section names from a supposed ELF
file,
this change makes gem5 print an error message as returned by libelf and
die.
Previously these sorts of errors would make gem5 segfault when it tried
to
access the section name through a NULL pointer.
diffstat:
src/base/loader/elf_object.cc | 20 ++--
1 files changed, 14 insertions(+), 6 deletions(-)
diffs (30 lines):
diff -r 931ef19535e0 -r 9fb150de362e src/base/loader/elf_object.cc
--- a/src/base/loader/elf_object.cc Sun Jun 12 23:51:59 2011 -0700
+++ b/src/base/loader/elf_object.cc Sun Jun 12 23:52:21 2011 -0700
@@ -266,12 +266,20 @@
gelf_getshdr(section, shdr);
char * secName = elf_strptr(elf, ehdr.e_shstrndx, shdr.sh_name);
-if (!strcmp(.text, secName)) {
-textSecStart = shdr.sh_addr;
-} else if (!strcmp(.data, secName)) {
-dataSecStart = shdr.sh_addr;
-} else if (!strcmp(.bss, secName)) {
-bssSecStart = shdr.sh_addr;
+if (secName) {
+if (!strcmp(.text, secName)) {
+textSecStart = shdr.sh_addr;
+} else if (!strcmp(.data, secName)) {
+dataSecStart = shdr.sh_addr;
+} else if (!strcmp(.bss, secName)) {
+bssSecStart = shdr.sh_addr;
+}
+} else {
+Elf_Error errorNum = (Elf_Error)elf_errno();
+if (errorNum != ELF_E_NONE) {
+const char *errorMessage = elf_errmsg(errorNum);
+fatal(Error from libelf: %s.\n, errorMessage);
+}
}
section = elf_getscn(elf, ++secIdx);
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
On 06/12/11 16:29, Steve Reinhardt wrote: On Sun, Jun 12, 2011 at 1:05 AM, Gabe Black gbl...@eecs.umich.edu wrote: I was thinking about this today, and if we expand the read/write functions to handle signed types too, we're really just expanding the arbitrary set of types they can handle, not removing the limitation that you have to stay within those types which is what I think you don't like. I think originally we supported memory accesses for any operand type you could define, but that stopped being true once you made the definitions extensible. My immediate concern is just to make sure that switching from the old explicit sign extensions to some implicit sign extensions that happened as a side effect of C type conversions is really doing the right thing, but having a cleaner way of doing memory accesses of arbitrary types is a good idea. Instead of extending what we already have as far as explicit instantiation, it would be nice to have a more automatic mechanism where we'd just feed a list of types and a template (you can pass templates as template arguments, sort of like function pointers but for templates) and have some widget that cranks out the actual instantiation without so much copy and paste coding. That sounds interesting, but seems like overkill... I just looked at the SimpleCPU code, and as far as I can tell, the memory access type (the arg type for read() and write()) is only used for two things: to determine the size of the access, and to control the data type in the InstRecord type for exec tracing (basically this is mostly setting the data_status enum, but also using the proper double vs int field in the data union). The actual type clearly doesn't matter at all for the first, and only a subset of types are supported for the second. There are actually three things, the third is to handle endianness. This is important if we delegate endian swapping to the read and write functions which we currently do, and helps when printing out what loads and stores returned at the CPU level. You get the same information, it's just easier to look at since it's a number and not a string of bytes. Moving that into the ISA desc would be feasible, and you could make a convincing argue that's where it should be in the first place. If ARM is configured to do loads/stores in the other endianness (which we support) then values could be swapped on the way in and then re-swapped before being used. The idea of non-byte sized values in memory and the endianness of them of is an ISA level concept. Other non-byte sized values like PCI registers are another story, of course, but that's a whole other issue. The original idea with the templates was that they might permit faster implementations for functional CPU models that communicated directly with memory. However, if anything we've gone in the other direction by implementing these templates in terms of readBytes() and writeBytes(). So my general feeling is that if we want to make significant changes to this interface, I'd be more inclined to streamline it and have the generated ISA code call readBytes() and writeBytes() directly with a size and some additional info to make exec tracing work (which should get rid of the templates entirely, I think) rather than expanding the template interface. Then the burden of converting from an untyped sequence of bytes to whatever the ISA wants could be done entirely in the ISA definition, which seems like a good place for it. Does that make sense? Do you think it's feasible or worthwhile? I think that makes sense, although the devil is in the details as always. For memory in traces, it would be nice to have it endian corrected, but it wouldn't necessarily have to be. We could just print out each byte one at a time and have the understanding that it's a blob of memory, low addresses on the left. Or at least I think we could do that... Also, while looking for information about Boost (in progress right now) I found their page where they talk about their license (link below). Looking through it, there are some ideas there which seem relevant to gem5. Specifically, I like the idea of a single license for everything, perhaps involving assigning copyright to a neutral body like a gem5 foundation or something, and then just referring to it in the actual source files. That would get rid of lots of redundant text, and they make a good point that all that text is the sort of thing lawyers might get their underwear in a bunch over. There may be (but isn't necessarily) subtle variation on a file by file basis, and it's probably a lot more work to go through if somebody ever needed to do that. We discussed this a long long time ago (when we first started distributing the code, IIRC), and while it does have the advantages you describe, the cost of further wrangling with lawyers is basically not worth it IMO. Maybe if we started a new project from scratch we would consider
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
On 06/04/11 09:32, Steve Reinhardt wrote: On Sat, Jun 4, 2011 at 1:57 AM, Gabe Black gbl...@eecs.umich.edu wrote: To clarify, is this signed/unsigned issue something we need to deal with before this patch goes in, or can it be dealt with separately later? I'd like to see it handled before the patch is committed, mostly because I'm still not 100% convinced that these changes don't break something. Also when something gets deferred like this there's always the uncertainty of when/if it's going to get taken care of... nothing personal, I'd feel the same way if it was my own patch. Steve ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev I was thinking about this today, and if we expand the read/write functions to handle signed types too, we're really just expanding the arbitrary set of types they can handle, not removing the limitation that you have to stay within those types which is what I think you don't like. Instead of extending what we already have as far as explicit instantiation, it would be nice to have a more automatic mechanism where we'd just feed a list of types and a template (you can pass templates as template arguments, sort of like function pointers but for templates) and have some widget that cranks out the actual instantiation without so much copy and paste coding. Some Googling indicates that Boost already does this with some macros it has for working with sequences. I'm looking into that more, and I'm thinking it would be best to figure out how they did it and then make our own, rather than introduce a great big new dependency. Combining that with the first point, I think a nice solution would be to have the ISA parser spit out a list of types that read/write need to support in whatever form is necessary (preliminary reading suggests a #define) and then have the CPU models all feed that into the instantiator widget to get the versions they need. Then we'd get exactly the functions we need and cut out a bunch of repetition in the source. This would be at the reasonable cost (in my opinion) of some additional complexity, although in some ways it would trade one type of complexity for another. Also, while looking for information about Boost (in progress right now) I found their page where they talk about their license (link below). Looking through it, there are some ideas there which seem relevant to gem5. Specifically, I like the idea of a single license for everything, perhaps involving assigning copyright to a neutral body like a gem5 foundation or something, and then just referring to it in the actual source files. That would get rid of lots of redundant text, and they make a good point that all that text is the sort of thing lawyers might get their underwear in a bunch over. There may be (but isn't necessarily) subtle variation on a file by file basis, and it's probably a lot more work to go through if somebody ever needed to do that. http://www.boost.org/users/license.html Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: LibElf: Build the error management code in libelf.
Ping On 06/04/11 11:39, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/735/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- LibElf: Build the error management code in libelf. This change makes some minor changes to get the error management code in libelf to build on Linux and to built it into the library. Diffs - ext/libelf/SConscript b9ba22cb23f2 ext/libelf/_libelf.h b9ba22cb23f2 ext/libelf/elf_errmsg.c b9ba22cb23f2 Diff: http://reviews.m5sim.org/r/735/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: Loader: Handle bad section names when loading an ELF file.
Ping On 06/04/11 11:42, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/736/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Loader: Handle bad section names when loading an ELF file. If there's a problem when reading the section names from a supposed ELF file, this change makes gem5 print an error message as returned by libelf and die. Previously these sorts of errors would make gem5 segfault when it tried to access the section name through a NULL pointer. Diffs - src/base/loader/elf_object.cc b9ba22cb23f2 Diff: http://reviews.m5sim.org/r/736/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Cron m5test@zizzer /z/m5/regression/do-regression quick
Merging the regression output (mainly stats) is probably not going to do what you want, so if those files had to be merged then that's probably part of the problem. Gabe On 06/10/11 01:16, Korey Sewell wrote: I was late in updating the repository. I think this may have happened since I was running stuff on zizzer while the regressions were loading up. What's the method of choice for rerunning the do-regression script? Also, when updating the simple cpu regressions, I had to hg merge the changesets, so it seems I had a merge that didnt propagate through. I'm finally able to regenerate the o3-timing error seen in the regressions. I wont be able to fix those 2 regressions just this second, but the simple-cpu ones should be updated and when I get into the lab today, I'll look again into the O3 ones and see what happened. On Fri, Jun 10, 2011 at 3:02 AM, Cron Daemon r...@zizzer.eecs.umich.eduwrote: scons: *** Cannot duplicate `src/SConscript' in `build/SPARC_SE': None. Stop. See /z/m5/regression/regress-2011-06-10-03:00:01 for details. ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: cpus/isa: add a != operator for pcstate
On 2011-06-08 22:52:15, Gabe Black wrote: I think you missed some (maybe just one) version of PC state defined in the ISAs themselves. ARM may be the only one, but you should double check to be sure. Also, for all these you could define them using ==, something like return !(*this == opc); Korey Sewell wrote: The !(*this == opc) is interesting. If you do it that way, it's definitely more programmable for the long term since one change to the equals operator definition will propagate. But, is that way adding two extra operations there? 1 to dereference the this pointer and then another to do the NOT operation? The == operator is the more used operator but if for some reason the != operator become popular within gem5 wouldnt it be slightly slower? Steve Reinhardt wrote: I think defining != in terms of == is much preferred for the reasons you said. The compiler should inline the == definition into != where appropriate, so the performance difference for optimized code should be somewhere between non-existent and negligible, and certainly worthwhile given how much simpler and more robust the code will be. You should just be able to define it once on the base class too which will simplify things even more. Be really careful just defining it in the base class. It's not virtual as far as I remember, and generally speaking the subclasses add extra things to check. You could end up using the base class version and not checking everything, but it would frequently be right and could slip by pretty easily. I'm not saying it won't work, just be very careful and verify it's doing exactly what you think it is in all the ISAs one by one. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/738/#review1303 --- On 2011-06-08 22:46:05, Korey Sewell wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/738/ --- (Updated 2011-06-08 22:46:05) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- cpus/isa: add a != operator for pcstate Diffs - src/arch/generic/types.hh 77d12d8f7971 Diff: http://reviews.m5sim.org/r/738/diff Testing --- Thanks, Korey ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: alpha: naming for dtb faults
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/741/#review1310 --- This seems reasonable to me, but we should consider if the old name was the actual name given by the architecture. I'm not that familiar with Alpha so I have no idea. If it was, then you could do something like dfault (paging) and dfault (perm) or similar and preserve the old name for the sake of looking things up in a manual. - Gabe On 2011-06-08 23:25:03, Korey Sewell wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/741/ --- (Updated 2011-06-08 23:25:03) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- alpha: naming for dtb faults Just dfault gets confusing while debugging. Why not differentiate whether it's an access violation or page fault Diffs - src/arch/alpha/faults.cc 77d12d8f7971 Diff: http://reviews.m5sim.org/r/741/diff Testing --- Thanks, Korey ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: inorder/dtb: make sure DTB translate correct address
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/743/#review1311 --- This isn't a review, just a thought on the question you're asking. If the access is speculative, is it ok to use a misspeculated pc since the instruction will be thrown out anyway? - Gabe On 2011-06-08 23:34:50, Korey Sewell wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/743/ --- (Updated 2011-06-08 23:34:50) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- inorder/dtb: make sure DTB translate correct address The DTB expects the correct PC in the ThreadContext but how if the memory accesses are speculative? Shouldn't we send along the requestor's PC to the translate functions? Diffs - src/cpu/inorder/resources/cache_unit.cc 77d12d8f7971 Diff: http://reviews.m5sim.org/r/743/diff Testing --- Thanks, Korey ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: Mem: Use sysconf to get the page size instead...
changeset e39a9c0493ad in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=e39a9c0493ad description: Mem: Use sysconf to get the page size instead of the PAGE_SIZE macro. diffstat: src/mem/physical.cc | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diffs (12 lines): diff -r 1810956fa5dc -r e39a9c0493ad src/mem/physical.cc --- a/src/mem/physical.cc Tue Jun 07 00:46:54 2011 -0700 +++ b/src/mem/physical.cc Wed Jun 08 00:57:50 2011 -0700 @@ -90,7 +90,7 @@ int fd = open(params()-file.c_str(), O_RDONLY); _size = lseek(fd, 0, SEEK_END); lseek(fd, 0, SEEK_SET); -pmemAddr = (uint8_t *)mmap(NULL, roundUp(size(), PAGE_SIZE), +pmemAddr = (uint8_t *)mmap(NULL, roundUp(size(), sysconf(_SC_PAGESIZE)), PROT_READ | PROT_WRITE, map_flags, fd, 0); } ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: cpus/isa: add a != operator for pcstate
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/738/#review1303 --- I think you missed some (maybe just one) version of PC state defined in the ISAs themselves. ARM may be the only one, but you should double check to be sure. Also, for all these you could define them using ==, something like return !(*this == opc); - Gabe On 2011-06-08 22:46:05, Korey Sewell wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/738/ --- (Updated 2011-06-08 22:46:05) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- cpus/isa: add a != operator for pcstate Diffs - src/arch/generic/types.hh 77d12d8f7971 Diff: http://reviews.m5sim.org/r/738/diff Testing --- Thanks, Korey ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: sparc: init. cache state in TLB
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/739/#review1304 --- Ship it! - Gabe On 2011-06-08 22:48:14, Korey Sewell wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/739/ --- (Updated 2011-06-08 22:48:14) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- sparc: init. cache state in TLB valgrind complains and its a potential source of instability, so go ahead and set it to 0 to start Diffs - src/arch/sparc/tlb.cc 77d12d8f7971 Diff: http://reviews.m5sim.org/r/739/diff Testing --- Thanks, Korey ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: gcc 4.0: Add some virtual destructors to make...
changeset 4d1005f78496 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4d1005f78496
description:
gcc 4.0: Add some virtual destructors to make gcc 4.0 happy.
diffstat:
src/base/stats/output.hh | 1 +
src/cpu/inorder/resource_pool.hh | 2 +-
2 files changed, 2 insertions(+), 1 deletions(-)
diffs (23 lines):
diff -r b9ba22cb23f2 -r 4d1005f78496 src/base/stats/output.hh
--- a/src/base/stats/output.hh Fri Jun 03 13:52:18 2011 -0500
+++ b/src/base/stats/output.hh Tue Jun 07 00:24:49 2011 -0700
@@ -46,6 +46,7 @@
struct Output
{
+virtual ~Output() {}
virtual void begin() = 0;
virtual void end() = 0;
virtual bool valid() const = 0;
diff -r b9ba22cb23f2 -r 4d1005f78496 src/cpu/inorder/resource_pool.hh
--- a/src/cpu/inorder/resource_pool.hh Fri Jun 03 13:52:18 2011 -0500
+++ b/src/cpu/inorder/resource_pool.hh Tue Jun 07 00:24:49 2011 -0700
@@ -122,7 +122,7 @@
public:
ResourcePool(InOrderCPU *_cpu, ThePipeline::Params *params);
-~ResourcePool();
+virtual ~ResourcePool();
std::string name();
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request: ISA parser: Loosen the regular expressions matching filenames.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/737/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ISA parser: Loosen the regular expressions matching filenames. The regular expressions matching filenames in the ##include directives and the internally generated ##newfile directives where only looking for filenames composed of alpha numeric characters, periods, and dashes. In Unix/Linux, the rules for what characters can be in a filename are much looser than that. This change replaces those expressions with ones that look for anything other than a quote character. Technically quote characters are allowed as well so we should allow escaping them somehow, but the additional complexity probably isn't worth it. Diffs - src/arch/isa_parser.py 4d1005f78496 Diff: http://reviews.m5sim.org/r/737/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: ISA parser: Loosen the regular expressions ma...
changeset 1810956fa5dc in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=1810956fa5dc description: ISA parser: Loosen the regular expressions matching filenames. The regular expressions matching filenames in the ##include directives and the internally generated ##newfile directives where only looking for filenames composed of alpha numeric characters, periods, and dashes. In Unix/Linux, the rules for what characters can be in a filename are much looser than that. This change replaces those expressions with ones that look for anything other than a quote character. Technically quote characters are allowed as well so we should allow escaping them somehow, but the additional complexity probably isn't worth it. diffstat: src/arch/isa_parser.py | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diffs (21 lines): diff -r 4d1005f78496 -r 1810956fa5dc src/arch/isa_parser.py --- a/src/arch/isa_parser.pyTue Jun 07 00:24:49 2011 -0700 +++ b/src/arch/isa_parser.pyTue Jun 07 00:46:54 2011 -0700 @@ -1215,7 +1215,7 @@ return t def t_NEWFILE(self, t): -r'^\#\#newfile\s+[\w/.-]*' +r'^\#\#newfile\s+[^]*' self.fileNameStack.push((t.value[11:-1], t.lexer.lineno)) t.lexer.lineno = 0 @@ -1998,7 +1998,7 @@ f.close() # This regular expression matches '##include' directives -includeRE = re.compile(r'^\s*##include\s+(?Pfilename[\w/.-]*).*$', +includeRE = re.compile(r'^\s*##include\s+(?Pfilename[^]*).*$', re.MULTILINE) def replace_include(self, matchobj, dirname): ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
On 05/31/11 00:13, Gabe Black wrote:
On 05/30/11 21:57, Steve Reinhardt wrote:
On Mon, May 30, 2011 at 1:33 PM, Gabe Black gbl...@eecs.umich.edu wrote:
On 05/30/11 09:47, Steve Reinhardt wrote:
Anyway, it seems very odd to have to say (int8_t)Mem.ub when we already
have a .sb operand type defined as int8_t. It seems like a weird
hidden restriction to say that there are operand types you can declare but
can't use on memory, and that you're pushing a somewhat arbitrary internal
implementation decision up to the level of language visibility, which is
going the wrong direction. I'm not sure what the right solution is, but
even if it's the brute force one of creating a bunch more read/write
function templates in the CPU implementations, I think that's preferable.
[...]
The unsigned thing is sort of a weird gotcha. I'd like to avoid adding a
lot of bulk to the CPU models since they're already fairly chunky and it
makes them harder to reason about looking through the code, but it would
be great to straighten this out. One possibility might be the technique
I used with the endianness changing functions in src/sim/byteswap.hh.
Things are built up in layers there:
[...]
I think some kind of additional set of template instantiations should
do it. I just noticed that we already have:
template
Fault
AtomicSimpleCPU::read(Addr addr, int32_t data, unsigned flags)
{
return read(addr, (uint32_t)data, flags);
}
template
Fault
AtomicSimpleCPU::write(int32_t data, Addr addr, unsigned flags, uint64_t
*res)
{
return write((uint32_t)data, addr, flags, res);
}
.. and similar for TimingSimpleCPU; do we just need to extend these to
int8_t and int16_t, and maybe add similar sets in
base_dynamic_inst.hh?
Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
Oh yeah, I guess we're already doing something like that, and that glob
of code was just instantiating different versions of the function. Could
we reduce the amount of text by moving it into the header? There isn't a
lot of actual information there in all that text, and we're talking
about doubling it. I don't have a better idea, though.
Gabe
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
To clarify, is this signed/unsigned issue something we need to deal with
before this patch goes in, or can it be dealt with separately later?
Gabe
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request: LibElf: Build the error management code in libelf.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/735/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- LibElf: Build the error management code in libelf. This change makes some minor changes to get the error management code in libelf to build on Linux and to built it into the library. Diffs - ext/libelf/SConscript b9ba22cb23f2 ext/libelf/_libelf.h b9ba22cb23f2 ext/libelf/elf_errmsg.c b9ba22cb23f2 Diff: http://reviews.m5sim.org/r/735/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request: Loader: Handle bad section names when loading an ELF file.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/736/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Loader: Handle bad section names when loading an ELF file. If there's a problem when reading the section names from a supposed ELF file, this change makes gem5 print an error message as returned by libelf and die. Previously these sorts of errors would make gem5 segfault when it tried to access the section name through a NULL pointer. Diffs - src/base/loader/elf_object.cc b9ba22cb23f2 Diff: http://reviews.m5sim.org/r/736/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/668/#review1294 --- SConstruct http://reviews.m5sim.org/r/668/#comment1761 This is pretty reasonable, but it would be nice to mention that if it's not OK you can use Ctrl+C to get out of it. Usually Ctrl+C is a fairly drastic last resort, so people might not think to use it. - Gabe On 2011-06-01 21:27:23, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/668/ --- (Updated 2011-06-01 21:27:23) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- SConstruct: automatically update .hg/hgrc with style hooks Seems easier than pestering people about it. Note also that path is now absolute, so you don't get errors when invoking hg from subdirectories. Diffs - SConstruct 3f49ed206f46 Diff: http://reviews.m5sim.org/r/668/diff Testing --- Thanks, Steve ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
On 05/30/11 21:57, Steve Reinhardt wrote:
On Mon, May 30, 2011 at 1:33 PM, Gabe Black gbl...@eecs.umich.edu wrote:
On 05/30/11 09:47, Steve Reinhardt wrote:
Anyway, it seems very odd to have to say (int8_t)Mem.ub when we already
have a .sb operand type defined as int8_t. It seems like a weird
hidden restriction to say that there are operand types you can declare but
can't use on memory, and that you're pushing a somewhat arbitrary internal
implementation decision up to the level of language visibility, which is
going the wrong direction. I'm not sure what the right solution is, but
even if it's the brute force one of creating a bunch more read/write
function templates in the CPU implementations, I think that's preferable.
[...]
The unsigned thing is sort of a weird gotcha. I'd like to avoid adding a
lot of bulk to the CPU models since they're already fairly chunky and it
makes them harder to reason about looking through the code, but it would
be great to straighten this out. One possibility might be the technique
I used with the endianness changing functions in src/sim/byteswap.hh.
Things are built up in layers there:
[...]
I think some kind of additional set of template instantiations should
do it. I just noticed that we already have:
template
Fault
AtomicSimpleCPU::read(Addr addr, int32_t data, unsigned flags)
{
return read(addr, (uint32_t)data, flags);
}
template
Fault
AtomicSimpleCPU::write(int32_t data, Addr addr, unsigned flags, uint64_t *res)
{
return write((uint32_t)data, addr, flags, res);
}
.. and similar for TimingSimpleCPU; do we just need to extend these to
int8_t and int16_t, and maybe add similar sets in
base_dynamic_inst.hh?
Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
Oh yeah, I guess we're already doing something like that, and that glob
of code was just instantiating different versions of the function. Could
we reduce the amount of text by moving it into the header? There isn't a
lot of actual information there in all that text, and we're talking
about doubling it. I don't have a better idea, though.
Gabe
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] registerThreadContext
On 05/31/11 22:43, Steve Reinhardt wrote: On Wed, May 25, 2011 at 10:57 AM, nathan binkert n...@binkert.org wrote: I see a few options for solving this problem: 1) Separate out the contextId allocation from registerThreadContext so things like DMA controllers and memtesters can get allocated a contextID. 2) Create a base class for ThreadContext that is far simpler than the current thread context and use that when registering. 3) Figure out contextID not by registration, but instead by doing a traversal of the memory system. This would require that we have some sort of indication differentiating memory objects that can generate requests and thus require a contextID and memory objects that can't (caches, dram, pio devices, etc.). We add a constructor parameter to the MemObject base class. 4) Add a separate registration function for non Thread Contexts. While #3 sounds nice, it's a relatively big change, and would have to be done in a way that works both for the m5 classic memory system and for Ruby, and ideally is robust and predictable in assigning contextIDs in the face of minor configuration changes. My guess is that it will end up being harder than it sounds... maybe Korey can speak up if his experience indicates otherwise. The others seem more reasonable, but possibly still overkill. The idea I had, that's kind of a hack but combines #2 and #4 in a degenerate sort of way, would be to allow non-ThreadContext objects to call registerThreadContext, but they would pass in NULL for the ThreadContext pointer. This would allow something like a DMA device or the memtester to reserve or allocate a context ID without being a ThreadContext, but without creating an additional base class. The only down side I see is that error tracking would be harder; if two non-ThreadContexts tried to claim the same ID, the System would not have a pointer to be able to identify the first one. Basically that's the only case I can see where it would be useful for the System object to hold on to some kind of pointer for something that's not an actual ThreadContext. Of course, the current ThreadContext pointers don't have any real debug info either, so even now you just get Cannot have two CPUs with the same id (%d)\n when you try to reuse a context ID, so apparently it hasn't been a big problem. If we cared about providing good error messages, we could extend registerThreadContext to take both a SimObject* and a ThreadContext* and track both of these, but make the latter optional. This would provide better error messages for everyone (though again, this hasn't been a problem that I've seen). registerThreadContext is only called from one place (BaseCPU::registerThreadContexts()) so it wouldn't be hard to change that. Just some ideas... Steve ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev One drawback I see with this sort of approach is that you have to be careful if you want to loop over all registered contexts and do something with their thread contexts. You'd have to be careful to check which ones were NULL, and if that's uncommon it might be easy to forget to do. Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
On 2011-05-28 21:58:27, Steve Reinhardt wrote: The description is a little general; can you be more specific about what this change is doing? I see that you're getting rid of the size from the memory access operands and encoding that in the ctype instead, which seems fine. However it seems like you've gotten rid of a lot of the signed vs. unsigned code, and made everything look unsigned, and I don't see how that still works. The reason I changed them to unsigned is that all the read/write functions have definitions generated for unsigned int operands but not signed ones. They were being forced to unsigned when the call was being made. All of the regressions passed with these changes and at the time I was convinced why this worked, but it's been about a month since then and I don't really remember the specifics. In SPARC and ARM, the Mem variables are being cast to an appropriate type in the assignment, and in Alpha there were apparently no signed Mem types. I'm guessing MIPS and Power don't have as extensive regressions so problems may have slipped through due to those instructions not being used or being used in a way that didn't expose a difference in behavior. The simple fix is to add casts to those too in the few places where it makes a difference, although there may be a reason that I'm not able to remember that it works as is. Generally speaking, signed vs. unsigned, size, and float vs. int information only makes sense if you're building up a few preselected types as the code is currently doing. By deferring more to the C++ type system, you can use whatever type you want and it should just work. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/655/#review1276 --- On 2011-04-25 03:03:53, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/655/ --- (Updated 2011-04-25 03:03:53) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ISA parser: Simplify operand type handling. This change simplifies the code surrounding operand type handling and makes it depend only on the ctype that goes with each operand type. Future changes will allow defining operand types by their ctypes directly, convert the ISAs over to that style of definition, and then remove support for the old style. These changes are to make it easier to use non-builtin types like classes or structures as the type for operands. Diffs - src/arch/alpha/isa/mem.isa de679a068dd8 src/arch/arm/isa/insts/ldr.isa de679a068dd8 src/arch/arm/isa/insts/mem.isa de679a068dd8 src/arch/arm/isa/insts/str.isa de679a068dd8 src/arch/arm/isa/templates/mem.isa de679a068dd8 src/arch/isa_parser.py de679a068dd8 src/arch/mips/isa/decoder.isa de679a068dd8 src/arch/mips/isa/formats/mem.isa de679a068dd8 src/arch/power/isa/decoder.isa de679a068dd8 src/arch/power/isa/formats/mem.isa de679a068dd8 src/arch/sparc/isa/decoder.isa de679a068dd8 src/arch/sparc/isa/formats/mem/swap.isa de679a068dd8 src/arch/sparc/isa/formats/mem/util.isa de679a068dd8 Diff: http://reviews.m5sim.org/r/655/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: X86: Convert operand types to the new style.
On 2011-05-28 22:04:57, Steve Reinhardt wrote: Looks fine, but shouldn't you be doing all the ISAs if you're going to get rid of the old style? I have, but I didn't post all the patches since they're pretty similar and I didn't want to clutter up review board. I mentioned that when I first posted these, but that was a while ago. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/657/#review1278 --- On 2011-04-25 03:04:20, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/657/ --- (Updated 2011-04-25 03:04:20) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: Convert operand types to the new style. Diffs - src/arch/x86/isa/operands.isa de679a068dd8 Diff: http://reviews.m5sim.org/r/657/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Stop supporting the old style operand types.
On 2011-05-28 22:09:01, Steve Reinhardt wrote: See previous reviews... this looks fine if all the ISAs are converted to use it. Once it's committed, the relevant documentation should also be updated (e.g., http://gem5.org/Code_parsing#Operand_type_qualifiers). Will do. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/658/#review1279 --- On 2011-04-25 03:04:33, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/658/ --- (Updated 2011-04-25 03:04:33) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ISA parser: Stop supporting the old style operand types. Diffs - src/arch/isa_parser.py de679a068dd8 Diff: http://reviews.m5sim.org/r/658/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/655/ --- (Updated 2011-05-30 00:18:31.344077) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ISA parser: Simplify operand type handling. This change simplifies the code surrounding operand type handling and makes it depend only on the ctype that goes with each operand type. Future changes will allow defining operand types by their ctypes directly, convert the ISAs over to that style of definition, and then remove support for the old style. These changes are to make it easier to use non-builtin types like classes or structures as the type for operands. Diffs (updated) - src/arch/alpha/isa/mem.isa 03cfd2ecf6bb src/arch/arm/isa/insts/ldr.isa 03cfd2ecf6bb src/arch/arm/isa/insts/mem.isa 03cfd2ecf6bb src/arch/arm/isa/insts/str.isa 03cfd2ecf6bb src/arch/arm/isa/templates/mem.isa 03cfd2ecf6bb src/arch/isa_parser.py 03cfd2ecf6bb src/arch/mips/isa/decoder.isa 03cfd2ecf6bb src/arch/mips/isa/formats/mem.isa 03cfd2ecf6bb src/arch/power/isa/decoder.isa 03cfd2ecf6bb src/arch/power/isa/formats/mem.isa 03cfd2ecf6bb src/arch/sparc/isa/decoder.isa 03cfd2ecf6bb src/arch/sparc/isa/formats/mem/swap.isa 03cfd2ecf6bb src/arch/sparc/isa/formats/mem/util.isa 03cfd2ecf6bb Diff: http://reviews.m5sim.org/r/655/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Allow defining operand types with a ctype directly.
On 2011-05-28 22:04:00, Steve Reinhardt wrote: This looks fine to me, but I'm confused... don't you delete this code completely two patches from now? Why bother changing it if you're going to get rid of it? Gabe Black wrote: Because this way both are available, and the ISAs can be converted one at a time and everything will work between every patch. Otherwise I'd have to break everything that wasn't (or was) updated, or do everything in one big change that does too much at once. Once all the ISAs are consistently on the new system, the old system isn't needed anymore and is deleted in that later patch. Steve Reinhardt wrote: OK, I see. Overall this seems like overkill; I can see how this code was useful while you were developing, but for committing, one complete patch that gets rid of the old way of doing things and fixes all the ISAs to use the new way would be preferable to me. I think it would be less confusing because all the related changes would be right there in one place. I don't have a problem with large changesets (in terms of lines of code or files touched), just ones that are not conceptually unified, and spreading this change across multiple csets goes against conceptually unified in the opposite direction, IMO. That makes sense, but then I think when changes get to be too big, they get to be too hard to understand all at once. By keeping each part relatively small it makes things easier to digest later. My enormous PC state change is an example of the opposite extreme, and it was probably a lot of work to get through and review and overwhelming to look at later in the history. I'd like to avoid that if possible. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/656/#review1277 --- On 2011-04-25 03:04:12, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/656/ --- (Updated 2011-04-25 03:04:12) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ISA parser: Allow defining operand types with a ctype directly. Diffs - src/arch/isa_parser.py de679a068dd8 Diff: http://reviews.m5sim.org/r/656/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.
On 05/30/11 09:47, Steve Reinhardt wrote: On 2011-05-28 21:58:27, Steve Reinhardt wrote: The description is a little general; can you be more specific about what this change is doing? I see that you're getting rid of the size from the memory access operands and encoding that in the ctype instead, which seems fine. However it seems like you've gotten rid of a lot of the signed vs. unsigned code, and made everything look unsigned, and I don't see how that still works. Gabe Black wrote: The reason I changed them to unsigned is that all the read/write functions have definitions generated for unsigned int operands but not signed ones. They were being forced to unsigned when the call was being made. All of the regressions passed with these changes and at the time I was convinced why this worked, but it's been about a month since then and I don't really remember the specifics. In SPARC and ARM, the Mem variables are being cast to an appropriate type in the assignment, and in Alpha there were apparently no signed Mem types. I'm guessing MIPS and Power don't have as extensive regressions so problems may have slipped through due to those instructions not being used or being used in a way that didn't expose a difference in behavior. The simple fix is to add casts to those too in the few places where it makes a difference, although there may be a reason that I'm not able to remember that it works as is. Generally speaking, signed vs. unsigned, size, and float vs. int information only makes sense if you're building up a few preselected types as the code is currently doing. By deferring more to the C++ type system, you can use whatever type you want and it should just work. Well, the fact that the old regressions didn't notice doesn't fill me with confidence, but at least it's more plausible that the updated version works. I guess your old version may have worked in the MIPS case because the LHS is signed, e.g., lh is Rt.sw = Mem.uw while lhu is Rt.uw = Mem.uw, but the LHS is not signed in the Power case, and even in MIPS the size on the LHS for lb is wrong (it's also Rt.sw and not Rt.sb), which may or may not matter. Anyway, it seems very odd to have to say (int8_t)Mem.ub when we already have a .sb operand type defined as int8_t. It seems like a weird hidden restriction to say that there are operand types you can declare but can't use on memory, and that you're pushing a somewhat arbitrary internal implementation decision up to the level of language visibility, which is going the wrong direction. I'm not sure what the right solution is, but even if it's the brute force one of creating a bunch more read/write function templates in the CPU implementations, I think that's preferable. I must say that other than this one inconsistency, I like this change, and I'm left scratching my head about why I made the original version so complicated in the first place. - Steve I'm replying in email instead of on review board since this is more of a general discussion than review feedback. The unsigned thing is sort of a weird gotcha. I'd like to avoid adding a lot of bulk to the CPU models since they're already fairly chunky and it makes them harder to reason about looking through the code, but it would be great to straighten this out. One possibility might be the technique I used with the endianness changing functions in src/sim/byteswap.hh. Things are built up in layers there: The swapping functions that depend on the guest byte order are templated on the type being swapped and are defined in terms of the functions that use a fixed endianness on the guest end. The functions that depend on the host and not the guest are templated on the type as well, and use the preprocessor to pick versions that do swapping or not based on the endianness of the host. They're defined using the swap_byte function which is also templated on the type. The functions that depend on neither the guest nor the host are defined unconditionally, templated on the type, and use swap_byte for their implementation. The swap_byte function is templated on the type it's swapping. It's implemented as an if that selects a swapping function based on the size of the type involved. Because it's inlined and templated on a particular type, the compiler should squash the if (and the function entirely) so that the right thing is being called. Then, finally, the swap_byteXX functions are called where XX is the type's size in bits. So with all these layers, the compiler can select the appropriate byte swapping operation (or none at all) based on the starting and ending orders, the host and guest orders, and the size of the type without having to actually know about the type ahead of time, assuming it's 1, 2, 4, or 8 bytes in size. We might be able to do something like this for the execution context functions so that they can read anything as long as it's a recognized
Re: [gem5-dev] Review Request: ISA parser: Allow defining operand types with a ctype directly.
On 2011-05-28 22:04:00, Steve Reinhardt wrote: This looks fine to me, but I'm confused... don't you delete this code completely two patches from now? Why bother changing it if you're going to get rid of it? Gabe Black wrote: Because this way both are available, and the ISAs can be converted one at a time and everything will work between every patch. Otherwise I'd have to break everything that wasn't (or was) updated, or do everything in one big change that does too much at once. Once all the ISAs are consistently on the new system, the old system isn't needed anymore and is deleted in that later patch. Steve Reinhardt wrote: OK, I see. Overall this seems like overkill; I can see how this code was useful while you were developing, but for committing, one complete patch that gets rid of the old way of doing things and fixes all the ISAs to use the new way would be preferable to me. I think it would be less confusing because all the related changes would be right there in one place. I don't have a problem with large changesets (in terms of lines of code or files touched), just ones that are not conceptually unified, and spreading this change across multiple csets goes against conceptually unified in the opposite direction, IMO. Gabe Black wrote: That makes sense, but then I think when changes get to be too big, they get to be too hard to understand all at once. By keeping each part relatively small it makes things easier to digest later. My enormous PC state change is an example of the opposite extreme, and it was probably a lot of work to get through and review and overwhelming to look at later in the history. I'd like to avoid that if possible. Then again, the changes after this one don't add much complexity to this one since they're pretty simple. Merging them is reasonable, and I'll do that once we're ready to put these in. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/656/#review1277 --- On 2011-04-25 03:04:12, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/656/ --- (Updated 2011-04-25 03:04:12) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ISA parser: Allow defining operand types with a ctype directly. Diffs - src/arch/isa_parser.py de679a068dd8 Diff: http://reviews.m5sim.org/r/656/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in m5: Misc: Remove the URL from warnings, fatals, pan...
changeset 03cfd2ecf6bb in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=03cfd2ecf6bb
description:
Misc: Remove the URL from warnings, fatals, panics, etc.
diffstat:
src/base/misc.cc | 15 ---
src/python/m5/util/__init__.py | 12
2 files changed, 0 insertions(+), 27 deletions(-)
diffs (88 lines):
diff -r dda2a88eb7c4 -r 03cfd2ecf6bb src/base/misc.cc
--- a/src/base/misc.cc Wed May 25 01:32:07 2011 -0700
+++ b/src/base/misc.cc Sun May 29 21:48:58 2011 -0700
@@ -28,8 +28,6 @@
* Authors: Nathan Binkert
*/
-#include zlib.h
-
#include cstdlib
#include iostream
#include string
@@ -70,20 +68,15 @@
default:
format += \n;
}
-
-uint32_t crc = crc32(0, (const Bytef*)fmt, strlen(fmt));
format += @ cycle %d\n[%s:%s, line %d]\n;
format += Memory Usage: %ld KBytes\n;
-format += For more information see: http://www.m5sim.org/%s/%x\n;;
args.push_back(curTick());
args.push_back(func);
args.push_back(file);
args.push_back(line);
args.push_back(memUsage());
-args.push_back(prefix);
-args.push_back(crc);
ccprintf(cerr, format.c_str(), args);
@@ -110,8 +103,6 @@
default:
format += \n;
}
-
-uint32_t crc = crc32(0, (const Bytef*)fmt, strlen(fmt));
if (verbose) {
format += @ cycle %d\n[%s:%s, line %d]\n;
@@ -121,11 +112,5 @@
args.push_back(line);
}
-if (strcmp(prefix, warn) == 0) {
-format += For more information see: http://www.m5sim.org/%s/%x\n;;
-args.push_back(prefix);
-args.push_back(crc);
-}
-
ccprintf(stream, format.c_str(), args);
}
diff -r dda2a88eb7c4 -r 03cfd2ecf6bb src/python/m5/util/__init__.py
--- a/src/python/m5/util/__init__.pyWed May 25 01:32:07 2011 -0700
+++ b/src/python/m5/util/__init__.pySun May 29 21:48:58 2011 -0700
@@ -42,22 +42,11 @@
from sorteddict import SortedDict
from region import neg_inf, pos_inf, Region, Regions
-# define this here so we can use it right away if necessary
-def errorURL(prefix, s):
-try:
-import zlib
-hashstr = %x % zlib.crc32(s)
-except:
-hashstr = UnableToHash
-return For more information see: http://www.m5sim.org/%s/%s; % \
-(prefix, hashstr)
-
# panic() should be called when something happens that should never
# ever happen regardless of what the user does (i.e., an acutal m5
# bug).
def panic(fmt, *args):
print sys.stderr, 'panic:', fmt % args
-print sys.stderr, errorURL('panic',fmt)
sys.exit(1)
# fatal() should be called when the simulation cannot continue due to
@@ -65,7 +54,6 @@
# arguments, etc.) and not a simulator bug.
def fatal(fmt, *args):
print sys.stderr, 'fatal:', fmt % args
-print sys.stderr, errorURL('fatal',fmt)
sys.exit(1)
class Singleton(type):
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: O3: Create a pipeline activity viewer for the O3 CPU model.
That does look nice. It can be a lot of work slogging through O3 trace output to figure out what's going on, and I expect this to make that a lot easier. I glanced through the code and didn't see anything particularly objectionable, although I wouldn't necessarily consider that a thorough review :-P. Gabe On 05/26/11 22:20, Steve Reinhardt wrote: Cool! I haven't looked at the code yet, but the output looks nice. Steve On Thu, May 26, 2011 at 8:19 PM, Ali Saidi sa...@umich.edu wrote: Anyone that is curious what this looks like... lets try the picture again... Ali On May 26, 2011, at 9:19 PM, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/721/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- O3: Create a pipeline activity viewer for the O3 CPU model. Implement a pipeline activity viewer as a python script (util/o3-pipeview.py) and modified O3 code base to support an extra trace flag (O3PipeView) for generating traces to be used as inputs by the tool. Diffs - src/cpu/SConscript 3f37cc5d25bc src/cpu/o3/commit_impl.hh 3f37cc5d25bc src/cpu/o3/decode_impl.hh 3f37cc5d25bc src/cpu/o3/dyn_inst.hh 3f37cc5d25bc src/cpu/o3/fetch_impl.hh 3f37cc5d25bc src/cpu/o3/iew_impl.hh 3f37cc5d25bc src/cpu/o3/inst_queue_impl.hh 3f37cc5d25bc src/cpu/o3/rename_impl.hh 3f37cc5d25bc util/o3-pipeview.py PRE-CREATION Diff: http://reviews.m5sim.org/r/721/diff Testing --- Thanks, Ali ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: Config: Add support for a Self.all proxy object
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/720/#review1262 --- What does Self.all do? What is it for? You have one use in the change so I have a basic idea, but it would be helpful to know the specifics. - Gabe On 2011-05-26 19:17:18, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/720/ --- (Updated 2011-05-26 19:17:18) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Config: Add support for a Self.all proxy object Diffs - src/python/m5/SimObject.py 3f37cc5d25bc src/python/m5/params.py 3f37cc5d25bc src/python/m5/proxy.py 3f37cc5d25bc src/sim/System.py 3f37cc5d25bc Diff: http://reviews.m5sim.org/r/720/diff Testing --- Thanks, Ali ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request: Misc: Remove the URL from warnings, fatals, panics, etc.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/719/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Misc: Remove the URL from warnings, fatals, panics, etc. Diffs - src/base/misc.cc dda2a88eb7c4 src/python/m5/util/__init__.py dda2a88eb7c4 Diff: http://reviews.m5sim.org/r/719/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request: Name: Replace M5 with gem5 in a few places it's printed on startup.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/713/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Name: Replace M5 with gem5 in a few places it's printed on startup. Diffs - src/python/m5/main.py 76095b05f4da Diff: http://reviews.m5sim.org/r/713/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: Name: Replace M5 with gem5 in a few places it's printed on startup.
I'm sure there are other places we need to fix, but this gets some of the most prominent ones. Gabe On 05/24/11 00:42, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/713/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Name: Replace M5 with gem5 in a few places it's printed on startup. Diffs - src/python/m5/main.py 76095b05f4da Diff: http://reviews.m5sim.org/r/713/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: Name: Replace M5 with gem5 in a few places it's printed on startup.
On 2011-05-24 18:41:14, Ali Saidi wrote: src/python/m5/main.py, line 40 http://reviews.m5sim.org/r/713/diff/1/?file=12646#file12646line40 This really should be the something else, maybe remove university of michigan and put see COPYRIGHT file (although that needs an update too). I think you're right, but that's a larger issue than what I'm trying to handle with this patch. We'd need to fix up the copyright file like we've talked about in the past. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/713/#review1257 --- On 2011-05-24 00:42:10, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/713/ --- (Updated 2011-05-24 00:42:10) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Name: Replace M5 with gem5 in a few places it's printed on startup. Diffs - src/python/m5/main.py 76095b05f4da Diff: http://reviews.m5sim.org/r/713/diff Testing --- Thanks, Gabe ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in m5: Name: Replace M5 with gem5 in a few places it's...
changeset dda2a88eb7c4 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=dda2a88eb7c4 description: Name: Replace M5 with gem5 in a few places it's printed on startup. diffstat: src/python/m5/main.py | 15 --- 1 files changed, 8 insertions(+), 7 deletions(-) diffs (45 lines): diff -r 8b0eee09deaf -r dda2a88eb7c4 src/python/m5/main.py --- a/src/python/m5/main.py Tue May 24 21:19:31 2011 -0700 +++ b/src/python/m5/main.py Wed May 25 01:32:07 2011 -0700 @@ -34,10 +34,10 @@ __all__ = [ 'options', 'arguments', 'main' ] -usage=%prog [m5 options] script.py [script options] +usage=%prog [gem5 options] script.py [script options] version=%prog 2.0 brief_copyright=''' -Copyright (c) 2001-2008 +Copyright (c) 2001-2011 The Regents of The University of Michigan All Rights Reserved ''' @@ -130,7 +130,7 @@ return options,arguments def interact(scope): -banner = M5 Interactive Console +banner = gem5 Interactive Console sys.argv = [] try: from IPython.Shell import IPShellEmbed @@ -263,14 +263,15 @@ verbose = options.verbose - options.quiet if options.verbose = 0: -print M5 Simulator System +print gem5 Simulator System print brief_copyright print -print M5 compiled %s % defines.compileDate; +print gem5 compiled %s % defines.compileDate; -print M5 started %s % datetime.datetime.now().strftime(%b %e %Y %X) -print M5 executing on %s % socket.gethostname() +print gem5 started %s % \ +datetime.datetime.now().strftime(%b %e %Y %X) +print gem5 executing on %s % socket.gethostname() print command line:, for argv in sys.argv: ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: Enabled instruction fetch pipelining.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/718/#review1260 --- Have you run the regressions for the various ISAs with this patch? Have you tried the applicable ISAs with fetch pipelines deeper than the default (one stage?). The fetch code is subjected to a lot of corner cases and would likely be easy to break in subtle ways, so we need to be really careful. Also, have you considered making this an external component to the CPU? O3 is already very complicated, so if it could make sense to compartmentalize this as another component that would help. - Gabe On 2011-05-24 12:01:29, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/718/ --- (Updated 2011-05-24 12:01:29) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Enabled instruction fetch pipelining. This patch is from one of our co-ops who has since finished her term, Yasuko Watanabe. I don't personally know much about it. In the end, I'll push in her name. Thanks. Diffs - src/cpu/o3/fetch.hh 54a65799e4c1 src/cpu/o3/fetch_impl.hh 54a65799e4c1 Diff: http://reviews.m5sim.org/r/718/diff Testing --- Thanks, Lisa ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Cron m5test@zizzer /z/m5/regression/do-regression quick
The regressions are failing with the error below. This suggests one of Tushar Krishna's two recent commits is either bad or requires a clean build. Would you please look into it, Tushar? Gabe SLICC Generator pass 1... SLICC Generator pass 2... MI_example-dir.sm:197: Warning: Unused action: a_sendWriteBackAck, Send writeback ack to requestor SLICC writing C++ files... KeyError: 'vnet_type': File /z/m5/regression/zizzer/m5/SConstruct, line 980: exports = 'env') File /usr/lib/scons/SCons/Script/SConscript.py, line 596: return apply(method, args, kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 533: return apply(_SConscript, [self.fs,] + files, subst_kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 256: exec _file_ in call_stack[-1].globals File /z/m5/regression/zizzer/m5/build/ALPHA_SE/SConscript, line 318: SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir) File /usr/lib/scons/SCons/Script/SConscript.py, line 596: return apply(method, args, kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 533: return apply(_SConscript, [self.fs,] + files, subst_kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 256: exec _file_ in call_stack[-1].globals File /z/m5/regression/zizzer/m5/build/ALPHA_SE/mem/protocol/SConscript, line 134: nodes = env.SLICC([], [ Value(protocol) ] + sources) File /usr/lib/scons/SCons/Environment.py, line 219: return apply(MethodWrapper.__call__, (self, target, source) + args, kw) File /usr/lib/scons/SCons/Environment.py, line 183: return apply(self.method, nargs, kwargs) File /usr/lib/scons/SCons/Builder.py, line 614: return self._execute(env, target, source, OverrideWarner(kw), ekw) File /usr/lib/scons/SCons/Builder.py, line 561: tlist, slist = self._create_nodes(env, target, source) File /usr/lib/scons/SCons/Builder.py, line 525: target, source = self.emitter(target=tlist, source=slist, env=env) File /z/m5/regression/zizzer/m5/build/ALPHA_SE/mem/protocol/SConscript, line 89: slicc.writeCodeFiles(pdir) File /z/m5/regression/zizzer/m5/src/mem/slicc/parser.py, line 117: self.symtab.writeCodeFiles(code_path) File /z/m5/regression/zizzer/m5/src/mem/slicc/symbols/SymbolTable.py, line 139: symbol.writeCodeFiles(path) File /z/m5/regression/zizzer/m5/src/mem/slicc/symbols/StateMachine.py, line 163: self.printControllerCC(path) File /z/m5/regression/zizzer/m5/src/mem/slicc/symbols/StateMachine.py, line 597: vnet_type = var[vnet_type] File /z/m5/regression/zizzer/m5/src/mem/slicc/util.py, line 47: return self.pairs[item] Child returned 2 When attemping to execute: scons --ignore-style -k USE_MYSQL=no EXTRAS=/z/m5/regression/zizzer/encumbered RUBY=True -j 7 -Q build/ALPHA_SE/tests/opt/quick build/ALPHA_SE_MOESI_hammer/tests/opt/quick build/ALPHA_SE_MESI_CMP_directory/tests/opt/quick build/ALPHA_SE_MOESI_CMP_directory/tests/opt/quick build/ALPHA_SE_MOESI_CMP_token/tests/opt/quick build/ALPHA_FS/tests/opt/quick build/MIPS_SE/tests/opt/quick build/POWER_SE/tests/opt/quick build/SPARC_SE/tests/opt/quick build/X86_SE/tests/opt/quick build/X86_FS/tests/opt/quick build/ARM_SE/tests/opt/quick build/ARM_FS/tests/opt/quick On 05/20/11 00:00, Cron Daemon wrote: See /z/m5/regression/regress-2011-05-20-03:00:01 for details. ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Cron m5test@zizzer /z/m5/regression/do-regression quick
Thanks for taking care of it quickly. Gabe On 05/20/11 02:19, Tushar Krishna wrote: Sorry about that. Just checked in a patch which should fix it. Had added vnet_type to all protocols, but forgot to add it to MI_example. And its default build does not use RUBY so I missed catching it earlier. On 5/20/2011 4:35 AM, Gabe Black wrote: The regressions are failing with the error below. This suggests one of Tushar Krishna's two recent commits is either bad or requires a clean build. Would you please look into it, Tushar? Gabe SLICC Generator pass 1... SLICC Generator pass 2... MI_example-dir.sm:197: Warning: Unused action: a_sendWriteBackAck, Send writeback ack to requestor SLICC writing C++ files... KeyError: 'vnet_type': File /z/m5/regression/zizzer/m5/SConstruct, line 980: exports = 'env') File /usr/lib/scons/SCons/Script/SConscript.py, line 596: return apply(method, args, kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 533: return apply(_SConscript, [self.fs,] + files, subst_kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 256: exec _file_ in call_stack[-1].globals File /z/m5/regression/zizzer/m5/build/ALPHA_SE/SConscript, line 318: SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir) File /usr/lib/scons/SCons/Script/SConscript.py, line 596: return apply(method, args, kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 533: return apply(_SConscript, [self.fs,] + files, subst_kw) File /usr/lib/scons/SCons/Script/SConscript.py, line 256: exec _file_ in call_stack[-1].globals File /z/m5/regression/zizzer/m5/build/ALPHA_SE/mem/protocol/SConscript, line 134: nodes = env.SLICC([], [ Value(protocol) ] + sources) File /usr/lib/scons/SCons/Environment.py, line 219: return apply(MethodWrapper.__call__, (self, target, source) + args, kw) File /usr/lib/scons/SCons/Environment.py, line 183: return apply(self.method, nargs, kwargs) File /usr/lib/scons/SCons/Builder.py, line 614: return self._execute(env, target, source, OverrideWarner(kw), ekw) File /usr/lib/scons/SCons/Builder.py, line 561: tlist, slist = self._create_nodes(env, target, source) File /usr/lib/scons/SCons/Builder.py, line 525: target, source = self.emitter(target=tlist, source=slist, env=env) File /z/m5/regression/zizzer/m5/build/ALPHA_SE/mem/protocol/SConscript, line 89: slicc.writeCodeFiles(pdir) File /z/m5/regression/zizzer/m5/src/mem/slicc/parser.py, line 117: self.symtab.writeCodeFiles(code_path) File /z/m5/regression/zizzer/m5/src/mem/slicc/symbols/SymbolTable.py, line 139: symbol.writeCodeFiles(path) File /z/m5/regression/zizzer/m5/src/mem/slicc/symbols/StateMachine.py, line 163: self.printControllerCC(path) File /z/m5/regression/zizzer/m5/src/mem/slicc/symbols/StateMachine.py, line 597: vnet_type = var[vnet_type] File /z/m5/regression/zizzer/m5/src/mem/slicc/util.py, line 47: return self.pairs[item] Child returned 2 When attemping to execute: scons --ignore-style -k USE_MYSQL=no EXTRAS=/z/m5/regression/zizzer/encumbered RUBY=True -j 7 -Q build/ALPHA_SE/tests/opt/quick build/ALPHA_SE_MOESI_hammer/tests/opt/quick build/ALPHA_SE_MESI_CMP_directory/tests/opt/quick build/ALPHA_SE_MOESI_CMP_directory/tests/opt/quick build/ALPHA_SE_MOESI_CMP_token/tests/opt/quick build/ALPHA_FS/tests/opt/quick build/MIPS_SE/tests/opt/quick build/POWER_SE/tests/opt/quick build/SPARC_SE/tests/opt/quick build/X86_SE/tests/opt/quick build/X86_FS/tests/opt/quick build/ARM_SE/tests/opt/quick build/ARM_FS/tests/opt/quick On 05/20/11 00:00, Cron Daemon wrote: See /z/m5/regression/regress-2011-05-20-03:00:01 for details. ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request: gcc: fix an uninitialized variable warning from G++ 4.5
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/705/#review1239 --- Ship it! Because of how the macroops/microops are structure this is technically not necessary for functional correctness, but it's probably necessary for determinism and to pacify gcc. It's also not a bad idea in case somebody reuses these microops in a way I didn't intend when I wrote them. - Gabe On 2011-05-17 11:48:15, Nathan Binkert wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/705/ --- (Updated 2011-05-17 11:48:15) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- gcc: fix an uninitialized variable warning from G++ 4.5 Diffs - src/arch/arm/isa/insts/macromem.isa fb0e525008c5 Diff: http://reviews.m5sim.org/r/705/diff Testing --- Thanks, Nathan ___ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
[m5-dev] SimObject generated includes
I'm working on some patches that pull the kernel stuff out of the System object into it's own thing, and I'm running into some circular includes in the generated files. I have a new Workload object which Process and Kernel inherit from and which has a pointer back to System, and System has a pointer to kernel. More or less the WorkloadParams header includes the SystemParams header includes the KernelParams header includes the WorkloadParams header, and things break. This could be fixed by still making inheritance include the base class header files but make parameter pointers use prototype declarations. Does anybody know how to actually implement that? Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] src/dest detection in the ISA descriptions
Ping... On 05/05/11 10:38, Steve Reinhardt wrote: On Wed, May 4, 2011 at 2:25 PM, Gabe Black gbl...@eecs.umich.edu wrote: Did that make sense? I see how that could work... I think I was more puzzled by how you would figure out that for (int i = 0; i 7; i++) Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i]; overwrote all of Dest, but for (int i = 0; i 4; i++) Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i]; wouldn't... but looking back I see now that you'd expect to need manual annotations in at least one of those cases. Do you think you'll be able to review those patches soonish? I'll try... thanks for the reminder, that definitely increases the probability :-). Steve ___ 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
Re: [m5-dev] SimObject generated includes
On 05/15/11 06:53, Gabe Black wrote:
I'm working on some patches that pull the kernel stuff out of the System
object into it's own thing, and I'm running into some circular includes
in the generated files. I have a new Workload object which Process and
Kernel inherit from and which has a pointer back to System, and System
has a pointer to kernel. More or less the WorkloadParams header includes
the SystemParams header includes the KernelParams header includes the
WorkloadParams header, and things break. This could be fixed by still
making inheritance include the base class header files but make
parameter pointers use prototype declarations. Does anybody know how to
actually implement that?
Gabe
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Ok, I think I sort of figured that part out (example patch attached) but
now SWIG is unhappy. It's including the param struct for an object which
no longer includes the param header files for all parameters it has, so
it doesn't have definitions for their Param structs, or their base class
Param structs. Then it sets up these conversion functions which cast
things around using those types. I don't know why or how to change it,
and I'm getting lost in all this SWIG goop, and python generating SWIG
files and python files getting turned into wrapper .cc files including
other generated files including each other, and my head a-splode. So if
somebody (Nate) could please look at this and figure out what's going on
that would be great. This sort of thing should work, so this is
something we should fix.
It would also make my life easier if I didn't actually write any of the
code for this yet. Testing any patches would probably be ok.
Gabe
# HG changeset patch
# Parent 685345c45fe5600c2d745ea86846688e8e36ade7
diff -r 685345c45fe5 -r 17f669336cf9 src/python/m5/SimObject.py
--- a/src/python/m5/SimObject.pySun May 15 03:20:19 2011 -0700
+++ b/src/python/m5/SimObject.pySun May 15 04:04:44 2011 -0700
@@ -98,7 +98,15 @@
instanceDict = {}
def default_cxx_predecls(cls, code):
-code('#include params/$cls.hh')
+class_path = cls._value_dict['cxx_class'].split('::')
+# A forward class declaration is sufficient since we are just
+# declaring a pointer.
+for ns in class_path[:-1]:
+code('namespace $ns {')
+code('class $0;', class_path[-1])
+for ns in reversed(class_path[:-1]):
+code('} // namespace $ns')
+code()
def default_swig_predecls(cls, code):
code('%import python/m5/internal/param_$cls.i')
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: stats: delete mysql support
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/691/#review1219 --- I don't know for sure if this is correct, but it's great to see the tree lose some weight :-). - Gabe On 2011-05-10 06:09:27, Nathan Binkert wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/691/ --- (Updated 2011-05-10 06:09:27) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- stats: delete mysql support We can add it back within python in some future change. Diffs - SConstruct 44f8c2507d85 src/base/SConscript 44f8c2507d85 src/base/mysql.hh 44f8c2507d85 src/base/mysql.cc 44f8c2507d85 src/base/stats/mysql.hh 44f8c2507d85 src/base/stats/mysql.cc 44f8c2507d85 src/base/stats/mysql_run.hh 44f8c2507d85 src/python/m5/stats/__init__.py PRE-CREATION src/python/swig/stats.i 44f8c2507d85 Diff: http://reviews.m5sim.org/r/691/diff Testing --- quick regressions pass (though most recently run with review 689 and 690) Thanks, Nathan ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Break up condition codes into normal flags, saturation, and simd.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/679/#review1213 --- Ship it! It looks ok to me, assuming it passes all your tests. - Gabe On 2011-05-04 18:42:58, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/679/ --- (Updated 2011-05-04 18:42:58) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Break up condition codes into normal flags, saturation, and simd. This change splits out the condcodes from being one monolithic register into three blocks that are updated independently. This allows CPUs to not have to do RMW operations on the flags registers for instructions that don't write all flags. Diffs - src/arch/arm/faults.cc 5a9a639ce16f src/arch/arm/intregs.hh 5a9a639ce16f src/arch/arm/isa/formats/fp.isa 5a9a639ce16f src/arch/arm/isa/formats/pred.isa 5a9a639ce16f src/arch/arm/isa/insts/data.isa 5a9a639ce16f src/arch/arm/isa/insts/fp.isa 5a9a639ce16f src/arch/arm/isa/insts/ldr.isa 5a9a639ce16f src/arch/arm/isa/insts/macromem.isa 5a9a639ce16f src/arch/arm/isa/insts/mem.isa 5a9a639ce16f src/arch/arm/isa/insts/misc.isa 5a9a639ce16f src/arch/arm/isa/insts/mult.isa 5a9a639ce16f src/arch/arm/isa/insts/str.isa 5a9a639ce16f src/arch/arm/isa/operands.isa 5a9a639ce16f src/arch/arm/isa/templates/pred.isa 5a9a639ce16f src/arch/arm/miscregs.hh 5a9a639ce16f src/arch/arm/nativetrace.cc 5a9a639ce16f Diff: http://reviews.m5sim.org/r/679/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Remove the saturating (Q) condition code from the renamed register.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/680/#review1214 --- You need to be careful here since miscregs won't forward results back to later instructions. Anything that depends on the q bit (and by extension the CPSR as a whole) needs to wait for all earlier instructions to complete before executing. I didn't see anything in this change that handles that, but it may have already been in there for some other reason. - Gabe On 2011-05-04 18:43:33, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/680/ --- (Updated 2011-05-04 18:43:33) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Remove the saturating (Q) condition code from the renamed register. Move the saturating bit (which is also saturating) from the renamed register that holds the flags to the CPSR miscreg and adds a allows setting it in a similar way to the FP saturating registers. This removes a dependency in instructions that don't write, but need to preserve the Q bit. Diffs - src/arch/arm/faults.cc 5a9a639ce16f src/arch/arm/intregs.hh 5a9a639ce16f src/arch/arm/isa.cc 5a9a639ce16f src/arch/arm/isa/insts/data.isa 5a9a639ce16f src/arch/arm/isa/insts/ldr.isa 5a9a639ce16f src/arch/arm/isa/insts/macromem.isa 5a9a639ce16f src/arch/arm/isa/insts/misc.isa 5a9a639ce16f src/arch/arm/isa/insts/mult.isa 5a9a639ce16f src/arch/arm/isa/operands.isa 5a9a639ce16f src/arch/arm/miscregs.hh 5a9a639ce16f src/arch/arm/nativetrace.cc 5a9a639ce16f Diff: http://reviews.m5sim.org/r/680/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Further break up condition code into NZ, C, V bits.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/681/#review1215 --- src/arch/arm/faults.cc http://reviews.m5sim.org/r/681/#comment1661 The CPSR type has elements for some of these. You could add one for nz. That would eliminate the need for the shifts, and is a large part of what the bitunion types are for. Also be careful of order of operations. src/arch/arm/isa/formats/pred.isa http://reviews.m5sim.org/r/681/#comment1664 Watch order of operations. Even if it's technically correct, it's not immediately obvious. You might want to throw in parenthesis just to be clearer to a reader. src/arch/arm/isa/formats/pred.isa http://reviews.m5sim.org/r/681/#comment1665 It's not part of your change, but there should be a space around the :, I think. src/arch/arm/isa/insts/data.isa http://reviews.m5sim.org/r/681/#comment1666 Parenthesis, bitfields. src/arch/arm/isa/insts/fp.isa http://reviews.m5sim.org/r/681/#comment1667 Doesn't this change the behavior of this instruction? It used to write to a gpr, now it writes to the condition codes. Is that on purpose? src/arch/arm/isa/insts/ldr.isa http://reviews.m5sim.org/r/681/#comment1668 Parenthesis, bitfields. src/arch/arm/isa/insts/macromem.isa http://reviews.m5sim.org/r/681/#comment1669 Parenthesis, bitfields. src/arch/arm/isa/insts/macromem.isa http://reviews.m5sim.org/r/681/#comment1670 Bitfields. src/arch/arm/isa/insts/macromem.isa http://reviews.m5sim.org/r/681/#comment1671 Bitfields. src/arch/arm/isa/templates/pred.isa http://reviews.m5sim.org/r/681/#comment1672 Spaces after commas. src/arch/arm/nativetrace.cc http://reviews.m5sim.org/r/681/#comment1662 Why the shift up above but not here? src/arch/arm/utility.hh http://reviews.m5sim.org/r/681/#comment1663 The == 0x2 isn't needed since (nz 0x2) will either be 0x2 or 0. That maps to true and false naturally. - Gabe On 2011-05-04 18:44:54, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/681/ --- (Updated 2011-05-04 18:44:54) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Further break up condition code into NZ, C, V bits. Break up the condition code bits into NZ, C, V registers. These are individually written and this removes some incorrect dependencies between instructions. Diffs - src/arch/arm/faults.cc 5a9a639ce16f src/arch/arm/intregs.hh 5a9a639ce16f src/arch/arm/isa/formats/fp.isa 5a9a639ce16f src/arch/arm/isa/formats/pred.isa 5a9a639ce16f src/arch/arm/isa/insts/data.isa 5a9a639ce16f src/arch/arm/isa/insts/fp.isa 5a9a639ce16f src/arch/arm/isa/insts/ldr.isa 5a9a639ce16f src/arch/arm/isa/insts/macromem.isa 5a9a639ce16f src/arch/arm/isa/insts/mem.isa 5a9a639ce16f src/arch/arm/isa/insts/misc.isa 5a9a639ce16f src/arch/arm/isa/insts/mult.isa 5a9a639ce16f src/arch/arm/isa/insts/str.isa 5a9a639ce16f src/arch/arm/isa/operands.isa 5a9a639ce16f src/arch/arm/isa/templates/pred.isa 5a9a639ce16f src/arch/arm/isa/templates/vfp.isa 5a9a639ce16f src/arch/arm/miscregs.hh 5a9a639ce16f src/arch/arm/nativetrace.cc 5a9a639ce16f src/arch/arm/utility.hh 5a9a639ce16f Diff: http://reviews.m5sim.org/r/681/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Trace: Allow printing ASIDs and selectively tracing based on user/kernel code.
On 05/04/11 22:17, Ali Saidi wrote: On 2011-05-04 21:08:25, Gabe Black wrote: CR3 might work. Does the kernel change it on every context switch (user program)? The main reason for having it is when tracing user code the kernel can context switch on you. If you want to see all the code that was executed in a process and compare it to what it should have executed you need some kind of identifier to disambiguate processess. The ASID is an easy way to do this, while rooting around in the Linux process structure is annoying. On 2011-05-04 21:08:25, Gabe Black wrote: src/cpu/SConscript, line 188 http://reviews.m5sim.org/r/678/diff/1/?file=12418#file12418line188 This isn't from your change, but should this be missing ExecSymbol? We don't really need this flag in any case since you can just use Exec and turn off ExecTicks. You're right, I didn't even realize. I always do Exec,-ExecTicks. Leave it, delete it, add it? Let's delete it in a separate change. - Ali --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/678/#review1208 --- On 2011-05-04 18:42:30, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/678/ --- (Updated 2011-05-04 18:42:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Trace: Allow printing ASIDs and selectively tracing based on user/kernel code. Debug flags are ExecUser, ExecKernel, and ExecAsid. ExecUser and ExecKernel are set by default when Exec is specified. Use minus sign with ExecUser or ExecKernel to remove user or kernel tracing respectively. Diffs - src/arch/alpha/utility.hh 5a9a639ce16f src/arch/alpha/utility.cc 5a9a639ce16f src/arch/arm/utility.hh 5a9a639ce16f src/arch/mips/utility.hh 5a9a639ce16f src/arch/power/utility.hh 5a9a639ce16f src/arch/sparc/utility.hh 5a9a639ce16f src/arch/x86/utility.hh 5a9a639ce16f src/cpu/SConscript 5a9a639ce16f src/cpu/exetrace.cc 5a9a639ce16f Diff: http://reviews.m5sim.org/r/678/diff Testing --- Thanks, Ali ___ 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
Re: [m5-dev] [m5-users] Tracing does not work
I think the reason those wouldn't be renamed is that you're only outputing something you could put in a file (in the traditional sense) if you're tracing. If you're turning on/off some other functionality like a debug breakpoint you're not tracing, you're debugging. It gets a little confusing, I think, because we're so used to thinking about using it for tracing and there aren't really true debug flags at the moment. Gabe On 05/07/11 15:15, Joel Hestness wrote: Hey guys, I wasn't sure what the intended outcome with tracing v. debugging was going to be. It sounds like the move is to debug as a more general term, though it will support all of the trace functionality. In that case, my confusion arose from the naming of the flags. Since trace-file and trace-start now go along with the other debug flags (i.e. you wouldn't use them unless you're using the debug flags), it probably makes sense to change the names to reflect the connection. For instance, debug-trace-file and debug-trace-start are clearer and still reflect that you'll be collecting a trace. Joel I was thinking og doing it since Nate is not around. I'll do it soon. instance, trace-flags, and trace-file are still accepted, but they don't do anything now. They should be eliminated from the message. We're also missing the equivalent of trace-start and trace-file. Do you mind cleaning that up? Are you sure that trace-file doesn't work? I've basically renamed --trace-help to --debug-help, so the former can be removed. Also I've renamed --trace-flags to --debug-flags, so that one can be removed too. (I intended to, I just screwed up.) The purpose of renaming trace flags to debug flags is that the flags themselves can be used for a lot more than tracing (I'm starting to use them to insert debugging breakpoints, they're used for exec trace which is really a different tracing facility, they can be used for whatever) and it seemed odd to have two different classes of flags (though we could do that if we wanted to). The only error that I know of right now is that --trace-help and --trace-flags still exist and silently act when they shouldn't. I'm compiling right now, but things are slow on my laptop. I'll test out --trace-file, but I'm not sure why that would have changed at all. Nate ___ 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] changeset in m5: X86: Fix the Lldt instructions so they load the...
changeset 3c628a51f6e1 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=3c628a51f6e1 description: X86: Fix the Lldt instructions so they load the ldtr and not the tr. diffstat: src/arch/x86/isa/insts/system/segmentation.py | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diffs (36 lines): diff -r f64b07758814 -r 3c628a51f6e1 src/arch/x86/isa/insts/system/segmentation.py --- a/src/arch/x86/isa/insts/system/segmentation.py Thu May 05 02:20:31 2011 -0400 +++ b/src/arch/x86/isa/insts/system/segmentation.py Fri May 06 01:00:32 2011 -0700 @@ -223,8 +223,8 @@ ld t2, tsg, [8, t4, t0], 8, dataSize=8 chks reg, t1, LDTCheck wrdh t3, t1, t2 -wrdl tr, t1, reg -wrbase tr, t3, dataSize=8 +wrdl tsl, t1, reg +wrbase tsl, t3, dataSize=8 end: fault NoFault }; @@ -241,8 +241,8 @@ ld t2, tsg, [8, t4, t0], 8, dataSize=8 chks t5, t1, LDTCheck wrdh t3, t1, t2 -wrdl tr, t1, t5 -wrbase tr, t3, dataSize=8 +wrdl tsl, t1, t5 +wrbase tsl, t3, dataSize=8 end: fault NoFault }; @@ -260,8 +260,8 @@ ld t2, tsg, [8, t4, t0], 8, dataSize=8 chks t5, t1, LDTCheck wrdh t3, t1, t2 -wrdl tr, t1, t5 -wrbase tr, t3, dataSize=8 +wrdl tsl, t1, t5 +wrbase tsl, t3, dataSize=8 end: fault NoFault }; ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: X86: Fix the Lldt instructions so they load the...
Whoops, I forgot to put your name on this Tim, sorry about that. Thanks again for your help, and next time you'll get the credit you deserve. Gabe On 05/06/11 04:09, Gabe Black wrote: changeset 3c628a51f6e1 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=3c628a51f6e1 description: X86: Fix the Lldt instructions so they load the ldtr and not the tr. diffstat: src/arch/x86/isa/insts/system/segmentation.py | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diffs (36 lines): diff -r f64b07758814 -r 3c628a51f6e1 src/arch/x86/isa/insts/system/segmentation.py --- a/src/arch/x86/isa/insts/system/segmentation.py Thu May 05 02:20:31 2011 -0400 +++ b/src/arch/x86/isa/insts/system/segmentation.py Fri May 06 01:00:32 2011 -0700 @@ -223,8 +223,8 @@ ld t2, tsg, [8, t4, t0], 8, dataSize=8 chks reg, t1, LDTCheck wrdh t3, t1, t2 -wrdl tr, t1, reg -wrbase tr, t3, dataSize=8 +wrdl tsl, t1, reg +wrbase tsl, t3, dataSize=8 end: fault NoFault }; @@ -241,8 +241,8 @@ ld t2, tsg, [8, t4, t0], 8, dataSize=8 chks t5, t1, LDTCheck wrdh t3, t1, t2 -wrdl tr, t1, t5 -wrbase tr, t3, dataSize=8 +wrdl tsl, t1, t5 +wrbase tsl, t3, dataSize=8 end: fault NoFault }; @@ -260,8 +260,8 @@ ld t2, tsg, [8, t4, t0], 8, dataSize=8 chks t5, t1, LDTCheck wrdh t3, t1, t2 -wrdl tr, t1, t5 -wrbase tr, t3, dataSize=8 +wrdl tsl, t1, t5 +wrbase tsl, t3, dataSize=8 end: fault NoFault }; ___ 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
Re: [m5-dev] src/dest detection in the ISA descriptions
On 04/27/11 22:14, Gabe Black wrote:
My idea is to be able to inherit from the standard op types like
IntRegOperand and allow them to install more than one index and/or
change how they're declared, read, and written. So say for example you
had a 128 bit wide SIMD instruction operating on four floating point
registers which are normally accessible as 32 bit, indexed through some
strange scheme. You could say that operand generates 4 indices,
determined by whatever weird formula or just sequentially. Then you
could define a structure which was the union of an array of 4 32 bit
ints and 4 floats or 2 doubles, or 16 bytes, or ... The constructor
could take the return of reading the 4 indices with
readFloatRegOperandBits to fill it's main array, and then the
instruction could access whatever other representation it needed. The
other direction would work as well. Then, assuming we get this
source/dest thing straightened out, you could have an instruction that,
say, did parallel byte adds defined like this:
for (int i = 0; i 7; i++)
Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i];
And all the other goop would figure itself out. If some other scheme was
needed (runtime selectable width, selectable float vs. int, etc) then
the IntRegOperand subclass combined with the composite operand type
could have the smarts to do the right thing all tidily hidden away after
it was set up. Note that Dest is just a dest, even though it uses a
structure field.
I'm not sure how you're envisioning this will work... are you assuming
there's a full C++ parser like gcc-xml? How would you know that in this
case you're overwriting all of Dest, but if the upper bound of the loop
was
6 and not 7 then it would be just a partial overwrite? That's a level of
compiler analysis I *really* don't want to get into. (Yes, it is
theoretically feasible and it would be pretty slick, but I'm sure there
are
many other things that would have a greater ROI than that.)
Could we do this with C++ operator overloading? Seems like you could
just
say Dest = Source1 + Source2; which would be obvious to our code
parser,
and then have C++ do the magic. The type extensions could be used as
casts
to make this more flexible, e.g., Dest = Source1@bv + Source2@bv could
do
byte-by-byte adds while Source1@fv + Source2@fv could do it by 32-bit
floats, or something like that.
The only tricky thing there is determining what is a source and what is
a dest, although it highlights how hard that can be.
I still don't really get your proposal, in terms of how the code sample you
have parsed above would get turned into a concrete set of source and dest
operand indices.
Very roughly, it would look something like this.
struct SimdData
{
union
{
uint32_t regBits[4];
uint8_t bytes[32];
};
};
def operand_types {{
'SimdData' : 'SimdData'
}};
class SimdOp(FloatRegOperand):
def makeConstructor(self):
return '''
_srcRegIdx[numSrcRegs++] = %s + 0 + FP_Base_DepTag;
_srcRegIdx[numSrcRegs++] = %s + 1 + FP_Base_DepTag;
_srcRegIdx[numSrcRegs++] = %s + 2 + FP_Base_DepTag;
_srcRegIdx[numSrcRegs++] = %s + 3 + FP_Base_DepTag;
'''
def makeRead(self):
return '''
%s.regBits[0] = xc-readFloatRegOperandBits(this, %d + 0,
final_val);
%s.regBits[1] = xc-readFloatRegOperandBits(this, %d + 1,
final_val);
%s.regBits[2] = xc-readFloatRegOperandBits(this, %d + 2,
final_val);
%s.regBits[3] = xc-readFloatRegOperandBits(this, %d + 3,
final_val);
''' % (self.base_name, self.idxStart (?))
def makeWrite(self):
return '''
xc-setFloatRegOperandBits(this, %d, %s.regBits[0]);
xc-setFloatRegOperandBits(this, %d, %s.regBits[1]);
xc-setFloatRegOperandBits(this, %d, %s.regBits[2]);
xc-setFloatRegOperandBits(this, %d, %s.regBits[3]);
''' % (self.idxStart, self.base_name)
def operands {{
'Dest' : ('SimdOp', 'SimdData', 'DEST', 'IsInteger', 1),
'Source1' : ('SimdOp', 'SimdData', 'SOURCE1', 'IsInteger', 2),
'Source2 : ('SimdOp', 'SimdData', 'SOURCE2', 'IsInteger', 3),
}};
Did that make sense? Do you think you'll be able to review those patches
soonish?
Gabe
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Trace: Allow printing ASIDs and selectively tracing based on user/kernel code.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/678/#review1207 --- src/cpu/exetrace.cc http://reviews.m5sim.org/r/678/#comment1657 Maybe put these ifs together with an or? It's not hugely better since the body is so short, but it's something to consider. - Gabe On 2011-05-04 18:42:30, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/678/ --- (Updated 2011-05-04 18:42:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Trace: Allow printing ASIDs and selectively tracing based on user/kernel code. Debug flags are ExecUser, ExecKernel, and ExecAsid. ExecUser and ExecKernel are set by default when Exec is specified. Use minus sign with ExecUser or ExecKernel to remove user or kernel tracing respectively. Diffs - src/arch/alpha/utility.hh 5a9a639ce16f src/arch/alpha/utility.cc 5a9a639ce16f src/arch/arm/utility.hh 5a9a639ce16f src/arch/mips/utility.hh 5a9a639ce16f src/arch/power/utility.hh 5a9a639ce16f src/arch/sparc/utility.hh 5a9a639ce16f src/arch/x86/utility.hh 5a9a639ce16f src/cpu/SConscript 5a9a639ce16f src/cpu/exetrace.cc 5a9a639ce16f Diff: http://reviews.m5sim.org/r/678/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Trace: Allow printing ASIDs and selectively tracing based on user/kernel code.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/678/#review1208 --- src/cpu/SConscript http://reviews.m5sim.org/r/678/#comment1658 This isn't from your change, but should this be missing ExecSymbol? We don't really need this flag in any case since you can just use Exec and turn off ExecTicks. - Gabe On 2011-05-04 18:42:30, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/678/ --- (Updated 2011-05-04 18:42:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Trace: Allow printing ASIDs and selectively tracing based on user/kernel code. Debug flags are ExecUser, ExecKernel, and ExecAsid. ExecUser and ExecKernel are set by default when Exec is specified. Use minus sign with ExecUser or ExecKernel to remove user or kernel tracing respectively. Diffs - src/arch/alpha/utility.hh 5a9a639ce16f src/arch/alpha/utility.cc 5a9a639ce16f src/arch/arm/utility.hh 5a9a639ce16f src/arch/mips/utility.hh 5a9a639ce16f src/arch/power/utility.hh 5a9a639ce16f src/arch/sparc/utility.hh 5a9a639ce16f src/arch/x86/utility.hh 5a9a639ce16f src/cpu/SConscript 5a9a639ce16f src/cpu/exetrace.cc 5a9a639ce16f Diff: http://reviews.m5sim.org/r/678/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: CPU: Fix a case where timing simple cpu faults can nest.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/670/#review1190 --- Could you please walk through when two faults will happen at the same time and why that's a problem? src/cpu/simple/timing.cc http://reviews.m5sim.org/r/670/#comment1646 The reschedule function (with always set) will do the de/rescheduling for you all in one shot. - Gabe On 2011-05-02 15:41:43, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/670/ --- (Updated 2011-05-02 15:41:43) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- CPU: Fix a case where timing simple cpu faults can nest. If we fault, change the state to faulting so that we don't fault again in the same cycle. Diffs - src/cpu/simple/base.hh 3f49ed206f46 src/cpu/simple/timing.cc 3f49ed206f46 Diff: http://reviews.m5sim.org/r/670/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: CPU: Fix a case where timing simple cpu faults can nest.
On 2011-05-03 11:13:49, Gabe Black wrote: Could you please walk through when two faults will happen at the same time and why that's a problem? Ali Saidi wrote: mem op - tlb miss - delayed translation - table walk - fault - fetch - tlb miss - table walk The table walker should never be called twice in one cycle. After the first fault we really want to unwind the call stack, let a cycle go by, and then start fetching handling the next instruction. These cases generate traces that are super hard to debug, unrealistic, and make debugging challenging so we should avoid them. Ah, ok. I think this is similar to that problem where the call stack wraps back around on itself too many times and tracedata gets deleted out from under an earlier frame by a later frame. It's good to break that cycle here, I think. I didn't see anything (other than my comment below) that was objectionable, so if you've tested it go ahead. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/670/#review1190 --- On 2011-05-02 15:41:43, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/670/ --- (Updated 2011-05-02 15:41:43) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- CPU: Fix a case where timing simple cpu faults can nest. If we fault, change the state to faulting so that we don't fault again in the same cycle. Diffs - src/cpu/simple/base.hh 3f49ed206f46 src/cpu/simple/timing.cc 3f49ed206f46 Diff: http://reviews.m5sim.org/r/670/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ruby-stats: support for dump_stats instruction
On 2011-05-03 17:45:41, Brad Beckmann wrote: Can we change the name of Time in base/time.hh instead of Time in Ruby? Right now this patch touches 50+ Ruby files and a bunch of lines within those files just to change Time to RTime. It seems that far fewer changes would be required to change the name of base/time.hh's version of Time. Nathan Binkert wrote: While I wouldn't be opposed to changing the time in base/time.hh if we needed both, but don't we need to move ruby away from its own version of Time anyway? Shouldn't we be using Tick? Also, we've generally never shied away from changes that can be done with a one line sed/perl script. I was about to say something along these lines. This would be a good chance to consolidate Ruby and M5 systems a little. It might be a good idea to do the simple find/replace change on its own so the non-simple non-find/replace stuff doesn't get lost as noise. I don't have deep feelings one way or the other, though. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/675/#review1195 --- On 2011-05-03 11:20:58, Korey Sewell wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/675/ --- (Updated 2011-05-03 11:20:58) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ruby-stats: support for dump_stats instruction *** NOTE: The core changes for this diff are in Profiler.cc/hh, stat_control.hh/cc, and pseudo_inst.cc This is a first pass toward getting dump-stats functionality to work cleanly for Ruby. As is, the patch works, but there needs to be discussion over: - Changing Ruby typedef Time to RTime because it conflicts with the Time class defined in base/time.hh (The majority of files are renames)... Is there a better name than RTime? - Where is the right place for this RubyStatEvent code? I hesitated to do any real cosmetic changes because of what impending stat changes might do. I have two thoughts: (1) If Ruby Stats will be registered like old M5 stats, then this code would nicely fold into the old statEvent code in sim_control.cc. Fold into maybe too strong of a phrase even, as most of it should just naturally work. (2) If Ruby Stats are not registered, then maybe placing this code into the RubySystem class. I realized late that the Profiler and Network have two different stats that they track so the RubySystem would be the right place along with calling the namespace RubyStats. Diffs - SConstruct 3f49ed206f46 src/cpu/testers/rubytest/RubyTester.hh 3f49ed206f46 src/cpu/testers/rubytest/RubyTester.cc 3f49ed206f46 src/mem/ruby/buffers/MessageBuffer.hh 3f49ed206f46 src/mem/ruby/buffers/MessageBuffer.cc 3f49ed206f46 src/mem/ruby/buffers/MessageBufferNode.hh 3f49ed206f46 src/mem/ruby/common/Consumer.hh 3f49ed206f46 src/mem/ruby/common/Global.hh 3f49ed206f46 src/mem/ruby/common/TypeDefines.hh 3f49ed206f46 src/mem/ruby/eventqueue/RubyEventQueue.hh 3f49ed206f46 src/mem/ruby/eventqueue/RubyEventQueue.cc 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.hh 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.hh 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.hh 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/OutVcState_d.hh 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/VirtualChannel_d.hh 3f49ed206f46 src/mem/ruby/network/garnet/fixed-pipeline/flit_d.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/FlexibleConsumer.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/InVcState.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/InVcState.cc 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/OutVcState.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/OutVcState.cc 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/Router.hh 3f49ed206f46 src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 3f49ed206f46
Re: [m5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks
On 2011-05-02 12:51:56, Nathan Binkert wrote: SConstruct, line 247 http://reviews.m5sim.org/r/668/diff/1/?file=12211#file12211line247 Should we prompt the user for permission? That way the user would at least know that something happened. Steve Reinhardt wrote: I doubt most people care. How many would really say no? Most people just want things to be automatic. If they really do care they can always edit it out later (the comments will let them know where the hooks came from). Nathan Binkert wrote: I agree that nobody would say no, but most people also would never notice that it had happened because the message would just fly by if there was no prompt. I guess that you're arguing that that would be a good thing :) Changing config files behind peoples back is a really bad idea in my opinion. I know I've stopped using entire distros (Suse) because they mucked with config files behind my back and perpetually broke my system. My configs are mine, and the minimal level of respect for that would be if we asked permission before we let ourselves in. I think leaving well enough alone and having the user go in and fix it themselves is actually the best approach. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/668/#review1173 --- On 2011-05-02 12:34:56, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/668/ --- (Updated 2011-05-02 12:34:56) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- SConstruct: automatically update .hg/hgrc with style hooks Seems easier than pestering people about it. Note also that path is now absolute, so you don't get errors when invoking hg from subdirectories. Diffs - SConstruct 66a3187a6714 Diff: http://reviews.m5sim.org/r/668/diff Testing --- Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Add vfpv3 support to native trace.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/669/#review1181 --- src/arch/arm/nativetrace.cc http://reviews.m5sim.org/r/669/#comment1619 You should divide by two here instead of shifting by 1. It's more obvious what you're doing, and the compiler will be smart enough to use a shift it it's faster. src/arch/arm/nativetrace.cc http://reviews.m5sim.org/r/669/#comment1618 spaces around the + util/statetrace/arch/arm/tracechild.cc http://reviews.m5sim.org/r/669/#comment1621 Hmm... I wonder how that got there? Good catch. util/statetrace/arch/arm/tracechild.cc http://reviews.m5sim.org/r/669/#comment1622 I don't know how easy this would be to accommodate, but you're going to be sending a bunch of extra zeros for int regs that aren't 64 bits wide. Can you make it so you send full 64 bit values only when the source is actually 64 bits wide? util/statetrace/arch/arm/tracechild.cc http://reviews.m5sim.org/r/669/#comment1623 The idea is to verify that you're not falling off of uregs. Maybe you could do something more flexible like sizeof(myregs.uregs) / sizeof (myregs.uregs[0]). util/statetrace/arch/arm/tracechild.cc http://reviews.m5sim.org/r/669/#comment1624 The same comment applies as in getRegs, except that you have to deal with an offset. It would probably be a good idea to define something in the enum to mark the start of the FP regs. You can move the assert to after the if and subtract out the offset right before indexing fpregs. util/statetrace/arch/arm/tracechild.cc http://reviews.m5sim.org/r/669/#comment1625 Just because libc would use a macro doesn't mean we have to. You should replace this with a constant of the appropriate type. util/statetrace/arch/arm/tracechild.cc http://reviews.m5sim.org/r/669/#comment1626 Is the cast actually necessary here? I can believe it is to avoid a warning, but you could try leaving it out if you haven't already. - Gabe On 2011-05-02 15:41:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/669/ --- (Updated 2011-05-02 15:41:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Add vfpv3 support to native trace. Diffs - src/arch/arm/nativetrace.hh 3f49ed206f46 src/arch/arm/nativetrace.cc 3f49ed206f46 util/statetrace/arch/arm/tracechild.hh 3f49ed206f46 util/statetrace/arch/arm/tracechild.cc 3f49ed206f46 Diff: http://reviews.m5sim.org/r/669/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: CPU: Add some useful debug message to the timing simple cpu.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/671/#review1184 --- Generally speaking, I'd like to see us move towards using the same traceflags across ISAs and CPUs. If I'm using the SimpleCPU, being able to select DPRINTFs specific to the SimpleCPU isn't that useful. I'm not going to hit an O3 DPRINTF whether I have them turned on or not. Instead, it might be better to, say, use a Fetch traceflag to go with Fetch actions. Then you can be more specific and have to remember fewer particulars of the thing you're working with. You could have a compound traceflag called SimpleCPU (or just CPU?) which would turn on all the basics. It might get a little tricky if you have multiple CPU types and wanted to turn on only the ones in the SimpleCPU, but I expect that to be relatively uncommon. I'd encourage you to think about changing things around like this, but I understand it expands the scope of what you were trying to do significantly. I'm ok if you leave it as is. - Gabe On 2011-05-02 15:41:50, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/671/ --- (Updated 2011-05-02 15:41:50) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- CPU: Add some useful debug message to the timing simple cpu. Diffs - src/cpu/simple/timing.cc 3f49ed206f46 Diff: http://reviews.m5sim.org/r/671/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: ARM: Add vfpv3 support to native trace.
On 2011-05-02 16:42:25, Gabe Black wrote: util/statetrace/arch/arm/tracechild.cc, line 79 http://reviews.m5sim.org/r/669/diff/1/?file=12215#file12215line79 I don't know how easy this would be to accommodate, but you're going to be sending a bunch of extra zeros for int regs that aren't 64 bits wide. Can you make it so you send full 64 bit values only when the source is actually 64 bits wide? Ali Saidi wrote: There is no reason to worry about sending 4 bytes down the wire. The speed issue isn't about sending 4 bytes, it's all about having to put a breakpoint after every instruction. The reason I implemented sending diffs of the state instead of sending the whole state is that what you send over the wire -does- matter, significantly. Breakpoints are slow too, but that doesn't mean everything else is irrelevant. On 2011-05-02 16:42:25, Gabe Black wrote: util/statetrace/arch/arm/tracechild.cc, line 104 http://reviews.m5sim.org/r/669/diff/1/?file=12215#file12215line104 The idea is to verify that you're not falling off of uregs. Maybe you could do something more flexible like sizeof(myregs.uregs) / sizeof (myregs.uregs[0]). Ali Saidi wrote: uregs is never going to get smaller than it is now and I don't see a reason to come up with a crazy assert to try and prove that it isn't. uregs changing size is irrelevant. That formula will be exactly right all the time and doesn't depend on the coincidence that the CPSR is last. On 2011-05-02 16:42:25, Gabe Black wrote: util/statetrace/arch/arm/tracechild.cc, line 111 http://reviews.m5sim.org/r/669/diff/1/?file=12215#file12215line111 The same comment applies as in getRegs, except that you have to deal with an offset. It would probably be a good idea to define something in the enum to mark the start of the FP regs. You can move the assert to after the if and subtract out the offset right before indexing fpregs. Ali Saidi wrote: There are clearly 32 float registers defined in the enum and in the struct. The assert just verifies that we're actually accessing a floating point register when we should be. We don't need to verify the structure size it's correct by construction. I wrote the original assert and know what it's for, verifying the index and not the structure. Again, it assumes F0 is the first FP reg which is arbitrary. On 2011-05-02 16:42:25, Gabe Black wrote: util/statetrace/arch/arm/tracechild.cc, line 129 http://reviews.m5sim.org/r/669/diff/1/?file=12215#file12215line129 Just because libc would use a macro doesn't mean we have to. You should replace this with a constant of the appropriate type. Ali Saidi wrote: I disagree... This will transition nicely as soon as libc gets it's act together. The fact that gcc uses macros is an unfortunate historical artifact, not a valid design decision. The transition won't be that nice either since it'll leave macro cruft in the source. We should either use their macro because we have to, or use our own thing that doesn't suck because it makes sense. Here we're combining the downsides of both of those approaches. On 2011-05-02 16:42:25, Gabe Black wrote: util/statetrace/arch/arm/tracechild.cc, line 151 http://reviews.m5sim.org/r/669/diff/1/?file=12215#file12215line151 Is the cast actually necessary here? I can believe it is to avoid a warning, but you could try leaving it out if you haven't already. Ali Saidi wrote: might as well be explicit It's not a huge thing, but if it's not required it's mostly visual noise. People will usually not care too much about exactly what type is being used, and if they do they can look at the function return type. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/669/#review1181 --- On 2011-05-02 15:41:28, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/669/ --- (Updated 2011-05-02 15:41:28) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- ARM: Add vfpv3 support to native trace. Diffs - src/arch/arm/nativetrace.hh 3f49ed206f46 src/arch/arm/nativetrace.cc 3f49ed206f46 util/statetrace/arch/arm/tracechild.hh 3f49ed206f46 util/statetrace/arch/arm/tracechild.cc 3f49ed206f46 Diff: http://reviews.m5sim.org/r/669/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks
On 2011-05-02 12:51:56, Nathan Binkert wrote: SConstruct, line 247 http://reviews.m5sim.org/r/668/diff/1/?file=12211#file12211line247 Should we prompt the user for permission? That way the user would at least know that something happened. Steve Reinhardt wrote: I doubt most people care. How many would really say no? Most people just want things to be automatic. If they really do care they can always edit it out later (the comments will let them know where the hooks came from). Nathan Binkert wrote: I agree that nobody would say no, but most people also would never notice that it had happened because the message would just fly by if there was no prompt. I guess that you're arguing that that would be a good thing :) Gabe Black wrote: Changing config files behind peoples back is a really bad idea in my opinion. I know I've stopped using entire distros (Suse) because they mucked with config files behind my back and perpetually broke my system. My configs are mine, and the minimal level of respect for that would be if we asked permission before we let ourselves in. I think leaving well enough alone and having the user go in and fix it themselves is actually the best approach. Nathan Binkert wrote: This isn't a config file in your homedir, it's the config file in the M5 repo and it will only be changed if you don't have it set up right, so it's not all that bad. I have the same leaning that you do, so I think I'd rather see a prompt for the user, but I don't feel that strongly about it. Steve Reinhardt wrote: Yea, I would feel differently about ~/.hgrc, but this is just a local config file, and it's one that will need updating each time you clone a new repo. I don't mind making the warning a little more prominent, but since the status quo is that we won't even try to compile if the hooks aren't there, it seems overkill to me to make too big a deal of it... is someone going to be so offended about this that they're going to go try a different simulator? Plus if we could find a way to get these hooks to be part of the m5 repo so that they were automatically in place whenever you cloned, we would have done that, so I don't see how this is much different. I'm just afraid of this happening, causing some problem, somebody spending a long time and discovering (or not discovering) that some config file was changed without their knowledge and that's what was breaking everything. I can't imagine somebody not being a little miffed about that. But if you think it's really really safe and you've made it obvious what happened then I guess it could be ok. Maybe make it stop scons after that warning prints if those lines aren't there so it's harder to miss? Then they can just run it again and it'll go through, but they at least know what happened. And yes, I have heard of people that have tried to use M5, got frustrated and gone elsewhere. It is very possible to be too annoying or difficult and to drive away users. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/668/#review1173 --- On 2011-05-02 12:34:56, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/668/ --- (Updated 2011-05-02 12:34:56) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- SConstruct: automatically update .hg/hgrc with style hooks Seems easier than pestering people about it. Note also that path is now absolute, so you don't get errors when invoking hg from subdirectories. Diffs - SConstruct 66a3187a6714 Diff: http://reviews.m5sim.org/r/668/diff Testing --- Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] symbol tables and object files
Hey, does anyone know why we don't have a pointer to the symbol table in the object file objects we have? If there's no symbol table then it could be NULL, but that seems like a pretty clear relationship and it would be handy to group those together instead of maintaining them in parallel. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] changeset in m5: Util: Replace mkblankimage.sh with the new gem5...
changeset 7939dd0c4ff2 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=7939dd0c4ff2
description:
Util: Replace mkblankimage.sh with the new gem5img.py.
This change replaces the mkblankimage.sh script, used for creating new
disk
images, with a new gem5img.py script. The new version is written in
python
instead of bash, takes its parameters from command line arguments
instead of
prompting for them, and finds a free loopback device dynamically
instead of
hardcoding /dev/loop1. The file system used is now optionally
configurable,
and the blank image is filled by a hole left by lseek and write
instead of
literally filling it with zeroes.
The functionality of the new script is broken into subcommands init,
mount, umount, new, partition, and format. init creates a
new file
of the appropriate size, partitions it, and then formats the first (and
only)
new parition. mount attaches a new loopback device to the first
parition of
the image file and mounts it to the specified mount point. umount
unmounts
the specified mount point and identifies and cleans up the underlying
loopback
device. new, partition, and format are the individual stages of
init
but broken out so they can be run individually. That's so an image can
be
reinitialized in place if needed.
Two features of the original script are being dropped. The first is the
ability to specify a source directory to copy into the new file system.
The
second is the ability to specify a list of commands to run which are
expected
to (but not required to) update the permissions of the files in the new
fs.
Both of these seem easy enough to do manually, especially given the
mount
and umount commands, that removing them would meaningfully simplify
the
script without making it less useful.
diffstat:
util/gem5img.py | 372 +++
util/mkblankimage.sh | 219 --
2 files changed, 372 insertions(+), 219 deletions(-)
diffs (truncated from 599 to 300 lines):
diff -r dba41dbef071 -r 7939dd0c4ff2 util/gem5img.py
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/gem5img.py Fri Apr 29 04:46:19 2011 -0400
@@ -0,0 +1,372 @@
+#!/usr/bin/python
+#
+# gem5img.py
+# Script for managing a gem5 disk image.
+#
+
+from optparse import OptionParser
+import os
+from os import environ as env
+import string
+from subprocess import CalledProcessError, Popen, PIPE, STDOUT
+from sys import exit, argv
+
+
+# Some constants.
+MaxLBACylinders = 16383
+MaxLBAHeads = 16
+MaxLBASectors = 63
+MaxLBABlocks = MaxLBACylinders * MaxLBAHeads * MaxLBASectors
+
+BlockSize = 512
+MB = 1024 * 1024
+
+# Setup PATH to look in the sbins.
+env['PATH'] += ':/sbin:/usr/sbin'
+
+# Whether to print debug output.
+debug = False
+
+# Figure out cylinders, heads and sectors from a size in blocks.
+def chsFromSize(sizeInBlocks):
+if sizeInBlocks = MaxLBABlocks:
+sizeInMBs = (sizeInBlocks * BlockSize) / MB
+print '%d MB is too big for LBA, truncating file.' % sizeInMBs
+return (MaxLBACylinders, MaxLBAHeads, MaxLBASectors)
+
+sectors = sizeInBlocks
+if sizeInBlocks 63:
+sectors = 63
+
+headSize = sizeInBlocks / sectors
+heads = 16
+if headSize 16:
+heads = sizeInBlocks
+
+cylinders = sizeInBlocks / (sectors * heads)
+
+return (cylinders, heads, sectors)
+
+
+# Figure out if we should use sudo.
+def needSudo():
+if not hasattr(needSudo, 'notRoot'):
+needSudo.notRoot = (os.geteuid() != 0)
+if needSudo.notRoot:
+print 'You are not root. Using sudo.'
+return needSudo.notRoot
+
+# Run an external command.
+def runCommand(command, inputVal=''):
+print %, ' '.join(command)
+proc = Popen(command, stdin=PIPE)
+proc.communicate(inputVal)
+return proc.returncode
+
+# Run an external command and capture its output. This is intended to be
+# used with non-interactive commands where the output is for internal use.
+def getOutput(command, inputVal=''):
+global debug
+if debug:
+print %, ' '.join(command)
+proc = Popen(command, stderr=STDOUT,
+ stdin=PIPE, stdout=PIPE)
+(out, err) = proc.communicate(inputVal)
+return (out, proc.returncode)
+
+# Run a command as root, using sudo if necessary.
+def runPriv(command, inputVal=''):
+realCommand = command
+if needSudo():
+realCommand = [findProg('sudo')] + command
+return runCommand(realCommand, inputVal)
+
+def privOutput(command, inputVal=''):
+realCommand = command
+if needSudo():
+realCommand = [findProg('sudo')] + command
+return getOutput(realCommand, inputVal)
+
+# Find the path to a program.
+def findProg(program, cleanupDev=None):
+(out, returncode) =
Re: [m5-dev] src/dest detection in the ISA descriptions
Before I mentioned using @, and I have a patch that makes that change in the parser and in all the ISA descs. It was less painful than you may assume. I haven't looked at your patches yet... is the main point of switching from . to @ to allow structure field accesses? Basically. It could be structure fields or the bitfields of a BitUnion. This had actually cropped up with the Twin(32|64)_t types SPARC uses, and to get around it we added clunky parenthesis so the .foo didn't sit directly next to the operand. gcc figured it out, but it could slip by the parser. I don't think there's a good way to figure out whether operands are a source and/or destination in a setup like this. In the past, I've tried to come up with a scheme that would force gcc/C++ to figure it out for us, but no matter how clever/depraved I allow my solutions to be, they still don't work. It might be possible to use a fancy compiler system like LLVM to build a new tool to do it, but that isn't a near term solution (although I wouldn't rule it out entirely). One potentially interesting angle to pursue is gcc-xml... I don't know much about it except that, as the name implies, it generates an xml representation rather than machine code, which should be reasonably analyzable. I bet it's still hairy, but it might be the least hairy alternative. I expect the big problem with most of these approaches is that the final compilation context for most of the C snippets is not known at the time the code is parsed by the parser for operands etc. Thus there will be symbols that you can't disambiguate as types etc. because the proper include files haven't been included (which could be undecidable if the proper include files are also being generated in the same isa_parser run, so maybe they don't even exist yet at the point where you need them). The XML thing is an interesting possibility, and it avoids having to make a whole new thing that understands C++. It would still mean we'd have to make a whole new thing that understands XML (which is much easier), but then there's the chicken and egg issue you mention. I don't think we do anything sneaky with deciding what header files to include, though, and we could probably tell the tool about the operands somehow, maybe by making them globals that get thrown away later. The current approach is far from 100% correct in that sense, so if we're a little fuzzy around the edges that should be ok. This is why the ad hoc yet (as you say) surprisingly effective heuristics were used. Perhaps the heuristics could simply be extended to deal with structure field accesses... if the thing after the symbol is a ., then you look past that to see if there's an equals sign or not, and behave appropriately. appropriately isn't clearly defined, really. There's an example in ARM where there's a bit broken out of a status register which is set if some condition happens. If you set it, you're really setting the whole thing even though it looks like you're just setting the bitfield. Treating that operand as a source isn't necessary in that case even though it would normally be. Another solution might be to explicitly list source and destination operands like inline assembly blocks. I definitely don't like this idea. One of the main motivations for the current implementation was the old SimpleScalar ISA definition macros, which required explicit listing of operands... and which as a result had subtle bugs that went undetected for years where particular dependencies weren't being enforced because someone left an operand out of the list. As you say, the current approach works remarkably well most of the time, so while I'm all for improving it and addressing its shortcomings, I see no need to throw it out and replace it with something that's more manual effort, less concise, and more error prone. An extension that allows you to manually override what the parser finds. to deal with those cases where it's getting confused, would be fine though... basically sort of what you've been doing already, except that instead of overriding its heuristics by coming up with awkward code sequences that trick it into doing the right thing, you have a more blunt and effective path to do that. That's probably not as big a step forward as you want, but to me it's preferable to just switching to a fully manual explicit scheme. The manual override would get us there, but one thing I worry about is that if BitUnions or composite operands are used often, you might need to have them all over the place. Some sort of inheritance or being able to associate the overrides with a blob of code once that gets reused a lot (like setting condition codes, checking predicates, etc.) would help. Any ideas how we can get this to work better? This is a big stumbling block for this sort of set up, and this will lead to operand types which can be automatically composited from component values instead of
Re: [m5-dev] Problem with LLDT on x86?
There's a good chance the implementation was copied from LTR and not
completely updated. Please go ahead and make the switch, and if it works
I'm willing to say that's what the problem is. I'll look it over once
more carefully if that works and we're going to commit it.
Gabe
On 04/27/11 04:47, Tim Harris (RESEARCH) wrote:
Hi,
I'm trying to debug some user-mode software that is using the LDT in ways
which AFAIK are correct on x86-64, but which are provoking a GPF when running
in FS mode on M5 (either the tip, or some older trees I've been using).
This is running on the Barrelfish OS, so the way the software's using the LDT
may well be different from Linux or other cases that have been used in the
past.
The GPF happens at a mov %ax, %fs instruction. %ax is 7 = RPL=3, and use
the first entry in the LDT. It looks like the LDT is 0, and the GPF occurs
when accessing address 0 to load the first entry. The program previously
initializes the LDT via an LLDT instruction in a system call, but it looks
like the value is not being written correctly.
Looking at LLDT_R in src/arch/x86/isa/insts/system/segmentation.py:
def macroop LLDT_R
{
.serializing
chks reg, t0, InGDTCheck, flags=(EZF,)
br label(end), flags=(CEZF,)
limm t4, 0, dataSize=8
srli t4, reg, 3, dataSize=2
ldst t1, tsg, [8, t4, t0], dataSize=8
ld t2, tsg, [8, t4, t0], 8, dataSize=8
chks reg, t1, LDTCheck
wrdh t3, t1, t2
wrdl tr, t1, reg
wrbase tr, t3, dataSize=8
end:
fault NoFault
};
Is there a bug with the final two stores to TR? I'd been expecting the code
to be setting the base and limit of TSL somewhere (by analogy with TSG in
LGDT). I'm not sure I understand x86 segmentation to be very confident of
writing a fix - e.g., if it's just TR-TSL, or if there's something more
subtle needed (or indeed if I'm completely wrong here!).
Thanks,
Tim
___
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
Re: [m5-dev] src/dest detection in the ISA descriptions
On 04/27/11 08:02, Steve Reinhardt wrote: On Wed, Apr 27, 2011 at 1:21 AM, Gabe Black gbl...@eecs.umich.edu wrote: The XML thing is an interesting possibility, and it avoids having to make a whole new thing that understands C++. It would still mean we'd have to make a whole new thing that understands XML (which is much easier), but then there's the chicken and egg issue you mention. I don't think we do anything sneaky with deciding what header files to include, though, and we could probably tell the tool about the operands somehow, maybe by making them globals that get thrown away later. The current approach is far from 100% correct in that sense, so if we're a little fuzzy around the edges that should be ok. My concern is that parsing C is a much more complex and strictly defined thing than what we currently do... the current approach degrades gracefully in the sense that it might not get the results you want, but it never fails completely. In contrast, if there's a situation where X is a type but gcc thinks it's a variable (or vice versa), the parsing could break down pretty completely. This is why the ad hoc yet (as you say) surprisingly effective heuristics were used. Perhaps the heuristics could simply be extended to deal with structure field accesses... if the thing after the symbol is a ., then you look past that to see if there's an equals sign or not, and behave appropriately. appropriately isn't clearly defined, really. There's an example in ARM where there's a bit broken out of a status register which is set if some condition happens. If you set it, you're really setting the whole thing even though it looks like you're just setting the bitfield. Treating that operand as a source isn't necessary in that case even though it would normally be. appropriately was just a placeholder for stuff I'm sure we can figure out later. Maybe there are two classes of fields, one that is a partial field (so that writing it means you have a source and a dest) and one that is a complete field (so that writing it implies just a dest, not a source). If you want to automatically detect that you've written a set of partial fields that covers the entire structure and thus you don't need to consider it a source, I think you're asking too much... that's a place where either a manual override or just using the current approach and adding an extra useless assignment to the entire dest would be best, I think. My point is that it -is- too complicated to try to figure out if you overwrite an entire dest or just part of it. That's why you need to be able to say which it is by hand. The manual override would get us there, but one thing I worry about is that if BitUnions or composite operands are used often, you might need to have them all over the place. Some sort of inheritance or being able to associate the overrides with a blob of code once that gets reused a lot (like setting condition codes, checking predicates, etc.) would help. Those all sound reasonable to me. My idea is to be able to inherit from the standard op types like IntRegOperand and allow them to install more than one index and/or change how they're declared, read, and written. So say for example you had a 128 bit wide SIMD instruction operating on four floating point registers which are normally accessible as 32 bit, indexed through some strange scheme. You could say that operand generates 4 indices, determined by whatever weird formula or just sequentially. Then you could define a structure which was the union of an array of 4 32 bit ints and 4 floats or 2 doubles, or 16 bytes, or ... The constructor could take the return of reading the 4 indices with readFloatRegOperandBits to fill it's main array, and then the instruction could access whatever other representation it needed. The other direction would work as well. Then, assuming we get this source/dest thing straightened out, you could have an instruction that, say, did parallel byte adds defined like this: for (int i = 0; i 7; i++) Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i]; And all the other goop would figure itself out. If some other scheme was needed (runtime selectable width, selectable float vs. int, etc) then the IntRegOperand subclass combined with the composite operand type could have the smarts to do the right thing all tidily hidden away after it was set up. Note that Dest is just a dest, even though it uses a structure field. I'm not sure how you're envisioning this will work... are you assuming there's a full C++ parser like gcc-xml? How would you know that in this case you're overwriting all of Dest, but if the upper bound of the loop was 6 and not 7 then it would be just a partial overwrite? That's a level of compiler analysis I *really* don't want to get into. (Yes, it is theoretically feasible and it would be pretty slick, but I'm sure there are many other things that would have
[m5-dev] Review Request: X86: Convert operand types to the new style.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/657/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: Convert operand types to the new style. Diffs - src/arch/x86/isa/operands.isa de679a068dd8 Diff: http://reviews.m5sim.org/r/657/diff Testing --- Thanks, Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/ --- (Updated 2011-04-24 03:48:03.724082) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Util: Replace mkblankimage.sh with the new gem5img.py. This change replaces the mkblankimage.sh script, used for creating new disk images, with a new gem5img.py script. The new version is written in python instead of bash, takes its parameters from command line arguments instead of prompting for them, and finds a free loopback device dynamically instead of hardcoding /dev/loop1. The file system used is now optionally configurable, and the blank image is filled by a hole left by lseek and write instead of literally filling it with zeroes. The functionality of the new script is broken into subcommands init, mount, umount, new, partition, and format. init creates a new file of the appropriate size, partitions it, and then formats the first (and only) new parition. mount attaches a new loopback device to the first parition of the image file and mounts it to the specified mount point. umount unmounts the specified mount point and identifies and cleans up the underlying loopback device. new, partition, and format are the individual stages of init but broken out so they can be run individually. That's so an image can be reinitialized in place if needed. Two features of the original script are being dropped. The first is the ability to specify a source directory to copy into the new file system. The second is the ability to specify a list of commands to run which are expected to (but not required to) update the permissions of the files in the new fs. Both of these seem easy enough to do manually, especially given the mount and umount commands, that removing them would meaningfully simplify the script without making it less useful. Diffs (updated) - util/gem5img.py PRE-CREATION util/mkblankimage.sh d8ec0a7b3f0c Diff: http://reviews.m5sim.org/r/644/diff Testing --- Thanks, Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
This new version adds a % prompt in front of echoed commands to make them more obvious, cleans up some minor redundancy in one of the functions, replaces the needSudo singleton with a simpler function, and now measures the offset of the first partition in the image instead of hard coding it. The hard coded value would be correct for images above a certain size where the number of sectors saturates at 63 and the DOS compatibility region is in place, but may not be correct otherwise. Gabe On 04/24/11 06:48, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/ --- (Updated 2011-04-24 03:48:03.724082) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Util: Replace mkblankimage.sh with the new gem5img.py. This change replaces the mkblankimage.sh script, used for creating new disk images, with a new gem5img.py script. The new version is written in python instead of bash, takes its parameters from command line arguments instead of prompting for them, and finds a free loopback device dynamically instead of hardcoding /dev/loop1. The file system used is now optionally configurable, and the blank image is filled by a hole left by lseek and write instead of literally filling it with zeroes. The functionality of the new script is broken into subcommands init, mount, umount, new, partition, and format. init creates a new file of the appropriate size, partitions it, and then formats the first (and only) new parition. mount attaches a new loopback device to the first parition of the image file and mounts it to the specified mount point. umount unmounts the specified mount point and identifies and cleans up the underlying loopback device. new, partition, and format are the individual stages of init but broken out so they can be run individually. That's so an image can be reinitialized in place if needed. Two features of the original script are being dropped. The first is the ability to specify a source directory to copy into the new file system. The second is the ability to specify a list of commands to run which are expected to (but not required to) update the permissions of the files in the new fs. Both of these seem easy enough to do manually, especially given the mount and umount commands, that removing them would meaningfully simplify the script without making it less useful. Diffs (updated) - util/gem5img.py PRE-CREATION util/mkblankimage.sh d8ec0a7b3f0c Diff: http://reviews.m5sim.org/r/644/diff Testing --- Thanks, Gabe ___ 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
Re: [m5-dev] Review Request: O3: Tighten memory order violation checking to 16 bytes.
I have a plan for how to fix this which won't be that difficult, but since it's going to be near the heart of all the x86 instruction machinery I'd like to be able to test it. I could put together a specialized test case which has a malformed instruction that should hit this even in the simple CPUs, but it would be easier if you can just give me a command line. If it takes a long time to run or I'd need extra patches then don't worry about it. Gabe On 04/22/11 10:25, Gabe Black wrote: It looks like this patch was committed, but it didn't have anything to do with the assert. It just happened to change whether or not that assert was hit. I'll take a look at it sometime soon. Gabe On 04/22/11 10:04, nathan binkert wrote: Did this ever get committed? I'm running into this bug with 20.parser. Nate On Wed, Mar 30, 2011 at 8:46 AM, Ali Saidi sa...@umich.edu wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/520/ I think the updated patch addresses all of your issues Gabe. I tested it with an opt binary and one problem jumped out in x86 for 20.parser an assert: m5.opt: build/X86_SE/arch/x86/emulenv.cc:49: void X86ISA::EmulEnv::doModRM(const X86ISA::ExtMachInst): Assertion `machInst.modRM.mod != 3' failed. It looks like the assert shouldn't be there and is hit during some miss speculation. - Ali On March 30th, 2011, 8:41 a.m., Ali Saidi wrote: Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Ali Saidi. *Updated 2011-03-30 08:41:48* Description O3: Tighten memory order violation checking to 16 bytes. The comment in the code suggests that the checking granularity should be 16 bytes, however in reality the shift by 8 is 256 bytes which seems much larger than required. Diffs - src/cpu/base_dyn_inst.hh (d54b7775a6b0) - src/cpu/o3/O3CPU.py (d54b7775a6b0) - src/cpu/o3/lsq_unit.hh (d54b7775a6b0) - src/cpu/o3/lsq_unit_impl.hh (d54b7775a6b0) View Diff http://reviews.m5sim.org/r/520/diff/ ___ 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
Re: [m5-dev] Review Request: O3: Tighten memory order violation checking to 16 bytes.
Never mind. The test was easy to write. Gabe On 04/23/11 13:55, Gabe Black wrote: I have a plan for how to fix this which won't be that difficult, but since it's going to be near the heart of all the x86 instruction machinery I'd like to be able to test it. I could put together a specialized test case which has a malformed instruction that should hit this even in the simple CPUs, but it would be easier if you can just give me a command line. If it takes a long time to run or I'd need extra patches then don't worry about it. Gabe On 04/22/11 10:25, Gabe Black wrote: It looks like this patch was committed, but it didn't have anything to do with the assert. It just happened to change whether or not that assert was hit. I'll take a look at it sometime soon. Gabe On 04/22/11 10:04, nathan binkert wrote: Did this ever get committed? I'm running into this bug with 20.parser. Nate On Wed, Mar 30, 2011 at 8:46 AM, Ali Saidi sa...@umich.edu wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/520/ I think the updated patch addresses all of your issues Gabe. I tested it with an opt binary and one problem jumped out in x86 for 20.parser an assert: m5.opt: build/X86_SE/arch/x86/emulenv.cc:49: void X86ISA::EmulEnv::doModRM(const X86ISA::ExtMachInst): Assertion `machInst.modRM.mod != 3' failed. It looks like the assert shouldn't be there and is hit during some miss speculation. - Ali On March 30th, 2011, 8:41 a.m., Ali Saidi wrote: Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Ali Saidi. *Updated 2011-03-30 08:41:48* Description O3: Tighten memory order violation checking to 16 bytes. The comment in the code suggests that the checking granularity should be 16 bytes, however in reality the shift by 8 is 256 bytes which seems much larger than required. Diffs - src/cpu/base_dyn_inst.hh (d54b7775a6b0) - src/cpu/o3/O3CPU.py (d54b7775a6b0) - src/cpu/o3/lsq_unit.hh (d54b7775a6b0) - src/cpu/o3/lsq_unit_impl.hh (d54b7775a6b0) View Diff http://reviews.m5sim.org/r/520/diff/ ___ 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
Re: [m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
On 2011-04-18 21:20:49, Nathan Binkert wrote: util/gem5img.py, line 64 http://reviews.m5sim.org/r/644/diff/1/?file=11664#file11664line64 This is pretty similar to m5.util.readCommand which made me think that it might be nice if we put your utility functions here in m5.util Gabe Black wrote: Can I use m5.util from an arbitrary python script? If I can that's good to know. Also, how does readCommand work? Does it pass through stdout/stderr or capture it? Depending on the answer it might be an appropriate replacement for this or the subsequent getOutput, but changing only one obscures the similarities between the two functions. If you're advocating adding a new version of readCommand that has the other behavior then that makes sense. I also funnel text into stdin for input, and I think sudo happens to still work because it goes around any redirection I set up. Nathan Binkert wrote: Yes, you can. Several scripts do that. You have to get it into your path though. Check out the style hook. I suggest just looking at readCommand and the examples to see exactly what it does. I looked at it and it is similar, but it's different enough that they can't be interchanged. My functions all allow passing a string in as standard input and return the error code, but readCommand doesn't. I could extend it to take an input string, but it's not clear how to return the output and the error code without changing all the call sites and complicating all the code that's working fine now without it. readCommand doesn't allow printing the command before it's run so I'd have to add a wrapper to handle that, and it doesn't handle adding 'sudo'. I think tying in readCommand would actually make everything bigger and more complicated because it's not quite what I need and I'd have to add a bunch of wrappers. Also wrapping all the possible variations of Popen people might need (not that that's what you're saying, but it heads that way) is probably also counter productive because at that point you might as well just use Popen directly. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/#review1133 --- On 2011-04-18 02:37:48, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/ --- (Updated 2011-04-18 02:37:48) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Util: Replace mkblankimage.sh with the new gem5img.py. This change replaces the mkblankimage.sh script, used for creating new disk images, with a new gem5img.py script. The new version is written in python instead of bash, takes its parameters from command line arguments instead of prompting for them, and finds a free loopback device dynamically instead of hardcoding /dev/loop1. The file system used is now optionally configurable, and the blank image is filled by a hole left by lseek and write instead of literally filling it with zeroes. The functionality of the new script is broken into subcommands init, mount, umount, new, partition, and format. init creates a new file of the appropriate size, partitions it, and then formats the first (and only) new parition. mount attaches a new loopback device to the first parition of the image file and mounts it to the specified mount point. umount unmounts the specified mount point and identifies and cleans up the underlying loopback device. new, partition, and format are the individual stages of init but broken out so they can be run individually. That's so an image can be reinitialized in place if needed. Two features of the original script are being dropped. The first is the ability to specify a source directory to copy into the new file system. The second is the ability to specify a list of commands to run which are expected to (but not required to) update the permissions of the files in the new fs. Both of these seem easy enough to do manually, especially given the mount and umount commands, that removing them would meaningfully simplify the script without making it less useful. Diffs - util/gem5img.py PRE-CREATION util/mkblankimage.sh d8ec0a7b3f0c Diff: http://reviews.m5sim.org/r/644/diff Testing --- Thanks, Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
On 04/23/11 23:49, nathan binkert wrote: I looked at it and it is similar, but it's different enough that they can't be interchanged. My functions all allow passing a string in as standard input and return the error code, but readCommand doesn't. I could extend it to take an input string, but it's not clear how to return the output and the error code without changing all the call sites and complicating all the code that's working fine now without it. readCommand doesn't allow printing the command before it's run so I'd have to add a wrapper to handle that, and it doesn't handle adding 'sudo'. I think tying in readCommand would actually make everything bigger and more complicated because it's not quite what I need and I'd have to add a bunch of wrappers. Also wrapping all the possible variations of Popen people might need (not that that's what you're saying, but it heads that way) is probably also counter productive because at that point you might as well just use Popen directly. I totally agree. If it doesn't work, it doesn't work. Is it something reusable that should go in m5.util? Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev I'm inclined to say no for two reasons. First, the whole sudo thing is still a little shady. The path thing is basically because sudo inherits the current environment, I think. You can pass -i to it to make it work like root just logged in and ran a command, but then that might clobber things people purposefully put in their environment like tweaking PATH. Appending /sbin and /usr/sbin is a workaround which has drawbacks which you've pointed out, but then forcing people to add them to their path when not root is also cumbersome. Running sudo automatically is probably not a universally good idea. So with all that sort of up in the air, I'm willing to let it slide for this one script because it's historically been that way and I don't have a better idea, but I don't want to make it institutional by putting it into a utility module. Second, there's some script specific plumbing here like the part where it prints out normally silent commands if you want to try to debug what's going on. You could make whether to print the command a parameter, but that over specializes the library functions, I think. Also, printing the command like I'm doing isn't quite right either. If you were to copy and paste one of the commands at a prompt (which is why they're frequently printed), it's conceivable it wouldn't work like it does in the script since there's no quoting of list elements with spaces in them. They get handled by Popen right because they're a single element in the list, but if you copy and paste the shell will split them up. All that said, if you want to take the script and move things into m5.util, or rework it so it uses m5.util, or make it handle paths better, or whatever, I certainly won't try to stop you. I think my version is an improvement over the original, but it's definitely not perfect. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
On 04/24/11 00:14, nathan binkert wrote: All that said, if you want to take the script and move things into m5.util, or rework it so it uses m5.util, or make it handle paths better, or whatever, I certainly won't try to stop you. I think my version is an improvement over the original, but it's definitely not perfect. I didn't mean the whole thing. I meant stuff that might be generally useful. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev Yes, that's what I'm talking about too. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: O3: Tighten memory order violation checking to 16 bytes.
It looks like this patch was committed, but it didn't have anything to do with the assert. It just happened to change whether or not that assert was hit. I'll take a look at it sometime soon. Gabe On 04/22/11 10:04, nathan binkert wrote: Did this ever get committed? I'm running into this bug with 20.parser. Nate On Wed, Mar 30, 2011 at 8:46 AM, Ali Saidi sa...@umich.edu wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/520/ I think the updated patch addresses all of your issues Gabe. I tested it with an opt binary and one problem jumped out in x86 for 20.parser an assert: m5.opt: build/X86_SE/arch/x86/emulenv.cc:49: void X86ISA::EmulEnv::doModRM(const X86ISA::ExtMachInst): Assertion `machInst.modRM.mod != 3' failed. It looks like the assert shouldn't be there and is hit during some miss speculation. - Ali On March 30th, 2011, 8:41 a.m., Ali Saidi wrote: Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Ali Saidi. *Updated 2011-03-30 08:41:48* Description O3: Tighten memory order violation checking to 16 bytes. The comment in the code suggests that the checking granularity should be 16 bytes, however in reality the shift by 8 is 256 bytes which seems much larger than required. Diffs - src/cpu/base_dyn_inst.hh (d54b7775a6b0) - src/cpu/o3/O3CPU.py (d54b7775a6b0) - src/cpu/o3/lsq_unit.hh (d54b7775a6b0) - src/cpu/o3/lsq_unit_impl.hh (d54b7775a6b0) View Diff http://reviews.m5sim.org/r/520/diff/ ___ 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
Re: [m5-dev] Introducing...InOrder AlphaFS!
Tada! Congratulations. Gabe On 4/21/2011 12:12 AM, Korey Sewell wrote: For 1-4 wide, InOrder Cores running in ALPHA FS mode... Observe terminal output: M5 console: m5AlphaAccess @ 0xFD02 Got Configuration 623 memsize 800 pages 4000 First free page after ROM 0xFC018000 HWRPB 0xFC018000 l1pt 0xFC04 l2pt 0xFC042000 l3pt_rpb 0xFC044000 l3pt_kernel 0xFC048000 l2reserv 0xFC046000 kstart = 0xFC31, kend = 0xFC855898, kentry = 0xFC31, numCPUs = 0x1 CPU Clock at 2000 MHz IntrClockFrequency=1024 Booting with 1 processor(s) ... Linux version 2.6.13 (h...@zed.eecs.umich.edu) (gcc version 3.4.3) #1 SMP Sun Oct 8 .. Brought up 1 CPUs ... init started: BusyBox v1.1.0 (2007.03.04-01:07+) multi-call binary mounting filesystems... EXT2-fs warning: checktime reached, running e2fsck is recommended loading script... * **Two Words: Yes Sir! *More Than Two Words: InOrder is booting Linux!!! Getting to this point took about30 patches, so I have a lot of patch-cleaning work to do before I can place it in the repo. Also, just because it boots doesn't mean I've tested it with any kind of real workload suite (e.g. SPEC or Parsec). I don't have the bandwidth to complete stress test through all the benchmarks and test but if someone tries and something goes wrong, please post to the mailing list and I'll do my best to help you debug. Either way, I think this is a good milestone and I'll be adding the patches and the regression test for ALPHA_FS/10.linux_boot/InOrder-CPU within the next week (or two) and then eventually updating more things in our handy-dandy M5 status matrix: http://m5sim.org/wiki/index.php/Status_Matrix The next big thing I want to get in InOrder before the gem5 official merge is complete is timing TLB translation and in the longer term microcode support. I can't say exactly when those will be added (again, not enough time for me to completely focus on this), but if anyone wants to try I have a good idea of what needs to be done and can assist. ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: scons: Allow the build directory live under an ...
It's probably a bit late to bring this up, but won't this remove -all-
directories called build, not just -the- build directory? There's an os
function that checks if two directories are the same. We can use that to
compare the actual build directory with whatever we're recursing down.
That would even catch hard links, I think, although hard linking your
build directory would be a little weird.
Gabe
On 4/20/2011 11:20 AM, Brad Danofsky wrote:
changeset f52ece27e20d in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=f52ece27e20d
description:
scons: Allow the build directory live under an EXTRAS directory
diffstat:
src/SConscript | 4
1 files changed, 4 insertions(+), 0 deletions(-)
diffs (14 lines):
diff -r ee4e795343bf -r f52ece27e20d src/SConscript
--- a/src/SConscriptTue Apr 19 18:45:23 2011 -0700
+++ b/src/SConscriptWed Apr 20 11:14:51 2011 -0700
@@ -320,6 +320,10 @@
for extra_dir in extras_dir_list:
prefix_len = len(dirname(extra_dir)) + 1
for root, dirs, files in os.walk(extra_dir, topdown=True):
+# if build lives in the extras directory, don't walk down it
+if 'build' in dirs:
+dirs.remove('build')
+
if 'SConscript' in files:
build_dir = joinpath(env['BUILDDIR'], root[prefix_len:])
SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir)
___
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
Re: [m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
On 2011-04-18 21:20:49, Nathan Binkert wrote: util/gem5img.py, line 25 http://reviews.m5sim.org/r/644/diff/1/?file=11664#file11664line25 This makes me feel uneasy for a script that you're likely to call using sudo. I know it's overly paranoid, but why not just simply give the user a tip if the program is not found (which you have to deal with anyway.) I'm not necessarily advocating doing things this way, but this is what the original script was doing. This script should not be called with sudo since it calls it internally, although I suppose it could. I don't know why, but I think if you use sudo to like the script does, you have to make sure you use an absolute path to the target binary. There were some posts to back that up online and that's also what the original script was doing. To get that path, the script calls which, and for that to find things that are normally only usable by root it needs to also search in /sbin and /usr/sbin. If a program isn't found the script will complain and die. On 2011-04-18 21:20:49, Nathan Binkert wrote: util/gem5img.py, line 51 http://reviews.m5sim.org/r/644/diff/1/?file=11664#file11664line51 I'm not going to make you change it or anything, but this whole class seems to me to be a bit overkill, no? __notRoot = None def needSudo(): if __notRoot is None: __notRoot = os.geteuid() != 0 return __notRoot BTW: we also have m5.util.Singleton Yes and no. This came about because the original script had a warning about using sudo which I wanted to preserve. I made it possible to run only part of the script, and if you run a part that doesn't need sudo (like just creating an appropriately sized file) then the warning might be confusing. I also didn't want the warning to show up multiple times if sudo was used more than once. It's a little clumsy, but I didn't see any obvious alternative that would get the behavior I wanted. Feel free to convince me to drop the warning. On 2011-04-18 21:20:49, Nathan Binkert wrote: util/gem5img.py, line 64 http://reviews.m5sim.org/r/644/diff/1/?file=11664#file11664line64 This is pretty similar to m5.util.readCommand which made me think that it might be nice if we put your utility functions here in m5.util Can I use m5.util from an arbitrary python script? If I can that's good to know. Also, how does readCommand work? Does it pass through stdout/stderr or capture it? Depending on the answer it might be an appropriate replacement for this or the subsequent getOutput, but changing only one obscures the similarities between the two functions. If you're advocating adding a new version of readCommand that has the other behavior then that makes sense. I also funnel text into stdin for input, and I think sudo happens to still work because it goes around any redirection I set up. On 2011-04-18 21:20:49, Nathan Binkert wrote: util/gem5img.py, line 72 http://reviews.m5sim.org/r/644/diff/1/?file=11664#file11664line72 This is basically readCommand() See above. On 2011-04-18 21:20:49, Nathan Binkert wrote: util/gem5img.py, line 106 http://reviews.m5sim.org/r/644/diff/1/?file=11664#file11664line106 Here is where you could suggest that it is in /sbin or /usr/sbin I'm not sure what telling them where it might be would accomplish since the script still wouldn't be able to find/use it. I may just not understand what you're getting at. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/#review1133 --- On 2011-04-18 02:37:48, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/ --- (Updated 2011-04-18 02:37:48) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Util: Replace mkblankimage.sh with the new gem5img.py. This change replaces the mkblankimage.sh script, used for creating new disk images, with a new gem5img.py script. The new version is written in python instead of bash, takes its parameters from command line arguments instead of prompting for them, and finds a free loopback device dynamically instead of hardcoding /dev/loop1. The file system used is now optionally configurable, and the blank image is filled by a hole left by lseek and write instead of literally filling it with zeroes. The functionality of the new script is broken into subcommands init, mount, umount, new, partition, and format. init creates a new file of the appropriate size, partitions it, and then formats the first (and only) new parition. mount attaches a new loopback device
[m5-dev] what scons can do
I was looking at some of the stuff in util, and it occurred to me that the m5 utility program is cross compiled using different Makefiles for different architectures. Statetrace used to be like that (sort of), but recently I converted it over to scons and set up some configuration variables that made it easier to work with. It would be nice to be able to share some of that with the m5 utility program, although I don't remember it being all that complicated. Anyway, it seems like it would be useful to be able to have multiple binaries that can be built by scons, specifically the utility stuff and unit tests. That way we could avoid having a hodge podge of small build systems which are either isolated or not in not quite the right ways. I know some of Nate's recent changes suggested this was going to get easier. Could you quickly summarize what that's all about, Nate? Speaking of which, the regressions are still broken. Since that's taking a little while, would you mind please backing out the problem change? Also, I was thinking about how to handle the dependencies/generated files/custom language issue a little while back, and what I kept coming back to were schemes where scons would use a cache of dependency information which it would regenerate if any of the input files which determined outputs and/or dependencies changed. The problem is that scons would need to run once and possibly regenerate its cache, and then run again to actually run. Is this sort of multi-pass setup possible somehow without major hacks? To explain more what I'm getting, lets say you have input file foo.isa which, when processed, generates the files foo_exec.cc and bar_exec.cc. What would happen is that you'd have a file like foo.isa.dep which would describe what would happen and make that depend on foo.isa. When you run for the first time, scons would see that foo.isa.dep doesn't exist. During it's build phase, it would run foo.isa through the system and see that it generated foo_exec.cc and bar_exec.cc and put that into foo.isa.dep (as actual SConscript type code, or flat data, or...). When scons ran the second time, it would read in foo.isa.dep and extract the dependencies from it and build that into the graph. It wouldn't construct foo.isa.dep again since all its inputs were the same, and it would still capture all those dependencies. This time around, the larger binary would see that it depended on foo_exec.cc and bar_exec.cc and that those depend on foo.isa.dep (as a convenient aggregation point of all *.isa files involved). If foo.isa changed later, foo.isa.dep would be out of date and have to be regenerated, and then foo_exec.cc and bar_exec.cc, and then the main binary. The net effect of this is that the thing that processed the .isa would only be run when necessary. In our current setup, that would mean SLICC wouldn't have to be run for every build, only ones where the SLICC input files changed. The problem here is that scons would need to basically call a nested instance of itself on foo.isa.dep, let that build a dep tree and run the build phase, then process foo.isa.dep in the parent dep phase, and then run the parent build phase. It could literally just call scons from scons (though that seems like a major hack) or it could, if scons has a facility for it, do some sort of fancy multi-pass thing. This is sort of related to the first thing (additional targets) because the dependency cache files are sort of like independent targets with their own invocations of scons. Also related to scons are those .pyc files that end up scattered around the source tree. I know I asked about those a long, long time ago, but why are they there? Why don't they end up in the build directories? Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] Review Request: Util: Replace mkblankimage.sh with the new gem5img.py.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/644/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Util: Replace mkblankimage.sh with the new gem5img.py. This change replaces the mkblankimage.sh script, used for creating new disk images, with a new gem5img.py script. The new version is written in python instead of bash, takes its parameters from command line arguments instead of prompting for them, and finds a free loopback device dynamically instead of hardcoding /dev/loop1. The file system used is now optionally configurable, and the blank image is filled by a hole left by lseek and write instead of literally filling it with zeroes. The functionality of the new script is broken into subcommands init, mount, umount, new, partition, and format. init creates a new file of the appropriate size, partitions it, and then formats the first (and only) new parition. mount attaches a new loopback device to the first parition of the image file and mounts it to the specified mount point. umount unmounts the specified mount point and identifies and cleans up the underlying loopback device. new, partition, and format are the individual stages of init but broken out so they can be run individually. That's so an image can be reinitialized in place if needed. Two features of the original script are being dropped. The first is the ability to specify a source directory to copy into the new file system. The second is the ability to specify a list of commands to run which are expected to (but not required to) update the permissions of the files in the new fs. Both of these seem easy enough to do manually, especially given the mount and umount commands, that removing them would meaningfully simplify the script without making it less useful. Diffs - util/gem5img.py PRE-CREATION util/mkblankimage.sh d8ec0a7b3f0c Diff: http://reviews.m5sim.org/r/644/diff Testing --- Thanks, Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Cron m5test@zizzer /z/m5/regression/do-regression --scratch all
Deleting build didn't seem to do anything. Traceback (most recent call last): File string, line 1, in module File /z/m5/regression/zizzer/m5/src/python/importer.py, line 73, in load_module exec code in mod.__dict__ File /z/m5/regression/zizzer/m5/src/python/m5/__init__.py, line 38, in module internal.core.__package__ AttributeError: 'module' object has no attribute '__package__' Traceback (most recent call last): File string, line 1, in module File /z/m5/regression/zizzer/m5/src/python/importer.py, line 73, in load_module exec code in mod.__dict__ File /z/m5/regression/zizzer/m5/src/python/m5/__init__.py, line 38, in module internal.core.__package__ AttributeError: 'module' object has no attribute '__package__' M5 exited with non-zero status 1 * build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/inorder-timing FAILED! On 04/17/11 03:51, Cron Daemon wrote: * build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/inorder-timing FAILED! * build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby FAILED! * build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby FAILED! * build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/inorder-timing FAILED! * build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/inorder-timing FAILED! * build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby FAILED! * build/ALPHA_SE/tests/fast/quick/01.hello-2T-smt/alpha/linux/o3-timing FAILED! * build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/inorder-timing FAILED! * build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-atomic-mp FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/o3-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby FAILED! * build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/inorder-timing FAILED! * build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-timing-mp FAILED! * build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-timing FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MESI_CMP_directory FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory FAILED! *
Re: [m5-dev] Cron m5test@zizzer /z/m5/regression/do-regression quick
/z/m5/regression/zizzer/m5/build has been deleted. I'm pretty sure that will do it, but let me know if I need to remove anything else. Gabe On 04/16/11 11:13, nathan binkert wrote: This is likely due to my push last night, though I ran the full regression suite before I pushed. My guess is that the directory needs to be blown away so a fresh build can happen. Can someone do that for me? I'm in the car about to go camping. Nate On Sat, Apr 16, 2011 at 12:56 AM, Cron Daemon r...@zizzer.eecs.umich.edu wrote: * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby FAILED! * build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby FAILED! * build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-timing-mp FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/o3-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-atomic-mp FAILED! * build/ALPHA_SE/tests/fast/quick/01.hello-2T-smt/alpha/linux/o3-timing FAILED! * build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-atomic FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing FAILED! * build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/o3-timing FAILED! * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/inorder-timing FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MESI_CMP_directory FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory FAILED! * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MESI_CMP_directory FAILED! * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_directory FAILED! * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory FAILED! * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory FAILED! * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory FAILED! * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_token FAILED! * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token FAILED! * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token FAILED! * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token FAILED! * build/ALPHA_FS/tests/fast/quick/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic FAILED! * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing FAILED! * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing-dual FAILED! * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual FAILED! * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic FAILED! * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-timing-ruby FAILED! * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-timing FAILED! * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/o3-timing FAILED! * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-atomic FAILED! * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/inorder-timing FAILED! * build/POWER_SE/tests/fast/quick/00.hello/power/linux/simple-atomic FAILED! *
Re: [m5-dev] Bug in changeset 8225 or 8227
Since we seem to have some folks using an older python, my vote would be to make it 2.4 compatible. I don't have a good intuition about how old 2.4 is, though. If it's really ancient we might want to bump up the required version and force Nilay (and anybody else) to upgrade. Gabe On 04/16/11 12:58, Steve Reinhardt wrote: Yea, if you look at the mentioned line, it's mode = 'r+' if inplace else 'r', and conditional assignments like this (basically python's version of C's ?:) weren't added until 2.5. Since we officially only require python 2.4 (not 2.5), I'm happy to call this Nate's fault. If you want to change the code to be 2.4 compatible you can, or you can upgrade your python, or you can just roll back to an earlier changeset in your local repo. Steve On Sat, Apr 16, 2011 at 12:10 PM, Nilay Vaish ni...@cs.wisc.edu wrote: On Sat, 16 Apr 2011, nathan binkert wrote: What version of python are you using? It could be that that syntax wasn't available until 2.5 and you're using something older. I can't do anything about it right now because I'm about to leave on a hike. Nate Nate, it seems one of your checkins from yesterday has a bug. I receive the following message on executing any merculrial command. *** failed to import extension style from ./util/style.py: invalid syntax (file_types.py, line 143) -- Nilay I am using python version 2.4.3. Nilay ___ 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
Re: [m5-dev] Review Request: Ruby: Add support for functional accesses
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/611/#review1113
---
Please fix these style issues, including the ones in this file I haven't
explicitly pointed out. You should be using M5 style generally, but especially
when your in M5 code. Also, please be sure to point this out to one of the
classic memory system experts (Nate, Steve, or Ali) and get them to sign off.
They might not see that this change touches outside of Ruby.
src/mem/packet.hh
http://reviews.m5sim.org/r/611/#comment1510
Don't add commented out code.
src/mem/packet.hh
http://reviews.m5sim.org/r/611/#comment1513
space after if, brace at the end of the line
src/mem/packet.hh
http://reviews.m5sim.org/r/611/#comment1511
space after if, brace on the end of the line not on a new one.
src/mem/packet.hh
http://reviews.m5sim.org/r/611/#comment1512
This should be } else {
- Gabe
On 2011-04-13 11:17:53, Nilay Vaish wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/611/
---
(Updated 2011-04-13 11:17:53)
Review request for Default.
Summary
---
Ruby: Add support for functional accesses
This patch is meant for implementing functional access support in Ruby.
Currently, the patch does not functional accesses for the PioPort.
Diffs
-
configs/example/ruby_mem_test.py 8b5f900233ee
configs/ruby/MESI_CMP_directory.py 8b5f900233ee
configs/ruby/Ruby.py 8b5f900233ee
src/cpu/testers/memtest/memtest.cc 8b5f900233ee
src/mem/packet.hh 8b5f900233ee
src/mem/packet.cc 8b5f900233ee
src/mem/protocol/MESI_CMP_directory-L1cache.sm 8b5f900233ee
src/mem/protocol/MESI_CMP_directory-L2cache.sm 8b5f900233ee
src/mem/protocol/MESI_CMP_directory-dir.sm 8b5f900233ee
src/mem/protocol/RubySlicc_Types.sm 8b5f900233ee
src/mem/ruby/network/Network.cc 8b5f900233ee
src/mem/ruby/network/Network.py 8b5f900233ee
src/mem/ruby/profiler/Profiler.cc 8b5f900233ee
src/mem/ruby/profiler/Profiler.py 8b5f900233ee
src/mem/ruby/recorder/Tracer.cc 8b5f900233ee
src/mem/ruby/recorder/Tracer.py 8b5f900233ee
src/mem/ruby/system/AbstractMemory.hh PRE-CREATION
src/mem/ruby/system/AbstractMemory.cc PRE-CREATION
src/mem/ruby/system/AbstractMemory.py PRE-CREATION
src/mem/ruby/system/Cache.py 8b5f900233ee
src/mem/ruby/system/CacheMemory.hh 8b5f900233ee
src/mem/ruby/system/CacheMemory.cc 8b5f900233ee
src/mem/ruby/system/DirectoryMemory.hh 8b5f900233ee
src/mem/ruby/system/DirectoryMemory.cc 8b5f900233ee
src/mem/ruby/system/DirectoryMemory.py 8b5f900233ee
src/mem/ruby/system/RubyPort.hh 8b5f900233ee
src/mem/ruby/system/RubyPort.cc 8b5f900233ee
src/mem/ruby/system/RubySystem.py 8b5f900233ee
src/mem/ruby/system/SConscript 8b5f900233ee
src/mem/ruby/system/Sequencer.cc 8b5f900233ee
src/mem/ruby/system/Sequencer.py 8b5f900233ee
src/mem/ruby/system/System.hh 8b5f900233ee
src/mem/ruby/system/System.cc 8b5f900233ee
src/mem/slicc/ast/MemberExprAST.py 8b5f900233ee
Diff: http://reviews.m5sim.org/r/611/diff
Testing
---
I have tested functional accesses with the ratio between functional
and timing accesses for different ratios -- 100:0, 99:1, 90:1, 50:50,
10:90, 1:99. It is working in all the cases.
Thanks,
Nilay
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] breaking up SConstruct, src/SConscript
I went digging through our SCons files today, and it occurs to me that the two main files, SConstruct and src/SConscript, are quite long and intimidating. They tend (as far as I could tell) to be split up into sections which is very helpful, but there's still a sea of text and it's easy to get disoriented and hard to get a high level idea of what's in there and what it's all for. Does anyone have any objections to breaking those files into several smaller ones, likely similar to how they're already grouped into sections? SCons supports a site_scons directory which automatically gets added to the python path and where you can put extra utility source files. That seems like a good place to spin sections off into. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Core: Add some documentation about the sim clocks.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/636/#review1105 --- src/sim/core.hh http://reviews.m5sim.org/r/636/#comment1478 The way this is worded makes it hard to understand. How about: These variables equal the number of ticks in the unit of time they're named after. - Gabe On 2011-04-10 16:53:18, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/636/ --- (Updated 2011-04-10 16:53:18) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- Core: Add some documentation about the sim clocks. Diffs - src/sim/core.hh 02cb69e5cfeb Diff: http://reviews.m5sim.org/r/636/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Cron m5test@zizzer /z/m5/regression/do-regression --scratch all
It looks like it was this change which was directly after the one I
pointed out before.
changeset: 8134:b01a51ff05fa
user:Ali Saidi ali.sa...@arm.com
date:Thu Mar 17 19:20:19 2011 -0500
summary: Mem: Fix issue with dirty block being lost when entire
block transferred to non-cache.
Could you take a look, Ali? The description doesn't necessarily sound
like something you'd expect to change the stats (it sounds like a corner
case), but I'm assuming you'll know.
Gabe
On 04/03/11 19:51, Gabe Black wrote:
Does anyone have any ideas about when X86_SE parser stopped working? The
last time it passed for sure was the end of February, but on March 16th
Ali updated the stats and so it was presumably working then too. I'm
running at that changeset right now to confirm that. There weren't any
X86 specific changes recently, but there were a few O3 ones which might
have changed the stats. The output is below, and you can see the biggest
change percentage wise was icache writebacks. Most of the changes are
related to memory somehow. After the stats is info about a change that
may have caused the problem.
= Statistics differences =
Maximum error magnitude: +133.33%
Reference New Value Abs Diff Pct Chg
Key statistics:
host_inst_rate 189714 148502 -41212 -21.72%
host_mem_usage 264736 268256 3520+1.33%
sim_insts 1527476062 15289887561512694+0.10%
sim_ticks 610952992000 612245337000 1292345000
+0.21%
system.cpu.commit.COM:count1527476062 15289887561512694+0.10%
Differences 0%:
system.cpu.icache.writebacks3 7 4 +133.33%
system.cpu.rename.RENAME:serializeStallCycles 19936
16025 -3911 -19.62%
system.cpu.l2cache.occ_%::0 0.213694 0.236362 0.022668 +10.61%
system.cpu.l2cache.occ_blocks::0 7002.339473 7745.103692 742.764219
+10.61%
system.cpu.rename.RENAME:tempSerializingInsts 2561
2314 -247-9.64%
system.cpu.icache.ReadReq_mshr_hits 1570 1427
-143-9.11%
system.cpu.icache.demand_mshr_hits 1570 1427 -143
-9.11%
system.cpu.icache.overall_mshr_hits 1570 1427
-143-9.11%
system.cpu.rename.RENAME:serializingInsts 2550 2345
-205-8.04%
system.cpu.l2cache.ReadReq_misses 316709 339091 22382
+7.07%
system.cpu.l2cache.ReadReq_mshr_misses 316709 339091
22382+7.07%
system.cpu.l2cache.ReadReq_mshr_miss_latency 9818903000 10512799000
693896000+7.07%
system.cpu.l2cache.ReadReq_miss_latency 10822415500 11584355000
761939500+7.04%
system.cpu.dcache.ReadReq_mshr_miss_latency 14062264500 14863694500
80143+5.70%
system.cpu.l2cache.ReadReq_miss_rate 0.182786 0.192941
0.010155+5.56%
system.cpu.l2cache.ReadReq_mshr_miss_rate 0.182786 0.192941
0.010155+5.56%
system.cpu.idleCycles24586339 256777931091454+4.44%
system.cpu.dcache.ReadReq_avg_mshr_miss_latency 8150.695480
8493.787248 343.091768+4.21%
system.cpu.l2cache.replacements 553099 575827 22728+4.11%
system.cpu.l2cache.demand_mshr_miss_latency 17475146000 18186565000
711419000+4.07%
[... showing top 20 errors only, additional errors omitted ...]
* build/X86_SE/tests/fast/long/20.parser/x86/linux/o3-timing FAILED!
changeset 9f704aa10eb4 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=9f704aa10eb4
description:
O3: Fix unaligned stores when cache blocked
Without this change the a store can be issued to the cache multiple
times.
If this case occurs when the l1 cache is out of mshrs (and thus blocked)
the processor will never make forward progress because each cycle it
will
send a single request using the recently freed mshr and not completing
the
multipart store. This will continue forever.
diffstat:
src/cpu/o3/lsq_unit_impl.hh | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diffs (14 lines):
diff -r 2af262e73961 -r 9f704aa10eb4 src/cpu/o3/lsq_unit_impl.hh
--- a/src/cpu/o3/lsq_unit_impl.hh Thu Mar 17 00:43:54 2011 -0400
+++ b/src/cpu/o3/lsq_unit_impl.hh Thu Mar 17 19:20:19 2011 -0500
@@ -1103,7 +1103,9 @@
dynamic_castLSQSenderState *(retryPkt-senderState);
// Don't finish the store unless this is the last packet.
-if (!TheISA::HasUnalignedMemAcc || !state-pktToSend) {
+if (!TheISA::HasUnalignedMemAcc || !state-pktToSend ||
+state-pendingPacket == retryPkt) {
+state-pktToSend = false;
storePostSend(retryPkt);
}
retryPkt = NULL
Re: [m5-dev] Review Request: patch from Vince Weaver for review
On 2011-03-18 13:54:12, Gabe Black wrote: It seems like we should be able to emulate the access system call fairly easily. It basically just checks if a file can be accessed in certain ways, I think. We could do that on the real file descriptor, rearrange the result if necessary, and pass it back. The other syscall changes here look ok. Lisa Hsu wrote: Would you object to committing/pushing as is and emulating access() if it ever becomes necessary in the future? It's not something I'm going to commit to doing, and I wouldn't want everything else to get held up because of it. How is access() being used right now? It would be dangerous to assume just because it didn't seem to cause a problem by ignoring it that it's actually safe to do so, and it must be used by something or that line wouldn't be necessary. I would rather we did this safely than expediently. This change, or at least that line, should be independent of any other change as far as I've seen and shouldn't hold anything else up. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/588/#review980 --- On 2011-03-17 16:05:56, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/588/ --- (Updated 2011-03-17 16:05:56) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: SE syscalls: patch from Vince Weaver for review Diffs - src/arch/x86/linux/syscalls.cc 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/588/diff Testing --- I've done minimal testing on these, i.e. I've pushed them to a clean tree and run X86 SPEC2k6 binaries on them, some of which didn't work prior to the patches but now do. Others remain broken. Vince, however, has done lots of testing and basically needed these to run SPEC2K workloads to completion for his thesis. In other words, I bet these patches are good, but not complete for the purposes of running SPEC2k6. Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: X86 ioctl: Another patch from Vince Weaver
On 2011-03-18 13:57:35, Gabe Black wrote: src/sim/syscall_emul.hh, line 503 http://reviews.m5sim.org/r/589/diff/1/?file=11013#file11013line503 Why is this change necessary? I'm not 100% sure why it was the way it was before, but I see no reason to change it either. Changing the fatal to a warn may be necessary to get some benchmark to run, but I'm talking specifically about the ones that would return -ENOTTY. Vince Weaver wrote: It's been a while, but let's see if I can see what's going on. Before we had a short list of ioctls we return -ENOTTY for, any not on the list gave a fatal error. ENOTTY if I recall correctly is the typical way the kernel reports an ioctl not being implemented. So by changing all ioctls to warn and then report ENOTTY we are saying that we don't support it, but the program can keep running assuming it handles an ENOTTY properly. For most of the SPEC benchmarks ioctl() calls aren't important for correctness, so this works. We could go back to having a whitelist of known-to-be-OK-to-ignore ioctls(), but it might turn out to be a lengthy list, and also we probably should implement things like isatty() properly as I do think some benchmarks depend on it returning the right value (see http://www.cs.binghamton.edu/~msim/wiki/index.php/TIOCISATTY ) Gabe Black wrote: isatty is actually pretty annoying, and it's important that it -not- always return the right answer. It needs to be consistent regardless of whether your piping output to a file or watching it on the console so runs are deterministic, and if it always thinks it's going to a tty it'll buffer properly for when it's actually going to a tty (and doesn't look like something's broken, basically) and just perform a little worse when it's not. As far as simulator performance it doesn't matter since I'm sure it's a long way from being the critical path, and it should be a reasonable situation as far as simulated performance. I think a whitelist is a good idea, and we don't have to have -all- the ok to ignore ones on there, just the ok to ignore ones that actually get called. That should be a lot more manageable list. That way you know if something out of the ordinary is happening (the simulator will bomb out) rather than something weird happening and the benchmark stumbling on to eventually die, potentially far from evidence of what happened. The later is a lot harder to debug. Steve Reinhardt wrote: ENOTTY is just the way the kernel reports a TTY-specific ioctl being invoked on a non-TTY device. Gabe is right that for repeatability you always want the simulator to always act like there's no tty for determinism. I would prefer to see us figure out what other ioctls need to be added to the TTY-specific list than to make a blanket assumption that any ioctl that's called is TTY-specific. Lisa Hsu wrote: So, what's the consensus here? No checkin until ioctls are better figured out? The change to syscalls.cc is fine since that's just hooking up the existing syscall functions and there isn't anything unusual or objectionable about how that's being done. The change to syscall_emul.hh is not correct as far as I can tell. If there are specific IOCTL constants that are being used, we'll need to find out what those are and then what to do with them. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/589/#review981 --- On 2011-03-17 16:06:13, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/589/ --- (Updated 2011-03-17 16:06:13) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86 ioctl: Another patch from Vince Weaver Diffs - src/arch/x86/linux/syscalls.cc 2e269d6fb3e6 src/sim/syscall_emul.hh 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/589/diff Testing --- Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: X86: fnstsw: Another patch from Vince Weaver
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/594/#review1094 --- src/arch/x86/isa/insts/x87/control/save_x87_status_word.py http://reviews.m5sim.org/r/594/#comment1453 Since FSTSW isn't an instruction it doesn't need to be mentioned. src/arch/x86/isa/insts/x87/control/save_x87_status_word.py http://reviews.m5sim.org/r/594/#comment1454 This information isn't useful here and shouldn't clutter the instruction definition. src/arch/x86/isa/insts/x87/control/save_x87_status_word.py http://reviews.m5sim.org/r/594/#comment1455 Reading the status register should automatically fold in the top register. If it doesn't it should. That shouldn't be implemented with microcode. Also, the string constants naming these registers can be set up in arch/x86/isa/microasm.isa by appending them to the assembler.symbols dict. Then they'll show up in the scope of the microcode and you can use rdval t1, fsw With those changes, there's also no reason to construct the value in a temporary microcode register t1 in the AX case (FNSTSW_R). src/arch/x86/isa/insts/x87/control/save_x87_status_word.py http://reviews.m5sim.org/r/594/#comment1456 datasize will already be 2 because of the rAw operand. The w is interpreted as word sized which means datasize is set to 2 bytes. That's what I remember and what looking at the code seems to confirm, but it's been a while so let me know if you know that's not true. src/arch/x86/isa/operands.isa http://reviews.m5sim.org/r/594/#comment1452 This is never used and shouldn't be added. src/arch/x86/regs/misc.hh http://reviews.m5sim.org/r/594/#comment1451 This already exists as MISCREG_FSW. MISCREG_FCW is the control word, and MISCREG_FTW is the tag word. - Gabe On 2011-03-17 16:07:24, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/594/ --- (Updated 2011-03-17 16:07:24) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: fnstsw: Another patch from Vince Weaver Diffs - src/arch/x86/isa/decoder/x87.isa 2e269d6fb3e6 src/arch/x86/isa/insts/x87/control/save_x87_status_word.py 2e269d6fb3e6 src/arch/x86/isa/operands.isa 2e269d6fb3e6 src/arch/x86/regs/misc.hh 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/594/diff Testing --- Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: X86: fsincos: Another patch from Vince Weaver
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/593/#review1095 --- We never really did decide how we wanted to structure the microops for x87, but this seems reasonable at least to start with. The structure of the microop definitions need to be cleaned up a little. src/arch/x86/isa/microops/fpop.isa http://reviews.m5sim.org/r/593/#comment1457 Cosfp, Sinfp and the existing Movfp could all inherit from a base class (FpUniOp for unary for instance) that has this other form of __init__. Then they wouldn't each define an essentially identical copy. - Gabe On 2011-03-17 16:07:17, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/593/ --- (Updated 2011-03-17 16:07:17) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: fsincos: Another patch from Vince Weaver Diffs - src/arch/x86/isa/decoder/x87.isa 2e269d6fb3e6 src/arch/x86/isa/insts/x87/transcendental_functions/trigonometric_functions.py 2e269d6fb3e6 src/arch/x86/isa/microops/fpop.isa 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/593/diff Testing --- Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: X86: haddps: Another patch from Vince Weaver
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/592/#review1096 --- src/arch/x86/isa/insts/simd128/floating_point/arithmetic/horizontal_addition.py http://reviews.m5sim.org/r/592/#comment1458 ext is no longer set to a raw bitvector that selects per instruction features like this since, as you can see, it's pretty opaque just looking at it. The maddf ext=1 becomes ext=Scalar. For msrli and mslli, ext=0 is the default and can be dropped. It would leave the ops as SIMD. Since they're already operating at the full width of the fp register type (a double) the value is especially redundant. src/arch/x86/isa/insts/simd128/floating_point/arithmetic/horizontal_addition.py http://reviews.m5sim.org/r/592/#comment1461 This implementation is a bit inefficient, although not terribly so. You have to be careful since the two operands may be the same registers and you don't want to overwrite something you still need, but, for instance, the maddf one line above, this shift of ufp4 and the maddf on line 60 could all update xmmh since all high halves of xmm registers have been read and no faults can happen. The moves that read out xmmlm could be moved higher, and xmml could also be updated directly. I think it -may- also be possible to do something clever and cut down the number of microops shifting things around to pack and unpack the results. I may have also suspected this was true when I wrote the much simpler 64 bit wide version of this instruction below this one where the components are whole registers and can be indexed directly, but then didn't come up with anything and punted for later. src/arch/x86/isa/insts/simd128/floating_point/arithmetic/horizontal_addition.py http://reviews.m5sim.org/r/592/#comment1459 This microop is changing architecturally visible state and effectively committing to completing the op before all the possibly faulting ops have executed, specifically the following loads. There are 8 microcode fp registers so you can just use the others and leave ufp3 around until the end. src/arch/x86/isa/insts/simd128/floating_point/arithmetic/horizontal_addition.py http://reviews.m5sim.org/r/592/#comment1460 Like above, this can't happen before the loads. - Gabe On 2011-03-17 16:07:08, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/592/ --- (Updated 2011-03-17 16:07:08) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: haddps: Another patch from Vince Weaver Diffs - src/arch/x86/isa/decoder/two_byte_opcodes.isa 2e269d6fb3e6 src/arch/x86/isa/insts/simd128/floating_point/arithmetic/horizontal_addition.py 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/592/diff Testing --- Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: X86: rlimit: Another patch from Vince Weaver
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/591/#review1097 --- src/arch/x86/linux/linux.hh http://reviews.m5sim.org/r/591/#comment1463 These RUSAGE and RLIMIT constants come from generic header files in Linux, not one specific to X86. If they aren't there already, they should be in a base class, not an x86 (or especially a 64 bit x86) subclass. src/arch/x86/linux/linux.hh http://reviews.m5sim.org/r/591/#comment1462 This constant is wrong according to http://lxr.linux.no/linux+*/include/asm-generic/resource.h#L26. This should be NPROC. Similarly 7 is NOFILE, 8 is MEMLOCK, and 9 is AS (VMEM doesn't seem to exist). There are some others which have been left out. src/arch/x86/linux/linux.hh http://reviews.m5sim.org/r/591/#comment1464 The indentation here is off. src/arch/x86/linux/linux.hh http://reviews.m5sim.org/r/591/#comment1465 These constants also need to be fixed, or removed if the 64 bit versions get promoted to a base class. - Gabe On 2011-03-17 16:06:30, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/591/ --- (Updated 2011-03-17 16:06:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: rlimit: Another patch from Vince Weaver Diffs - src/arch/x86/linux/linux.hh 2e269d6fb3e6 src/arch/x86/linux/syscalls.cc 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/591/diff Testing --- Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: patch from Vince Weaver for review
On 2011-03-18 13:54:12, Gabe Black wrote: It seems like we should be able to emulate the access system call fairly easily. It basically just checks if a file can be accessed in certain ways, I think. We could do that on the real file descriptor, rearrange the result if necessary, and pass it back. The other syscall changes here look ok. Lisa Hsu wrote: Would you object to committing/pushing as is and emulating access() if it ever becomes necessary in the future? It's not something I'm going to commit to doing, and I wouldn't want everything else to get held up because of it. Gabe Black wrote: How is access() being used right now? It would be dangerous to assume just because it didn't seem to cause a problem by ignoring it that it's actually safe to do so, and it must be used by something or that line wouldn't be necessary. I would rather we did this safely than expediently. This change, or at least that line, should be independent of any other change as far as I've seen and shouldn't hold anything else up. Ali Saidi wrote: well... all it seems like you would need to do is return 0 when it's called. That means you can access the file and I doubt we would ever have a case where you couldn't. That sounds about right, and that should be really easy to implement. We should make sure we put a comment in there explaining why we're always returning 0. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/588/#review980 --- On 2011-03-17 16:05:56, Lisa Hsu wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/588/ --- (Updated 2011-03-17 16:05:56) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- X86: SE syscalls: patch from Vince Weaver for review Diffs - src/arch/x86/linux/syscalls.cc 2e269d6fb3e6 Diff: http://reviews.m5sim.org/r/588/diff Testing --- I've done minimal testing on these, i.e. I've pushed them to a clean tree and run X86 SPEC2k6 binaries on them, some of which didn't work prior to the patches but now do. Others remain broken. Vince, however, has done lots of testing and basically needed these to run SPEC2K workloads to completion for his thesis. In other words, I bet these patches are good, but not complete for the purposes of running SPEC2k6. Thanks, Lisa ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Cron m5test@zizzer /z/m5/regression/do-regression --scratch all
Does anyone have any ideas about when X86_SE parser stopped working? The
last time it passed for sure was the end of February, but on March 16th
Ali updated the stats and so it was presumably working then too. I'm
running at that changeset right now to confirm that. There weren't any
X86 specific changes recently, but there were a few O3 ones which might
have changed the stats. The output is below, and you can see the biggest
change percentage wise was icache writebacks. Most of the changes are
related to memory somehow. After the stats is info about a change that
may have caused the problem.
= Statistics differences =
Maximum error magnitude: +133.33%
Reference New Value Abs Diff Pct Chg
Key statistics:
host_inst_rate 189714 148502 -41212 -21.72%
host_mem_usage 264736 268256 3520+1.33%
sim_insts 1527476062 15289887561512694+0.10%
sim_ticks 610952992000 612245337000 1292345000
+0.21%
system.cpu.commit.COM:count1527476062 15289887561512694+0.10%
Differences 0%:
system.cpu.icache.writebacks3 7 4 +133.33%
system.cpu.rename.RENAME:serializeStallCycles 19936
16025 -3911 -19.62%
system.cpu.l2cache.occ_%::0 0.213694 0.236362 0.022668 +10.61%
system.cpu.l2cache.occ_blocks::0 7002.339473 7745.103692 742.764219
+10.61%
system.cpu.rename.RENAME:tempSerializingInsts 2561
2314 -247-9.64%
system.cpu.icache.ReadReq_mshr_hits 1570 1427
-143-9.11%
system.cpu.icache.demand_mshr_hits 1570 1427 -143
-9.11%
system.cpu.icache.overall_mshr_hits 1570 1427
-143-9.11%
system.cpu.rename.RENAME:serializingInsts 2550 2345
-205-8.04%
system.cpu.l2cache.ReadReq_misses 316709 339091 22382
+7.07%
system.cpu.l2cache.ReadReq_mshr_misses 316709 339091
22382+7.07%
system.cpu.l2cache.ReadReq_mshr_miss_latency 9818903000 10512799000
693896000+7.07%
system.cpu.l2cache.ReadReq_miss_latency 10822415500 11584355000
761939500+7.04%
system.cpu.dcache.ReadReq_mshr_miss_latency 14062264500 14863694500
80143+5.70%
system.cpu.l2cache.ReadReq_miss_rate 0.182786 0.192941
0.010155+5.56%
system.cpu.l2cache.ReadReq_mshr_miss_rate 0.182786 0.192941
0.010155+5.56%
system.cpu.idleCycles24586339 256777931091454+4.44%
system.cpu.dcache.ReadReq_avg_mshr_miss_latency 8150.695480
8493.787248 343.091768+4.21%
system.cpu.l2cache.replacements 553099 575827 22728+4.11%
system.cpu.l2cache.demand_mshr_miss_latency 17475146000 18186565000
711419000+4.07%
[... showing top 20 errors only, additional errors omitted ...]
* build/X86_SE/tests/fast/long/20.parser/x86/linux/o3-timing FAILED!
changeset 9f704aa10eb4 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=9f704aa10eb4
description:
O3: Fix unaligned stores when cache blocked
Without this change the a store can be issued to the cache multiple
times.
If this case occurs when the l1 cache is out of mshrs (and thus blocked)
the processor will never make forward progress because each cycle it
will
send a single request using the recently freed mshr and not completing
the
multipart store. This will continue forever.
diffstat:
src/cpu/o3/lsq_unit_impl.hh | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diffs (14 lines):
diff -r 2af262e73961 -r 9f704aa10eb4 src/cpu/o3/lsq_unit_impl.hh
--- a/src/cpu/o3/lsq_unit_impl.hh Thu Mar 17 00:43:54 2011 -0400
+++ b/src/cpu/o3/lsq_unit_impl.hh Thu Mar 17 19:20:19 2011 -0500
@@ -1103,7 +1103,9 @@
dynamic_castLSQSenderState *(retryPkt-senderState);
// Don't finish the store unless this is the last packet.
-if (!TheISA::HasUnalignedMemAcc || !state-pktToSend) {
+if (!TheISA::HasUnalignedMemAcc || !state-pktToSend ||
+state-pendingPacket == retryPkt) {
+state-pktToSend = false;
storePostSend(retryPkt);
}
retryPkt = NULL;
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
On 04/03/11 09:44, Cron Daemon wrote:
* build/X86_SE/tests/fast/long/20.parser/x86/linux/o3-timing FAILED!
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic
passed.
* build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby
passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-timing
passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby
passed.
*
Re: [m5-dev] Review Request: O3: Tighten memory order violation checking to 16 bytes.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/520/#review1034 --- I can't verify 100% that the code in your new function is correct, but I don't see anything obviously wrong. I really like that you consolidated the same code in two places down to the one. There's one issue which is pointed out below. src/cpu/base_dyn_inst.hh http://reviews.m5sim.org/r/520/#comment1405 This comment is inaccurate. It's really the largest address that's part of the request, which is the effective address plus the size and then minus one. Also, this feels like a temporary variable promoted to too large of a scope and/or permanence. size seems like it would be more generally useful, it would be more immediately obvious what it is, and you can go from one to the other easily like you are elsewhere in this change. - Gabe On 2011-03-30 08:41:48, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/520/ --- (Updated 2011-03-30 08:41:48) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- O3: Tighten memory order violation checking to 16 bytes. The comment in the code suggests that the checking granularity should be 16 bytes, however in reality the shift by 8 is 256 bytes which seems much larger than required. Diffs - src/cpu/base_dyn_inst.hh d54b7775a6b0 src/cpu/o3/O3CPU.py d54b7775a6b0 src/cpu/o3/lsq_unit.hh d54b7775a6b0 src/cpu/o3/lsq_unit_impl.hh d54b7775a6b0 Diff: http://reviews.m5sim.org/r/520/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: CPU: Remove references to memory copy operations
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/612/#review1035 --- Looks good to me. - Gabe On 2011-03-30 08:58:56, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/612/ --- (Updated 2011-03-30 08:58:56) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- CPU: Remove references to memory copy operations Diffs - src/cpu/base_dyn_inst.hh d54b7775a6b0 src/cpu/inorder/inorder_dyn_inst.hh d54b7775a6b0 src/cpu/o3/commit_impl.hh d54b7775a6b0 src/cpu/ozone/lw_back_end_impl.hh d54b7775a6b0 src/cpu/static_inst.hh d54b7775a6b0 src/cpu/thread_state.hh d54b7775a6b0 Diff: http://reviews.m5sim.org/r/612/diff Testing --- Thanks, Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: CPU: Remove references to memory copy operations
This does interact with Korey's change a bit, so hopefully we don't end up stepping on each other too much. Since somebody's going to have to update a patch anyway, I'll look at whether that result stuff in the dyninst can go away too. Gabe On 03/30/11 12:09, Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/612/#review1035 --- Looks good to me. - Gabe On 2011-03-30 08:58:56, Ali Saidi wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/612/ --- (Updated 2011-03-30 08:58:56) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- CPU: Remove references to memory copy operations Diffs - src/cpu/base_dyn_inst.hh d54b7775a6b0 src/cpu/inorder/inorder_dyn_inst.hh d54b7775a6b0 src/cpu/o3/commit_impl.hh d54b7775a6b0 src/cpu/ozone/lw_back_end_impl.hh d54b7775a6b0 src/cpu/static_inst.hh d54b7775a6b0 src/cpu/thread_state.hh d54b7775a6b0 Diff: http://reviews.m5sim.org/r/612/diff Testing --- Thanks, Ali ___ 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
