Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sink regression on devel: cannot compile the generated C code anymore #12722

Closed
mratsim opened this issue Nov 24, 2019 · 4 comments
Closed

Sink regression on devel: cannot compile the generated C code anymore #12722

mratsim opened this issue Nov 24, 2019 · 4 comments

Comments

@mratsim
Copy link
Collaborator

mratsim commented Nov 24, 2019

In the last 3 days there was a regression in devel on the codegen for sink proc:

DeepinScreenshot_select-area_20191124150943

I don't have a full self-contained reproducible example at the moment but the colontmpD_ and blitTmp variable are not declared before use.

proc nextTask*(childTask: bool): Task {.inline.} =

  profile(enq_deq_task):
    if childTask:
      result = myWorker().deque.popFirstIfChild(myTask())
    else:
      result = myWorker().deque.popFirst()

  # TODO: steal early

  shareWork()

  # Check if someone requested to steal from us
  var req: StealRequest
  while recv(req):
    # If we just popped a loop task, we may split it here
    # It makes dispatching tasks simpler
    if myWorker().deque.isEmpty() and result.isSplittable():
      if req.thiefID != myID():
        splitAndSend(result, req)
      else:
        forget(req)
    else:
      dispatchTasks(req)

C code (with focus on related paragraph)

static N_INLINE(tyObject_TaskcolonObjectType___1Z1XSKW5drk9cOAphS9cyhbg*, nextTask__UE7GE7k3y9crMoWUP5T9boBQscheduler)(NIM_BOOL childTask) {	

       ///////////////////////////////////////
       ...
       ///////////////////////////////////////

	req = (tyObject_StealRequestcolonObjectType___LTgtnfBhL5TdUm2ePZ4M6Q*)0;
	{
		nimln_(68, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
		while (1) {
			NIM_BOOL T8_;
			T8_ = (NIM_BOOL)0;
			T8_ = recv__rh6W1iOJIA0BqMW3EaQ72gvictims(&req);
			if (!T8_) goto LA7;
			nimln_(71, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
			{
				NIM_BOOL T11_;
				T11_ = (NIM_BOOL)0;
				T11_ = isEmpty__zTHBqKFOE8nGHllwGgDJrgprell_deques(localCtx__xr8jkofR2rSp69czCGO79bqQ.worker.deque);
				if (!(T11_)) goto LA12_;
				T11_ = isSplittable__N8IdLdi5T7pGxk1tzH003g(result);
				LA12_: ;
				if (!T11_) goto LA13_;
				nimln_(72, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
				{
					tyObject_StealRequestcolonObjectType___LTgtnfBhL5TdUm2ePZ4M6Q* blitTmp;
					if (!!(((*req).thiefID == localCtx__xr8jkofR2rSp69czCGO79bqQ.worker.ID))) goto LA17_;
					nimln_(73, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
					colontmpD_ = 0;
					colontmpD_ = req;
					blitTmp = colontmpD_;
					colontmpD_ = 0;
					splitAndSend__YufgLt6fiFyPy0arOhE8tA(result, blitTmp);
				}
				goto LA15_;
				LA17_: ;
				{
					tyObject_StealRequestcolonObjectType___LTgtnfBhL5TdUm2ePZ4M6Q* blitTmp_2;
					nimln_(75, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
					colontmpD__2 = 0;
					colontmpD__2 = req;
					blitTmp_2 = colontmpD__2;
					colontmpD__2 = 0;
					forget__FoJB8hpGPM8HP8Am6rgcTQ_2(blitTmp_2);
				}
				LA15_: ;
			}
			goto LA9_;
			LA13_: ;
			{
				tyObject_StealRequestcolonObjectType___LTgtnfBhL5TdUm2ePZ4M6Q* blitTmp_3;
				nimln_(77, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
				colontmpD__3 = 0;
				colontmpD__3 = req;
				blitTmp_3 = colontmpD__3;
				colontmpD__3 = 0;
				dispatchTasks__bgTUscn2shIVJXNChVXG9bg(blitTmp_3);
			}
			LA9_: ;
		} LA7: ;
	}
	popFrame();
	return result;
}

Commit 1b2c1bc from Thursday was working fine and generated the following instead

static N_INLINE(tyObject_TaskcolonObjectType___1Z1XSKW5drk9cOAphS9cyhbg*, nextTask__UE7GE7k3y9crMoWUP5T9boBQscheduler)(NIM_BOOL childTask) {	

       ///////////////////////////////////////
       ...
       ///////////////////////////////////////

	req = (tyObject_StealRequestcolonObjectType___LTgtnfBhL5TdUm2ePZ4M6Q*)0;
	{
		nimln_(68, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
		while (1) {
			NIM_BOOL T8_;
			T8_ = (NIM_BOOL)0;
			T8_ = recv__rh6W1iOJIA0BqMW3EaQ72gvictims(&req);
			if (!T8_) goto LA7;
			nimln_(71, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
			{
				NIM_BOOL T11_;
				T11_ = (NIM_BOOL)0;
				T11_ = isEmpty__zTHBqKFOE8nGHllwGgDJrgprell_deques(localCtx__xr8jkofR2rSp69czCGO79bqQ.worker.deque);
				if (!(T11_)) goto LA12_;
				T11_ = isSplittable__N8IdLdi5T7pGxk1tzH003g(result);
				LA12_: ;
				if (!T11_) goto LA13_;
				nimln_(72, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
				{
					if (!!(((*req).thiefID == localCtx__xr8jkofR2rSp69czCGO79bqQ.worker.ID))) goto LA17_;
					nimln_(73, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
					splitAndSend__YufgLt6fiFyPy0arOhE8tA(result, req);
				}
				goto LA15_;
				LA17_: ;
				{
					nimln_(75, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
					forget__FoJB8hpGPM8HP8Am6rgcTQ_2(req);
				}
				LA15_: ;
			}
			goto LA9_;
			LA13_: ;
			{
				nimln_(77, "/home/beta/Programming/Nim/weave/weave/scheduler.nim");
				dispatchTasks__bgTUscn2shIVJXNChVXG9bg(req);
			}
			LA9_: ;
		} LA7: ;
	}
	popFrame();
	return result;
}

Source: https://github.com/mratsim/weave/blob/e3a1f2944a1bf3bdd25155a4c220b7aabdf38a93/weave/scheduler.nim#L54-L77

Repro:

git clone https://github.com/mratsim/weave
cd weave
mkdir build
# in weave/async.nim at the bottom comment out main2() and comment in main()
# unless your PC is fast you want to heat your home by computing fib(40) the naive way in debug mode
nim c --threads:on -o:build/async weave/async.nim
@mratsim mratsim changed the title Sink regression: cannot compile anymore Sink regression: cannot compile the generated C code anymore Nov 24, 2019
mratsim added a commit to mratsim/weave that referenced this issue Nov 24, 2019
Rename to WV_CacheLinePadding

Unfortunately nim-lang/Nim#12722 broke Nim devel in the past 3 days, commit nim-lang/Nim@1b2c1bc is good.

Also C proc signatures changed to csize_t nim-lang/Nim#12497
@mratsim mratsim changed the title Sink regression: cannot compile the generated C code anymore Sink regression on devel: cannot compile the generated C code anymore Nov 24, 2019
@mratsim
Copy link
Collaborator Author

mratsim commented Nov 24, 2019

After bisecting, it seems like the culprit is this "bugfix" that now injects destructors ;) c85e266

@Araq
Copy link
Member

Araq commented Nov 25, 2019

Some snippet would really be nice in order to tackle this...

@mratsim
Copy link
Collaborator Author

mratsim commented Nov 29, 2019

Dropping it to lower priority, seemingly unrelated changes (no change in type, proc or even file) removed the issue in later commits in my repo.

It seems like tough bug to extract.

@mratsim
Copy link
Collaborator Author

mratsim commented Nov 29, 2019

Apparently removing inline from the proc that have issues worked so maybe related to #12770

@mratsim mratsim mentioned this issue Nov 29, 2019
@Araq Araq closed this as completed Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants