{"action":"created","comment":{"url":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments/68804406","id":68804406,"diff_hunk":"@@ -0,0 +1,57 @@\n+/*\n+ * Copyright 2004-2016 Cray Inc.\n+ * Other additional copyright holders may be indicated within.\n+ * \n+ * The entirety of this work is licensed under the Apache License,\n+ * Version 2.0 (the \"License\"); you may not use this file except\n+ * in compliance with the License.\n+ * \n+ * You may obtain a copy of the License at\n+ * \n+ *     http://www.apache.org/licenses/LICENSE-2.0\n+ * \n+ * Unless required by applicable law or agreed to in writing, software\n+ * distributed under the License is distributed on an \"AS IS\" BASIS,\n+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n+ * See the License for the specific language governing permissions and\n+ * limitations under the License.\n+ */\n+\n+#ifndef _chpl_align_h_\n+#define _chpl_align_h_\n+\n+#include <inttypes.h>\n+#include <string.h>\n+#include <assert.h>\n+#include <stdint.h>\n+#include <limits.h>\n+\n+// If we have a mask representing 2^n - 1,\n+// round an offset down to a multiple of 2^n.\n+static inline\n+uintptr_t round_down_to_mask(uintptr_t p, uintptr_t mask)\n+{\n+ return p - (p & mask);","path":"runtime/include/chpl-align.h","position":34,"original_position":34,"commit_id":"b80da99f4441ebea473b85e6df398771a74aa4d8","original_commit_id":"b80da99f4441ebea473b85e6df398771a74aa4d8","user":{"login":"gbtitus","id":7740880,"avatar_url":"https://avatars.githubusercontent.com/u/7740880?v=3","gravatar_id":"","url":"https://api.github.com/users/gbtitus","html_url":"https://github.com/gbtitus","followers_url":"https://api.github.com/users/gbtitus/followers","following_url":"https://api.github.com/users/gbtitus/following{/other_user}","gists_url":"https://api.github.com/users/gbtitus/gists{/gist_id}","starred_url":"https://api.github.com/users/gbtitus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gbtitus/subscriptions","organizations_url":"https://api.github.com/users/gbtitus/orgs","repos_url":"https://api.github.com/users/gbtitus/repos","events_url":"https://api.github.com/users/gbtitus/events{/privacy}","received_events_url":"https://api.github.com/users/gbtitus/received_events","type":"User","site_admin":false},"body":"one more space here","created_at":"2016-06-28T17:35:04Z","updated_at":"2016-06-28T17:35:04Z","html_url":"https://github.com/chapel-lang/chapel/pull/4074#discussion_r68804406","pull_request_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074","_links":{"self":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments/68804406"},"html":{"href":"https://github.com/chapel-lang/chapel/pull/4074#discussion_r68804406"},"pull_request":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074"}}},"pull_request":{"url":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074","id":75120155,"html_url":"https://github.com/chapel-lang/chapel/pull/4074","diff_url":"https://github.com/chapel-lang/chapel/pull/4074.diff","patch_url":"https://github.com/chapel-lang/chapel/pull/4074.patch","issue_url":"https://api.github.com/repos/chapel-lang/chapel/issues/4074","number":4074,"state":"open","locked":false,"title":"Stack allocate on and task function argument bundles","user":{"login":"mppf","id":3653132,"avatar_url":"https://avatars.githubusercontent.com/u/3653132?v=3","gravatar_id":"","url":"https://api.github.com/users/mppf","html_url":"https://github.com/mppf","followers_url":"https://api.github.com/users/mppf/followers","following_url":"https://api.github.com/users/mppf/following{/other_user}","gists_url":"https://api.github.com/users/mppf/gists{/gist_id}","starred_url":"https://api.github.com/users/mppf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mppf/subscriptions","organizations_url":"https://api.github.com/users/mppf/orgs","repos_url":"https://api.github.com/users/mppf/repos","events_url":"https://api.github.com/users/mppf/events{/privacy}","received_events_url":"https://api.github.com/users/mppf/received_events","type":"User","site_admin":false},"body":"We've been heap allocating argument bundles for task bodies and for on statements.\r\nThis may have once been necessary but it does not seem to be necessary any longer.\r\n\r\nFor on statements:\r\n * GASNet already has a model, for AMRequestMedium (which is what we eventually call to do an on statement), in which the passed buffer can be reused immediately after that GASNet function returns. Additionally, the buffer need not be in communication registered memory.\r\n * Generally, we are trying to get to task stacks being allocated from communication-registered memory anyway. Additionally, the communication layer is free to copy into a heap allocated area if necessary. Lastly, on \"on\" statement causes the arguments to be copied over the network, so it is a fiction that by heap allocating them we preserve any sense of identity.\r\n\r\nFor tasks:\r\n * tasks-qthreads.c already calls qthread_fork_copyargs and more generally qthread_spawn copies arguments to a thread (and heap allocates in some case if they are too big).\r\n * for FIFO, the tasking layer was already calling the allocator for each spawned task in order to create a new task pool entry. This allocation can easily be extended to include space for the argument bundle.\r\n * Any tasking layer that does not have the capability to copy some variable-length arguments to the task can heap-allocate and copy the arguments. While pthreads is not a Chapel tasking layer, it is a thread package that does not have this ability - arguments to pthreads are just a single void *. Nonetheless, it is much easier for the runtime to add this copy when it is necessary than it is to remove a copy that the compiler is already adding.\r\n\r\nOne issue with no longer heap-allocating the argument bundles is that the tasking and communication layer need to pass some additional information to the remote locale / executing task. For example, comm-gasnet.c needs to pass the requesting nodeID in order to handle execute_on_nb. Since the network communication will copy a single buffer, if the size of this comm-layer information was private, the comm layer would have to heap or stack allocate a new the bundle (that it includes the extra information) and copy in the old bundle's information. At the same time, since the Chapel comm layer interface is for the code generated by the compiler (vs a user-facing API), there is not much encapsulation benefit to making the size of the comm-layer information private in this way. Therefore this PR uses the alternative strategy, which is to allocate all argument bundles with a runtime header field already included. The runtime uses this space as needed in order to pass data (like the requesting nodeID). This extra field is included as the first field in the argument bundle class. The entire argument bundle is passed to on/task wrapper functions and this extra field is ignored at that point.\r\n\r\nBecause tasks creation and execute_on require different runtime headers, the generated code differentiates between these cases. Since execute_on and execute_on_nb are generally implemented by creating tasks, the execute_on runtime header includes the tasking runtime header as its first element. In this way, the task header from any argument bundle can be read by the runtime regardless of the type.\r\n\r\nHere is a tour through the changes in this PR:\r\n * Adds *PRIM_STACK_ALLOCATE_CLASS* primitive and adjusts codegenPrimitive to support it. This primitive will create stack space for a class instance. Since the Chapel compiler normally heap allocates class instances, the class instance type is a pointer type. I considered an alternative of storing argument bundles as classes containing a record, but that seemed to be too awkward a strategy. Also, this primitive might be useful in other cases. believe that converting heap allocations to stack allocations is an interesting optimization and this primitive might be able to help with that case.\r\n * Adjusts bundleArgs in the compiler to call PRIM_STACK_ALLOCATE_CLASS.\r\n * Adds an *args_size* argument to runtime task and execute_on interfaces and adjust codegenInvokeTaskFun and codegenInvokeOnFun accordingly. This args_size argument is critical to enabling the runtime to copy the argument bundle.\r\n * Changes the args argument to runtime task and execute_on functions to be the pointer type for the runtime header. The generated code puts this runtime header as the first element of the argument bundle struct and further arguments are in following structure members. There are actually two runtime headers: *chpl_comm_on_bundle_t* and *chpl_task_bundle_t*. Adjusts the compiler to become aware of these types and to store them as the first element of the argument bundles.\r\n * Adjusts comm-none.c, comm-gasnet.c, tasks-fifo.c, and tasks-qthreads.c to support the new interface and to be sure to copy the argument bundles before the relevant runtime call returns. Generally take care to minimize the number of allocations.\r\n\r\nMore about the format of the runtime headers:\r\n * chpl-comm.h declares  chpl_comm_on_bundle_t and chpl-tasks.h declares chpl_task_bundle_t. Each has a struct member (comm or task_prv respectively) that is defined by the communications or tasking layer, respectively. The comm layers define this in chpl-comm-task-decls.h. The tasking layers define this in tasks-fifo.h / tasks-qthreads.h etc.\r\n * I took the strategy of storing arguments that were common among the various tasking layers into the struct defined in chpl-tasks.h. The comm layer can use these fields too when they are the same (so e.g. serial_state is only stored once). Then, I added the fid argument to the chpl_comm_on_bundle_t since I think it's inherent to the way our execute_on support works.\r\n\r\nQuestions for reviewers:\r\n * in tasks-qthreads.c, wrap_callbacks will setup a task ID if it has not yet been set. It can do this for chpl_task_cb_event_kind_create. I did not store the task ID in the qthreads task runtime header, so if it is set before the task launches, there is currently no way to communicate that. Is it necessary to support this functionality? Is calling the callback after the task starts to run sufficient?\r\n * we could more radically change chpl-comm.h and chpl-tasks.h. In particular, many of the arguments to these functions (serial_state, requested function, line, file) could be stored in the argument bundle directly. I did not attempt to do so because I don't see a clear advantage but in some ways it would be simpler. Instead, the current implementation sets these elements of the argument bundle within the task/comm runtime layer call.\r\n * While I did my best with the names and contents of  chpl_comm_on_bundle_t and chpl_task_bundle_t, I appreciate a second opinion.\r\n * I moved chpl_task_taskCallFTable to chpl-comm.h since with the new argument types it is more convenient for it to be defined there. It's primary use is to run a local task (e.g. in executeOnNB when the destination locale is local). But, since now it is in chpl-comm.h, I wonder if it should have a different name. Likewise, I wonder if this (or other inline helper fns) belong in chpl-comm.h.\r\n\r\nInitial Performance Results:\r\n * miniMD with compopts 1 or explicit version are 1.2x faster; other versions show less benefit.\r\n * release lulesh ran 1.10x or 1.13x faster depending on configuration.\r\n\r\nEarlier version passed full fifo testing.\r\nEarlier version passed qthreads and fifo release/examples testing.\r\nPassed full local testing except for 2 tests in runtime/gbt/tasks/callbacks (see question above).\r\n","created_at":"2016-06-24T14:14:26Z","updated_at":"2016-06-28T17:35:04Z","closed_at":null,"merged_at":null,"merge_commit_sha":"bd1a7ff306bfda670075516c343ae43714b2fde3","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074/commits","review_comments_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074/comments","review_comment_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments{/number}","comments_url":"https://api.github.com/repos/chapel-lang/chapel/issues/4074/comments","statuses_url":"https://api.github.com/repos/chapel-lang/chapel/statuses/b80da99f4441ebea473b85e6df398771a74aa4d8","head":{"label":"mppf:no-heap-alloc-bundles","ref":"no-heap-alloc-bundles","sha":"b80da99f4441ebea473b85e6df398771a74aa4d8","user":{"login":"mppf","id":3653132,"avatar_url":"https://avatars.githubusercontent.com/u/3653132?v=3","gravatar_id":"","url":"https://api.github.com/users/mppf","html_url":"https://github.com/mppf","followers_url":"https://api.github.com/users/mppf/followers","following_url":"https://api.github.com/users/mppf/following{/other_user}","gists_url":"https://api.github.com/users/mppf/gists{/gist_id}","starred_url":"https://api.github.com/users/mppf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mppf/subscriptions","organizations_url":"https://api.github.com/users/mppf/orgs","repos_url":"https://api.github.com/users/mppf/repos","events_url":"https://api.github.com/users/mppf/events{/privacy}","received_events_url":"https://api.github.com/users/mppf/received_events","type":"User","site_admin":false},"repo":{"id":22106313,"name":"chapel","full_name":"mppf/chapel","owner":{"login":"mppf","id":3653132,"avatar_url":"https://avatars.githubusercontent.com/u/3653132?v=3","gravatar_id":"","url":"https://api.github.com/users/mppf","html_url":"https://github.com/mppf","followers_url":"https://api.github.com/users/mppf/followers","following_url":"https://api.github.com/users/mppf/following{/other_user}","gists_url":"https://api.github.com/users/mppf/gists{/gist_id}","starred_url":"https://api.github.com/users/mppf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mppf/subscriptions","organizations_url":"https://api.github.com/users/mppf/orgs","repos_url":"https://api.github.com/users/mppf/repos","events_url":"https://api.github.com/users/mppf/events{/privacy}","received_events_url":"https://api.github.com/users/mppf/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/mppf/chapel","description":"a Productive Parallel Programming Language","fork":true,"url":"https://api.github.com/repos/mppf/chapel","forks_url":"https://api.github.com/repos/mppf/chapel/forks","keys_url":"https://api.github.com/repos/mppf/chapel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mppf/chapel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mppf/chapel/teams","hooks_url":"https://api.github.com/repos/mppf/chapel/hooks","issue_events_url":"https://api.github.com/repos/mppf/chapel/issues/events{/number}","events_url":"https://api.github.com/repos/mppf/chapel/events","assignees_url":"https://api.github.com/repos/mppf/chapel/assignees{/user}","branches_url":"https://api.github.com/repos/mppf/chapel/branches{/branch}","tags_url":"https://api.github.com/repos/mppf/chapel/tags","blobs_url":"https://api.github.com/repos/mppf/chapel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mppf/chapel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mppf/chapel/git/refs{/sha}","trees_url":"https://api.github.com/repos/mppf/chapel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mppf/chapel/statuses/{sha}","languages_url":"https://api.github.com/repos/mppf/chapel/languages","stargazers_url":"https://api.github.com/repos/mppf/chapel/stargazers","contributors_url":"https://api.github.com/repos/mppf/chapel/contributors","subscribers_url":"https://api.github.com/repos/mppf/chapel/subscribers","subscription_url":"https://api.github.com/repos/mppf/chapel/subscription","commits_url":"https://api.github.com/repos/mppf/chapel/commits{/sha}","git_commits_url":"https://api.github.com/repos/mppf/chapel/git/commits{/sha}","comments_url":"https://api.github.com/repos/mppf/chapel/comments{/number}","issue_comment_url":"https://api.github.com/repos/mppf/chapel/issues/comments{/number}","contents_url":"https://api.github.com/repos/mppf/chapel/contents/{+path}","compare_url":"https://api.github.com/repos/mppf/chapel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mppf/chapel/merges","archive_url":"https://api.github.com/repos/mppf/chapel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mppf/chapel/downloads","issues_url":"https://api.github.com/repos/mppf/chapel/issues{/number}","pulls_url":"https://api.github.com/repos/mppf/chapel/pulls{/number}","milestones_url":"https://api.github.com/repos/mppf/chapel/milestones{/number}","notifications_url":"https://api.github.com/repos/mppf/chapel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mppf/chapel/labels{/name}","releases_url":"https://api.github.com/repos/mppf/chapel/releases{/id}","deployments_url":"https://api.github.com/repos/mppf/chapel/deployments","created_at":"2014-07-22T14:05:45Z","updated_at":"2016-01-11T15:01:10Z","pushed_at":"2016-06-28T16:43:50Z","git_url":"git://github.com/mppf/chapel.git","ssh_url":"git@github.com:mppf/chapel.git","clone_url":"https://github.com/mppf/chapel.git","svn_url":"https://github.com/mppf/chapel","homepage":"http://chapel.cray.com/","size":450357,"stargazers_count":1,"watchers_count":1,"language":"Chapel","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"chapel-lang:master","ref":"master","sha":"3a9a8b53bab735f0f358242e72545e385be6638f","user":{"login":"chapel-lang","id":7597261,"avatar_url":"https://avatars.githubusercontent.com/u/7597261?v=3","gravatar_id":"","url":"https://api.github.com/users/chapel-lang","html_url":"https://github.com/chapel-lang","followers_url":"https://api.github.com/users/chapel-lang/followers","following_url":"https://api.github.com/users/chapel-lang/following{/other_user}","gists_url":"https://api.github.com/users/chapel-lang/gists{/gist_id}","starred_url":"https://api.github.com/users/chapel-lang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chapel-lang/subscriptions","organizations_url":"https://api.github.com/users/chapel-lang/orgs","repos_url":"https://api.github.com/users/chapel-lang/repos","events_url":"https://api.github.com/users/chapel-lang/events{/privacy}","received_events_url":"https://api.github.com/users/chapel-lang/received_events","type":"Organization","site_admin":false},"repo":{"id":21663285,"name":"chapel","full_name":"chapel-lang/chapel","owner":{"login":"chapel-lang","id":7597261,"avatar_url":"https://avatars.githubusercontent.com/u/7597261?v=3","gravatar_id":"","url":"https://api.github.com/users/chapel-lang","html_url":"https://github.com/chapel-lang","followers_url":"https://api.github.com/users/chapel-lang/followers","following_url":"https://api.github.com/users/chapel-lang/following{/other_user}","gists_url":"https://api.github.com/users/chapel-lang/gists{/gist_id}","starred_url":"https://api.github.com/users/chapel-lang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chapel-lang/subscriptions","organizations_url":"https://api.github.com/users/chapel-lang/orgs","repos_url":"https://api.github.com/users/chapel-lang/repos","events_url":"https://api.github.com/users/chapel-lang/events{/privacy}","received_events_url":"https://api.github.com/users/chapel-lang/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/chapel-lang/chapel","description":"a Productive Parallel Programming Language","fork":false,"url":"https://api.github.com/repos/chapel-lang/chapel","forks_url":"https://api.github.com/repos/chapel-lang/chapel/forks","keys_url":"https://api.github.com/repos/chapel-lang/chapel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/chapel-lang/chapel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/chapel-lang/chapel/teams","hooks_url":"https://api.github.com/repos/chapel-lang/chapel/hooks","issue_events_url":"https://api.github.com/repos/chapel-lang/chapel/issues/events{/number}","events_url":"https://api.github.com/repos/chapel-lang/chapel/events","assignees_url":"https://api.github.com/repos/chapel-lang/chapel/assignees{/user}","branches_url":"https://api.github.com/repos/chapel-lang/chapel/branches{/branch}","tags_url":"https://api.github.com/repos/chapel-lang/chapel/tags","blobs_url":"https://api.github.com/repos/chapel-lang/chapel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/chapel-lang/chapel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/chapel-lang/chapel/git/refs{/sha}","trees_url":"https://api.github.com/repos/chapel-lang/chapel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/chapel-lang/chapel/statuses/{sha}","languages_url":"https://api.github.com/repos/chapel-lang/chapel/languages","stargazers_url":"https://api.github.com/repos/chapel-lang/chapel/stargazers","contributors_url":"https://api.github.com/repos/chapel-lang/chapel/contributors","subscribers_url":"https://api.github.com/repos/chapel-lang/chapel/subscribers","subscription_url":"https://api.github.com/repos/chapel-lang/chapel/subscription","commits_url":"https://api.github.com/repos/chapel-lang/chapel/commits{/sha}","git_commits_url":"https://api.github.com/repos/chapel-lang/chapel/git/commits{/sha}","comments_url":"https://api.github.com/repos/chapel-lang/chapel/comments{/number}","issue_comment_url":"https://api.github.com/repos/chapel-lang/chapel/issues/comments{/number}","contents_url":"https://api.github.com/repos/chapel-lang/chapel/contents/{+path}","compare_url":"https://api.github.com/repos/chapel-lang/chapel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/chapel-lang/chapel/merges","archive_url":"https://api.github.com/repos/chapel-lang/chapel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/chapel-lang/chapel/downloads","issues_url":"https://api.github.com/repos/chapel-lang/chapel/issues{/number}","pulls_url":"https://api.github.com/repos/chapel-lang/chapel/pulls{/number}","milestones_url":"https://api.github.com/repos/chapel-lang/chapel/milestones{/number}","notifications_url":"https://api.github.com/repos/chapel-lang/chapel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/chapel-lang/chapel/labels{/name}","releases_url":"https://api.github.com/repos/chapel-lang/chapel/releases{/id}","deployments_url":"https://api.github.com/repos/chapel-lang/chapel/deployments","created_at":"2014-07-09T18:15:54Z","updated_at":"2016-06-28T16:22:26Z","pushed_at":"2016-06-28T17:27:03Z","git_url":"git://github.com/chapel-lang/chapel.git","ssh_url":"git@github.com:chapel-lang/chapel.git","clone_url":"https://github.com/chapel-lang/chapel.git","svn_url":"https://github.com/chapel-lang/chapel","homepage":"http://chapel.cray.com/","size":475617,"stargazers_count":283,"watchers_count":283,"language":"Chapel","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":104,"mirror_url":null,"open_issues_count":13,"forks":104,"open_issues":13,"watchers":283,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074"},"html":{"href":"https://github.com/chapel-lang/chapel/pull/4074"},"issue":{"href":"https://api.github.com/repos/chapel-lang/chapel/issues/4074"},"comments":{"href":"https://api.github.com/repos/chapel-lang/chapel/issues/4074/comments"},"review_comments":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074/comments"},"review_comment":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/4074/commits"},"statuses":{"href":"https://api.github.com/repos/chapel-lang/chapel/statuses/b80da99f4441ebea473b85e6df398771a74aa4d8"}}}}
{"actor":{"display_login":"gbtitus"}}