Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-07 Thread Gabe Black 

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


Most of these comments are pretty minor, but I think my comment for timing.cc 
is fairly important. It's not life or death, but there's some low hanging 
performance there.


src/arch/alpha/isa/mem.isa


You might want to leave out this new blank line.



src/arch/alpha/isa/mem.isa


And getting rid of this blank line.



src/arch/alpha/isa/mem.isa


This doesn't need to be on its own line.



src/cpu/simple/timing.cc


Could this be supressed in initiateAcc itself? That won't work in other 
models where the fault doesn't go through initiateAcc on its way back to the 
CPU, but it would make this check unnecessary.



src/cpu/static_inst.hh


You could use isInstPrefetch and isDataPrefetch internally here. It's not 
likely to ever make a difference, but if those other functions ever get more 
complicated isPrefetch would pick it up automatically. It's fine if you don't, 
but I thought I'd suggest it.


- Gabe


On 2010-11-05 17:26:22, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/
> ---
> 
> (Updated 2010-11-05 17:26:22)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
> 
> This change modifies the way prefetches work. They are now like normal loads
> that don't writeback a register. Previously prefetches were supposed to call
> prefetch() on the exection context, so they executed with execute() methods
> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
> CPUs
> are blank, meaning that they get executed, but don't actually do anything.
> 
> On Alpha dead cache copy code was removed and prefetches are now normal ops.
> They count as executed operations, but still don't do anything and IsMemRef is
> not longer set on them.
> 
> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
> instructions. The timing simple CPU doesn't try to do anything special for
> prefetches now and they execute with the normal memory code path.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>   src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>   src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>   src/arch/mips/isa/formats/mem.isa 9d60c5339ae5 
>   src/cpu/base_dyn_inst.hh 9d60c5339ae5 
>   src/cpu/base_dyn_inst_impl.hh 9d60c5339ae5 
>   src/cpu/checker/cpu.hh 9d60c5339ae5 
>   src/cpu/checker/cpu.cc 9d60c5339ae5 
>   src/cpu/exec_context.hh 9d60c5339ae5 
>   src/cpu/inorder/cpu.hh 9d60c5339ae5 
>   src/cpu/inorder/cpu.cc 9d60c5339ae5 
>   src/cpu/inorder/inorder_dyn_inst.hh 9d60c5339ae5 
>   src/cpu/inorder/inorder_dyn_inst.cc 9d60c5339ae5 
>   src/cpu/inorder/resource.hh 9d60c5339ae5 
>   src/cpu/inorder/resources/cache_unit.hh 9d60c5339ae5 
>   src/cpu/inorder/resources/cache_unit.cc 9d60c5339ae5 
>   src/cpu/ozone/cpu.hh 9d60c5339ae5 
>   src/cpu/ozone/cpu_impl.hh 9d60c5339ae5 
>   src/cpu/simple/base.hh 9d60c5339ae5 
>   src/cpu/simple/base.cc 9d60c5339ae5 
>   src/cpu/simple/timing.cc 9d60c5339ae5 
>   src/cpu/static_inst.hh 9d60c5339ae5 
> 
> Diff: http://reviews.m5sim.org/r/256/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/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-06 Thread Gabe Black 
I've been meaning to look at it. Please wait.

Gabe

Ali Saidi wrote:
> Any comments? Can I push this?
>
> Ali
>
> On Nov 5, 2010, at 7:27 PM, Ali Saidi wrote:
>
>   
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/256/#review433
>> ---
>>
>>
>>
>> src/cpu/simple/timing.cc
>> 
>>
>>I still didn't fix this change, but it is fixed in my local patch
>>
>>
>> - Ali
>>
>>
>> On 2010-11-05 17:26:22, Ali Saidi wrote:
>> 
>>> ---
>>> This is an automatically generated e-mail. To reply, visit:
>>> http://reviews.m5sim.org/r/256/
>>> ---
>>>
>>> (Updated 2010-11-05 17:26:22)
>>>
>>>
>>> Review request for Default.
>>>
>>>
>>> Summary
>>> ---
>>>
>>> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
>>>
>>> This change modifies the way prefetches work. They are now like normal loads
>>> that don't writeback a register. Previously prefetches were supposed to call
>>> prefetch() on the exection context, so they executed with execute() methods
>>> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
>>> CPUs
>>> are blank, meaning that they get executed, but don't actually do anything.
>>>
>>> On Alpha dead cache copy code was removed and prefetches are now normal ops.
>>> They count as executed operations, but still don't do anything and IsMemRef 
>>> is
>>> not longer set on them.
>>>
>>> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
>>> instructions. The timing simple CPU doesn't try to do anything special for
>>> prefetches now and they execute with the normal memory code path.
>>>
>>>
>>> Diffs
>>> -
>>>
>>>  src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>>>  src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>>>  src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>>>  src/arch/mips/isa/formats/mem.isa 9d60c5339ae5 
>>>  src/cpu/base_dyn_inst.hh 9d60c5339ae5 
>>>  src/cpu/base_dyn_inst_impl.hh 9d60c5339ae5 
>>>  src/cpu/checker/cpu.hh 9d60c5339ae5 
>>>  src/cpu/checker/cpu.cc 9d60c5339ae5 
>>>  src/cpu/exec_context.hh 9d60c5339ae5 
>>>  src/cpu/inorder/cpu.hh 9d60c5339ae5 
>>>  src/cpu/inorder/cpu.cc 9d60c5339ae5 
>>>  src/cpu/inorder/inorder_dyn_inst.hh 9d60c5339ae5 
>>>  src/cpu/inorder/inorder_dyn_inst.cc 9d60c5339ae5 
>>>  src/cpu/inorder/resource.hh 9d60c5339ae5 
>>>  src/cpu/inorder/resources/cache_unit.hh 9d60c5339ae5 
>>>  src/cpu/inorder/resources/cache_unit.cc 9d60c5339ae5 
>>>  src/cpu/ozone/cpu.hh 9d60c5339ae5 
>>>  src/cpu/ozone/cpu_impl.hh 9d60c5339ae5 
>>>  src/cpu/simple/base.hh 9d60c5339ae5 
>>>  src/cpu/simple/base.cc 9d60c5339ae5 
>>>  src/cpu/simple/timing.cc 9d60c5339ae5 
>>>  src/cpu/static_inst.hh 9d60c5339ae5 
>>>
>>> Diff: http://reviews.m5sim.org/r/256/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
>   

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

Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-06 Thread Ali Saidi 
Any comments? Can I push this?

Ali

On Nov 5, 2010, at 7:27 PM, Ali Saidi wrote:

> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/#review433
> ---
> 
> 
> 
> src/cpu/simple/timing.cc
> 
> 
>I still didn't fix this change, but it is fixed in my local patch
> 
> 
> - Ali
> 
> 
> On 2010-11-05 17:26:22, Ali Saidi wrote:
>> 
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/256/
>> ---
>> 
>> (Updated 2010-11-05 17:26:22)
>> 
>> 
>> Review request for Default.
>> 
>> 
>> Summary
>> ---
>> 
>> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
>> 
>> This change modifies the way prefetches work. They are now like normal loads
>> that don't writeback a register. Previously prefetches were supposed to call
>> prefetch() on the exection context, so they executed with execute() methods
>> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
>> CPUs
>> are blank, meaning that they get executed, but don't actually do anything.
>> 
>> On Alpha dead cache copy code was removed and prefetches are now normal ops.
>> They count as executed operations, but still don't do anything and IsMemRef 
>> is
>> not longer set on them.
>> 
>> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
>> instructions. The timing simple CPU doesn't try to do anything special for
>> prefetches now and they execute with the normal memory code path.
>> 
>> 
>> Diffs
>> -
>> 
>>  src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>>  src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>>  src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>>  src/arch/mips/isa/formats/mem.isa 9d60c5339ae5 
>>  src/cpu/base_dyn_inst.hh 9d60c5339ae5 
>>  src/cpu/base_dyn_inst_impl.hh 9d60c5339ae5 
>>  src/cpu/checker/cpu.hh 9d60c5339ae5 
>>  src/cpu/checker/cpu.cc 9d60c5339ae5 
>>  src/cpu/exec_context.hh 9d60c5339ae5 
>>  src/cpu/inorder/cpu.hh 9d60c5339ae5 
>>  src/cpu/inorder/cpu.cc 9d60c5339ae5 
>>  src/cpu/inorder/inorder_dyn_inst.hh 9d60c5339ae5 
>>  src/cpu/inorder/inorder_dyn_inst.cc 9d60c5339ae5 
>>  src/cpu/inorder/resource.hh 9d60c5339ae5 
>>  src/cpu/inorder/resources/cache_unit.hh 9d60c5339ae5 
>>  src/cpu/inorder/resources/cache_unit.cc 9d60c5339ae5 
>>  src/cpu/ozone/cpu.hh 9d60c5339ae5 
>>  src/cpu/ozone/cpu_impl.hh 9d60c5339ae5 
>>  src/cpu/simple/base.hh 9d60c5339ae5 
>>  src/cpu/simple/base.cc 9d60c5339ae5 
>>  src/cpu/simple/timing.cc 9d60c5339ae5 
>>  src/cpu/static_inst.hh 9d60c5339ae5 
>> 
>> Diff: http://reviews.m5sim.org/r/256/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] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-05 Thread Ali Saidi 

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



src/cpu/simple/timing.cc


I still didn't fix this change, but it is fixed in my local patch


- Ali


On 2010-11-05 17:26:22, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/
> ---
> 
> (Updated 2010-11-05 17:26:22)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
> 
> This change modifies the way prefetches work. They are now like normal loads
> that don't writeback a register. Previously prefetches were supposed to call
> prefetch() on the exection context, so they executed with execute() methods
> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
> CPUs
> are blank, meaning that they get executed, but don't actually do anything.
> 
> On Alpha dead cache copy code was removed and prefetches are now normal ops.
> They count as executed operations, but still don't do anything and IsMemRef is
> not longer set on them.
> 
> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
> instructions. The timing simple CPU doesn't try to do anything special for
> prefetches now and they execute with the normal memory code path.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>   src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>   src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>   src/arch/mips/isa/formats/mem.isa 9d60c5339ae5 
>   src/cpu/base_dyn_inst.hh 9d60c5339ae5 
>   src/cpu/base_dyn_inst_impl.hh 9d60c5339ae5 
>   src/cpu/checker/cpu.hh 9d60c5339ae5 
>   src/cpu/checker/cpu.cc 9d60c5339ae5 
>   src/cpu/exec_context.hh 9d60c5339ae5 
>   src/cpu/inorder/cpu.hh 9d60c5339ae5 
>   src/cpu/inorder/cpu.cc 9d60c5339ae5 
>   src/cpu/inorder/inorder_dyn_inst.hh 9d60c5339ae5 
>   src/cpu/inorder/inorder_dyn_inst.cc 9d60c5339ae5 
>   src/cpu/inorder/resource.hh 9d60c5339ae5 
>   src/cpu/inorder/resources/cache_unit.hh 9d60c5339ae5 
>   src/cpu/inorder/resources/cache_unit.cc 9d60c5339ae5 
>   src/cpu/ozone/cpu.hh 9d60c5339ae5 
>   src/cpu/ozone/cpu_impl.hh 9d60c5339ae5 
>   src/cpu/simple/base.hh 9d60c5339ae5 
>   src/cpu/simple/base.cc 9d60c5339ae5 
>   src/cpu/simple/timing.cc 9d60c5339ae5 
>   src/cpu/static_inst.hh 9d60c5339ae5 
> 
> Diff: http://reviews.m5sim.org/r/256/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/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-05 Thread Ali Saidi 

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

(Updated 2010-11-05 17:26:22.029174)


Review request for Default.


Summary
---

ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

This change modifies the way prefetches work. They are now like normal loads
that don't writeback a register. Previously prefetches were supposed to call
prefetch() on the exection context, so they executed with execute() methods
instead of initiateAcc() completeAcc(). The prefetch() methods for all the CPUs
are blank, meaning that they get executed, but don't actually do anything.

On Alpha dead cache copy code was removed and prefetches are now normal ops.
They count as executed operations, but still don't do anything and IsMemRef is
not longer set on them.

On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
instructions. The timing simple CPU doesn't try to do anything special for
prefetches now and they execute with the normal memory code path.


Diffs (updated)
-

  src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
  src/arch/alpha/isa/mem.isa 9d60c5339ae5 
  src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
  src/arch/mips/isa/formats/mem.isa 9d60c5339ae5 
  src/cpu/base_dyn_inst.hh 9d60c5339ae5 
  src/cpu/base_dyn_inst_impl.hh 9d60c5339ae5 
  src/cpu/checker/cpu.hh 9d60c5339ae5 
  src/cpu/checker/cpu.cc 9d60c5339ae5 
  src/cpu/exec_context.hh 9d60c5339ae5 
  src/cpu/inorder/cpu.hh 9d60c5339ae5 
  src/cpu/inorder/cpu.cc 9d60c5339ae5 
  src/cpu/inorder/inorder_dyn_inst.hh 9d60c5339ae5 
  src/cpu/inorder/inorder_dyn_inst.cc 9d60c5339ae5 
  src/cpu/inorder/resource.hh 9d60c5339ae5 
  src/cpu/inorder/resources/cache_unit.hh 9d60c5339ae5 
  src/cpu/inorder/resources/cache_unit.cc 9d60c5339ae5 
  src/cpu/ozone/cpu.hh 9d60c5339ae5 
  src/cpu/ozone/cpu_impl.hh 9d60c5339ae5 
  src/cpu/simple/base.hh 9d60c5339ae5 
  src/cpu/simple/base.cc 9d60c5339ae5 
  src/cpu/simple/timing.cc 9d60c5339ae5 
  src/cpu/static_inst.hh 9d60c5339ae5 

Diff: http://reviews.m5sim.org/r/256/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/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-04 Thread nathan binkert 
I was planning to send an e-mail that more or less says these same
things, so I agree with Steve.

  Nate

> I saw your earlier mail and you have a good point but after some reflection
> I still don't agree.  It sounds good to say that every revision should pass
> regressions, but in practice I don't know how much that matters.  I
> certainly think that every revision should compile and run, because
> otherwise 'hg bisect' is a pain, but I've never used bisect to track down
> when a regression started failing... I guess it's possible, but if
> regressions are running regularly then we should catch failures soon enough
> that it's not such a mystery where things went wrong.
>
> Meanwhile it's nice to just be able to ignore the csets that are labeled as
> stats updates and not wade through them to find actual code changes.
> Perhaps even more compelling is that there are a number of times where
> people (including me) have made some pretty substantive changes that are
> spread out across several csets, and it's a big enough pain to regenerate
> stats that you really only want to do it once for a related sequence of
> changes, and once you cross that bridge then you're admitting that you're
> not going to have regressions pass at every rev.
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-04 Thread Steve Reinhardt 
On Thu, Nov 4, 2010 at 9:41 AM, Ali Saidi  wrote:

>
>
> > On 2010-11-03 20:08:42, Steve Reinhardt wrote:
> > > Please split out the code changes and the stats changes into separate
> changesets (as is our habit, if not yet our official policy)... the stats
> changes are just too unwieldy and make it awkward to browse the code
> changes.  Thanks!
>
> I'll post the code changes separately, but when it comes to committing the
> change I think we should commit code and stats changes together. The goal
> should be the repository passes regressions at every revision.
>

I saw your earlier mail and you have a good point but after some reflection
I still don't agree.  It sounds good to say that every revision should pass
regressions, but in practice I don't know how much that matters.  I
certainly think that every revision should compile and run, because
otherwise 'hg bisect' is a pain, but I've never used bisect to track down
when a regression started failing... I guess it's possible, but if
regressions are running regularly then we should catch failures soon enough
that it's not such a mystery where things went wrong.

Meanwhile it's nice to just be able to ignore the csets that are labeled as
stats updates and not wade through them to find actual code changes.
Perhaps even more compelling is that there are a number of times where
people (including me) have made some pretty substantive changes that are
spread out across several csets, and it's a big enough pain to regenerate
stats that you really only want to do it once for a related sequence of
changes, and once you cross that bridge then you're admitting that you're
not going to have regressions pass at every rev.


>
>
> > On 2010-11-03 20:08:42, Steve Reinhardt wrote:
> > > src/cpu/simple/timing.cc, line 765
> > > 
> > >
> > > Does swapping the order of this OR really matter?
>
> Sorry, no... that is what I get for not paying enough attention to my diff.
> I'll remove that part of it.
>
>
> > On 2010-11-03 20:08:42, Steve Reinhardt wrote:
> > > src/arch/alpha/isa/decoder.isa, line 791
> > > 
> > >
> > > Is there still a writeHint() method out there that just doesn't
> ever get called now?  Seems like if we're going to change the API to get rid
> of writeHint() and prefetch() (which seems reasonable to me) we should make
> sure we eliminate them thoroughly.
>
> prefetch() I'm happy with axing. If we wanted to do a write hint type
> operation in the future, how would we do it? We could do a cache block sized
> store, in which case I'm happy to axe it too...
>

I don't think writeHint() does anything anyway.  If we want to add it back
in we could just add a special flag to a write operation, which would be
more consistent with other prefetches anyway.

Steve


>
>
> - Ali
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/#review431
> ---
>
>
> On 2010-11-03 16:13:11, Ali Saidi wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviews.m5sim.org/r/256/
> > ---
> >
> > (Updated 2010-11-03 16:13:11)
> >
> >
> > Review request for Default.
> >
> >
> > Summary
> > ---
> >
> > ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
> >
> > This change modifies the way prefetches work. They are now like normal
> loads
> > that don't writeback a register. Previously prefetches were supposed to
> call
> > prefetch() on the exection context, so they executed with execute()
> methods
> > instead of initiateAcc() completeAcc(). The prefetch() methods for all
> the CPUs
> > are blank, meaning that they get executed, but don't actually do
> anything.
> >
> > On Alpha dead cache copy code was removed and prefetches are now normal
> ops.
> > They count as executed operations, but still don't do anything and
> IsMemRef is
> > not longer set on them.
> >
> > On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
> > instructions. The timing simple CPU doesn't try to do anything special
> for
> > prefetches now and they execute with the normal memory code path.
> >
> >
> > Diffs
> > -
> >
> >   src/arch/alpha/isa/decoder.isa 9d60c5339ae5
> >   src/arch/alpha/isa/mem.isa 9d60c5339ae5
> >   src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5
> >   src/cpu/simple/timing.cc 9d60c5339ae5
> >   src/cpu/static_inst.hh 9d60c5339ae5
> >   tests/long/00.gzip/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5
> >   tests/long/00.gzip/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5
> >   tests/long/00.gzip/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5
> >   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/config.

Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-04 Thread Ali Saidi 


> On 2010-11-03 20:08:42, Steve Reinhardt wrote:
> > Please split out the code changes and the stats changes into separate 
> > changesets (as is our habit, if not yet our official policy)... the stats 
> > changes are just too unwieldy and make it awkward to browse the code 
> > changes.  Thanks!

I'll post the code changes separately, but when it comes to committing the 
change I think we should commit code and stats changes together. The goal 
should be the repository passes regressions at every revision. 


> On 2010-11-03 20:08:42, Steve Reinhardt wrote:
> > src/cpu/simple/timing.cc, line 765
> > 
> >
> > Does swapping the order of this OR really matter?

Sorry, no... that is what I get for not paying enough attention to my diff. 
I'll remove that part of it.


> On 2010-11-03 20:08:42, Steve Reinhardt wrote:
> > src/arch/alpha/isa/decoder.isa, line 791
> > 
> >
> > Is there still a writeHint() method out there that just doesn't ever 
> > get called now?  Seems like if we're going to change the API to get rid of 
> > writeHint() and prefetch() (which seems reasonable to me) we should make 
> > sure we eliminate them thoroughly.

prefetch() I'm happy with axing. If we wanted to do a write hint type operation 
in the future, how would we do it? We could do a cache block sized store, in 
which case I'm happy to axe it too... 


- Ali


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


On 2010-11-03 16:13:11, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/
> ---
> 
> (Updated 2010-11-03 16:13:11)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
> 
> This change modifies the way prefetches work. They are now like normal loads
> that don't writeback a register. Previously prefetches were supposed to call
> prefetch() on the exection context, so they executed with execute() methods
> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
> CPUs
> are blank, meaning that they get executed, but don't actually do anything.
> 
> On Alpha dead cache copy code was removed and prefetches are now normal ops.
> They count as executed operations, but still don't do anything and IsMemRef is
> not longer set on them.
> 
> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
> instructions. The timing simple CPU doesn't try to do anything special for
> prefetches now and they execute with the normal memory code path.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>   src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>   src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>   src/cpu/simple/timing.cc 9d60c5339ae5 
>   src/cpu/static_inst.hh 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/config.ini 
> 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/simout 
> 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/stats.txt 
> 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/config.ini 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/simout 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/stats.txt 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>   tests/long/30

Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-03 Thread Ali Saidi 
I can do that, but I think it is a bad habit that we should change. I'm happy 
to do it for review, but for committing it is the wrong plan because it means 
that there are changesets in the repo that do not pass regressions.

Ali

On Nov 3, 2010, at 10:08 PM, Steve Reinhardt wrote:

> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/#review431
> ---
> 
> 
> Please split out the code changes and the stats changes into separate 
> changesets (as is our habit, if not yet our official policy)... the stats 
> changes are just too unwieldy and make it awkward to browse the code changes. 
>  Thanks!
> 
> 
> src/arch/alpha/isa/decoder.isa
> 
> 
>Is there still a writeHint() method out there that just doesn't ever get 
> called now?  Seems like if we're going to change the API to get rid of 
> writeHint() and prefetch() (which seems reasonable to me) we should make sure 
> we eliminate them thoroughly.
> 
> 
> 
> src/cpu/simple/timing.cc
> 
> 
>Does swapping the order of this OR really matter?
> 
> 
> - Steve
> 
> 
> On 2010-11-03 16:13:11, Ali Saidi wrote:
>> 
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/256/
>> ---
>> 
>> (Updated 2010-11-03 16:13:11)
>> 
>> 
>> Review request for Default.
>> 
>> 
>> Summary
>> ---
>> 
>> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
>> 
>> This change modifies the way prefetches work. They are now like normal loads
>> that don't writeback a register. Previously prefetches were supposed to call
>> prefetch() on the exection context, so they executed with execute() methods
>> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
>> CPUs
>> are blank, meaning that they get executed, but don't actually do anything.
>> 
>> On Alpha dead cache copy code was removed and prefetches are now normal ops.
>> They count as executed operations, but still don't do anything and IsMemRef 
>> is
>> not longer set on them.
>> 
>> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
>> instructions. The timing simple CPU doesn't try to do anything special for
>> prefetches now and they execute with the normal memory code path.
>> 
>> 
>> Diffs
>> -
>> 
>>  src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>>  src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>>  src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>>  src/cpu/simple/timing.cc 9d60c5339ae5 
>>  src/cpu/static_inst.hh 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
>>  tests/long/00.gzip/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
>>  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/config.ini 
>> 9d60c5339ae5 
>>  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/simout 
>> 9d60c5339ae5 
>>  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/stats.txt 
>> 9d60c5339ae5 
>>  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/config.ini 9d60c5339ae5 
>>  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/simout 9d60c5339ae5 
>>  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/stats.txt 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
>>  tests/long/30.eon/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
>>  tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>>  tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>>  tests/long/40.perlbmk/ref/alpha/tr

Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-03 Thread Steve Reinhardt 

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


Please split out the code changes and the stats changes into separate 
changesets (as is our habit, if not yet our official policy)... the stats 
changes are just too unwieldy and make it awkward to browse the code changes.  
Thanks!


src/arch/alpha/isa/decoder.isa


Is there still a writeHint() method out there that just doesn't ever get 
called now?  Seems like if we're going to change the API to get rid of 
writeHint() and prefetch() (which seems reasonable to me) we should make sure 
we eliminate them thoroughly.



src/cpu/simple/timing.cc


Does swapping the order of this OR really matter?


- Steve


On 2010-11-03 16:13:11, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/256/
> ---
> 
> (Updated 2010-11-03 16:13:11)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.
> 
> This change modifies the way prefetches work. They are now like normal loads
> that don't writeback a register. Previously prefetches were supposed to call
> prefetch() on the exection context, so they executed with execute() methods
> instead of initiateAcc() completeAcc(). The prefetch() methods for all the 
> CPUs
> are blank, meaning that they get executed, but don't actually do anything.
> 
> On Alpha dead cache copy code was removed and prefetches are now normal ops.
> They count as executed operations, but still don't do anything and IsMemRef is
> not longer set on them.
> 
> On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
> instructions. The timing simple CPU doesn't try to do anything special for
> prefetches now and they execute with the normal memory code path.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
>   src/arch/alpha/isa/mem.isa 9d60c5339ae5 
>   src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
>   src/cpu/simple/timing.cc 9d60c5339ae5 
>   src/cpu/static_inst.hh 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
>   tests/long/00.gzip/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/config.ini 
> 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/simout 
> 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/stats.txt 
> 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/config.ini 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/simout 9d60c5339ae5 
>   tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/stats.txt 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
>   tests/long/30.eon/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
>   tests/long/40.perlbmk/ref/alpha/tru

Re: [m5-dev] Review Request: ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

2010-11-03 Thread Ali Saidi 

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

(Updated 2010-11-03 16:13:11.575399)


Review request for Default.


Summary (updated)
---

ARM/Alpha/Cpu: Change prefetchs to be more like normal loads.

This change modifies the way prefetches work. They are now like normal loads
that don't writeback a register. Previously prefetches were supposed to call
prefetch() on the exection context, so they executed with execute() methods
instead of initiateAcc() completeAcc(). The prefetch() methods for all the CPUs
are blank, meaning that they get executed, but don't actually do anything.

On Alpha dead cache copy code was removed and prefetches are now normal ops.
They count as executed operations, but still don't do anything and IsMemRef is
not longer set on them.

On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch
instructions. The timing simple CPU doesn't try to do anything special for
prefetches now and they execute with the normal memory code path.


Diffs (updated)
-

  src/arch/alpha/isa/decoder.isa 9d60c5339ae5 
  src/arch/alpha/isa/mem.isa 9d60c5339ae5 
  src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 
  src/cpu/simple/timing.cc 9d60c5339ae5 
  src/cpu/static_inst.hh 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
  tests/long/00.gzip/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/config.ini 
9d60c5339ae5 
  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/simout 9d60c5339ae5 
  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/stats.txt 
9d60c5339ae5 
  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/config.ini 9d60c5339ae5 
  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/simout 9d60c5339ae5 
  tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/stats.txt 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
  tests/long/30.eon/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 
  tests/long/40.perlbmk/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/inorder-timing/config.ini 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/inorder-timing/simerr 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/inorder-timing/simout 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/inorder-timing/stats.txt 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 
  tests/long/50.vortex/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 
  tests/l