Re: [m5-dev] changeset in m5: style: clean up the Packet stuff
It's true that I added some extra assertions, but I worry that this
assertion actually caught a real bug, not a bogus assertion.
If you're calling allocate and data is nonzero but there is no valid
data flag, I assert that something else is going wrong.
Or does my logic not make sense?
On Wed, Nov 12, 2008 at 4:48 PM, Beckmann, Brad [EMAIL PROTECTED] wrote:
The allocate() change implements the previous logic of allocate() before
your update. I'm not certain whether my allocate change is what you had
intended. I just know that I needed the previous behavior of allocate()
to run NetperfMaerts. Otherwise I encountered the following assertion
error:
build/ALPHA_FS/mem/packet.hh:705: void Packet::allocate(): Assertion
`flags.none(STATIC_DATA|DYNAMIC_DATA)' failed.
Brad
-Original Message-
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of nathan
binkert
Sent: Wednesday, November 12, 2008 4:42 PM
To: Beckmann, Brad
Cc: M5 Developer List
Subject: Re: [m5-dev] changeset in m5: style: clean up the Packet
stuff
The VALID_DST change is correct, but I don't quite understand the
allocate() change. You got rid of the assertion that there is valid
data if data is true. You also added an assertion to make sure that
there is no static data if data is false.
Why are those checks better? Are you not allowed to switch from
static data to dynamic data?
Please go ahead and commit the VALID_DST change. Sorry about that. If
you're certain about the allocate() change, that's fine with me.
Interestingly, I passed all of the regressions. Don't you love that?
Nate
On Wed, Nov 12, 2008 at 4:33 PM, Beckmann, Brad
[EMAIL PROTECTED]
wrote:
Hi Nate,
I noticed that these changes introduced a couple bugs. I believe
the
Packet constructors should set the VALID_DST flag and the logic of
the
allocate function is incorrect. I believe we should make the
following
changes to packet.hh. Would you like me to go ahead and update the
m5-dev repository?
Brad
diff -r 4f5d99098862 src/mem/packet.hh
--- a/src/mem/packet.hh Tue Nov 11 11:41:19 2008 -0800
+++ b/src/mem/packet.hh Wed Nov 12 16:31:55 2008 -0800
@@ -461,8 +461,8 @@ class Packet : public FastAlloc, public
* supplied.
*/
Packet(Request *_req, MemCmd _cmd, NodeID _dest)
-: cmd(_cmd), req(_req), data(NULL), addr(_req-paddr),
- size(_req-size), dest(_dest), time(curTick),
senderState(NULL)
+: flags(VALID_DST), cmd(_cmd), req(_req), data(NULL),
addr(_req-paddr),
+ size(_req-size), dest(_dest), time(curTick),
senderState(NULL)
{
}
@@ -472,7 +472,7 @@ class Packet : public FastAlloc, public
* req. this allows for overriding the size/addr of the req.
*/
Packet(Request *_req, MemCmd _cmd, NodeID _dest, int _blkSize)
-: cmd(_cmd), req(_req), data(NULL),
+: flags(VALID_DST), cmd(_cmd), req(_req), data(NULL),
addr(_req-paddr ~(_blkSize - 1)), size(_blkSize),
dest(_dest),
time(curTick), senderState(NULL)
{
@@ -701,12 +701,11 @@ class Packet : public FastAlloc, public
void
allocate()
{
-if (data) {
-assert(flags.none(STATIC_DATA|DYNAMIC_DATA));
-} else {
-flags.set(DYNAMIC_DATA|ARRAY_DATA);
-data = new uint8_t[getSize()];
-}
+if (data)
+return;
+assert(flags.none(STATIC_DATA));
+flags.set(DYNAMIC_DATA|ARRAY_DATA);
+data = new uint8_t[getSize()];
}
-Original Message-
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On
Behalf Of nathan binkert
Sent: Monday, November 10, 2008 6:55 PM
To: m5-dev@m5sim.org
Subject: Re: [m5-dev] changeset in m5: style: clean up the Packet
stuff
This diff went way beyond style cleanup. Unfortunately, I didn't
remember to update the commit message. One of the big things I did
was get rid of all of the individual boolean variable flags and
create
a single flags variable. I also made all of the flag values static
members of their classes instead of globals.
Nate
On Mon, Nov 10, 2008 at 6:49 PM, Nathan Binkert [EMAIL PROTECTED]
wrote:
changeset a88e8e7dec75 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=a88e8e7dec75
description:
style: clean up the Packet stuff
diffstat:
3 files changed, 24 insertions(+), 32 deletions(-)
src/mem/packet.cc |1
src/mem/packet.hh |1
src/mem/request.hh | 54
+--
-
diffs (truncated from 1501 to 300 lines):
diff -r f3733e2b19d5 -r a88e8e7dec75 src/mem/packet.cc
--- a/src/mem/packet.cc Mon Nov 10 11:51:17 2008 -0800
+++ b/src/mem/packet.cc Mon Nov 10 11:51:17 2008 -0800
@@ -41,6 +41,8 @@
#include base/misc.hh
#include base/trace.hh
Re: [m5-dev] changeset in m5: style: clean up the Packet stuff
The packet in question is a ReadReq to the iobus. Below is the Pkt information and the call stack. If you notice, the pkt is created in the function AtomicSimpleCPU::read() and the pkt's static data flag should be set after the pkt.dataStatic(dataPtr) call. I'm not sure what is going on. I will investigate further and let you know. Brad 1653925: testsys.iobus: recvAtomic: packet src 0 dest -1 addr 0x802 cmd ReadReq #0 0x002a9628e745 in raise () from /lib64/tls/libc.so.6 #1 0x002a9628feb3 in abort () from /lib64/tls/libc.so.6 #2 0x002a96287dc9 in __assert_fail () from /lib64/tls/libc.so.6 #3 0x0054051d in Packet::allocate (this=0x7fbfffc9d0) at build/ALPHA_FS/mem/packet.hh:711 #4 0x006629e8 in AlphaBackdoor::read (this=0x18d2180, pkt=0x7fbfffc9d0) at build/ALPHA_FS/dev/alpha/backdoor.cc:109 #5 0x005de359 in PioPort::recvAtomic (this=0x1cd96f0, pkt=0x7fbfffc9d0) at build/ALPHA_FS/dev/io_device.cc:46 #6 0x005261a0 in Port::sendAtomic (this=0x1cd9830, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/port.hh:194 #7 0x0068c8f2 in Bus::recvAtomic (this=0x18dd010, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/bus.cc:406 #8 0x006987b8 in Bus::BusPort::recvAtomic (this=0x1cd88c0, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/bus.hh:94 #9 0x005261a0 in Port::sendAtomic (this=0x18ade70, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/port.hh:194 #10 0x0067f4d5 in Bridge::BridgePort::recvAtomic (this=0x18adf40, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/bridge.cc:313 #11 0x005261a0 in Port::sendAtomic (this=0x1cd8320, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/port.hh:194 #12 0x0068c8f2 in Bus::recvAtomic (this=0x18a0ea0, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/bus.cc:406 #13 0x006987b8 in Bus::BusPort::recvAtomic (this=0x1cd8650, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/bus.hh:94 #14 0x005261a0 in Port::sendAtomic (this=0x18d0298, pkt=0x7fbfffc9d0) at build/ALPHA_FS/mem/port.hh:194 #15 0x005388b7 in AtomicSimpleCPU::readunsigned int (this=0x18d, addr=18446740783764602880, [EMAIL PROTECTED], flags=0) at build/ALPHA_FS/cpu/simple/atomic.cc:332 #16 0x00780b88 in AlphaISAInst::Ldl::execute (this=0x1d4d540, xc=0x18d, traceData=0x0) at build/ALPHA_FS/arch/alpha/atomic_simple_cpu_exec.cc:1269 #17 0x0052f130 in AtomicSimpleCPU::tick (this=0x18d) at build/ALPHA_FS/cpu/simple/atomic.cc:759 #18 0x0052f4bb in AtomicSimpleCPU::TickEvent::process (this=0x18d0210) at build/ALPHA_FS/cpu/simple/atomic.cc:54 #19 0x006e8e25 in EventQueue::serviceOne (this=0xcc1100) at build/ALPHA_FS/sim/eventq.cc:186 #20 0x0071525e in simulate (num_cycles=9223372036854775807) at build/ALPHA_FS/sim/simulate.cc:73 -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of nathan binkert Sent: Wednesday, November 12, 2008 4:54 PM To: Beckmann, Brad Cc: M5 Developer List Subject: Re: [m5-dev] changeset in m5: style: clean up the Packet stuff It's true that I added some extra assertions, but I worry that this assertion actually caught a real bug, not a bogus assertion. If you're calling allocate and data is nonzero but there is no valid data flag, I assert that something else is going wrong. Or does my logic not make sense? On Wed, Nov 12, 2008 at 4:48 PM, Beckmann, Brad [EMAIL PROTECTED] wrote: The allocate() change implements the previous logic of allocate() before your update. I'm not certain whether my allocate change is what you had intended. I just know that I needed the previous behavior of allocate() to run NetperfMaerts. Otherwise I encountered the following assertion error: build/ALPHA_FS/mem/packet.hh:705: void Packet::allocate(): Assertion `flags.none(STATIC_DATA|DYNAMIC_DATA)' failed. Brad -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of nathan binkert Sent: Wednesday, November 12, 2008 4:42 PM To: Beckmann, Brad Cc: M5 Developer List Subject: Re: [m5-dev] changeset in m5: style: clean up the Packet stuff The VALID_DST change is correct, but I don't quite understand the allocate() change. You got rid of the assertion that there is valid data if data is true. You also added an assertion to make sure that there is no static data if data is false. Why are those checks better? Are you not allowed to switch from static data to dynamic data? Please go ahead and commit the VALID_DST change. Sorry about that. If you're certain about the allocate() change, that's fine with me. Interestingly, I passed all of the regressions. Don't you love that? Nate On Wed, Nov 12, 2008 at 4:33 PM, Beckmann, Brad [EMAIL PROTECTED] wrote: Hi Nate, I noticed that these changes introduced a couple bugs. I believe the Packet constructors should set the VALID_DST flag and the logic of the allocate function is incorrect
Re: [m5-dev] changeset in m5: style: clean up the Packet stuff
Isn't the assertion backwards? Looks to me that if data is nonzero you're asserting that neither STATIC_DATA nor DYNAMIC_DATA is set. Don't you need a '!' in that assertion? Or change it to flags.any() or whatever the right operator is? You're right of course. It should be flags.any(). Maybe you should run your regressions with m5.opt to catch assertion errors, especially when you're mucking with lots of assertions... I generally do run opt, but clearly I did not in this case. Sorry for the hassle. If you change the none to any in the first part of the if block, I think things should pass brad. If you like I can take care of it if you need. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: clean up the Packet stuff
This diff went way beyond style cleanup. Unfortunately, I didn't
remember to update the commit message. One of the big things I did
was get rid of all of the individual boolean variable flags and create
a single flags variable. I also made all of the flag values static
members of their classes instead of globals.
Nate
On Mon, Nov 10, 2008 at 6:49 PM, Nathan Binkert [EMAIL PROTECTED] wrote:
changeset a88e8e7dec75 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=a88e8e7dec75
description:
style: clean up the Packet stuff
diffstat:
3 files changed, 24 insertions(+), 32 deletions(-)
src/mem/packet.cc |1
src/mem/packet.hh |1
src/mem/request.hh | 54 +---
diffs (truncated from 1501 to 300 lines):
diff -r f3733e2b19d5 -r a88e8e7dec75 src/mem/packet.cc
--- a/src/mem/packet.cc Mon Nov 10 11:51:17 2008 -0800
+++ b/src/mem/packet.cc Mon Nov 10 11:51:17 2008 -0800
@@ -41,6 +41,8 @@
#include base/misc.hh
#include base/trace.hh
#include mem/packet.hh
+
+using namespace std;
// The one downside to bitsets is that static initializers can get ugly.
#define SET1(a1) (1 (a1))
@@ -133,35 +135,6 @@
{ SET2(IsRequest, IsPrint), InvalidCmd, PrintReq }
};
-
-/** delete the data pointed to in the data pointer. Ok to call to matter how
- * data was allocted. */
-void
-Packet::deleteData()
-{
-assert(staticData || dynamicData);
-if (staticData)
-return;
-
-if (arrayData)
-delete [] data;
-else
-delete data;
-}
-
-/** If there isn't data in the packet, allocate some. */
-void
-Packet::allocate()
-{
-if (data)
-return;
-assert(!staticData);
-dynamicData = true;
-arrayData = true;
-data = new uint8_t[getSize()];
-}
-
-
bool
Packet::checkFunctional(Printable *obj, Addr addr, int size, uint8_t *data)
{
@@ -193,7 +166,7 @@
if (isRead()) {
if (func_start = val_start func_end = val_end) {
allocate();
-std::memcpy(getPtruint8_t(), data + offset, getSize());
+memcpy(getPtruint8_t(), data + offset, getSize());
makeResponse();
return true;
} else {
@@ -208,11 +181,12 @@
}
} else if (isWrite()) {
if (offset = 0) {
-std::memcpy(data + offset, getPtruint8_t(),
-(std::min(func_end, val_end) - func_start) + 1);
-} else { // val_start func_start
-std::memcpy(data, getPtruint8_t() - offset,
-(std::min(func_end, val_end) - val_start) + 1);
+memcpy(data + offset, getPtruint8_t(),
+ (min(func_end, val_end) - func_start) + 1);
+} else {
+// val_start func_start
+memcpy(data, getPtruint8_t() - offset,
+ (min(func_end, val_end) - val_start) + 1);
}
} else {
panic(Don't know how to handle command %s\n, cmdString());
@@ -222,22 +196,18 @@
return false;
}
-
void
-Packet::print(std::ostream o, const int verbosity,
- const std::string prefix) const
+Packet::print(ostream o, const int verbosity, const string prefix) const
{
ccprintf(o, %s[%x:%x] %s\n, prefix,
getAddr(), getAddr() + getSize() - 1, cmdString());
}
-
-Packet::PrintReqState::PrintReqState(std::ostream _os, int _verbosity)
-: curPrefixPtr(new std::string()), os(_os), verbosity(_verbosity)
+Packet::PrintReqState::PrintReqState(ostream _os, int _verbosity)
+: curPrefixPtr(new string()), os(_os), verbosity(_verbosity)
{
labelStack.push_back(LabelStackEntry(, curPrefixPtr));
}
-
Packet::PrintReqState::~PrintReqState()
{
@@ -246,21 +216,17 @@
delete curPrefixPtr;
}
-
Packet::PrintReqState::
-LabelStackEntry::LabelStackEntry(const std::string _label,
- std::string *_prefix)
+LabelStackEntry::LabelStackEntry(const string _label, string *_prefix)
: label(_label), prefix(_prefix), labelPrinted(false)
{
}
-
void
-Packet::PrintReqState::pushLabel(const std::string lbl,
- const std::string prefix)
+Packet::PrintReqState::pushLabel(const string lbl, const string prefix)
{
labelStack.push_back(LabelStackEntry(lbl, curPrefixPtr));
-curPrefixPtr = new std::string(*curPrefixPtr);
+curPrefixPtr = new string(*curPrefixPtr);
*curPrefixPtr += prefix;
}
diff -r f3733e2b19d5 -r a88e8e7dec75 src/mem/packet.hh
--- a/src/mem/packet.hh Mon Nov 10 11:51:17 2008 -0800
+++ b/src/mem/packet.hh Mon Nov 10 11:51:17 2008 -0800
@@ -42,8 +42,10 @@
#include list
#include bitset
+#include base/cast.hh
#include base/compiler.hh
#include base/fast_alloc.hh
+#include base/flags.hh
#include base/misc.hh
#include base/printable.hh
#include mem/request.hh
@@ -58,9 +60,12 @@
Re: [m5-dev] changeset in m5: style: Make a style pass over the whole arch/al...
I thought that we had agreed to always use braces for control structures
(for, if, while, etc.) since that makes it easier to add/remove lines
without worrying about adding/removing braces too. I don't see it mentioned
either way on the coding style page, but I know I've developed the habit of
using braces unconditionally based on my recollection of that decision.
I don't really have a strong opinion either way; the #1 thing is that we
should agree and get it down on the wiki page so that these style updates
converge rather than oscillating.
Steve
On Sat, Sep 27, 2008 at 9:04 PM, Nathan Binkert [EMAIL PROTECTED] wrote:
void
copyIprs(ThreadContext *src, ThreadContext *dest)
{
-for (int i = 0; i NumInternalProcRegs; ++i) {
+for (int i = 0; i NumInternalProcRegs; ++i)
dest-setMiscRegNoEffect(i, src-readMiscRegNoEffect(i));
-}
}
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: Make a style pass over the whole arch/al...
Also, please make these sorts of large scale formatting changes
judiciously. There's a large collection of patches out there and it can
be non-trivial to keep them applying correctly.
Gabe
Steve Reinhardt wrote:
I thought that we had agreed to always use braces for control
structures (for, if, while, etc.) since that makes it easier to
add/remove lines without worrying about adding/removing braces too. I
don't see it mentioned either way on the coding style page, but I know
I've developed the habit of using braces unconditionally based on my
recollection of that decision.
I don't really have a strong opinion either way; the #1 thing is that
we should agree and get it down on the wiki page so that these style
updates converge rather than oscillating.
Steve
On Sat, Sep 27, 2008 at 9:04 PM, Nathan Binkert [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
void
copyIprs(ThreadContext *src, ThreadContext *dest)
{
-for (int i = 0; i NumInternalProcRegs; ++i) {
+for (int i = 0; i NumInternalProcRegs; ++i)
dest-setMiscRegNoEffect(i, src-readMiscRegNoEffect(i));
-}
}
___
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] changeset in m5: style: Make a style pass over the whole arch/al...
I thought that we had agreed to always use braces for control structures (for, if, while, etc.) since that makes it easier to add/remove lines without worrying about adding/removing braces too. I don't see it mentioned either way on the coding style page, but I know I've developed the habit of using braces unconditionally based on my recollection of that decision. oh, I thought the agreement was that you use braces if there is an else, but if it's just a simple two liner, you don't have to. I don't really have a strong opinion either way; the #1 thing is that we should agree and get it down on the wiki page so that these style updates converge rather than oscillating. Agreed. What do you think about my above statement? If the whole expression fits in two lines, no braces required. More than two requires braces. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: Make a style pass over the whole arch/al...
I don't really have a strong opinion either way; the #1 thing is that we should agree and get it down on the wiki page so that these style updates converge rather than oscillating. Agreed. What do you think about my above statement? If the whole expression fits in two lines, no braces required. More than two requires braces. I think that sounds fine. Does no braces required also mean no braces allowed, or is that something left up to the implementers discretion? I think it should be optional rather than forbidden. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: Make a style pass over the whole arch/al...
I think that sounds fine. Does no braces required also mean no braces allowed, or is that something left up to the implementers discretion? I think it should be optional rather than forbidden. I'd agree with optional. ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: Make a style pass over the whole arch/al...
On Sat, Sep 27, 2008 at 9:35 PM, nathan binkert [EMAIL PROTECTED] wrote: I think that sounds fine. Does no braces required also mean no braces allowed, or is that something left up to the implementers discretion? I think it should be optional rather than forbidden. I'd agree with optional. Optional is OK with me, but in that case it's not something that should be fixed in a style update. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: Make a style pass over the whole arch/al...
Optional is OK with me, but in that case it's not something that should be fixed in a style update. True, my bad. I was just going really fast. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style: These files didn't even come close to fo...
yeah, I just did this quickly and I guess I missed switch in my
regexp. I just committed fix.
I've been toying around with an emacs script to go through and fix up
some of the basics.
It might be worth spending some effort to get a beautifier working.
There's one that seems to still be getting updated called uncrustify.
It's in the ubuntu distros.
Nate
2008/9/26 Steve Reinhardt [EMAIL PROTECTED]:
Are you still missing spaces in switch (foo) in a couple places?
On Fri, Sep 26, 2008 at 3:29 PM, Nathan Binkert [EMAIL PROTECTED] wrote:
changeset cb98f0fcc6c6 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=cb98f0fcc6c6
description:
style: These files didn't even come close to following the M5 style
guide.
diffstat:
2 files changed, 27 insertions(+), 145 deletions(-)
src/arch/mips/dsp.cc | 32 ---
src/arch/mips/dsp.hh | 140
+-
diffs (truncated from 1956 to 300 lines):
diff -r 03c186e416aa -r cb98f0fcc6c6 src/arch/mips/dsp.cc
--- a/src/arch/mips/dsp.cc Fri Sep 26 07:44:07 2008 -0700
+++ b/src/arch/mips/dsp.cc Fri Sep 26 08:18:53 2008 -0700
@@ -40,92 +40,84 @@
using namespace std;
int32_t
-MipsISA::bitrev( int32_t value )
+MipsISA::bitrev(int32_t value)
{
int32_t result = 0;
-int i, shift;
+int shift;
-for( i=0; i16; i++ )
-{
-shift = 2*i - 15;
+for (int i = 0; i 16; i++) {
+shift = 2 * i - 15;
-if( shift 0 )
-result |= (value 1Li) -shift;
+if (shift 0)
+result |= (value 1 i) -shift;
else
-result |= (value 1Li) shift;
+result |= (value 1 i) shift;
}
return result;
}
uint64_t
-MipsISA::dspSaturate( uint64_t value, int32_t fmt, int32_t sign, uint32_t
*overflow )
+MipsISA::dspSaturate(uint64_t value, int32_t fmt, int32_t sign,
+uint32_t *overflow)
{
-int64_t svalue;
+int64_t svalue = (int64_t)value;
-svalue = (int64_t)value;
-
-switch( sign )
-{
+switch(sign) {
case SIGNED:
-if( svalue (int64_t)FIXED_SMAX[fmt] )
-{
+if (svalue (int64_t)FIXED_SMAX[fmt]) {
*overflow = 1;
svalue = (int64_t)FIXED_SMAX[fmt];
-}
-else if( svalue (int64_t)FIXED_SMIN[fmt] )
-{
+} else if (svalue (int64_t)FIXED_SMIN[fmt]) {
*overflow = 1;
svalue = (int64_t)FIXED_SMIN[fmt];
}
break;
case UNSIGNED:
-if( svalue (int64_t)FIXED_UMAX[fmt] )
-{
+if (svalue (int64_t)FIXED_UMAX[fmt]) {
*overflow = 1;
svalue = FIXED_UMAX[fmt];
-}
-else if( svalue (int64_t)FIXED_UMIN[fmt] )
-{
+} else if (svalue (int64_t)FIXED_UMIN[fmt]) {
*overflow = 1;
svalue = FIXED_UMIN[fmt];
}
break;
}
-return( (uint64_t)svalue );
+return (uint64_t)svalue;
}
uint64_t
-MipsISA::checkOverflow( uint64_t value, int32_t fmt, int32_t sign,
uint32_t *overflow )
+MipsISA::checkOverflow(uint64_t value, int32_t fmt, int32_t sign,
+uint32_t *overflow)
{
-int64_t svalue;
+int64_t svalue = (int64_t)value;
-svalue = (int64_t)value;
-
-switch( sign )
+switch(sign)
{
case SIGNED:
-if( svalue (int64_t)FIXED_SMAX[fmt] || svalue
(int64_t)FIXED_SMIN[fmt] )
+if (svalue (int64_t)FIXED_SMAX[fmt] ||
+svalue (int64_t)FIXED_SMIN[fmt])
*overflow = 1;
break;
case UNSIGNED:
-if( svalue (int64_t)FIXED_UMAX[fmt] || svalue
(int64_t)FIXED_UMIN[fmt] )
+if (svalue (int64_t)FIXED_UMAX[fmt] ||
+svalue (int64_t)FIXED_UMIN[fmt])
*overflow = 1;
break;
}
-return( (uint64_t)svalue );
+return (uint64_t)svalue;
}
uint64_t
-MipsISA::signExtend( uint64_t value, int32_t fmt )
+MipsISA::signExtend(uint64_t value, int32_t fmt)
{
int32_t signpos = SIMD_NBITS[fmt];
-uint64_t sign = uint64_t(1)(signpos-1);
+uint64_t sign = uint64_t(1) (signpos - 1);
uint64_t ones = ~(0ULL);
-if( value sign )
+if (value sign)
value |= (ones signpos); // extend with ones
else
value = (ones (64 - signpos)); // extend with zeros
@@ -134,231 +126,230 @@
}
uint64_t
-MipsISA::addHalfLsb( uint64_t value, int32_t lsbpos )
+MipsISA::addHalfLsb(uint64_t value, int32_t lsbpos)
{
-return( value += ULL(1) (lsbpos-1) );
+return value += ULL(1) (lsbpos - 1);
}
int32_t
-MipsISA::dspAbs( int32_t a, int32_t fmt, uint32_t *dspctl )
+MipsISA::dspAbs(int32_t a, int32_t fmt, uint32_t *dspctl)
{
-int i = 0;
int nvals = SIMD_NVALS[fmt];
int32_t result;
int64_t svalue;
uint32_t ouflag = 0;
uint64_t a_values[SIMD_MAX_VALS];
-
Re: [m5-dev] changeset in m5: style: Remove non-leading tabs everywhere they ...
I really hope this isn't going to make applying all my patches significantly harder... Gabe Quoting Ali Saidi [EMAIL PROTECTED]: changeset 3af77710f397 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=3af77710f397 description: style: Remove non-leading tabs everywhere they shouldn't be. Developers should configure their editors to not insert tabs diffstat: 69 files changed, 427 insertions(+), 682 deletions(-) configs/common/Benchmarks.py | 17 src/arch/alpha/aout_machdep.h | 21 src/arch/alpha/floatregfile.hh |1 src/arch/alpha/ipr.cc | 53 -- src/arch/alpha/ipr.hh | 54 +- src/arch/alpha/isa_traits.hh |4 src/arch/alpha/linux/linux.cc | 16 src/arch/alpha/linux/linux.hh | 14 src/arch/alpha/miscregfile.hh |4 src/arch/alpha/osfpal.cc | 193 ++- src/arch/alpha/pagetable.hh|8 src/arch/alpha/regfile.hh |1 src/arch/alpha/system.cc |2 src/arch/alpha/tru64/tru64.cc | 16 src/arch/alpha/tru64/tru64.hh | 14 src/arch/mips/isa_traits.hh|4 src/arch/mips/linux/linux.cc | 16 src/arch/mips/linux/linux.hh | 14 src/arch/mips/regfile/regfile.hh |6 src/arch/mips/system.cc|2 src/arch/mips/tlb.hh |2 src/arch/sparc/linux/linux.cc | 16 src/arch/sparc/linux/linux.hh | 14 src/arch/sparc/miscregfile.hh | 22 src/arch/sparc/regfile.hh |1 src/arch/sparc/solaris/solaris.cc | 18 src/arch/sparc/solaris/solaris.hh | 10 src/arch/sparc/sparc_traits.hh |1 src/arch/x86/isa/insts/general_purpose/cache_and_memory_management.py |4 src/arch/x86/isa/insts/general_purpose/data_conversion/ascii_adjust.py |2 src/arch/x86/isa/insts/general_purpose/load_segment_registers.py |4 src/arch/x86/isa/insts/general_purpose/system_calls.py |2 src/arch/x86/linux/linux.hh| 14 src/base/crc.cc|1 src/base/inifile.hh|1 src/base/loader/coff_symconst.h| 12 src/base/stats/flags.hh|9 src/cpu/base_dyn_inst.hh |1 src/cpu/checker/cpu_impl.hh|2 src/cpu/memtest/memtest.hh |2 src/cpu/o3/alpha/dyn_inst.hh |1 src/cpu/o3/mips/dyn_inst.hh|1 src/cpu/simple_thread.cc |1 src/cpu/static_inst.hh | 16 src/dev/alpha/access.h | 14 src/dev/etherdump.cc |8 src/dev/mips/access.h | 16 src/dev/ns_gige.hh |3 src/dev/pcireg.h | 11 src/kern/linux/linux.hh| 28 - src/kern/operatingsystem.hh|1 src/kern/solaris/solaris.hh| 22 src/kern/tru64/mbuf.hh | 33 - src/kern/tru64/tru64_syscalls.cc | 243 -- src/mem/cache/blk.hh |1 src/mem/cache/builder.cc | 26 - src/mem/cache/prefetch/stride.cc |2
Re: [m5-dev] changeset in m5: style: Remove non-leading tabs everywhere they ...
run expand on the patch files before trying to apply them... Everything should work fine. Ali On Sep 10, 2008, at 4:44 PM, [EMAIL PROTECTED] wrote: I really hope this isn't going to make applying all my patches significantly harder... Gabe Quoting Ali Saidi [EMAIL PROTECTED]: changeset 3af77710f397 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=3af77710f397 description: style: Remove non-leading tabs everywhere they shouldn't be. Developers should configure their editors to not insert tabs diffstat: 69 files changed, 427 insertions(+), 682 deletions(-) configs/common/ Benchmarks.py | 17 src/arch/alpha/ aout_machdep.h | 21 src/arch/alpha/ floatregfile.hh |1 src/arch/alpha/ ipr.cc | 53 -- src/arch/alpha/ ipr.hh | 54 +- src/arch/alpha/ isa_traits.hh |4 src/arch/alpha/linux/ linux.cc | 16 src/arch/alpha/linux/ linux.hh | 14 src/arch/alpha/ miscregfile.hh |4 src/arch/alpha/ osfpal.cc | 193 ++- src/arch/alpha/ pagetable.hh|8 src/arch/alpha/ regfile.hh |1 src/arch/alpha/ system.cc |2 src/arch/alpha/tru64/ tru64.cc | 16 src/arch/alpha/tru64/ tru64.hh | 14 src/arch/mips/ isa_traits.hh|4 src/arch/mips/linux/ linux.cc | 16 src/arch/mips/linux/ linux.hh | 14 src/arch/mips/regfile/ regfile.hh |6 src/arch/mips/ system.cc|2 src/arch/mips/ tlb.hh |2 src/arch/sparc/linux/ linux.cc | 16 src/arch/sparc/linux/ linux.hh | 14 src/arch/sparc/ miscregfile.hh | 22 src/arch/sparc/ regfile.hh |1 src/arch/sparc/solaris/ solaris.cc | 18 src/arch/sparc/solaris/ solaris.hh | 10 src/arch/sparc/ sparc_traits.hh |1 src/arch/x86/isa/insts/general_purpose/ cache_and_memory_management.py |4 src/arch/x86/isa/insts/general_purpose/data_conversion/ ascii_adjust.py |2 src/arch/x86/isa/insts/general_purpose/ load_segment_registers.py |4 src/arch/x86/isa/insts/general_purpose/ system_calls.py |2 src/arch/x86/linux/ linux.hh| 14 src/base/ crc.cc|1 src/base/ inifile.hh|1 src/base/loader/ coff_symconst.h| 12 src/base/stats/ flags.hh|9 src/cpu/ base_dyn_inst.hh |1 src/cpu/checker/ cpu_impl.hh|2 src/cpu/memtest/ memtest.hh |2 src/cpu/o3/alpha/ dyn_inst.hh |1 src/cpu/o3/mips/ dyn_inst.hh|1 src/cpu/ simple_thread.cc |1 src/cpu/ static_inst.hh | 16 src/dev/alpha/ access.h | 14 src/dev/ etherdump.cc |8 src/dev/mips/ access.h | 16 src/dev/ ns_gige.hh |3 src/dev/ pcireg.h | 11 src/kern/linux/ linux.hh| 28 - src/kern/ operatingsystem.hh|1 src/kern/solaris/ solaris.hh| 22 src/kern/tru64/ mbuf.hh | 33 - src/kern/tru64/
Re: [m5-dev] changeset in m5: style: Remove non-leading tabs everywhere they ...
we've hacked on remote_gdb so much that preserving the tabs wouldn't really help. Ali On Sep 10, 2008, at 4:48 PM, [EMAIL PROTECTED] wrote: Also I notice some of these are changes to the *bsd copyrights on some files, for instance the remote_gdbs. We probably shouldn't change those. Gabe Quoting Ali Saidi [EMAIL PROTECTED]: changeset 3af77710f397 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=3af77710f397 description: style: Remove non-leading tabs everywhere they shouldn't be. Developers should configure their editors to not insert tabs diffstat: 69 files changed, 427 insertions(+), 682 deletions(-) configs/common/ Benchmarks.py | 17 src/arch/alpha/ aout_machdep.h | 21 src/arch/alpha/ floatregfile.hh |1 src/arch/alpha/ ipr.cc | 53 -- src/arch/alpha/ ipr.hh | 54 +- src/arch/alpha/ isa_traits.hh |4 src/arch/alpha/linux/ linux.cc | 16 src/arch/alpha/linux/ linux.hh | 14 src/arch/alpha/ miscregfile.hh |4 src/arch/alpha/ osfpal.cc | 193 ++- src/arch/alpha/ pagetable.hh|8 src/arch/alpha/ regfile.hh |1 src/arch/alpha/ system.cc |2 src/arch/alpha/tru64/ tru64.cc | 16 src/arch/alpha/tru64/ tru64.hh | 14 src/arch/mips/ isa_traits.hh|4 src/arch/mips/linux/ linux.cc | 16 src/arch/mips/linux/ linux.hh | 14 src/arch/mips/regfile/ regfile.hh |6 src/arch/mips/ system.cc|2 src/arch/mips/ tlb.hh |2 src/arch/sparc/linux/ linux.cc | 16 src/arch/sparc/linux/ linux.hh | 14 src/arch/sparc/ miscregfile.hh | 22 src/arch/sparc/ regfile.hh |1 src/arch/sparc/solaris/ solaris.cc | 18 src/arch/sparc/solaris/ solaris.hh | 10 src/arch/sparc/ sparc_traits.hh |1 src/arch/x86/isa/insts/general_purpose/ cache_and_memory_management.py |4 src/arch/x86/isa/insts/general_purpose/data_conversion/ ascii_adjust.py |2 src/arch/x86/isa/insts/general_purpose/ load_segment_registers.py |4 src/arch/x86/isa/insts/general_purpose/ system_calls.py |2 src/arch/x86/linux/ linux.hh| 14 src/base/ crc.cc|1 src/base/ inifile.hh|1 src/base/loader/ coff_symconst.h| 12 src/base/stats/ flags.hh|9 src/cpu/ base_dyn_inst.hh |1 src/cpu/checker/ cpu_impl.hh|2 src/cpu/memtest/ memtest.hh |2 src/cpu/o3/alpha/ dyn_inst.hh |1 src/cpu/o3/mips/ dyn_inst.hh|1 src/cpu/ simple_thread.cc |1 src/cpu/ static_inst.hh | 16 src/dev/alpha/ access.h | 14 src/dev/ etherdump.cc |8 src/dev/mips/ access.h | 16 src/dev/ ns_gige.hh |3 src/dev/ pcireg.h | 11 src/kern/linux/ linux.hh| 28 - src/kern/ operatingsystem.hh|1 src/kern/solaris/ solaris.hh| 22 src/kern/tru64/ mbuf.hh
Re: [m5-dev] changeset in m5: style
On the topic of style violations, I've seen an increasing number of 'if('
and 'for(' (with no space) creeping in (see below). Another thing I've seen
that irritates me (but isn't officially banned by the style code ... yet
...) is C++-style comments with no space between the // and the comment:
int foo; //this is a foo
Anyone else bugged by this?
Steve
% find src -name '*.cc' -o -name '*.hh' | xargs util/chkformat -nv | grep
'improper spacing' | sed -e 's/:.*//' | sort -u
src/arch/alpha/faults.cc
src/arch/alpha/ipr.cc
src/arch/alpha/tlb.cc
src/arch/mips/dsp.cc
src/arch/mips/faults.cc
src/arch/mips/isa_traits.hh
src/arch/mips/regfile/int_regfile.cc
src/arch/mips/regfile/misc_regfile.cc
src/arch/mips/system.cc
src/arch/mips/tlb.cc
src/arch/mips/utility.hh
src/arch/mips/vtophys.cc
src/arch/sparc/faults.cc
src/arch/sparc/intregfile.cc
src/arch/sparc/linux/syscalls.cc
src/arch/sparc/predecoder.hh
src/arch/sparc/process.cc
src/arch/sparc/regfile.cc
src/arch/sparc/remote_gdb.cc
src/arch/sparc/tlb.cc
src/arch/x86/faults.cc
src/arch/x86/insts/microldstop.cc
src/arch/x86/insts/microregop.cc
src/arch/x86/insts/static_inst.cc
src/arch/x86/insts/static_inst.hh
src/arch/x86/pagetable_walker.cc
src/arch/x86/predecoder.cc
src/arch/x86/predecoder.hh
src/arch/x86/process.cc
src/arch/x86/types.hh
src/base/loader/elf_object.cc
src/base/loader/hex_file.cc
src/cpu/legiontrace.cc
src/cpu/nativetrace.cc
src/cpu/nativetrace.hh
src/cpu/o3/inst_queue_impl.hh
src/cpu/o3/rename_impl.hh
src/cpu/simple/atomic.cc
src/cpu/simple/base.cc
src/dev/alpha/tsunami_cchip.cc
src/dev/i8254xGBe.hh
src/dev/intel_8254_timer.cc
src/dev/mips/malta_cchip.cc
src/dev/mips/malta_io.cc
src/kern/tru64/tru64_events.cc
src/mem/bus.cc
src/mem/dram.cc
src/mem/packet.hh
src/mem/page_table.cc
src/sim/process.cc
src/sim/sim_object.cc
src/sim/tlb.cc
src/unittest/rangemaptest2.cc
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] changeset in m5: style
On the topic of style violations, I've seen an increasing number of 'if('
and 'for(' (with no space) creeping in (see below). Another thing I've seen
that irritates me (but isn't officially banned by the style code ... yet
...) is C++-style comments with no space between the // and the comment:
int foo; //this is a foo
Anyone else bugged by this?
Yes. I'd prefer that they're fixed. All of these things make grep
more difficult. My personal pet peeve is of course the over 80 column
lines. I fix these all too often. There is rarely a case where there
isn't some very simple fix. Often it does involve creating a
temporary variable or temporary typedef, but that makes the code more
readable anyway.
I can add the if( for( while( check to the style hook.
Nate
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
