> On Oct. 17, 2016, 10:25 p.m., Jason Lowe-Power wrote:
> > src/sim/process.cc, line 593
> > <http://reviews.gem5.org/r/3671/diff/1/?file=61630#file61630line593>
> >
> >     Any reason not to move this code into ProcessParams::create(), and 
> > eliminate this static function?
> Brandon Potter wrote:
>     Jason, I'm not sure what you want me to do here.
>     I put some gdb breaks on the create function to see how this is all 
> invoked. I get a backtrace where _wrap_ProcessParams::create 
> (build/X86/python/m5/internal/param_Process_wrap.cc\:5201) calls the static 
> function. It looks like Python is wrapped around the C++ create function and 
> is calling it.
>     The C++ method is static because the Process object (which ends up being 
> a derived type object who's type depends on the ISA) has not been created 
> yet; you can't call methods on an object that aren't statically declared. So, 
> the static function in the base class acts as a generator for the derived 
> Process class. Also, the ProcessParams must have already been initialized 
> because they're used within the Process::create function (so any new code 
> that was added would have to be put into the end of ProcessParams 
> initialization to make these variables available).
>     Regarding moving it into the Python code for params, it may be possible, 
> but I'm not sure how to do it. The macro "THE_ISA" is used to help figure out 
> what type of derived Process class to return and that's obvious in the code 
> how it's done in C++. I don't know of any examples for Python or how to go 
> about doing it.
>     Is there a particular reason that you want to remove the static function. 
> Have you run into problems with similar code in the past?
> Jason Lowe-Power wrote:
>     I certainly could be missing something here, but it seems like you could 
> take all of the code in `Process::create(ProcessParams * params)` and put it 
> in the body of `ProcessParams::create()`. You would need to replace the 
> `params` variable with `this`, but other than that, I don't see any changes. 
> Am I missing something?
>     It wasn't this that before, which I assumed was because LiveProcess was 
> one of multiple types of Process classes, but there could have been a good 
> reason. I haven't run into any problems with this code before. I was just 
> suggesting a little more housekeeping while you were here.

Really, I'd rather not alter how this is currently set up. Have a hard time 
seeing how I'd implement this and I don't believe that this will be a simple 
copy-paste and make alterations job. It will probably involve sleuthing around 
the Python code while straddling the SWIG boundary. I prefer to stay away from 
that __black magic__ when possible.

Regarding housekeeping, I don't know why it's better to move this from C++ into 
Python. The same code needs to exist in one form or another in either C++ or 

If you feel strongly about it, I'd be happy to review an implementation of it. 
However, I have no desire to actually go and do it myself.

- Brandon

This is an automatically generated e-mail. To reply, visit:

On Oct. 17, 2016, 3:17 p.m., Brandon Potter wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3671/
> -----------------------------------------------------------
> (Updated Oct. 17, 2016, 3:17 p.m.)
> Review request for Default.
> Repository: gem5
> Description
> -------
> Changeset 11693:05fc28ce62a5
> ---------------------------
> syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead
> The EIOProcess class was removed recently and it was the only other class
> which derived from Process. Since every Process invocation is also a
> LiveProcess invocation, it makes sense to simplify the who organization
> by combining the fields from LiveProcess into Process.
> Diffs
> -----
>   src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/splash2/run.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/splash2/cluster.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/learning_gem5/part1/two_level.py 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/learning_gem5/part1/simple.py 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/example/se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/example/apu_se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/common/cpu2000.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_desc.cc PRE-CREATION 
>   src/sim/syscall_desc.hh PRE-CREATION 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/emul_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/operatingsystem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/operatingsystem.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/freebsd/freebsd.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/gpu-compute/cl_driver.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/gpu-compute/cl_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> Diff: http://reviews.gem5.org/r/3671/diff/
> Testing
> -------
> Thanks,
> Brandon Potter

gem5-dev mailing list

Reply via email to