> 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?

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.


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3671/#review8885
-----------------------------------------------------------


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to