> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > Hi Gabe,
> > 
> > Thanks for taking the time to making everything work with 4.6.1. I have a 
> > couple of questions about what you're changing in the ARM ISA. It wasn't 
> > immediately clear to me what you're doing or why. Have you tried 4.6 
> > support for link time optimization or whole program optimization? It would 
> > be nice to get a speedup with it.
> > 
> > 
> >

Nope, I haven't tried those optimizations. I don't have access to the machine 
with 4.6.1 on it right now, but I could give it a try maybe this weekend.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/alpha/isa/decoder.isa, line 791
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14831#file14831line791>
> >
> >     Does the parser still pick this up as a source if it's not in the 
> > expression?
> >

Yes. It's not that smart. Also, this way Rb is always used and the preprocessor 
won't chop out the use.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/arm/isa/formats/fp.isa, line 567
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14833#file14833line567>
> >
> >     Could you describe what you're doing here? I can't quite grok it from 
> > inspection. 
> >     
> >

I don't remember for sure (I did this patch over a long time), but I don't 
think VpminQ exists as an instruction. I think also the US version of this 
function changes the versions generated, but I don't remember how. The function 
names aren't great.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/arm/isa/formats/fp.isa, line 1547
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14833#file14833line1547>
> >
> >     Same here.

Ditto.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/arm/isa/insts/m5ops.isa, line 206
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14835#file14835line206>
> >
> >     You don't need to do this, but it would be nice if we added a 
> > PseudoInst::initParam() and called it just as every other one.

That's a good idea, but it should be in a separate change.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/arm/isa/insts/macromem.isa, line 566
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14836#file14836line566>
> >
> >     short description?

I don't remember for sure, but I think just so the size is available while 
iterating in the loops so only valid instructions are generated. Otherwise you 
get versions that 4.6.1 gets upset about because they overflow arrays. There 
isn't really much to say. It's a bit ugly, but there isn't really anything that 
interesting going on.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/arm/isa/insts/neon.isa, line 656
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14837#file14837line656>
> >
> >     same please

It's doing whatever it used to be doing. This is just simplifying the code with 
some basic refactoring.


> On 2011-09-06 10:00:39, Ali Saidi wrote:
> > src/arch/mips/isa/decoder.isa, line 570
> > <http://reviews.m5sim.org/r/843/diff/1/?file=14839#file14839line570>
> >
> >     What is  the point of this?

Oh, I thought I took this back out. This is to hide the operand name from the 
parser since it didn't use to ignore names in strings. It's not needed any 
more, but apparently I forgot to refresh my patch. I guess I won't be able to 
push this until I get home and get at the current version of the patch.


- Gabe


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


On 2011-09-05 22:34:59, Gabe Black wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/843/
> -----------------------------------------------------------
> 
> (Updated 2011-09-05 22:34:59)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> -------
> 
> GCC: Get everything working with gcc 4.6.1.
> 
> And by "everything" I mean all the quick regressions.
> 
> 
> Diffs
> -----
> 
>   src/SConscript 1f95c9a0bb2f 
>   src/arch/alpha/ev5.cc 1f95c9a0bb2f 
>   src/arch/alpha/isa/decoder.isa 1f95c9a0bb2f 
>   src/arch/alpha/isa/mem.isa 1f95c9a0bb2f 
>   src/arch/arm/isa/formats/fp.isa 1f95c9a0bb2f 
>   src/arch/arm/isa/insts/fp.isa 1f95c9a0bb2f 
>   src/arch/arm/isa/insts/m5ops.isa 1f95c9a0bb2f 
>   src/arch/arm/isa/insts/macromem.isa 1f95c9a0bb2f 
>   src/arch/arm/isa/insts/neon.isa 1f95c9a0bb2f 
>   src/arch/arm/isa/templates/mem.isa 1f95c9a0bb2f 
>   src/arch/mips/isa/decoder.isa 1f95c9a0bb2f 
>   src/arch/mips/isa/formats/control.isa 1f95c9a0bb2f 
>   src/arch/mips/isa/formats/mt.isa 1f95c9a0bb2f 
>   src/arch/mips/isa/includes.isa 1f95c9a0bb2f 
>   src/arch/mips/tlb.cc 1f95c9a0bb2f 
>   src/arch/power/isa/formats/mem.isa 1f95c9a0bb2f 
>   src/arch/power/tlb.cc 1f95c9a0bb2f 
>   src/arch/sparc/isa/formats/mem/util.isa 1f95c9a0bb2f 
>   src/arch/x86/isa/microops/base.isa 1f95c9a0bb2f 
>   src/base/inet.cc 1f95c9a0bb2f 
>   src/cpu/base.cc 1f95c9a0bb2f 
>   src/cpu/inorder/cpu.cc 1f95c9a0bb2f 
>   src/cpu/legiontrace.cc 1f95c9a0bb2f 
>   src/cpu/o3/cpu.cc 1f95c9a0bb2f 
>   src/cpu/o3/rename_impl.hh 1f95c9a0bb2f 
>   src/mem/cache/tags/iic.cc 1f95c9a0bb2f 
>   src/mem/ruby/network/orion/Clock.cc 1f95c9a0bb2f 
>   src/mem/ruby/system/PersistentTable.hh 1f95c9a0bb2f 
>   src/mem/ruby/system/PseudoLRUPolicy.hh 1f95c9a0bb2f 
>   src/python/m5/params.py 1f95c9a0bb2f 
>   src/python/swig/pyobject.cc 1f95c9a0bb2f 
>   src/sim/pseudo_inst.hh 1f95c9a0bb2f 
> 
> Diff: http://reviews.m5sim.org/r/843/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gabe
> 
>

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

Reply via email to