Lets put it on reviewboard. The first comment is too long. The
information about the fix is fine, but it should go in a commit message,
not a comment in the code. We don't put attributions in comments.
storeCommitted and youngest_store use different style which is not
correct. I don't remember which is correct (member variable vs. local
variable?) The second comment isn't necessary. No one is going to know
what "the deadlock" is. Same with the third comment. In that block of
code, I don't know whether that's functionally correct. Someone that
knows O3 better will have to comment. Watch for lines that are too long
though. I'm on my parent's computer, and wordpad isn't being very
helpful in counting characters. There should be spaces between the
commas and "tid", even though O3 isn't very good about following that
rule generally. The last line of change in the diff shouldn't comment
out code, it should delete it, again assuming doing that is functionally
correct.
Do we have somebody who's taken ownership of O3 now that Kevin is off
doing other things? I've hacked on it before, but it's pretty intricate
and somebody would really need to study it to understand what's going on
and what all the implications of changes would be. We should at least
make sure all our regressions still pass, although that can be a fairly
crude check for O3.
Gabe
On 11/26/2010 8:51 PM, Ali Saidi wrote:
Anyone have comments on this? The comments would need some work to
meet the style guidelines, but otherwise.
Thanks,
Ali
On Oct 14, 2010, at 10:51 AM, Liang Wang wrote:
I found the deadlock during my development of modified o3 model based
on M5. The deadlock condition rarely happened so that M5's o3 could
work with all splash2 kernels. However, the deadlock did show up in
my modified version of o3 when running Cholesky. After reviewing M5's
o3, the deadlock may apply to original o3 as well.
I attach a diff file generated by "hg export" based on the m5-r7088.
I tried the fix with Choelsky, FFT, Radix, all using default
configurations in M5 release. Those three kernels work ed fine with
the fix.
Thanks.
Liang
/------------------
Wang, Liang
Dept. of Computer Science, SEAS
Univ. of Virginia, Charlottesville, VA/
On Wed, Oct 13, 2010 at 11:42 PM, Ali Saidi <[email protected]
<mailto:[email protected]>> wrote:
HI Liang,
Thank you for reporting the issue. It appears as though you've
also included a fix in the source file. Have you tried the fix?
Does it work? Are there any issues? Regression changes? Would you
provide us with a diff?
Thanks,
Ali
On Oct 11, 2010, at 8:56 PM, Liang Wang wrote:
> The problematic code segment is in src/cpu/o3/iew_impl.hh with
latest m5 (r7704).
>
> Details are presented in the source as comment at line started
form 1508.
> ------------------
> Wang, Liang
> Dept. of Computer Science, SEAS
> Univ. of Virginia, Charlottesville, VA
> <iew_impl.hh>_______________________________________________
> m5-users mailing list
> [email protected] <mailto:[email protected]>
> http://m5sim.org/cgi-bin/mailman/listinfo/m5-users
_______________________________________________
m5-users mailing list
[email protected] <mailto:[email protected]>
http://m5sim.org/cgi-bin/mailman/listinfo/m5-users
<deadlock.diff>_______________________________________________
m5-users mailing list
[email protected] <mailto:[email protected]>
http://m5sim.org/cgi-bin/mailman/listinfo/m5-users
_______________________________________________
m5-users mailing list
[email protected]
http://m5sim.org/cgi-bin/mailman/listinfo/m5-users
_______________________________________________
m5-users mailing list
[email protected]
http://m5sim.org/cgi-bin/mailman/listinfo/m5-users
