> On Jan. 6, 2016, 1:39 p.m., Andreas Hansson wrote:
> > I still find that the design needs a far better explanation, and at least
> > one simple example. I would suggest implementing the SwapReq as an atomic
> > op (as I guess it is our only example). Also, surely we need to impose
> > restrictions on the granularity? If we use SwapReq as an example then
> > things would be much more clear.
> >
> > I am also not sure about Joel's comments having been addressed.
Our GPU code has lots of examples for AtomicOps, and the concept is pretty
simple: use function pointers to perform the atomic RMW op instead of adding a
bunch of conditions to check all possible SwapReqs. Here is how our GPU CAS is
implemented, and you can see all AtomicOps in gpu_dyn_inst.hh:
template<typename T>
class AtomicOpCAS : public TypedAtomicOpFunctor<T>
{
public:
T c;
T s;
ComputeUnit *computeUnit;
AtomicOpCAS(T _c, T _s, ComputeUnit *compute_unit)
: c(_c), s(_s), computeUnit(compute_unit) { }
void
execute(T *b)
{
computeUnit->numCASOps++;
if (*b == c) {
*b = s;
} else {
computeUnit->numFailedCASOps++;
}
if (computeUnit->xact_cas_mode) {
computeUnit->xactCasLoadMap.clear();
}
}
};
A call to the functor will call execute(). In this case the ISA inst objects
set the swap value 's' based on one of its src operands, and if the current
value 'b' hasn't changed from the orignial 'c' (another src operand), b gets s.
I think implementing the SwapReq as an AtomicOp would be as simple as moving
the logic in if (pkt->cmd == MemCmd::SwapReq) in abstract_mem.cc to a derived
AtomicOpFunctor class and calling it, but I would prefer not to make that
change part of this patch. I'm not sure how the AtomicOpFunctor would be setup
by the CPU ISAs here and I don't have the time or the infrastructure to test
the changes sufficiently to ensure they work properly for the CPU ISAs that use
this (ARM and SPARC?), so I think it'd be safer to leave it as it is for now.
Getting this patch in is necessary for us to push our GPU code, which is pretty
important to lots of people in the community - at least based on the e-mails
from users I've been getting - and I would hope this issue wouldn't hold that
up.
As far as restricting the granularity, I can't imagine a case where one would
try to perform a RMW on a size greater than a cache line, but to ensure it I
can add a fatal if the size is greater than a cache block.
- Tony
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3205/#review7813
-----------------------------------------------------------
On Jan. 6, 2016, 1:12 p.m., Tony Gutierrez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3205/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 1:12 p.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 11280:f5e328bd5233
> ---------------------------
> mem: support for gpu-style RMWs in ruby
>
> This patch adds support for GPU-style read-modify-write (RMW) operations in
> ruby. Such atomic operations are traditionally executed at the memory
> controller
> (instead of through an L1 cache using cache-line locking).
>
> Currently, this patch works by propogating operation functors through the
> memory
> system.
>
>
> Diffs
> -----
>
> src/base/types.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd
> src/mem/abstract_mem.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd
> src/mem/packet.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd
> src/mem/protocol/RubySlicc_Exports.sm
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd
> src/mem/request.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd
>
> Diff: http://reviews.gem5.org/r/3205/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tony Gutierrez
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
