Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On oct. 27, 2016, 12:11 après-midi, Andreas Hansson wrote:
> > thanks for getting this in shape

Hi all,

Could someone commit this patch, please ?
Thanks


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8995
---


On oct. 27, 2016, 11:25 matin, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated oct. 27, 2016, 11:25 matin)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11688:1a792798e845
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True",
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f10951 
>   configs/common/O3_ARM_v7a.py 4aac82f10951 
>   configs/example/arm/devices.py 4aac82f10951 
>   configs/learning_gem5/part1/caches.py 4aac82f10951 
>   src/mem/cache/Cache.py 4aac82f10951 
>   src/mem/cache/base.hh 4aac82f10951 
>   src/mem/cache/base.cc 4aac82f10951 
>   src/mem/cache/tags/Tags.py 4aac82f10951 
>   src/mem/cache/tags/base.hh 4aac82f10951 
>   src/mem/cache/tags/base.cc 4aac82f10951 
>   src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.cc 4aac82f10951 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Andreas Hansson]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8995
---

Ship it!


thanks for getting this in shape

- Andreas Hansson


On Oct. 27, 2016, 11:25 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated Oct. 27, 2016, 11:25 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11688:1a792798e845
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True",
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f10951 
>   configs/common/O3_ARM_v7a.py 4aac82f10951 
>   configs/example/arm/devices.py 4aac82f10951 
>   configs/learning_gem5/part1/caches.py 4aac82f10951 
>   src/mem/cache/Cache.py 4aac82f10951 
>   src/mem/cache/base.hh 4aac82f10951 
>   src/mem/cache/base.cc 4aac82f10951 
>   src/mem/cache/tags/Tags.py 4aac82f10951 
>   src/mem/cache/tags/base.hh 4aac82f10951 
>   src/mem/cache/tags/base.cc 4aac82f10951 
>   src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.cc 4aac82f10951 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated Oct. 27, 2016, 11:25 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11688:1a792798e845
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True",
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  configs/common/Caches.py 4aac82f10951 
  configs/common/O3_ARM_v7a.py 4aac82f10951 
  configs/example/arm/devices.py 4aac82f10951 
  configs/learning_gem5/part1/caches.py 4aac82f10951 
  src/mem/cache/Cache.py 4aac82f10951 
  src/mem/cache/base.hh 4aac82f10951 
  src/mem/cache/base.cc 4aac82f10951 
  src/mem/cache/tags/Tags.py 4aac82f10951 
  src/mem/cache/tags/base.hh 4aac82f10951 
  src/mem/cache/tags/base.cc 4aac82f10951 
  src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
  src/mem/cache/tags/fa_lru.hh 4aac82f10951 
  src/mem/cache/tags/fa_lru.cc 4aac82f10951 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Andreas Hansson]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8986
---



src/mem/cache/tags/base_set_assoc.hh (line 233)


If you don't mind changing it, could we keep the operator on the first line?

The same goes for the condition in the if statement.

I am not sure if this is actually captured in the style, but it is 
definitley convention.

The same goes for the copy in FA-LRU

Thanks!


- Andreas Hansson


On Oct. 25, 2016, 9:18 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated Oct. 25, 2016, 9:18 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11688:74be5cba513a
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True",
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f10951 
>   configs/common/O3_ARM_v7a.py 4aac82f10951 
>   configs/example/arm/devices.py 4aac82f10951 
>   configs/learning_gem5/part1/caches.py 4aac82f10951 
>   src/mem/cache/Cache.py 4aac82f10951 
>   src/mem/cache/base.hh 4aac82f10951 
>   src/mem/cache/base.cc 4aac82f10951 
>   src/mem/cache/tags/Tags.py 4aac82f10951 
>   src/mem/cache/tags/base.hh 4aac82f10951 
>   src/mem/cache/tags/base.cc 4aac82f10951 
>   src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.cc 4aac82f10951 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated Oct. 25, 2016, 9:18 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11688:74be5cba513a
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True",
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  configs/common/Caches.py 4aac82f10951 
  configs/common/O3_ARM_v7a.py 4aac82f10951 
  configs/example/arm/devices.py 4aac82f10951 
  configs/learning_gem5/part1/caches.py 4aac82f10951 
  src/mem/cache/Cache.py 4aac82f10951 
  src/mem/cache/base.hh 4aac82f10951 
  src/mem/cache/base.cc 4aac82f10951 
  src/mem/cache/tags/Tags.py 4aac82f10951 
  src/mem/cache/tags/base.hh 4aac82f10951 
  src/mem/cache/tags/base.cc 4aac82f10951 
  src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
  src/mem/cache/tags/fa_lru.hh 4aac82f10951 
  src/mem/cache/tags/fa_lru.cc 4aac82f10951 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Andreas Hansson]

> On July 27, 2016, 4:52 p.m., Andreas Hansson wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 228
> > 
> >
> > Could you add a comment here?
> > 
> > It seems to me this code is not right, as it checks if the data is 
> > technically written now, but we only need the data at time T.
> > 
> > Should we not rather add the dataLatency to the blk->whenReady and then 
> > do the plus or max opteration?
> 
> Sophiane SENNI wrote:
> I actually don't really understand what this code represents, which was 
> already present before applying the patch. Because it seems to show a case 
> where the cache latency is greater than accessLatency, when the lat variable 
> is updated as follows:
> lat = cache->ticksToCycles(blk->whenReady - curTick())
> Can this situation actually occur ?
> 
> Andreas Hansson wrote:
> blk->whenReady represents the fact that the block is technically not 
> available yet. Due to how we do timing modelling we annotate the block when 
> it arrives, but have to remember when it is _actually_ availalbe. Thus, 
> anything we do here should add on top of the blk->whenReady. Same for fa_lru
> 
> Sophiane SENNI wrote:
> Ok. So if I understood, we actually need to apply the accessLatency on 
> top of the blk->whenReady. Hence, the good code would be as follows:
> 
> if (blk->whenReady > curTick()
> && cache->ticksToCycles(blk->whenReady - curTick())
> > accessLatency) {
> lat = cache->ticksToCycles(blk->whenReady - curTick()) + 
> accessLatency;
> }
> 
> Does this change make more sense ?

Yes. Also, could you add a comment to explain what is happening here.


> On July 27, 2016, 4:52 p.m., Andreas Hansson wrote:
> > src/mem/cache/tags/fa_lru.cc, line 188
> > 
> >
> > here we don't care about blk->whenReady?
> 
> Sophiane SENNI wrote:
> I we care about blk->whenReady in base_set_assoc.hh, I assume we have 
> also to care about it here.

Agreed.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8552
---


On Oct. 24, 2016, 2:56 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated Oct. 24, 2016, 2:56 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11688:9dba209f1590
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True",
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/base.cc 4aac82f10951 
>   src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.cc 4aac82f10951 
>   configs/common/Caches.py 4aac82f10951 
>   configs/common/O3_ARM_v7a.py 4aac82f10951 
>   configs/example/arm/devices.py 4aac82f10951 
>   configs/learning_gem5/part1/caches.py 4aac82f10951 
>   src/mem/cache/Cache.py 4aac82f10951 
>   src/mem/cache/base.hh 4aac82f10951 
>   src/mem/cache/base.cc 4aac82f10951 
>   src/mem/cache/tags/Tags.py 4aac82f10951 
>   src/mem/cache/tags/base.hh 4aac82f10951 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On oct. 21, 2016, 1:29 après-midi, Pierre-Yves Péneau wrote:
> > Hi,
> > 
> > Someone can commit this patch ? I don't have right access on the 
> > repository, either Sophiane.
> > Thank you.
> 
> Jason Lowe-Power wrote:
> Sorry we've been so slow on this patch. A couple of questions before I 
> commit.
> 
> 1. Are all of Andreas H.'s comments resolved? I'd like to see a "Ship It" 
> from him.
> 2. You need to make sure the regressions are passing. I understand that 
> our regression testing is poor, but I know that the learning_gem5 regression 
> is failing because of this patch. The file 
> configs/learning_gem5/part1/caches.py needs to be updated. There are likely 
> other files that need to be updated as well (configs/examples/arm/devices.py 
> comes to mind, there may be others).
> 
> Pierre-Yves Péneau wrote:
> 1. Sophiane answered to Andreas H.' issues but I did not respond (quote: 
> "Please go ahead with the patch as is"). I assume it's ok even without a 
> "Ship It" from him.
> 2. Regression tests have been done. Failures are due to missing CPU2000 
> benchmarks. The review will be update soon.

The regression tests passed, except the ones that require proprietary binaries.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8960
---


On oct. 24, 2016, 2:56 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated oct. 24, 2016, 2:56 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11688:9dba209f1590
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True",
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/base.cc 4aac82f10951 
>   src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
>   src/mem/cache/tags/fa_lru.cc 4aac82f10951 
>   configs/common/Caches.py 4aac82f10951 
>   configs/common/O3_ARM_v7a.py 4aac82f10951 
>   configs/example/arm/devices.py 4aac82f10951 
>   configs/learning_gem5/part1/caches.py 4aac82f10951 
>   src/mem/cache/Cache.py 4aac82f10951 
>   src/mem/cache/base.hh 4aac82f10951 
>   src/mem/cache/base.cc 4aac82f10951 
>   src/mem/cache/tags/Tags.py 4aac82f10951 
>   src/mem/cache/tags/base.hh 4aac82f10951 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated Oct. 24, 2016, 2:56 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11688:9dba209f1590
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True",
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  src/mem/cache/tags/base.cc 4aac82f10951 
  src/mem/cache/tags/base_set_assoc.hh 4aac82f10951 
  src/mem/cache/tags/fa_lru.cc 4aac82f10951 
  configs/common/Caches.py 4aac82f10951 
  configs/common/O3_ARM_v7a.py 4aac82f10951 
  configs/example/arm/devices.py 4aac82f10951 
  configs/learning_gem5/part1/caches.py 4aac82f10951 
  src/mem/cache/Cache.py 4aac82f10951 
  src/mem/cache/base.hh 4aac82f10951 
  src/mem/cache/base.cc 4aac82f10951 
  src/mem/cache/tags/Tags.py 4aac82f10951 
  src/mem/cache/tags/base.hh 4aac82f10951 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Pierre-Yves Péneau]

> On Oct. 21, 2016, 3:29 p.m., Pierre-Yves Péneau wrote:
> > Hi,
> > 
> > Someone can commit this patch ? I don't have right access on the 
> > repository, either Sophiane.
> > Thank you.
> 
> Jason Lowe-Power wrote:
> Sorry we've been so slow on this patch. A couple of questions before I 
> commit.
> 
> 1. Are all of Andreas H.'s comments resolved? I'd like to see a "Ship It" 
> from him.
> 2. You need to make sure the regressions are passing. I understand that 
> our regression testing is poor, but I know that the learning_gem5 regression 
> is failing because of this patch. The file 
> configs/learning_gem5/part1/caches.py needs to be updated. There are likely 
> other files that need to be updated as well (configs/examples/arm/devices.py 
> comes to mind, there may be others).

1. Sophiane answered to Andreas H.' issues but I did not respond (quote: 
"Please go ahead with the patch as is"). I assume it's ok even without a "Ship 
It" from him.
2. Regression tests have been done. Failures are due to missing CPU2000 
benchmarks. The review will be update soon.


- Pierre-Yves


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8960
---


On July 28, 2016, 12:31 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 28, 2016, 12:31 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base_set_assoc.hh 
> 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Jason Lowe-Power]

> On Oct. 21, 2016, 1:29 p.m., Pierre-Yves Péneau wrote:
> > Hi,
> > 
> > Someone can commit this patch ? I don't have right access on the 
> > repository, either Sophiane.
> > Thank you.

Sorry we've been so slow on this patch. A couple of questions before I commit.

1. Are all of Andreas H.'s comments resolved? I'd like to see a "Ship It" from 
him.
2. You need to make sure the regressions are passing. I understand that our 
regression testing is poor, but I know that the learning_gem5 regression is 
failing because of this patch. The file configs/learning_gem5/part1/caches.py 
needs to be updated. There are likely other files that need to be updated as 
well (configs/examples/arm/devices.py comes to mind, there may be others).


- Jason


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8960
---


On July 28, 2016, 10:31 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 28, 2016, 10:31 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base_set_assoc.hh 
> 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Pierre-Yves Péneau]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8960
---


Hi,

Someone can commit this patch ? I don't have right access on the repository, 
either Sophiane.
Thank you.

- Pierre-Yves Péneau


On July 28, 2016, 12:31 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 28, 2016, 12:31 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base_set_assoc.hh 
> 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 27, 2016, 4:52 après-midi, Andreas Hansson wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 228
> > 
> >
> > Could you add a comment here?
> > 
> > It seems to me this code is not right, as it checks if the data is 
> > technically written now, but we only need the data at time T.
> > 
> > Should we not rather add the dataLatency to the blk->whenReady and then 
> > do the plus or max opteration?
> 
> Sophiane SENNI wrote:
> I actually don't really understand what this code represents, which was 
> already present before applying the patch. Because it seems to show a case 
> where the cache latency is greater than accessLatency, when the lat variable 
> is updated as follows:
> lat = cache->ticksToCycles(blk->whenReady - curTick())
> Can this situation actually occur ?
> 
> Andreas Hansson wrote:
> blk->whenReady represents the fact that the block is technically not 
> available yet. Due to how we do timing modelling we annotate the block when 
> it arrives, but have to remember when it is _actually_ availalbe. Thus, 
> anything we do here should add on top of the blk->whenReady. Same for fa_lru

Ok. So if I understood, we actually need to apply the accessLatency on top of 
the blk->whenReady. Hence, the good code would be as follows:

if (blk->whenReady > curTick()
&& cache->ticksToCycles(blk->whenReady - curTick())
> accessLatency) {
lat = cache->ticksToCycles(blk->whenReady - curTick()) + 
accessLatency;
}

Does this change make more sense ?


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8552
---


On juil. 28, 2016, 10:31 matin, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 28, 2016, 10:31 matin)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base_set_assoc.hh 
> 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Andreas Hansson]

> On July 27, 2016, 4:52 p.m., Andreas Hansson wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 228
> > 
> >
> > Could you add a comment here?
> > 
> > It seems to me this code is not right, as it checks if the data is 
> > technically written now, but we only need the data at time T.
> > 
> > Should we not rather add the dataLatency to the blk->whenReady and then 
> > do the plus or max opteration?
> 
> Sophiane SENNI wrote:
> I actually don't really understand what this code represents, which was 
> already present before applying the patch. Because it seems to show a case 
> where the cache latency is greater than accessLatency, when the lat variable 
> is updated as follows:
> lat = cache->ticksToCycles(blk->whenReady - curTick())
> Can this situation actually occur ?

blk->whenReady represents the fact that the block is technically not available 
yet. Due to how we do timing modelling we annotate the block when it arrives, 
but have to remember when it is _actually_ availalbe. Thus, anything we do here 
should add on top of the blk->whenReady. Same for fa_lru


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8552
---


On July 28, 2016, 10:31 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 28, 2016, 10:31 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base_set_assoc.hh 
> 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 25, 2016, 1:18 après-midi, Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.
> 
> Sophiane SENNI wrote:
> Hi Jason,
> 
> I did not run any regression test. I only checked (with 
> --debug-flags=Cache) if the cache access latencies were correct for:
> - hits and misses
> - parallel and sequential accesses
> Regarding the stats, I only checked if the "sim_seconds" changes were 
> intuitive when modifying:
> - tag_latency and data_latency
> - cache access mode (i.e. sequential_access = True or False)
> 
> Jason Lowe-Power wrote:
> Please run all of the regression tests that you can (e.g., the ones that 
> don't require proprietary binaries). I know there are some other config files 
> that will need to be changed (e.g., the learning_gem5 scripts), and there may 
> be others.
> 
> Also, I expect this will require another patch to update the stats, too. 
> But you don't have to post that one on reviewboard :).
> 
> Sophiane SENNI wrote:
> I ran the following command: "scons build/ARM/tests/opt/quick/se"
> The output seems to be the same as when compiling gem5 with the command 
> "scons build/ARM/gem5.opt". Is it normal ?
> I expected to get outputs like the following one: "* 
> build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: passed."
> 
> Sophiane SENNI wrote:
> I successfully launched the regression tests. You are right. Some config 
> files need to be changed. For instance, the "O3_ARM_v7a.py" file where the 
> "hit_latency" has to be replaced by the two new parameters "tag_latency" and 
> "data_latency". I will try to make all necessary changes according to the 
> regression tests results.
> 
> Sophiane SENNI wrote:
> Jason,
> 
> Below are the results of the regression tests I ran:
> 
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: 
> passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing: passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing-checker: 
> passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-atomic: 
> passed.
> * 
> build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-atomic-dummychecker: 
> passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-timing: 
> passed.
> * 
> build/ARM/tests/opt/quick/se/03.learning-gem5/arm/linux/learning-gem5-p1-simple:
>  passed.
> * 
> build/ARM/tests/opt/quick/se/03.learning-gem5/arm/linux/learning-gem5-p1-two-level:
>  FAILED!
> 
> The last test failed with the following error (Do you know how to resolve 
> it ?):
> 
> File 
> "/home/senni/Partage/shared_folder_gem5/gem5/gem5_from_mercurial/gem5/tests/testing/../../configs/learning_gem5/part1/two_level.py",
>  line 93, in 
> system.cpu.icache = L1ICache(opts)
> NameError: name 'L1ICache' is not defined
> 
> 
> For the fs mode regression tests, it fails with the following error:
> 
> File 
> "/home/senni/Partage/shared_folder_gem5/gem5/gem5_from_mercurial/gem5/configs/common/SysPaths.py",
>  line 69, in system
> raise IOError, "Can't find a path to system files."
> IOError: Can't find a path to system files.
> 
> I have this error even if the $M5_PATH variable is correctly set.
> 
> Jason Lowe-Power wrote:
> I don't know why you're seeing that error for the FS tests. I would 
> suggest adding some print statements to the python config scripts. 
> Additionally, it may be helpful to run gem5 without the regression scripts to 
> track down these issues.
> 
> For learning_gem5... It should be working. I just ran the test on the 
> head and it passed. I imagine you need to modify 
> learning_gem5/part1/caches.py to add the tag/data latencies.

Ok for the FS tests.
For learning_gem5, that is odd. I replaced the hit_latencies by tag/data 
latencies, but the error is still there.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On juil. 28, 2016, 10:31 matin, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated 

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Jason Lowe-Power]

> On July 25, 2016, 1:18 p.m., Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.
> 
> Sophiane SENNI wrote:
> Hi Jason,
> 
> I did not run any regression test. I only checked (with 
> --debug-flags=Cache) if the cache access latencies were correct for:
> - hits and misses
> - parallel and sequential accesses
> Regarding the stats, I only checked if the "sim_seconds" changes were 
> intuitive when modifying:
> - tag_latency and data_latency
> - cache access mode (i.e. sequential_access = True or False)
> 
> Jason Lowe-Power wrote:
> Please run all of the regression tests that you can (e.g., the ones that 
> don't require proprietary binaries). I know there are some other config files 
> that will need to be changed (e.g., the learning_gem5 scripts), and there may 
> be others.
> 
> Also, I expect this will require another patch to update the stats, too. 
> But you don't have to post that one on reviewboard :).
> 
> Sophiane SENNI wrote:
> I ran the following command: "scons build/ARM/tests/opt/quick/se"
> The output seems to be the same as when compiling gem5 with the command 
> "scons build/ARM/gem5.opt". Is it normal ?
> I expected to get outputs like the following one: "* 
> build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: passed."
> 
> Sophiane SENNI wrote:
> I successfully launched the regression tests. You are right. Some config 
> files need to be changed. For instance, the "O3_ARM_v7a.py" file where the 
> "hit_latency" has to be replaced by the two new parameters "tag_latency" and 
> "data_latency". I will try to make all necessary changes according to the 
> regression tests results.
> 
> Sophiane SENNI wrote:
> Jason,
> 
> Below are the results of the regression tests I ran:
> 
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: 
> passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing: passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing-checker: 
> passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-atomic: 
> passed.
> * 
> build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-atomic-dummychecker: 
> passed.
> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-timing: 
> passed.
> * 
> build/ARM/tests/opt/quick/se/03.learning-gem5/arm/linux/learning-gem5-p1-simple:
>  passed.
> * 
> build/ARM/tests/opt/quick/se/03.learning-gem5/arm/linux/learning-gem5-p1-two-level:
>  FAILED!
> 
> The last test failed with the following error (Do you know how to resolve 
> it ?):
> 
> File 
> "/home/senni/Partage/shared_folder_gem5/gem5/gem5_from_mercurial/gem5/tests/testing/../../configs/learning_gem5/part1/two_level.py",
>  line 93, in 
> system.cpu.icache = L1ICache(opts)
> NameError: name 'L1ICache' is not defined
> 
> 
> For the fs mode regression tests, it fails with the following error:
> 
> File 
> "/home/senni/Partage/shared_folder_gem5/gem5/gem5_from_mercurial/gem5/configs/common/SysPaths.py",
>  line 69, in system
> raise IOError, "Can't find a path to system files."
> IOError: Can't find a path to system files.
> 
> I have this error even if the $M5_PATH variable is correctly set.

I don't know why you're seeing that error for the FS tests. I would suggest 
adding some print statements to the python config scripts. Additionally, it may 
be helpful to run gem5 without the regression scripts to track down these 
issues.

For learning_gem5... It should be working. I just ran the test on the head and 
it passed. I imagine you need to modify learning_gem5/part1/caches.py to add 
the tag/data latencies.


- Jason


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On July 28, 2016, 10:31 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 28, 2016, 10:31 a.m.)
> 
> 
> Review request for Default.
> 
> 
> 

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 25, 2016, 1:18 après-midi, Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.
> 
> Sophiane SENNI wrote:
> Hi Jason,
> 
> I did not run any regression test. I only checked (with 
> --debug-flags=Cache) if the cache access latencies were correct for:
> - hits and misses
> - parallel and sequential accesses
> Regarding the stats, I only checked if the "sim_seconds" changes were 
> intuitive when modifying:
> - tag_latency and data_latency
> - cache access mode (i.e. sequential_access = True or False)
> 
> Jason Lowe-Power wrote:
> Please run all of the regression tests that you can (e.g., the ones that 
> don't require proprietary binaries). I know there are some other config files 
> that will need to be changed (e.g., the learning_gem5 scripts), and there may 
> be others.
> 
> Also, I expect this will require another patch to update the stats, too. 
> But you don't have to post that one on reviewboard :).
> 
> Sophiane SENNI wrote:
> I ran the following command: "scons build/ARM/tests/opt/quick/se"
> The output seems to be the same as when compiling gem5 with the command 
> "scons build/ARM/gem5.opt". Is it normal ?
> I expected to get outputs like the following one: "* 
> build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: passed."
> 
> Sophiane SENNI wrote:
> I successfully launched the regression tests. You are right. Some config 
> files need to be changed. For instance, the "O3_ARM_v7a.py" file where the 
> "hit_latency" has to be replaced by the two new parameters "tag_latency" and 
> "data_latency". I will try to make all necessary changes according to the 
> regression tests results.

Jason,

Below are the results of the regression tests I ran:

* build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: passed.
* build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing: passed.
* build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing-checker: passed.
* build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-atomic: passed.
* 
build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-atomic-dummychecker: 
passed.
* build/ARM/tests/opt/quick/se/00.hello/arm/linux/simple-timing: passed.
* 
build/ARM/tests/opt/quick/se/03.learning-gem5/arm/linux/learning-gem5-p1-simple:
 passed.
* 
build/ARM/tests/opt/quick/se/03.learning-gem5/arm/linux/learning-gem5-p1-two-level:
 FAILED!

The last test failed with the following error (Do you know how to resolve it ?):

File 
"/home/senni/Partage/shared_folder_gem5/gem5/gem5_from_mercurial/gem5/tests/testing/../../configs/learning_gem5/part1/two_level.py",
 line 93, in 
system.cpu.icache = L1ICache(opts)
NameError: name 'L1ICache' is not defined


For the fs mode regression tests, it fails with the following error:

File 
"/home/senni/Partage/shared_folder_gem5/gem5/gem5_from_mercurial/gem5/configs/common/SysPaths.py",
 line 69, in system
raise IOError, "Can't find a path to system files."
IOError: Can't find a path to system files.

I have this error even if the $M5_PATH variable is correctly set.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On juil. 28, 2016, 10:31 matin, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 28, 2016, 10:31 matin)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Andreas Hansson]

> On July 27, 2016, 4:52 p.m., Andreas Hansson wrote:
> >

To be clear: Please go ahead with the patch as is. I merely wanted to 
hightlight that there is room for further patches and improvements.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8552
---


On July 28, 2016, 10:31 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 28, 2016, 10:31 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/base_set_assoc.hh 
> 4aac82f109517217e6bfb3812689280e7a8fa842 
>   src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated juil. 28, 2016, 10:31 matin)


Review request for Default.


Repository: gem5


Description
---

Changeset 11536:1a3a96d435ed
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter 
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True", 
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  configs/common/Caches.py 4aac82f109517217e6bfb3812689280e7a8fa842 
  configs/common/O3_ARM_v7a.py 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/Cache.py 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/tags/Tags.py 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/tags/base.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/tags/base.cc 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/tags/base_set_assoc.hh 4aac82f109517217e6bfb3812689280e7a8fa842 
  src/mem/cache/tags/fa_lru.cc 4aac82f109517217e6bfb3812689280e7a8fa842 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 27, 2016, 4:52 après-midi, Andreas Hansson wrote:
> > src/mem/cache/tags/Tags.py, line 57
> > 
> >
> > Why would the tags care about the data latency?

This is required to initialize the "accessLatency" variable in tags/base.cc
If I am right, the "accessLatency" variable declared in tags/base.hh is used to 
consider the cache latency on every access (i.e. considering tags and data 
accesses). Previously, "accessLatency" was always assigned to "hit_latency". 
With this patch, "accessLatency" value is initialized according to the cache 
access mode (parallel or sequential).


> On juil. 27, 2016, 4:52 après-midi, Andreas Hansson wrote:
> > src/mem/cache/tags/base.hh, line 75
> > 
> >
> > Seems odd that the tags need to track this. Is this still the best 
> > division? Perhaps explain why it's here.

This can be removed. We actually do not need it since data latency is already 
included in accessLatency.


> On juil. 27, 2016, 4:52 après-midi, Andreas Hansson wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 228
> > 
> >
> > Could you add a comment here?
> > 
> > It seems to me this code is not right, as it checks if the data is 
> > technically written now, but we only need the data at time T.
> > 
> > Should we not rather add the dataLatency to the blk->whenReady and then 
> > do the plus or max opteration?

I actually don't really understand what this code represents, which was already 
present before applying the patch. Because it seems to show a case where the 
cache latency is greater than accessLatency, when the lat variable is updated 
as follows:
lat = cache->ticksToCycles(blk->whenReady - curTick())
Can this situation actually occur ?


> On juil. 27, 2016, 4:52 après-midi, Andreas Hansson wrote:
> > src/mem/cache/tags/fa_lru.cc, line 188
> > 
> >
> > here we don't care about blk->whenReady?

I we care about blk->whenReady in base_set_assoc.hh, I assume we have also to 
care about it here.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8552
---


On juil. 25, 2016, 1:16 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 25, 2016, 1:16 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Andreas Hansson]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8552
---



src/mem/cache/tags/Tags.py (line 57)


Why would the tags care about the data latency?



src/mem/cache/tags/base.hh (line 75)


Seems odd that the tags need to track this. Is this still the best 
division? Perhaps explain why it's here.



src/mem/cache/tags/base_set_assoc.hh (line 227)


Could you add a comment here?

It seems to me this code is not right, as it checks if the data is 
technically written now, but we only need the data at time T.

Should we not rather add the dataLatency to the blk->whenReady and then do 
the plus or max opteration?



src/mem/cache/tags/fa_lru.cc (line 188)


here we don't care about blk->whenReady?


Some minor concerns that hopefully make sense.

- Andreas Hansson


On July 25, 2016, 1:16 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 25, 2016, 1:16 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 25, 2016, 1:18 après-midi, Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.
> 
> Sophiane SENNI wrote:
> Hi Jason,
> 
> I did not run any regression test. I only checked (with 
> --debug-flags=Cache) if the cache access latencies were correct for:
> - hits and misses
> - parallel and sequential accesses
> Regarding the stats, I only checked if the "sim_seconds" changes were 
> intuitive when modifying:
> - tag_latency and data_latency
> - cache access mode (i.e. sequential_access = True or False)
> 
> Jason Lowe-Power wrote:
> Please run all of the regression tests that you can (e.g., the ones that 
> don't require proprietary binaries). I know there are some other config files 
> that will need to be changed (e.g., the learning_gem5 scripts), and there may 
> be others.
> 
> Also, I expect this will require another patch to update the stats, too. 
> But you don't have to post that one on reviewboard :).
> 
> Sophiane SENNI wrote:
> I ran the following command: "scons build/ARM/tests/opt/quick/se"
> The output seems to be the same as when compiling gem5 with the command 
> "scons build/ARM/gem5.opt". Is it normal ?
> I expected to get outputs like the following one: "* 
> build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: passed."

I successfully launched the regression tests. You are right. Some config files 
need to be changed. For instance, the "O3_ARM_v7a.py" file where the 
"hit_latency" has to be replaced by the two new parameters "tag_latency" and 
"data_latency". I will try to make all necessary changes according to the 
regression tests results.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On juil. 25, 2016, 1:16 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 25, 2016, 1:16 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 25, 2016, 1:18 après-midi, Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.
> 
> Sophiane SENNI wrote:
> Hi Jason,
> 
> I did not run any regression test. I only checked (with 
> --debug-flags=Cache) if the cache access latencies were correct for:
> - hits and misses
> - parallel and sequential accesses
> Regarding the stats, I only checked if the "sim_seconds" changes were 
> intuitive when modifying:
> - tag_latency and data_latency
> - cache access mode (i.e. sequential_access = True or False)
> 
> Jason Lowe-Power wrote:
> Please run all of the regression tests that you can (e.g., the ones that 
> don't require proprietary binaries). I know there are some other config files 
> that will need to be changed (e.g., the learning_gem5 scripts), and there may 
> be others.
> 
> Also, I expect this will require another patch to update the stats, too. 
> But you don't have to post that one on reviewboard :).

I ran the following command: "scons build/ARM/tests/opt/quick/se"
The output seems to be the same as when compiling gem5 with the command "scons 
build/ARM/gem5.opt". Is it normal ?
I expected to get outputs like the following one: "* 
build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing: passed."


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On juil. 25, 2016, 1:16 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 25, 2016, 1:16 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Jason Lowe-Power]

> On July 25, 2016, 1:18 p.m., Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.
> 
> Sophiane SENNI wrote:
> Hi Jason,
> 
> I did not run any regression test. I only checked (with 
> --debug-flags=Cache) if the cache access latencies were correct for:
> - hits and misses
> - parallel and sequential accesses
> Regarding the stats, I only checked if the "sim_seconds" changes were 
> intuitive when modifying:
> - tag_latency and data_latency
> - cache access mode (i.e. sequential_access = True or False)

Please run all of the regression tests that you can (e.g., the ones that don't 
require proprietary binaries). I know there are some other config files that 
will need to be changed (e.g., the learning_gem5 scripts), and there may be 
others.

Also, I expect this will require another patch to update the stats, too. But 
you don't have to post that one on reviewboard :).


- Jason


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On July 25, 2016, 1:16 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 25, 2016, 1:16 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 25, 2016, 1:18 après-midi, Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.
> 
> Jason Lowe-Power wrote:
> Hi Sophanie,
> 
> Have you run all of the regression tests? Are there changes to the stats 
> (I would expect so)? Have you checked to make sure the stat changes make 
> intuitive sense? Thanks!
> 
> Also, it would be nice to see Andreas H., or Steve, or someone who has 
> been modifying the cache code lately to take a quick look at this patch 
> before it's committed.

Hi Jason,

I did not run any regression test. I only checked (with --debug-flags=Cache) if 
the cache access latencies were correct for:
- hits and misses
- parallel and sequential accesses
Regarding the stats, I only checked if the "sim_seconds" changes were intuitive 
when modifying:
- tag_latency and data_latency
- cache access mode (i.e. sequential_access = True or False)


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On juil. 25, 2016, 1:16 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 25, 2016, 1:16 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Jason Lowe-Power]

> On July 25, 2016, 1:18 p.m., Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?
> 
> Nikos Nikoleris wrote:
> You can't commit it youself, one of the maintainers will have to commit 
> it for you.

Hi Sophanie,

Have you run all of the regression tests? Are there changes to the stats (I 
would expect so)? Have you checked to make sure the stat changes make intuitive 
sense? Thanks!

Also, it would be nice to see Andreas H., or Steve, or someone who has been 
modifying the cache code lately to take a quick look at this patch before it's 
committed.


- Jason


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On July 25, 2016, 1:16 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 25, 2016, 1:16 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

> On July 25, 2016, 1:18 p.m., Nikos Nikoleris wrote:
> > Ship It!
> 
> Sophiane SENNI wrote:
> How can I commit the patch ? I am not sure I have the commit access ?

You can't commit it youself, one of the maintainers will have to commit it for 
you.


- Nikos


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On July 25, 2016, 1:16 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 25, 2016, 1:16 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 25, 2016, 1:18 après-midi, Nikos Nikoleris wrote:
> > Ship It!

How can I commit the patch ? I am not sure I have the commit access ?


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---


On juil. 25, 2016, 1:16 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 25, 2016, 1:16 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Pierre-Yves Péneau]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8527
---

Ship it!


Ship It!

- Pierre-Yves Péneau


On July 25, 2016, 3:16 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 25, 2016, 3:16 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juil. 23, 2016, 2:05 après-midi, Nikos Nikoleris wrote:
> > src/mem/cache/tags/Tags.py, line 68
> > 
> >
> > This stale argument causes the problems you're facing please remove it, 
> > I don't think it is needed

You are right. This change resolved the problem. Thanks Nikos.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8519
---


On juil. 25, 2016, 1:16 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juil. 25, 2016, 1:16 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8525
---

Ship it!


Ship It!

- Nikos Nikoleris


On July 25, 2016, 1:16 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 25, 2016, 1:16 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated juil. 25, 2016, 1:16 après-midi)


Review request for Default.


Repository: gem5


Description
---

Changeset 11536:1a3a96d435ed
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter 
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True", 
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base_set_assoc.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8519
---


Thanks for all the effort you've put into this. Please make this last set of 
changes and it should be good to go.


src/mem/cache/tags/Tags.py (line 68)


This stale argument causes the problems you're facing please remove it, I 
don't think it is needed



src/mem/cache/tags/base.cc (line 61)


Please remove the comment here. If you would like move the comment in the 
header to explain what the access latency is.



src/mem/cache/tags/base.cc (line 63)


no need for parenthesis here



src/mem/cache/tags/base.cc (line 66)


you don't need the parenthesis here over


- Nikos Nikoleris


On July 20, 2016, 1:32 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated July 20, 2016, 1:32 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated juil. 20, 2016, 1:32 après-midi)


Review request for Default.


Repository: gem5


Description
---

Changeset 11536:1a3a96d435ed
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter 
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True", 
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base_set_assoc.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

> On June 27, 2016, 5 p.m., Nikos Nikoleris wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 218
> > 
> >
> > Would it make sense to move most of this code in the constructor? The 
> > flag sequentialAccess and the condition lookupLatency >= dataLantency 
> > shouldn't change during the simulation.
> 
> Sophiane SENNI wrote:
> I think this code should be left here. Because the total access latency 
> now depends on both tag_latency and data_latency, the value of the 'lat' 
> parameter specifying the access latency is not correct when the accessBlock() 
> function is called. As a result, the 'lat' value should be updated inside the 
> function depending on if it is a sequential or parallel access.
> 
> Sophiane SENNI wrote:
> Regarding your suggestion, I am agree with you that it would be better to 
> move the code in the constructor since as you mentioned the flag 
> sequentialAccess and the access latency value should not change during the 
> simulation. I will see how I can modify the patch accordingly.
> 
> Nikos Nikoleris wrote:
> Thanks for doing this! I think you could create a new variable in the 
> base class (BaseTags) and use that as the latency on every access. In any 
> case, in the code the latency is not dependent on the replacement policy. 
> Mind though that if the access is a miss the latency should always be 
> tagLatency, even when we've enabled the parallel access. We could also move 
> the sequentialAccess variable to BaseCache although I am not sure how 
> applicable a parallel lookup to the tag and the data array is for a fully 
> associative cache.
> 
> Nikos Nikoleris wrote:
> Sophiane, how are things progressing? It would be really great to get 
> done with this and commit it.
> 
> Sophiane SENNI wrote:
> Hi Nikos,
> 
> I was and I am still quite busy with some other work I have to finish. 
> But I will try to publish the new version of the patch as soon as possible. 
> Sorry for the delay.
> 
> Sophiane SENNI wrote:
> Nikos,
> 
> For the new patch, I use the "accessLatency" variable, as the latency on 
> every access, in the base class (BaseTags). I tried to initialize this 
> variable in the constructor according to the access mode as follows:
> 
> BaseTags::BaseTags(const Params *p)
> : ClockedObject(p), blkSize(p->block_size), size(p->size),
>   lookupLatency(p->tag_latency), dataLatency(p->data_latency),
>   accessLatency(p->sequential_access ? 
>   // If sequential access, sum tag lookup and data access latencies
> (p->tag_latency + p->data_latency) :
>   // If parallel access, take the max latency
>   // between tag lookup and data access   
> 
> (p->tag_latency >= p->data_latency ? 
>   p->tag_latency : p->data_latency )),
>   cache(nullptr), warmupBound(0),
>   warmedUp(false), numBlocks(0)
> {
> }
> 
> However, when checking by simulation, the value of "sequential_access" 
> parameter is always taken as "False". Even if this paramater is set to "True" 
> in configs/common/Caches.py
> 
> Do you have a solution to correctly initialize the "accessLatency" 
> variable in the initialization list ?
> 
> Thanks.

Hi Sohpianne, Can you post the patch so that I can have a look? I guess you run 
into an ordering problem with how we initialize the objects. BTW instead of the 
a >= b ? a  : b you can always use the std::max(a, b)


- Nikos


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8441
---


On June 28, 2016, 1:06 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated June 28, 2016, 1:06 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juin 27, 2016, 5 après-midi, Nikos Nikoleris wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 218
> > 
> >
> > Would it make sense to move most of this code in the constructor? The 
> > flag sequentialAccess and the condition lookupLatency >= dataLantency 
> > shouldn't change during the simulation.
> 
> Sophiane SENNI wrote:
> I think this code should be left here. Because the total access latency 
> now depends on both tag_latency and data_latency, the value of the 'lat' 
> parameter specifying the access latency is not correct when the accessBlock() 
> function is called. As a result, the 'lat' value should be updated inside the 
> function depending on if it is a sequential or parallel access.
> 
> Sophiane SENNI wrote:
> Regarding your suggestion, I am agree with you that it would be better to 
> move the code in the constructor since as you mentioned the flag 
> sequentialAccess and the access latency value should not change during the 
> simulation. I will see how I can modify the patch accordingly.
> 
> Nikos Nikoleris wrote:
> Thanks for doing this! I think you could create a new variable in the 
> base class (BaseTags) and use that as the latency on every access. In any 
> case, in the code the latency is not dependent on the replacement policy. 
> Mind though that if the access is a miss the latency should always be 
> tagLatency, even when we've enabled the parallel access. We could also move 
> the sequentialAccess variable to BaseCache although I am not sure how 
> applicable a parallel lookup to the tag and the data array is for a fully 
> associative cache.
> 
> Nikos Nikoleris wrote:
> Sophiane, how are things progressing? It would be really great to get 
> done with this and commit it.
> 
> Sophiane SENNI wrote:
> Hi Nikos,
> 
> I was and I am still quite busy with some other work I have to finish. 
> But I will try to publish the new version of the patch as soon as possible. 
> Sorry for the delay.

Nikos,

For the new patch, I use the "accessLatency" variable, as the latency on every 
access, in the base class (BaseTags). I tried to initialize this variable in 
the constructor according to the access mode as follows:

BaseTags::BaseTags(const Params *p)
: ClockedObject(p), blkSize(p->block_size), size(p->size),
  lookupLatency(p->tag_latency), dataLatency(p->data_latency),
  accessLatency(p->sequential_access ? 
  // If sequential access, sum tag lookup and data access latencies
(p->tag_latency + p->data_latency) :
  // If parallel access, take the max latency
  // between tag lookup and data access 
(p->tag_latency >= p->data_latency ? 
  p->tag_latency : p->data_latency )),
  cache(nullptr), warmupBound(0),
  warmedUp(false), numBlocks(0)
{
}

However, when checking by simulation, the value of "sequential_access" 
parameter is always taken as "False". Even if this paramater is set to "True" 
in configs/common/Caches.py

Do you have a solution to correctly initialize the "accessLatency" variable in 
the initialization list ?

Thanks.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8441
---


On juin 28, 2016, 1:06 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juin 28, 2016, 1:06 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juin 27, 2016, 5 après-midi, Nikos Nikoleris wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 218
> > 
> >
> > Would it make sense to move most of this code in the constructor? The 
> > flag sequentialAccess and the condition lookupLatency >= dataLantency 
> > shouldn't change during the simulation.
> 
> Sophiane SENNI wrote:
> I think this code should be left here. Because the total access latency 
> now depends on both tag_latency and data_latency, the value of the 'lat' 
> parameter specifying the access latency is not correct when the accessBlock() 
> function is called. As a result, the 'lat' value should be updated inside the 
> function depending on if it is a sequential or parallel access.
> 
> Sophiane SENNI wrote:
> Regarding your suggestion, I am agree with you that it would be better to 
> move the code in the constructor since as you mentioned the flag 
> sequentialAccess and the access latency value should not change during the 
> simulation. I will see how I can modify the patch accordingly.
> 
> Nikos Nikoleris wrote:
> Thanks for doing this! I think you could create a new variable in the 
> base class (BaseTags) and use that as the latency on every access. In any 
> case, in the code the latency is not dependent on the replacement policy. 
> Mind though that if the access is a miss the latency should always be 
> tagLatency, even when we've enabled the parallel access. We could also move 
> the sequentialAccess variable to BaseCache although I am not sure how 
> applicable a parallel lookup to the tag and the data array is for a fully 
> associative cache.
> 
> Nikos Nikoleris wrote:
> Sophiane, how are things progressing? It would be really great to get 
> done with this and commit it.

Hi Nikos,

I was and I am still quite busy with some other work I have to finish. But I 
will try to publish the new version of the patch as soon as possible. Sorry for 
the delay.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8441
---


On juin 28, 2016, 1:06 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juin 28, 2016, 1:06 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

> On June 27, 2016, 5 p.m., Nikos Nikoleris wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 218
> > 
> >
> > Would it make sense to move most of this code in the constructor? The 
> > flag sequentialAccess and the condition lookupLatency >= dataLantency 
> > shouldn't change during the simulation.
> 
> Sophiane SENNI wrote:
> I think this code should be left here. Because the total access latency 
> now depends on both tag_latency and data_latency, the value of the 'lat' 
> parameter specifying the access latency is not correct when the accessBlock() 
> function is called. As a result, the 'lat' value should be updated inside the 
> function depending on if it is a sequential or parallel access.
> 
> Sophiane SENNI wrote:
> Regarding your suggestion, I am agree with you that it would be better to 
> move the code in the constructor since as you mentioned the flag 
> sequentialAccess and the access latency value should not change during the 
> simulation. I will see how I can modify the patch accordingly.
> 
> Nikos Nikoleris wrote:
> Thanks for doing this! I think you could create a new variable in the 
> base class (BaseTags) and use that as the latency on every access. In any 
> case, in the code the latency is not dependent on the replacement policy. 
> Mind though that if the access is a miss the latency should always be 
> tagLatency, even when we've enabled the parallel access. We could also move 
> the sequentialAccess variable to BaseCache although I am not sure how 
> applicable a parallel lookup to the tag and the data array is for a fully 
> associative cache.

Sophiane, how are things progressing? It would be really great to get done with 
this and commit it.


- Nikos


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8441
---


On June 28, 2016, 1:06 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated June 28, 2016, 1:06 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

> On June 27, 2016, 5 p.m., Nikos Nikoleris wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 218
> > 
> >
> > Would it make sense to move most of this code in the constructor? The 
> > flag sequentialAccess and the condition lookupLatency >= dataLantency 
> > shouldn't change during the simulation.
> 
> Sophiane SENNI wrote:
> I think this code should be left here. Because the total access latency 
> now depends on both tag_latency and data_latency, the value of the 'lat' 
> parameter specifying the access latency is not correct when the accessBlock() 
> function is called. As a result, the 'lat' value should be updated inside the 
> function depending on if it is a sequential or parallel access.
> 
> Sophiane SENNI wrote:
> Regarding your suggestion, I am agree with you that it would be better to 
> move the code in the constructor since as you mentioned the flag 
> sequentialAccess and the access latency value should not change during the 
> simulation. I will see how I can modify the patch accordingly.

Thanks for doing this! I think you could create a new variable in the base 
class (BaseTags) and use that as the latency on every access. In any case, in 
the code the latency is not dependent on the replacement policy. Mind though 
that if the access is a miss the latency should always be tagLatency, even when 
we've enabled the parallel access. We could also move the sequentialAccess 
variable to BaseCache although I am not sure how applicable a parallel lookup 
to the tag and the data array is for a fully associative cache.


- Nikos


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8441
---


On June 28, 2016, 1:06 p.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated June 28, 2016, 1:06 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

> On juin 27, 2016, 5 après-midi, Nikos Nikoleris wrote:
> > src/mem/cache/tags/base_set_assoc.hh, line 218
> > 
> >
> > Would it make sense to move most of this code in the constructor? The 
> > flag sequentialAccess and the condition lookupLatency >= dataLantency 
> > shouldn't change during the simulation.
> 
> Sophiane SENNI wrote:
> I think this code should be left here. Because the total access latency 
> now depends on both tag_latency and data_latency, the value of the 'lat' 
> parameter specifying the access latency is not correct when the accessBlock() 
> function is called. As a result, the 'lat' value should be updated inside the 
> function depending on if it is a sequential or parallel access.

Regarding your suggestion, I am agree with you that it would be better to move 
the code in the constructor since as you mentioned the flag sequentialAccess 
and the access latency value should not change during the simulation. I will 
see how I can modify the patch accordingly.


- Sophiane


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8441
---


On juin 28, 2016, 1:06 après-midi, Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated juin 28, 2016, 1:06 après-midi)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> If the cache access mode is parallel, i.e. "sequential_access" parameter 
> is set to "False", tags and data are accessed in parallel. Therefore,
> the hit_latency is the maximum latency between tag_latency and
> data_latency. On the other hand, if the cache access mode is
> sequential, i.e. "sequential_access" parameter is set to "True", 
> tags and data are accessed sequentially. Therefore, the hit_latency
> is the sum of tag_latency plus data_latency.
> 
> 
> Diffs
> -
> 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated juin 28, 2016, 1:06 après-midi)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11536:1a3a96d435ed
---
mem: Split the hit_latency into tag_latency and data_latency

If the cache access mode is parallel, i.e. "sequential_access" parameter 
is set to "False", tags and data are accessed in parallel. Therefore,
the hit_latency is the maximum latency between tag_latency and
data_latency. On the other hand, if the cache access mode is
sequential, i.e. "sequential_access" parameter is set to "True", 
tags and data are accessed sequentially. Therefore, the hit_latency
is the sum of tag_latency plus data_latency.


Diffs (updated)
-

  configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base_set_assoc.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Nikos Nikoleris]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3502/#review8444
---


Sorry, that was probably unclear from my comment but the commit message body 
should be wrapped such that no line is more than 75 characters rather than the 
whole body being limited to 76 char.

- Nikos Nikoleris


On June 28, 2016, 9:57 a.m., Sophiane SENNI wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3502/
> ---
> 
> (Updated June 28, 2016, 9:57 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11536:1a3a96d435ed
> ---
> mem: Split the hit_latency into tag_latency and data_latency
> 
> Sum the two latency values for sequential access. Otherwise, take the max.
> 
> 
> Diffs
> -
> 
>   src/mem/cache/tags/base_set_assoc.hh 
> 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
>   src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
> 
> Diff: http://reviews.gem5.org/r/3502/diff/
> 
> 
> Testing
> ---
> 
> Tested using --Debug-flags=Cache
> 
> 
> Thanks,
> 
> Sophiane SENNI
> 
>

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

Re: [gem5-dev] Review Request 3502: mem: Split the hit_latency into tag_latency and data_latency

[Sophiane SENNI]

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

(Updated juin 28, 2016, 9:57 matin)


Review request for Default.


Summary (updated)
-

mem: Split the hit_latency into tag_latency and data_latency


Repository: gem5


Description (updated)
---

Changeset 11536:1a3a96d435ed
---
mem: Split the hit_latency into tag_latency and data_latency

Sum the two latency values for sequential access. Otherwise, take the max.


Diffs (updated)
-

  src/mem/cache/tags/base_set_assoc.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/fa_lru.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/Cache.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  configs/common/Caches.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/fa_lru.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/Tags.py 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.hh 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 
  src/mem/cache/tags/base.cc 80e79ae636ca6b021cbf7aa985b5fd56cb5b2708 

Diff: http://reviews.gem5.org/r/3502/diff/


Testing
---

Tested using --Debug-flags=Cache


Thanks,

Sophiane SENNI

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