> On Dec. 24, 2015, 9:18 a.m., Andreas Hansson wrote: > > This is a major change to the memory system, and I find the description > > rather misleading. It would be good to get some more examples, and make > > sure that this works as a general solution for "in-memory-system > > operations". What are the assumptions? Cache line sized only? What is the > > relation between packets and these requests? > > > > Also, is there a specific reason why you have opted for the crazily generic > > functor, as opposed to hard-coding the supported functions? > > > > I'd like to also understand how this relates to something like ARM v8.1 far > > atomics (and answers to the questions above would help tremendously with > > that). > > Tony Gutierrez wrote: > Q1) What are the assumptions, i.e., cache line size only? > A1) There is no assumption on the size of the data. The functors are > templated, > and can therefore perform arbitrary actions on arbitrary types; it's up > to the > user define their own functors as needed. > > Q2) Relation between packets and requests? > A2) The packets we use are MemCmd::SwapReq, and therefore we could put > the logic > for executing the atomic ops inside the original if (pkt->cmd == > MemCmd::SwapReq) check in AbstractMemory::access(), and just check if > it's an > ATOMIC_*_OP request. So I don't think it's a major change to the memory > sytem > itself, it's more like we're generalizing atomic ops so we can have others > besides CAS. For example, we derive from the atomic functors to provide > several > atomic ops: logical ops, CAS, min/max, arithmetic, etc. > > Q3) Why the generic functor name? > A3) It's generic because it's a purely virtua class, meant to define the > API > only. The idea is to derive from these and implement your own atomic ops. > E.g., > we derive from these to create AtomicOpAnd, AtomicOpOr, etc. So you can > see > the derived functors have much more meaningful names. If we hard-coded > every > possible op's implementation it could require us adding more code than > necessary. With this implementation we can simply add a few ATOMIC request > types, then we just check if it's ATOMIC, and execute the generic atomic > functor's execute() method, which should be overwritten by the derived op > we're actually using. > > Q4) General enough? How doees it relate to ARM v8.1, e.g.? > A4) I think the implementation is sufficiently generalized to support any > ISA > atomics. Assuming the ISA atomics perform their atomic ops at the memory > controllers it should be as simple as adding support in the ISA > instructions to > create the proper packets/requests, and define your own AtomicOp > Functors. A > similar approach could be used to implement the atomics at a cache > controller > if need be.
Thanks for the clarifications. Two requests from my end: 1) Could you capture this in the code comments somewhere? 2) I assume this does not negatively impact simulation performance, but it would be good to know for sure. Have you checked? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3205/#review7750 ----------------------------------------------------------- On Dec. 23, 2015, 6:34 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3205/ > ----------------------------------------------------------- > > (Updated Dec. 23, 2015, 6:34 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11279:ea817032d082 > --------------------------- > 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/mem/request.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/protocol/RubySlicc_Exports.sm > d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/packet.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/abstract_mem.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/base/types.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
