Re: [m5-dev] Review Request: x86: Timing support for pagetable walker

[Gabe Black]

> On 2011-01-07 05:51:30, Gabe Black wrote:
> > The code seems ok, but why do we need to have multiple outstanding page 
> > walks in timing mode again?
> 
> Gabe Black wrote:
> Actually, I wrote the above before I'd read it carefully. My question 
> still stands, but there are some areas that need to be fixed up. Also, since 
> translation is very much on the critical path, make sure you measure how much 
> this change affects performance. I expect with the addition indirection at 
> least there will be some slow down, and we should know what that is before we 
> commit anything.
> 
> Joel Hestness wrote:
> In timing mode x86, if a memory address translation misses in the TLB AND 
> happens to be an unaligned access (one that straddles a page boundary), the 
> TLB promptly fires both of the requests to the page table walker.  The old 
> implementation of the walker doesn't support multiple outstanding requests, 
> so it immediately crashes simulation with a state assertion failure (I asked 
> a few questions about this in June and July, back when I made the changes to 
> the walker).  The implementation in this patch can queue the requests and 
> service them sequentially.  It should be a simple future extension to service 
> them concurrently.
> 
> I modeled this implementation after the ARM implementation in 
> arch/arm/table_walker.*.
> 
> Concerning the slowdown, the frequency of unaligned accesses that miss in 
> the TLB is extremely rare (<10 in seconds of simulated system time).  Since 
> timing mode doesn't work without this fix, there isn't a way to compare 
> performance against a baseline.

Ah, ok. I remembered there was a discussion about something along these lines 
before, but I'd forgotten all the details. I initially was thinking this would 
affect every translation, but that's not true since this only changes page 
table walks which will (hopefully) be a lot less frequent. While that means a 
slow down would be less of an issue, since there are some new for loops and 
non-trivial data structures it would still be nice to know for sure whether it 
makes a difference. You could run without the patch until the simulation 
crashes and then run again with the patch up to that tick. It's not earth 
shatteringly critical to measure it, but it would be good for peace of mind.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/396/#review649
---


On 2011-01-06 16:12:34, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/396/
> ---
> 
> (Updated 2011-01-06 16:12:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Timing support for pagetable walker
> 
> Move page table walker state to its own object type, and make the
> walker instantiate state for each outstanding walk. By storing the
> states in a queue, the walker is able to handle multiple outstanding
> timing requests. Note that functional walks use separate state
> elements.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/pagetable_walker.hh 9f9e10967912 
>   src/arch/x86/pagetable_walker.cc 9f9e10967912 
>   src/arch/x86/tlb.hh 9f9e10967912 
>   src/arch/x86/tlb.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/396/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: Timing support for pagetable walker

[Gabe Black]

> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.cc, line 77
> > 
> >
> > These should use FastAlloc if at all possible since they're on a 
> > critical path and the heap is slow.
> 
> Joel Hestness wrote:
> Is this as simple as having WalkerState inherit from FastAlloc?

Yes I think so. I've never used FastAlloc myself, but it looks like it 
basically allocates a big pool of memory in certain sized chunks, and then 
rather than allocating them on the heap it just grabs a free one and fills it 
in. That implies your class has to be a fixed size or less (512 bytes according 
to the header). If it's too big then "new" falls back to the heap, so it'd 
actually be a little slower than going to the heap directly. Packets use 
FastAlloc if you want an example.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/396/#review649
---


On 2011-01-06 16:12:34, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/396/
> ---
> 
> (Updated 2011-01-06 16:12:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Timing support for pagetable walker
> 
> Move page table walker state to its own object type, and make the
> walker instantiate state for each outstanding walk. By storing the
> states in a queue, the walker is able to handle multiple outstanding
> timing requests. Note that functional walks use separate state
> elements.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/pagetable_walker.hh 9f9e10967912 
>   src/arch/x86/pagetable_walker.cc 9f9e10967912 
>   src/arch/x86/tlb.hh 9f9e10967912 
>   src/arch/x86/tlb.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/396/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ruby: x86 fs config support

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/417/#review693
---

Ship it!


Muuuch better.


configs/common/FSConfig.py


2011?



configs/example/ruby_fs.py


I really don't care personally, but should this be 2011?


- Gabe


On 2011-01-07 16:43:48, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/417/
> ---
> 
> (Updated 2011-01-07 16:43:48)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ruby: x86 fs config support
> 
> 
> Diffs
> -
> 
>   configs/common/FSConfig.py 9f9e10967912 
>   configs/example/ruby_fs.py 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/417/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: MessagePort: implemented virtual recvTiming avoiding double delete

[Gabe Black]

> On 2011-01-07 04:21:05, Gabe Black wrote:
> > I think there are two problems with this patch. First, if at all possible 
> > we should avoid the code duplication we'd now have for the recvTiming 
> > function. Second, while this probably does fix the legitimate problem of 
> > deleting packets twice, I think it creates a memory leak in the process. I 
> > suspect if you leave your other changes in place but get rid of your custom 
> > recvTiming function, things will still work. The packet won't be deleted by 
> > the device, won't be deleted after being received as a request in either 
> > atomic or timing mode, but will be deleted in both modes after being 
> > received as a response. The "virtual" you added in tport.hh could almost 
> > certainly go away then too.
> 
> Brad Beckmann wrote:
> Joel is the one who actually wrote this patch, so hopefully he can 
> elaborate on the possible the memory leak.  I'll hold off on this patch until 
> he can respond.
> 
> Joel Hestness wrote:
> Actually, the double delete problem still exists if we removed the 
> (almost) replicated recvTiming code.  This is because pkt->needsResponse() 
> returns false when the message type is MemCmd::MessageResp, which causes 
> execution of the needsResponse else clause in SimpleTimingPort::recvTiming.  
> It would be freed there, as well as in recvAtomic.
> 
> I think when I tested this with Valgrind, I didn't see the memory leak 
> (doesn't mean it doesn't exist).  However, I don't think I was able to 
> justify to myself why it didn't occur.
> 
> I remember that I spent a while trying to figure out how to make this 
> work nicely, but the inheritance SimpleTimingPort -> MessagePort -> IntPort, 
> and the overloading that that implies makes this quite difficult to analyze.  
> For instance, I'm still not clear why the new MemCmd, MessageReq/Resp, needed 
> to be defined for this.

It's been a while, but I think the idea was that MessageReq and MessageResp 
mean that no overlap between messages should happen. If you write 0x10 bytes to 
0x0 and 0x10 bytes to 0x8, those writes will interact. If you write a 0x10 
message to address 0x0 and a 0x10 message to 0x8, then since those are messages 
written to ports essentially, no interaction should happen. I don't know if it 
actually works that way in practice, but I kind of remember that being my 
thinking.

As far as the over/under deleting, I think the problem is that you're not 
supposed to add deletes to recvAtomic, aka leave that like it was and 
recvTiming like it was. I wasn't aware of this before, but digging through 
other bits of code it looks like recvAtomic doesn't delete packets that are 
being received. The idea is likely that since the call is being done in place, 
the calling code can easily clean up after itself. For recvTiming the source 
may no longer even exist when a packet is being handled.

So continue to get rid of the deletes in recvResponse, leave the recvAtomic and 
recvTiming alone, and everything should be straightened out I think. You'll 
also need to delete the packet built in the function that originally called 
sendAtomic, which appears to be sendMessage in IntPort.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/382/#review639
---


On 2011-01-06 15:56:19, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/382/
> ---
> 
> (Updated 2011-01-06 15:56:19)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> MessagePort: implemented virtual recvTiming avoiding double delete
> 
> Double packet delete problem is due to an interrupt device deleting a packet
> that the SimpleTimingPort also deletes. Since MessagePort descends from
> SimpleTimingPort, simply reimplement the failing code from SimpleTimingPort:
> recvTiming.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/mem/mport.hh 9f9e10967912 
>   src/mem/mport.cc 9f9e10967912 
>   src/mem/tport.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/382/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: page table walker functional support

[nathan binkert]

>
> src/arch/x86/vtophys.cc
>  (Diff
> revision 1)
>
>70
>
> Fault fault = walker->startFunctional(tc, NULL, req, mode);
>
>   Having a temporary variable here seems unnecessary unless it's to prevent 
> having to wrap the next line. It's not a big deal, though.
>
>  On January 7th, 2011, 5:07 p.m., *Joel Hestness* wrote:
>
> As far as I can tell, convention in ALL other code is to store the fault as a 
> temporary variable, even if it could simply be pushed into the if-clause.
>
>  I'd call that coincidence and not convention, and definitely not enough to 
> justify doing it here. Then again, you don't really need to justify it 
> because it's not really wrong.
>
> FWIW, I think temporary variables are a fantastic way to make code more
readable.  It is much harder for a temporary to get out of date (compared to
a comment), and they can be used to break down a complex expression into
more manageable chunks.

  Nate
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Fix corner cases where multiple squash/fetch redirects occur that

[Ali Saidi]

> On 2010-12-08 23:43:07, Gabe Black wrote:
> > Lots of braces on the wrong lines, lines that are too long, and the commit 
> > message is malformed.

fixed


- Ali


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/345/#review525
---


On 2010-12-06 16:13:52, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/345/
> ---
> 
> (Updated 2010-12-06 16:13:52)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Fix corner cases where multiple squash/fetch redirects occur that
> overwrite the squash wires.
> 
> 
> Diffs
> -
> 
>   src/cpu/o3/iew_impl.hh 2b5fbdcbfb5d 
>   src/cpu/o3/lsq_unit_impl.hh 2b5fbdcbfb5d 
> 
> Diff: http://reviews.m5sim.org/r/345/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: The ARM decoder should not panic when decoding undefined holes in the

[Ali Saidi]

> On 2010-12-08 23:56:50, Gabe Black wrote:
> > src/arch/arm/isa/formats/fp.isa, line 762
> > 
> >
> > The style of this comment looks unusual, although I don't think I can 
> > say it's wrong. I think the line with /* should at least be otherwise 
> > blank, but really // style comments would work better. Gramatically this is 
> > actually two sentences. I think the comment in general is probably not 
> > necessary since the name immValid makes it pretty clear what's going on.

cleaned up with //


> On 2010-12-08 23:56:50, Gabe Black wrote:
> > src/arch/arm/isa/formats/fp.isa, line 767
> > 
> >
> > Misplaced brace

done


> On 2010-12-08 23:56:50, Gabe Black wrote:
> > src/arch/arm/isa/insts/neon.isa, line 930
> > 
> >
> > delete, don't comment out

all fixed


- Ali


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/346/#review526
---


On 2010-12-06 16:14:12, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/346/
> ---
> 
> (Updated 2010-12-06 16:14:12)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> The ARM decoder should not panic when decoding undefined holes in the
> Architecture, as this can abort simulations when the fetch unit runs
> ahead and speculatively decodes instructions that are off the execution
> path or that are generated dynamically.
> 
> 
> Diffs
> -
> 
>   src/arch/arm/insts/macromem.cc 2b5fbdcbfb5d 
>   src/arch/arm/insts/pred_inst.hh 2b5fbdcbfb5d 
>   src/arch/arm/isa/formats/fp.isa 2b5fbdcbfb5d 
>   src/arch/arm/isa/formats/misc.isa 2b5fbdcbfb5d 
>   src/arch/arm/isa/insts/neon.isa 2b5fbdcbfb5d 
>   src/arch/arm/miscregs.cc 2b5fbdcbfb5d 
> 
> Diff: http://reviews.m5sim.org/r/346/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: O3: Keep around the last committed instruction and use for squashing.

[Ali Saidi]

> On 2010-12-21 22:18:34, Steve Reinhardt wrote:
> > I agree with Gabe, the explanation in the commit message is kind of 
> > confusing, but the patch itself looks OK.

O3: Keep around the last committed instruction and use for squashing.

Without this change 0 is always used for the youngest sequence number if
a squash occured and the ROB was empty (E.g. an instruction is marked
serializeAfter or a fetch stall prevents other instructions from issuing).
Using 0 there is a race to rename where an instruction that committed the
same cycle as the squashing instruction can have it's renamed state undone 
by the squash using sequence number 0.


- Ali


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/348/#review561
---


On 2010-12-06 16:14:46, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/348/
> ---
> 
> (Updated 2010-12-06 16:14:46)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> O3: Keep around the last committed instruction and use for squashing.
> 
> Before this, 0 was used for the youngest sequence number to squash
> and there was a race to rename. In the case of a instruction marked
> serializeAfter that squashes the ROB is empty and 0 is passed to the rename.
> If multiple instructions commited that same cycle rename it could undo some
> renames beacuse of the 0 being passed which would undo some state.
> 
> 
> Diffs
> -
> 
>   src/cpu/o3/commit.hh 2b5fbdcbfb5d 
>   src/cpu/o3/commit_impl.hh 2b5fbdcbfb5d 
> 
> Diff: http://reviews.m5sim.org/r/348/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: page table walker functional support

[Gabe Black]

> On 2011-01-07 04:45:16, Gabe Black wrote:
> > src/arch/x86/vtophys.cc, line 70
> > 
> >
> > Having a temporary variable here seems unnecessary unless it's to 
> > prevent having to wrap the next line. It's not a big deal, though.
> 
> Joel Hestness wrote:
> As far as I can tell, convention in ALL other code is to store the fault 
> as a temporary variable, even if it could simply be pushed into the if-clause.

I'd call that coincidence and not convention, and definitely not enough to 
justify doing it here. Then again, you don't really need to justify it because 
it's not really wrong.


> On 2011-01-07 04:45:16, Gabe Black wrote:
> > src/arch/x86/vtophys.cc, line 73
> > 
> >
> > This is very suspicious. The request size was set to 0 when you 
> > constructed the request object, so this is anding the original address with 
> > -1. That doesn't do anything, so you're really just oring the addresses 
> > together. The TLB will already have taken care of any page offset/page 
> > number munging that you need. Actually, this whole function is suspect (not 
> > because of your code) since there's no guarantee code/data and/or different 
> > forms of data will be translated the same, or that flags aren't important.
> 
> Brad Beckmann wrote:
> I agree, something seems off here.  However, I'll let Joel respond before 
> changing it.  At least there needs to be a comment explaining why this 
> calculation is necessary.
> 
> Joel Hestness wrote:
> The size field of the request is set in the functional portion of 
> Walker::WalkerState::startWalk in my other patch for review.  The physical 
> address that is returned from vtophys needs to include the offset into the 
> page, which in x86 can have multiple different sizes.  The page table 
> contains the information about the page size, so it needs to be passed in the 
> request object through startFunctional().

So you're using the size field of the request object to pass back the size of 
the page from a functional page walk? I don't like that. It's pretty sneaky and 
I can easily imagine someone tripping over that perhaps expecting size to be 
what it's supposed to be or assuming it isn't anything, basically like Brad and 
I were. Since you're making up a functional function and it doesn't have to 
make sense in other contexts, just go ahead and add a page size parameter 
that's pass by reference. Also, if the request doesn't do anything but 
communicate an address for this function, go ahead and make it an address. If 
it needs to be a request to prevent the walker's internals from getting too 
messy then it's fine to leave it that way.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/385/#review642
---


On 2011-01-06 15:59:24, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/385/
> ---
> 
> (Updated 2011-01-06 15:59:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: page table walker functional support
> 
> src/arch/x86/pagetable_walker.hh: Added method to functionally walk page table
> src/arch/x86/pagetable_walker.cc: Added method to functionally walk page table
> src/arch/x86/tlb.cc: Added method to return pointer to walker
> src/arch/x86/tlb.hh: Added method to return pointer to walker
> src/arch/x86/vtophys.cc: Calls walker to look up virt. to phys. page mapping
> 
> 
> Diffs
> -
> 
>   src/arch/x86/vtophys.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/385/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: O3: Support timing translations for O3 CPU fetch.

[Ali Saidi]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/340/#review687
---



src/cpu/o3/fetch.hh


done



src/cpu/o3/fetch.hh


done



src/cpu/o3/fetch_impl.hh


It's not gone, it's moved... Need to check that the physical address still 
is what we think it is after translation. 



src/cpu/o3/fetch_impl.hh


it's really dated.. done



src/cpu/o3/fetch_impl.hh


you're right buildInst should have been used and then it will be ~ the same 
size



src/cpu/o3/fetch_impl.hh


done



src/cpu/o3/fetch_impl.hh


yes


- Ali


On 2010-12-06 16:11:45, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/340/
> ---
> 
> (Updated 2010-12-06 16:11:45)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> O3: Support timing translations for O3 CPU fetch.
> 
> 
> Diffs
> -
> 
>   src/cpu/o3/fetch.hh 2b5fbdcbfb5d 
>   src/cpu/o3/fetch_impl.hh 2b5fbdcbfb5d 
> 
> Diff: http://reviews.m5sim.org/r/340/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 support for moving predicated false dest operands from sources.

[Ali Saidi]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/339/
---

(Updated 2011-01-09 19:14:19.250408)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

ARM: Add support for moving predicated false dest operands from sources.


Diffs (updated)
-

  src/arch/arm/isa/insts/misc.isa 5d0f62927d75 
  src/arch/arm/isa/templates/basic.isa 5d0f62927d75 
  src/arch/arm/isa/templates/branch.isa 5d0f62927d75 
  src/arch/arm/isa/templates/macromem.isa 5d0f62927d75 
  src/arch/arm/isa/templates/mem.isa 5d0f62927d75 
  src/arch/arm/isa/templates/misc.isa 5d0f62927d75 
  src/arch/arm/isa/templates/mult.isa 5d0f62927d75 
  src/arch/arm/isa/templates/neon.isa 5d0f62927d75 
  src/arch/arm/isa/templates/pred.isa 5d0f62927d75 
  src/arch/arm/isa/templates/vfp.isa 5d0f62927d75 
  src/arch/arm/registers.hh 5d0f62927d75 
  src/cpu/o3/dyn_inst.hh 5d0f62927d75 
  src/cpu/o3/iew_impl.hh 5d0f62927d75 
  src/cpu/o3/lsq_unit_impl.hh 5d0f62927d75 

Diff: http://reviews.m5sim.org/r/339/diff


Testing
---


Thanks,

Ali

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: m5: added work completed monitoring support

[Ali Saidi]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/418/#review686
---



src/sim/pseudo_inst.cc


There needs to be a good comment about why you would use this here and the 
wiki page needs to be updated



src/sim/system.hh


Maybe here would be more appropriate.


- Ali


On 2011-01-07 16:44:00, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/418/
> ---
> 
> (Updated 2011-01-07 16:44:00)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> m5: added work completed monitoring support
> 
> 
> Diffs
> -
> 
>   configs/common/FSConfig.py 9f9e10967912 
>   configs/common/Options.py 9f9e10967912 
>   configs/example/fs.py 9f9e10967912 
>   configs/example/ruby_fs.py 9f9e10967912 
>   src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912 
>   src/cpu/base.hh 9f9e10967912 
>   src/cpu/base.cc 9f9e10967912 
>   src/sim/SConscript 9f9e10967912 
>   src/sim/System.py 9f9e10967912 
>   src/sim/pseudo_inst.hh 9f9e10967912 
>   src/sim/pseudo_inst.cc 9f9e10967912 
>   src/sim/system.hh 9f9e10967912 
>   src/sim/system.cc 9f9e10967912 
>   util/m5/m5op_x86.S 9f9e10967912 
>   util/m5/m5ops.h 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/418/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Update the Ruby request type names for LL/SC

[Ali Saidi]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/391/#review685
---


Wasn't the alpha name for this load locked? That name or the ARM name (load 
exclusive) seems to make more sense to me than load linked.

- Ali


On 2011-01-06 16:07:31, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/391/
> ---
> 
> (Updated 2011-01-06 16:07:31)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Ruby: Update the Ruby request type names for LL/SC
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/libruby.hh 9f9e10967912 
>   src/mem/ruby/libruby.cc 9f9e10967912 
>   src/mem/ruby/recorder/TraceRecord.cc 9f9e10967912 
>   src/mem/ruby/system/DMASequencer.cc 9f9e10967912 
>   src/mem/ruby/system/RubyPort.cc 9f9e10967912 
>   src/mem/ruby/system/Sequencer.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/391/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: packet: Added public method to identify valid data

[Ali Saidi]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/399/#review684
---


I'm not sure this change does what you think it does. those flags only mean 
that the packet has data storage allocated, it doesn't say anything about the 
validity of that data. I can't think of a good reason why someone external 
would need to know that the packet has data allocated. If that is what you're 
going for, then it should have a name that makes it clear that this just means 
storage, nothing about the protocol and a good doxygen comment.

- Ali


On 2011-01-06 16:13:49, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/399/
> ---
> 
> (Updated 2011-01-06 16:13:49)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> packet: Added public method to identify valid data
> 
> 
> Diffs
> -
> 
>   src/mem/packet.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/399/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: dev: fixed bugs to extend interrupt capability beyond 15 cores

[Ali Saidi]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/397/#review683
---



src/dev/x86/intdev.hh


I think this is correct. It should be fine, but checking with the chekpoint 
verifier script would be good before committing.


- Ali


On 2011-01-06 16:12:56, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/397/
> ---
> 
> (Updated 2011-01-06 16:12:56)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> dev: fixed bugs to extend interrupt capability beyond 15 cores
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/i82094aa.hh 9f9e10967912 
>   src/dev/x86/i82094aa.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/dev/x86/intdev.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/397/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Printing statistics

[Nilay]

Is there a way to print statistics in an order other than the alphabetical
one in which they get printed currently?


--
Nilay

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: stats: Add a histogram statistic type

[Nilay Vaish]

> On 2011-01-08 22:26:47, Nathan Binkert wrote:
> > src/base/statistics.hh, line 1462
> > 
> >
> > Why do you want this?  Generally, we don't expose the internals of 
> > stats and I'm reluctant to do so.

Since the average is being printed, I am fine with not having such a function.


- Nilay


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/357/#review679
---


On 2010-12-21 11:15:24, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/357/
> ---
> 
> (Updated 2010-12-21 11:15:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> stats: Add a histogram statistic type
> 
> 
> Diffs
> -
> 
>   src/base/statistics.hh 4a3bddd74f36 
>   src/base/statistics.cc 4a3bddd74f36 
>   src/base/stats/info.hh 4a3bddd74f36 
>   src/base/stats/text.cc 4a3bddd74f36 
>   src/unittest/stattest.cc 4a3bddd74f36 
> 
> Diff: http://reviews.m5sim.org/r/357/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nathan
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: stats: Add a histogram statistic type

[Nilay Vaish]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/357/#review682
---


With each bucket, both the name and description of the histogram are printed. I 
think this is unnecessary. Should each bucket's count be printed in a new line? 
Again, seems unnecessary to me.

- Nilay


On 2010-12-21 11:15:24, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/357/
> ---
> 
> (Updated 2010-12-21 11:15:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> stats: Add a histogram statistic type
> 
> 
> Diffs
> -
> 
>   src/base/statistics.hh 4a3bddd74f36 
>   src/base/statistics.cc 4a3bddd74f36 
>   src/base/stats/info.hh 4a3bddd74f36 
>   src/base/stats/text.cc 4a3bddd74f36 
>   src/unittest/stattest.cc 4a3bddd74f36 
> 
> Diff: http://reviews.m5sim.org/r/357/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nathan
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Cron /z/m5/regression/do-regression --scratch all

[Cron Daemon]

* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-atomic 
passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/inorder-timing 
passed.
* build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest passed.
* build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-timing-mp 
passed.
* build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-atomic-mp 
passed.
* build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/01.hello-2T-smt/alpha/linux/o3-timing 
passed.
* build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/simple-atomic 
passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/inorder-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-timing 
passed.
* build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/simple-timing 
passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/inorder-timing 
passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MESI_CMP_directory
 passed.
* build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/simple-timing passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_directory
 passed.
* build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/o3-timing passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual
 passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing 
passed.
* 
build/ALPHA_FS/tests/fast/quick/10.lin