Re: [gem5-dev] Review Request 3535: gpu: Adding serializing methods to Wavefront

[Alexandru Dutu]

> On June 30, 2016, 7:17 a.m., Andreas Hansson wrote:
> > src/gpu-compute/wavefront.cc, line 948
> > 
> >
> > Why the custom format? Why not stick to gem5's established methods for 
> > serializing and deserializing scalars and containers?
> 
> Alexandru Dutu wrote:
> Andreas, are you refering to the Serializable class? If so, my 
> understanding is that Serializable is used in the context of creating a 
> Checkpoint, therefore it includes the names of the classes and class members 
> in the serialized stream. This something which is not desired in our case as 
> we want the serialized context to be something available to the actual 
> simulated hardware and not just to the simulator. It seems that even 
> SERIALIZE_CONTAINER calls arrayParamOut which includes names in the output 
> stream. Serializable seems quite heavy weight, as it includes this extra 
> class for handling checkpoints (ScopedCheckpointSection) and it does not do 
> what we need.
> 
> Andreas Hansson wrote:
> Ok, then I really do not understand what the patch is trying to do :-). 
> Perhaps you can explain the use-case in the description?

The functionality which we want to enable is context switching, so this patch 
adds the methods that get the context of a wavefront ready to be written to the 
simulated memory. I will update the description.


- Alexandru


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


On June 29, 2016, 4:02 p.m., Tony Gutierrez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3535/
> ---
> 
> (Updated June 29, 2016, 4:02 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11556:e46e9a28e2aa
> ---
> gpu: Adding serializing methods to Wavefront
> This patch adds methods to serialize the context of a particular wavefront.
> It should be a neat feature to have for wavefront preemption.
> 
> 
> Diffs
> -
> 
>   src/gpu-compute/wavefront.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
>   src/gpu-compute/wavefront.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
> 
> Diff: http://reviews.gem5.org/r/3535/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Re: [gem5-dev] Review Request 3535: gpu: Adding serializing methods to Wavefront

[Andreas Hansson]

> On June 30, 2016, 7:17 a.m., Andreas Hansson wrote:
> > src/gpu-compute/wavefront.cc, line 948
> > 
> >
> > Why the custom format? Why not stick to gem5's established methods for 
> > serializing and deserializing scalars and containers?
> 
> Alexandru Dutu wrote:
> Andreas, are you refering to the Serializable class? If so, my 
> understanding is that Serializable is used in the context of creating a 
> Checkpoint, therefore it includes the names of the classes and class members 
> in the serialized stream. This something which is not desired in our case as 
> we want the serialized context to be something available to the actual 
> simulated hardware and not just to the simulator. It seems that even 
> SERIALIZE_CONTAINER calls arrayParamOut which includes names in the output 
> stream. Serializable seems quite heavy weight, as it includes this extra 
> class for handling checkpoints (ScopedCheckpointSection) and it does not do 
> what we need.

Ok, then I really do not understand what the patch is trying to do :-). Perhaps 
you can explain the use-case in the description?


- Andreas


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


On June 29, 2016, 4:02 p.m., Tony Gutierrez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3535/
> ---
> 
> (Updated June 29, 2016, 4:02 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11556:e46e9a28e2aa
> ---
> gpu: Adding serializing methods to Wavefront
> This patch adds methods to serialize the context of a particular wavefront.
> It should be a neat feature to have for wavefront preemption.
> 
> 
> Diffs
> -
> 
>   src/gpu-compute/wavefront.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
>   src/gpu-compute/wavefront.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
> 
> Diff: http://reviews.gem5.org/r/3535/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Re: [gem5-dev] Review Request 3535: gpu: Adding serializing methods to Wavefront

[Alexandru Dutu]

> On June 30, 2016, 7:17 a.m., Andreas Hansson wrote:
> > src/gpu-compute/wavefront.cc, line 948
> > 
> >
> > Why the custom format? Why not stick to gem5's established methods for 
> > serializing and deserializing scalars and containers?

Andreas, are you refering to the Serializable class? If so, my understanding is 
that Serializable is used in the context of creating a Checkpoint, therefore it 
includes the names of the classes and class members in the serialized stream. 
This something which is not desired in our case as we want the serialized 
context to be something available to the actual simulated hardware and not just 
to the simulator. It seems that even SERIALIZE_CONTAINER calls arrayParamOut 
which includes names in the output stream. Serializable seems quite heavy 
weight, as it includes this extra class for handling checkpoints 
(ScopedCheckpointSection) and it does not do what we need.


> On June 30, 2016, 7:17 a.m., Andreas Hansson wrote:
> > src/gpu-compute/wavefront.cc, line 946
> > 
> >
> > Not too many bonus points for comments here...that's for sure.

I have added a comment in the header file which should describe the behaviour 
of this method and added comments inside it where things are not straight 
forward (i.e. recovergence stack and LDS). Please let me know what is not clear 
and requires a comment, would be happy to add some more.


- Alexandru


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


On June 29, 2016, 4:02 p.m., Tony Gutierrez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3535/
> ---
> 
> (Updated June 29, 2016, 4:02 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11556:e46e9a28e2aa
> ---
> gpu: Adding serializing methods to Wavefront
> This patch adds methods to serialize the context of a particular wavefront.
> It should be a neat feature to have for wavefront preemption.
> 
> 
> Diffs
> -
> 
>   src/gpu-compute/wavefront.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
>   src/gpu-compute/wavefront.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
> 
> Diff: http://reviews.gem5.org/r/3535/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Re: [gem5-dev] Review Request 3535: gpu: Adding serializing methods to Wavefront

[Andreas Hansson]

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



src/gpu-compute/wavefront.hh (line 389)


Surely one of these should be const



src/gpu-compute/wavefront.cc (line 946)


Not too many bonus points for comments here...that's for sure.



src/gpu-compute/wavefront.cc (line 948)


Why the custom format? Why not stick to gem5's established methods for 
serializing and deserializing scalars and containers?


- Andreas Hansson


On June 29, 2016, 4:02 p.m., Tony Gutierrez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3535/
> ---
> 
> (Updated June 29, 2016, 4:02 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11556:e46e9a28e2aa
> ---
> gpu: Adding serializing methods to Wavefront
> This patch adds methods to serialize the context of a particular wavefront.
> It should be a neat feature to have for wavefront preemption.
> 
> 
> Diffs
> -
> 
>   src/gpu-compute/wavefront.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
>   src/gpu-compute/wavefront.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
> 
> Diff: http://reviews.gem5.org/r/3535/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

[gem5-dev] Review Request 3535: gpu: Adding serializing methods to Wavefront

[Tony Gutierrez]

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

Review request for Default.


Repository: gem5


Description
---

Changeset 11556:e46e9a28e2aa
---
gpu: Adding serializing methods to Wavefront
This patch adds methods to serialize the context of a particular wavefront.
It should be a neat feature to have for wavefront preemption.


Diffs
-

  src/gpu-compute/wavefront.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c 
  src/gpu-compute/wavefront.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c 

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


Testing
---


Thanks,

Tony Gutierrez

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