Ah I see. I'm still new to the code base so I'm giving the benefit of the
doubt to cleverness.

I'm still prodding around -- I'll try to submit patches once I figure out
how to use mercurial (coming from git so hopefully it won't be too bad).

On Mon, Jan 9, 2012 at 3:49 PM, Steve Reinhardt <[email protected]> wrote:

> Outdated is always a safer bet than clever... looks like the original
> comment dates all the way back to 2006, at which point there were 3 iop
> objects generated, ea_iop, memacc_iop, and just plain iop:
> http://repo.gem5.org/gem5/annotate/8e2506a2beb6/arch/alpha/isa/mem.isa#l353
>
>
> Looks like Korey got rid of ea_iop here without updating the comment (easy
> enough to do):
> http://repo.gem5.org/gem5/diff/19fedb1e5ded/src/arch/alpha/isa/mem.isa
> And then got rid of the last use of memacc_iop here, but didn't notice
> that it was no longer needed:
> http://repo.gem5.org/gem5/diff/6cd5f0282d8a/src/arch/alpha/isa/mem.isa
>
> Paul, if you're up for it, it would be great if you update any bad
> comments you find and then submit a patch to reviewboard.  Even just
> deleting the bad comments would be a help.
>
> Thanks,
>
> Steve
>
> On Mon, Jan 9, 2012 at 11:35 AM, Paul Rosenfeld <[email protected]>wrote:
>
>> Looking at the code for the ALPHA ISA, I've run across something that's
>> either an outdated comment or something I don't quite understand.
>>
>> In particular I'm looking at the code for LoadStoreBase in
>> src/arch/alpha/isa/mem.isa:
>>
>> 468     # Some CPU models execute the memory operation as an atomic unit,
>> 469     # while others want to separate them into an effective address
>> 470     # computation and a memory access operation.  As a result, we need
>> 471     # to generate three StaticInst objects.  Note that the latter two
>> 472     # are nested inside the larger "atomic" one.
>> 473
>> 474     # *Generate InstObjParams for each of the three objects.*  Note
>> that
>> 475     # they differ only in the set of code objects contained (which in
>> 476     # turn affects the object's overall operand list).
>> 477     iop = InstObjParams(name, Name, base_class,
>> 478                         { 'ea_code':ea_code,
>> 'memacc_code':memacc_code, 'postacc_code':postacc_code },
>> 479                         inst_flags)
>> 480     memacc_iop = InstObjParams(name, Name, base_class,
>> 481                         { 'memacc_code':memacc_code,
>> 'postacc_code':postacc_code },
>> 482                         inst_flags)
>>
>> The first part that has me scratching my head is that the comment says
>> that it will generate 3 instances of InstObjParams, but it looks like only
>> 2 are created down below.
>>
>> The second part that has me confused is that it doesn't seem that
>> memacc_iop is actually used anywhere down below. Doesn't an InstObjParam
>> only have an effect if it is passed through the subst() method of some
>> template? Or am I missing something?
>>
>> Thanks!
>> Paul
>>
>> _______________________________________________
>> gem5-users mailing list
>> [email protected]
>> http://m5sim.org/cgi-bin/mailman/listinfo/gem5-users
>>
>
>
> _______________________________________________
> gem5-users mailing list
> [email protected]
> http://m5sim.org/cgi-bin/mailman/listinfo/gem5-users
>
_______________________________________________
gem5-users mailing list
[email protected]
http://m5sim.org/cgi-bin/mailman/listinfo/gem5-users

Reply via email to