On 04/22/12 20:21, Steve Reinhardt wrote: > On Sun, Apr 22, 2012 at 7:24 PM, Nilay Vaish <[email protected]> wrote: > >> Also, totally outside the scope of this patch, but as a longer-term goal it >> would be nice to take these really big hunks of python and just put them in >> .py files and import them indirectly... >> >> Conversely, it would be nice if the microcode files that are .py files but >> which each exist solely to define a single really long string constant were >> redone as .isa files or something similar. >> >> Sorry for the big tangent, but this just reminded me of those thoughts. >> >> I don't get your metric for deciding between what should be in .isa files, >> and what goes into .py files. >> >> (Switching to email since this is unrelated to the patch under review.) > It's mostly subjective, but basically it's more convenient and intuitive if > most of the code in a particular language (or equivalently that's input for > a particular tool) is in a file whose extension indicates what language > it's in. > > Originally "let" was just kind of a hack to allow declaration of some > common python helper functions in the context of the isa parser, but it > really wasn't intended to be a gateway to enabling inclusion of huge > amounts of python. For example, in the entire Alpha ISA description, there > are just two uses of "let", both to define functions that factor common > code out of multiple isa-description-language "def format" blocks, and the > longer of the two is still less than 50 lines long. All of the other > pure-python heavy lifting for Alpha is done in isa_parser.py itself, > basically. To a large part this worked out because the Alpha ISA > description was co-designed with the language itself, so if it ever looked > like the language wasn't powerful enough, I just added more features to the > language. The fact that decoding Alpha instructions is relatively simple > (almost trivial compared to x86) helped a lot too. > > So to my eyes, it's an abuse of the original intent to have a let block > with ~1300 lines of python in it, particularly when it's in a .isa file > that's only ~1500 lines long, so 86% of this .isa file is actually pure > python code. The microop files are even worse, where you have a .py file > that (excluding the license comment at the top and the lines with the > triple quotes to make it a string) is 100% input for the micro assembler. > > Of course, decoding x86 at all is pretty much an abuse of the original > intent of the isa parser ;-), and I'm impressed and pleased that Gabe got > it to work anyway, so I'm not complaining. At some point it would be nice > to go back and fix up the isa parser to provide more first-class support > for what x86 needs, or (as I know Gabe has suggested, if I understood him > properly) get rid of it entirely and redo it as a pure python library. > > In the meantime though, I just think it would be cleaner and more intuitive > if more of the code was kept in files that reflect the code's actual > format. As I said, it's really a more long-term goal, it's just that > looking at regops.isa reminded me of it. > > Steve > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev
To throw in my two cents, I basically agree. I try to move C++ code out of the parser into normal C++ files so people that know C++ but not python or the ISA description language can at least figure out that part, and it's less obscured by the code gluing it into the description. Base instruction classes are an example of code I've separated like that. I think some of the python stuff has been separated out (the micro assembler at least) but there are two problems I ran into/can remember. First, scons can't figure out what python files matter as far as rebuilding things. If it doesn't know that foo.py is imported by bar.py is imported by main.isa's let block, changes to foo.py will not cause things to be rebuilt without a lot of arm twisting. Because the isa file include mechanism is more explicit and easier for scons to follow, it works better in that sense. Also I think there were problems with the python search path. As you mention, Steve, I'd ultimately like to turn the whole thing inside out so that the ISA description stuff is embedded in normal python code, probably as a python library. Gabe _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
