On Wed, Jul 31, 2013 at 9:28 PM, Nilay Vaish <[email protected]> wrote:

> On Wed, 31 Jul 2013, Steve Reinhardt wrote:
>
> It seems particularly confusing to me that at some points in time the
>>
> offset for FP regs is FP_Base_DepTag, while at other points in time it's
>> NumIntRegs (and similarly Ctrl_Base_DepTag and NumIntRegs+NumFloatRegs).
>> Does anyone have any insight on why this is necessary?
>>
>
> Can point out the code? I looked through o3/cpu.cc and o3/rename_impl.hh.
> Both seemed fine to me.


See for example this line, which is at
http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l962:

   962 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l962>
            flat_src_reg = src_reg - TheISA::Ctrl_Base_DepTag +

   963 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l963>
                           TheISA::NumFloatRegs + TheISA::NumIntRegs;


So src_reg *was* offset by Ctrl_Base_DepTag, but now we're tweaking it to
be offset by NumFloatRegs+NumIntRegs instead.  For most ISAs this is a
no-op, since Ctrl_Base_DepTag == (NumFloatRegs+NumIntRegs), but for ARM and
x86 that's not the case.  So now we have a situation where clearly
sometimes it's expected to be adjusted by one offset, and sometimes by the
other.

The case for FP regs is a little more convoluted; if you look just a couple
of lines above there, the code is:

   956 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l956>
            src_reg = src_reg - TheISA::FP_Base_DepTag;

   957 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l957>
            flat_src_reg = inst->tcBase()->flattenFloatIndex(src_reg);

   958 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l958>
            DPRINTF(Rename, "Flattening index %d to %d.\n",

   959 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l959>
                    (int)src_reg, (int)flat_src_reg);

   960 
<http://repo.gem5.org/gem5/file/2492d7ccda7e/src/cpu/o3/rename_impl.hh#l960>
            flat_src_reg += TheISA::NumIntRegs;


So again we're removing an offset of FP_Base_DepTag and then adding back an
offset of NumIntRegs, though this time there's a flattening call in the
middle.  But again, if one expects that FP_Base_DepTag == NumIntRegs, it
doesn't make sense to use different values in different places.


>
>
> Meanwhile I think I'll change FP_Base_DepTag to be equal to NumIntRegs in
>> those architectures where it's not and see if the regressions pass...
>>
>
> I suspect x86 regressions would fail. In operands.isa we explicitly set
> the index for each register.
>

You're right, I made the change and the x86 O3 regressions are the only
ones that fail.  I don't get why though, as the indices in operands.isa
don't seem that well aligned with the constants in arch/x86/registers.hh.
 For example, the control regs in operands.isa seem to start at 100, while
Ctrl_Base_DepTag is the result of a complex equation that happens to work
out to 184.

Steve
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to