-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/297/#review453
-----------------------------------------------------------


I found one instance where I think the line got too long, but there was ALOT of 
stuff here, and I stopped really looking carefully after a while. I have a 
comment as far as the quantity of new classes (below). Also, I think you may 
have overly applied the SIMDness. VFP3 instructions are "vector" instructions 
in the sense that they apply the same operation multiple times, but I got the 
impression that they literally were applied multiple times serially instead of 
in parallel by repeating them with different register indexes. I think I 
implemented them that way using microcode, but it's been a while.


src/arch/arm/isa/insts/fp.isa
<http://reviews.m5sim.org/r/297/#comment658>

    My counting may be off, but I think these are now more than 80 characters 
long. If they are, please wrap them.



src/cpu/FuncUnit.py
<http://reviews.m5sim.org/r/297/#comment665>

    What exactly is an add and accumulate? Isn't that a little redundant? Since 
shift isn't broken out in the non-simd classes, it can probably fold into Alu, 
as can ShiftAcc. Do you think MultAcc is a genuinely different operation from 
Mult? Generally I like the idea of new Simd op classes, but I think you might 
have been overly specific.


- Gabe


On 2010-11-08 15:36:44, Ali Saidi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/297/
> -----------------------------------------------------------
> 
> (Updated 2010-11-08 15:36:44)
> 
> 
> Review request for Default.
> 
> 
> Summary
> -------
> 
> ARM/CPU: Add op classes for SIMD type instructions and use them in ARM ISA 
> description.
> 
> 
> Diffs
> -----
> 
>   src/arch/arm/isa/insts/div.isa f61e079ad05e 
>   src/arch/arm/isa/insts/fp.isa f61e079ad05e 
>   src/arch/arm/isa/insts/mult.isa f61e079ad05e 
>   src/arch/arm/isa/insts/neon.isa f61e079ad05e 
>   src/cpu/FuncUnit.py f61e079ad05e 
>   src/cpu/o3/FUPool.py f61e079ad05e 
>   src/cpu/o3/FuncUnitConfig.py f61e079ad05e 
>   src/cpu/op_class.hh f61e079ad05e 
> 
> Diff: http://reviews.m5sim.org/r/297/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ali
> 
>

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

Reply via email to