src/arch/trips/SConscript (Diff revision 1)

Your (either you personally or your institution) copyright should be added to the files.

In the cases where the file is completely re-written, it's unlikely that the Michigan copyright should remain.

src/arch/trips/constants.hh (Diff revision 1)

You should add yourself as the author of this file. 

src/arch/trips/faults.hh (Diff revision 1)

If you're only made a minor change, or search and replace on a file it's fine to leave the original authors, however if you have made significant changes you should add your name to the Authors list. It's treated as a list of people who know what the file should do and are responsible for it. In a case where you have almost completely re-written the file, it's also acceptable to remove the authors. 

src/arch/trips/faults.cc (Diff revision 1)

Does TRIPS actually have a vector number (e.g. in full system), or is this just an artifact of starting with SPARC or some other ISA? If not it should probably be removed.

src/arch/trips/isa.hh (Diff revision 1)

These comments would be better as Doxygen comments. e.g.
/** floating point condition codes */

Doxygen picks up the /** and generates the documentation at http://www.m5sim.org/docs from it.

src/arch/trips/isa.hh (Diff revision 1)

//Fix me is rarely helpful, it's much better to describe what is broken.

src/arch/trips/isa/base.isa (Diff revision 1)

the spaces around ( ) with the if isn't within the m5 stytle guidlines

src/arch/trips/isa_traits.hh (Diff revision 1)

Again doxygen comments would be prefered

src/arch/trips/isa_traits.hh (Diff revision 1)

These are all artifacts of Alpha and probably should just be removed.

src/arch/trips/isa_traits.hh (Diff revision 1)

Name with these

src/arch/trips/isa_traits.hh (Diff revision 1)

These comments are wrong

src/arch/trips/linux/process.cc (Diff revision 1)

It looks like these came from Alpha and were slightly changed. If there is an official list of syscall numbers (maybe in the linux port for TRIPS), you should use those instead. 

src/arch/trips/linux/threadinfo.hh (Diff revision 1)

This is also full system alpha cruft

src/arch/trips/locked_mem.hh (Diff revision 1)

Does TRIPS have a locked memory access? If not, does this file need to exist?

src/arch/trips/pagetable.hh (Diff revision 1)

There is a lot of dead code in here from Alpha... it should really be removed.

src/arch/trips/pagetable.hh (Diff revision 1)

This is really the only needed code for an SE mode TLB

src/arch/trips/predecoder.hh (Diff revision 1)

This is more cruft from Alpha that should go away

src/arch/trips/stacktrace.cc (Diff revision 1)

This is all Alpha specific full system code that should be removed. 

src/arch/trips/vtophys.cc (Diff revision 1)

This il all Alpha cruft as well.

This is also a first pass review. I focused only on the ISA description files since Ali looked at the other ones. I'll want to look at those too in a later iteration. These files all need to be brought in line with the M5 style guidelines.

src/arch/trips/isa/base.isa (Diff revision 1)

Since this class definition doesn't really use any features of the isa parser, it would be better to move it out into regular C++. Things in the isa description inherently have one extra layer of stuff to figure out, making it harder to understand what's going on.

src/arch/trips/isa/decoder.isa (Diff revision 1)

The indentation in this file is off. Ali's comments about style apply here too.

src/arch/trips/isa/decoder.isa (Diff revision 1)

This line (and many others) is way too long. You get 80 characters, although Nate would only give you 79 (or 78? I forget).

src/arch/trips/isa/formats/b.isa (Diff revision 1)

The same as before, these hard coded class definitions should be outside the isa description.