[gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/ --- Review request for Default. Repository: gem5 Description --- Changeset 11195

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7428 --- Impressive! Should it not be src/dev/gpu rather than src/gpu? Also, cou

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Oct. 30, 2015, 3:27 p.m., Andreas Hansson wrote: > > Impressive! > > > > Should it not be src/dev/gpu rather than src/gpu? > > > > Also, could you please make it a subdirectory in the gpu dir (as from > > above). There are more GPUs in this world...in fact there is already the > > NoMali

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Oct. 30, 2015, 10:27 p.m., Andreas Hansson wrote: > > Impressive! > > > > Should it not be src/dev/gpu rather than src/gpu? > > > > Also, could you please make it a subdirectory in the gpu dir (as from > > above). There are more GPUs in this world...in fact there is already the > > NoMal

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Oct. 30, 2015, 10:27 p.m., Andreas Hansson wrote: > > Impressive! > > > > Should it not be src/dev/gpu rather than src/gpu? > > > > Also, could you please make it a subdirectory in the gpu dir (as from > > above). There are more GPUs in this world...in fact there is already the > > NoMal

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Oct. 30, 2015, 10:27 p.m., Andreas Hansson wrote: > > Impressive! > > > > Should it not be src/dev/gpu rather than src/gpu? > > > > Also, could you please make it a subdirectory in the gpu dir (as from > > above). There are more GPUs in this world...in fact there is already the > > NoMal

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7539 --- This is indeed very impressive! However, as it is, this patch is basicall

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7453 --- Looking forward to seeing the GPU in action! I'm mostly fine with new fil

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 13, 2015, 3:03 p.m., Joel Hestness wrote: > > src/gpu/gpu_mem_packet.cc, line 214 > > > > > > Also noted in the GPU atomics patch: Why not just return a pointer to > > the type-specific functions rather than dynam

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7623 --- src/mem/protocol/RubySlicc_Types.sm (line 136)

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/ --- (Updated Dec. 23, 2015, 9:11 a.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Oct. 30, 2015, 10:27 p.m., Andreas Hansson wrote: > > Impressive! > > > > Should it not be src/dev/gpu rather than src/gpu? > > > > Also, could you please make it a subdirectory in the gpu dir (as from > > above). There are more GPUs in this world...in fact there is already the > > NoMal

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Oct. 30, 2015, 3:27 p.m., Andreas Hansson wrote: > > Impressive! > > > > Should it not be src/dev/gpu rather than src/gpu? > > > > Also, could you please make it a subdirectory in the gpu dir (as from > > above). There are more GPUs in this world...in fact there is already the > > NoMali

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 11:51 a.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconce

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 11:51 a.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconce

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 20, 2015, 7:59 p.m., Joel Hestness wrote: > > src/mem/protocol/RubySlicc_Types.sm, line 136 > > > > > > These don't appear to be defined or used anywhere. Can you please > > remove them? Sure thing. These can be

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 13, 2015, 11:03 p.m., Joel Hestness wrote: > > src/mem/protocol/RubySlicc_Exports.sm, line 243 > > > > > > Please describe these more clearly. What is a CorePair? What do "TCP", > > "TCC", "SQC" stand for? Etc. >

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

Steve, I believe that the entirety of the license came from our legal department at some point. Probably it was what they came up with as a good 3-clause BSD license. It should be consistent throughout our code base as well. Joe From: Steve Reinhardt on beh

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 13, 2015, 11:03 p.m., Joel Hestness wrote: > > src/mem/ruby/system/Sequencer.py, line 77 > > > > > > Why is this default initialized to 16? > > Brad Beckmann wrote: > Thank you for pointing this out. We will r

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/ --- (Updated Jan. 13, 2016, 1:25 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7882 --- ...and yes, please split the patch to make it reviewable (if that is a wo

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7883 --- configs/ruby/MemCntrlConstructor.py (line 44)

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 11 p.m., Andreas Hansson wrote: > > configs/ruby/MemCntrlConstructor.py, line 44 > > > > > > I thought we concluded that one fixed latency memory controller is > > enough? I'd suggest to use SimpleMemory

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Nov. 10, 2015, 7:51 p.m., Andreas Sandberg wrote: > > This is indeed very impressive! However, as it is, this patch is basically > > the definition of unreviewable. Could you split it into a handful of > > different patches for review purposes? ReviewBoard makes all kinds of > > disconcer

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). Andreas, we simply do not have the time and resources to do so. We have spent a lot of time addressing Joel's changes and we have and will address the co

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 2:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes and

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7897 --- SConstruct (line 1244)

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7899 --- So src/mem/ruby/structures/CacheMemory.cc (line 38)

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 13, 2016, 10:47 p.m., Andreas Hansson wrote: > > ...and yes, please split the patch to make it reviewable (if that is a > > word). > > Brad Beckmann wrote: > Andreas, we simply do not have the time and resources to do so. We have > spent a lot of time addressing Joel's changes an

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > configs/example/apu_se.py, line 289 > > > > > > any reason why minor is not here? > > > > Why not use CpuConfig? Up until a few months ago we didn't even h

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > configs/ruby/GPU_VIPER_Baseline.py, line 126 > > > > > > again...there is a lot of repetition > > > > these files really need some modularity We agree, how

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce s

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:41 a.m., Andreas Hansson wrote: > > src/mem/ruby/system/GPUCoalescer.cc, line 77 > > > > > > seems like an enum would be a better option for the request scope While our current implementation doesn't e

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce s

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:41 p.m., Andreas Hansson wrote: > > src/mem/ruby/structures/CacheMemory.cc, line 38 > > > > > > This seems like a piece of functionality that is independent from the > > compute-gpu. A patch on its own?

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > configs/common/GPUTLBConfig.py, line 76 > > > > > > the location of the file, and the name of the functions make it seem > > quite generic, when in fact it is very sp

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce s

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > configs/common/GPUTLBOptions.py, line 42 > > > > > > I think we need some form of prefix, apu/compute-gpu/gpgpu etc. > > > > Most, if not all, existing option

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > configs/ruby/GPU_RfO.py, line 80 > > > > > > there seems to be an awful lot of repetition of boilerplate code > > involved in these scripts, can the common parts not

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce s

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:41 p.m., Andreas Hansson wrote: > > src/mem/ruby/system/Sequencer.py, line 81 > > > > > > Is this GPU related? > > Brad Beckmann wrote: > Not necessarily. We will take this out of this patch and Mi

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > configs/ruby/GPU_RfO.py, line 80 > > > > > > there seems to be an awful lot of repetition of boilerplate code > > involved in these scripts, can the common parts not

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > configs/ruby/GPU_RfO.py, line 80 > > > > > > there seems to be an awful lot of repetition of boilerplate code > > involved in these scripts, can the common parts not

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > configs/ruby/GPU_RfO.py, line 80 > > > > > > there seems to be an awful lot of repetition of boilerplate code > > involved in these scripts, can the common parts not

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 6:26 p.m., Andreas Hansson wrote: > > configs/ruby/GPU_RfO.py, line 80 > > > > > > there seems to be an awful lot of repetition of boilerplate code > > involved in these scripts, can the common parts not

Re: [gem5-dev] Review Request 3189: gpu: AMD's baseline GPU model

> On Jan. 14, 2016, 10:26 a.m., Andreas Hansson wrote: > > SConstruct, line 1244 > > > > > > This makes it look like the two are not orthogonal, could you > > explain/comment? > > Tony Gutierrez wrote: > We introduce