GitHub PullRequestReviewCommentEvent at 2014-10-28 15:44:58
Repository: Halide
Actor: dsharletg
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/halide/Halide/pulls/comments/19479739","id":19479739,"diff_hunk":"@@ -0,0 +1,44 @@\n+#include \"Deinterleave.h\"\n+#include \"IRMutator.h\"\n+\n+namespace Halide {\n+namespace Internal {\n+\n+class BlockFlattener : public IRMutator {\n+private:\n+\n+    using IRMutator::visit;\n+\n+    void visit(const Block *op) {\n+        /* First we dig into the block traversing down the 'first'\n+         * stmt until we find one that is not a block. We push all of\n+         * the rest stmt's into the 'rest' stmt of the top-level\n+         * block, and then fix up the 'rest' stmt recursively at the\n+         * end. The result of this mutation is an equivalent Block\n+         * node that does not contain any Block nodes in a 'first' stmt.\n+         */\n+        Stmt first = op->first;\n+        Stmt rest  = op->rest;\n+        while(const Block *first_block = op->first.as<Block>()) {","path":"src/BlockFlattening.cpp","position":null,"original_position":22,"commit_id":"ccf0c4f56b55fb0188f64cc4a1419775f813fbaf","original_commit_id":"0506ab6921df5503d03f4828b2e5e0f1ff25e388","user":{"login":"dsharletg","id":7494796,"avatar_url":"https://avatars.githubusercontent.com/u/7494796?v=2","gravatar_id":"","url":"https://api.github.com/users/dsharletg","html_url":"https://github.com/dsharletg","followers_url":"https://api.github.com/users/dsharletg/followers","following_url":"https://api.github.com/users/dsharletg/following{/other_user}","gists_url":"https://api.github.com/users/dsharletg/gists{/gist_id}","starred_url":"https://api.github.com/users/dsharletg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dsharletg/subscriptions","organizations_url":"https://api.github.com/users/dsharletg/orgs","repos_url":"https://api.github.com/users/dsharletg/repos","events_url":"https://api.github.com/users/dsharletg/events{/privacy}","received_events_url":"https://api.github.com/users/dsharletg/received_events","type":"User","site_admin":false},"body":"Playing with this test in the debugger, I'm guessing it is simply a stack\r\nsmashing happening somewhere. Are you able to reproduce this?\r\n\r\nOn Tue Oct 28 2014 at 8:35:23 AM Dillon Sharlet <dsharlet@google.com> wrote:\r\n\r\n> I'm now seeing **intermittent** failures of test_interleave. It does pass\r\n> sometimes (very strange!). Here are a few of the outputs I've seen:\r\n>\r\n> cd tmp ; DYLD_LIBRARY_PATH=:../bin LD_LIBRARY_PATH=:../bin\r\n> ../bin/test_interleave\r\n> Internal error at src/CodeGen.cpp:537 triggered by user code at\r\n> test/correctness/interleave.cpp:181:\r\n> Symbol not found: outp\r\n> bash: line 1: 52480 Abort trap: 6 DYLD_LIBRARY_PATH=:../bin\r\n> LD_LIBRARY_PATH=:../bin ../bin/test_interleave\r\n> make: *** [test_interleave] Error 134\r\n>\r\n> cd tmp ; DYLD_LIBRARY_PATH=:../bin LD_LIBRARY_PATH=:../bin\r\n> ../bin/test_interleave\r\n> bash: line 1: 52621 Segmentation fault: 11 DYLD_LIBRARY_PATH=:../bin\r\n> LD_LIBRARY_PATH=:../bin ../bin/test_interleave\r\n> make: *** [test_interleave] Error 139\r\n>\r\n>\r\n> Value still in symbol table! Type = 'bash: line 1: 52292 Segmentation\r\n> fault: 11 DYLD_LIBRARY_PATH=:../bin LD_LIBRARY_PATH=:../bin\r\n> ../bin/test_interleave\r\n> make: *** [test_interleave] Error 139\r\n>\r\n>\r\n> So there are at least 3 distinct failure modes in addition to succeeding!\r\n>\r\n> On Mon Oct 27 2014 at 10:40:35 PM Kree Cole-McLaughlin <\r\n> notifications@github.com> wrote:\r\n>\r\n>> In src/BlockFlattening.cpp:\r\n>>\r\n>> > +class BlockFlattener : public IRMutator {\r\n>> > +private:\r\n>> > +\r\n>> > +    using IRMutator::visit;\r\n>> > +\r\n>> > +    void visit(const Block *op) {\r\n>> > +        /* First we dig into the block traversing down the 'first'\r\n>> > +         * stmt until we find one that is not a block. We push all of\r\n>> > +         * the rest stmt's into the 'rest' stmt of the top-level\r\n>> > +         * block, and then fix up the 'rest' stmt recursively at the\r\n>> > +         * end. The result of this mutation is an equivalent Block\r\n>> > +         * node that does not contain any Block nodes in a 'first' stmt.\r\n>> > +         */\r\n>> > +        Stmt first = op->first;\r\n>> > +        Stmt rest  = op->rest;\r\n>> > +        while(const Block *first_block = op->first.as<Block>()) {\r\n>>\r\n>> This should be fixed now with the latest push. All tests passed for me.\r\n>>\r\n>> —\r\n>> Reply to this email directly or view it on GitHub\r\n>> <https://github.com/halide/Halide/pull/510/files#r19454071>.\r\n>>\r\n>","created_at":"2014-10-28T15:44:58Z","updated_at":"2014-10-28T15:44:58Z","html_url":"https://github.com/halide/Halide/pull/510#discussion_r19479739","pull_request_url":"https://api.github.com/repos/halide/Halide/pulls/510","_links":{"self":{"href":"https://api.github.com/repos/halide/Halide/pulls/comments/19479739"},"html":{"href":"https://github.com/halide/Halide/pull/510#discussion_r19479739"},"pull_request":{"href":"https://api.github.com/repos/halide/Halide/pulls/510"}}},"pull_request":{"url":"https://api.github.com/repos/halide/Halide/pulls/510","id":22669872,"html_url":"https://github.com/halide/Halide/pull/510","diff_url":"https://github.com/halide/Halide/pull/510.diff","patch_url":"https://github.com/halide/Halide/pull/510.patch","issue_url":"https://api.github.com/repos/halide/Halide/issues/510","number":510,"state":"open","locked":false,"title":"Added a case to deinterleave that works when transposing fixed sized blo...","user":{"login":"kree-colemcalughlin","id":8183542,"avatar_url":"https://avatars.githubusercontent.com/u/8183542?v=2","gravatar_id":"","url":"https://api.github.com/users/kree-colemcalughlin","html_url":"https://github.com/kree-colemcalughlin","followers_url":"https://api.github.com/users/kree-colemcalughlin/followers","following_url":"https://api.github.com/users/kree-colemcalughlin/following{/other_user}","gists_url":"https://api.github.com/users/kree-colemcalughlin/gists{/gist_id}","starred_url":"https://api.github.com/users/kree-colemcalughlin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kree-colemcalughlin/subscriptions","organizations_url":"https://api.github.com/users/kree-colemcalughlin/orgs","repos_url":"https://api.github.com/users/kree-colemcalughlin/repos","events_url":"https://api.github.com/users/kree-colemcalughlin/events{/privacy}","received_events_url":"https://api.github.com/users/kree-colemcalughlin/received_events","type":"User","site_admin":false},"body":"Added a case to deinterleave that works when transposing fixed sized blocks of data. In this case we can safely interleave if the collected stores have unit stride, and each store is simply a load with a stride equal to the vector width. Adding this case makes this optimization a little easier to trigger when doing a simple transpose.\r\n\r\nThe last test in test/correctness/interleave.cpp demonstrates this. Prior to this change trans1 would not trigger the call to interleave_vector, whereas it is probably the more likely code that people would want to write. This change essentially allows us to interleave blocks of stores of the form:\r\n```\r\n    trans1[ramp(0, 1, 8)] = square[ramp(0, 8, 8)]\r\n    trans1[ramp(8, 1, 8)] = square[ramp(1, 8, 8)]\r\n    trans1[ramp(16, 1, 8)] = square[ramp(2, 8, 8)]\r\n    trans1[ramp(24, 1, 8)] = square[ramp(3, 8, 8)]\r\n    trans1[ramp(32, 1, 8)] = square[ramp(4, 8, 8)]\r\n    trans1[ramp(40, 1, 8)] = square[ramp(5, 8, 8)]\r\n    trans1[ramp(48, 1, 8)] = square[ramp(6, 8, 8)]\r\n    trans1[ramp(56, 1, 8)] = square[ramp(7, 8, 8)]\r\n```\r\nwhereas before we were only interleaving:\r\n```\r\n    trans2[ramp(0, 8, 8)] = square[ramp(0, 1, 8)]\r\n    trans2[ramp(1, 8, 8)] = square[ramp(8, 1, 8)]\r\n    trans2[ramp(2, 8, 8)] = square[ramp(16, 1, 8)]\r\n    trans2[ramp(3, 8, 8)] = square[ramp(24, 1, 8)]\r\n    trans2[ramp(4, 8, 8)] = square[ramp(32, 1, 8)]\r\n    trans2[ramp(5, 8, 8)] = square[ramp(40, 1, 8)]\r\n    trans2[ramp(6, 8, 8)] = square[ramp(48, 1, 8)]\r\n    trans2[ramp(7, 8, 8)] = square[ramp(56, 1, 8)]\r\n```","created_at":"2014-10-13T20:25:04Z","updated_at":"2014-10-28T15:44:58Z","closed_at":null,"merged_at":null,"merge_commit_sha":"cbe08ee13509089e915b7fe7e9534e0799902e9c","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/halide/Halide/pulls/510/commits","review_comments_url":"https://api.github.com/repos/halide/Halide/pulls/510/comments","review_comment_url":"https://api.github.com/repos/halide/Halide/pulls/comments/{number}","comments_url":"https://api.github.com/repos/halide/Halide/issues/510/comments","statuses_url":"https://api.github.com/repos/halide/Halide/statuses/ccf0c4f56b55fb0188f64cc4a1419775f813fbaf","head":{"label":"halide:symmetric_interleave","ref":"symmetric_interleave","sha":"ccf0c4f56b55fb0188f64cc4a1419775f813fbaf","user":{"login":"halide","id":1548551,"avatar_url":"https://avatars.githubusercontent.com/u/1548551?v=2","gravatar_id":"","url":"https://api.github.com/users/halide","html_url":"https://github.com/halide","followers_url":"https://api.github.com/users/halide/followers","following_url":"https://api.github.com/users/halide/following{/other_user}","gists_url":"https://api.github.com/users/halide/gists{/gist_id}","starred_url":"https://api.github.com/users/halide/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/halide/subscriptions","organizations_url":"https://api.github.com/users/halide/orgs","repos_url":"https://api.github.com/users/halide/repos","events_url":"https://api.github.com/users/halide/events{/privacy}","received_events_url":"https://api.github.com/users/halide/received_events","type":"Organization","site_admin":false},"repo":{"id":5241312,"name":"Halide","full_name":"halide/Halide","owner":{"login":"halide","id":1548551,"avatar_url":"https://avatars.githubusercontent.com/u/1548551?v=2","gravatar_id":"","url":"https://api.github.com/users/halide","html_url":"https://github.com/halide","followers_url":"https://api.github.com/users/halide/followers","following_url":"https://api.github.com/users/halide/following{/other_user}","gists_url":"https://api.github.com/users/halide/gists{/gist_id}","starred_url":"https://api.github.com/users/halide/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/halide/subscriptions","organizations_url":"https://api.github.com/users/halide/orgs","repos_url":"https://api.github.com/users/halide/repos","events_url":"https://api.github.com/users/halide/events{/privacy}","received_events_url":"https://api.github.com/users/halide/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/halide/Halide","description":"a language for image processing and computational photography","fork":false,"url":"https://api.github.com/repos/halide/Halide","forks_url":"https://api.github.com/repos/halide/Halide/forks","keys_url":"https://api.github.com/repos/halide/Halide/keys{/key_id}","collaborators_url":"https://api.github.com/repos/halide/Halide/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/halide/Halide/teams","hooks_url":"https://api.github.com/repos/halide/Halide/hooks","issue_events_url":"https://api.github.com/repos/halide/Halide/issues/events{/number}","events_url":"https://api.github.com/repos/halide/Halide/events","assignees_url":"https://api.github.com/repos/halide/Halide/assignees{/user}","branches_url":"https://api.github.com/repos/halide/Halide/branches{/branch}","tags_url":"https://api.github.com/repos/halide/Halide/tags","blobs_url":"https://api.github.com/repos/halide/Halide/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/halide/Halide/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/halide/Halide/git/refs{/sha}","trees_url":"https://api.github.com/repos/halide/Halide/git/trees{/sha}","statuses_url":"https://api.github.com/repos/halide/Halide/statuses/{sha}","languages_url":"https://api.github.com/repos/halide/Halide/languages","stargazers_url":"https://api.github.com/repos/halide/Halide/stargazers","contributors_url":"https://api.github.com/repos/halide/Halide/contributors","subscribers_url":"https://api.github.com/repos/halide/Halide/subscribers","subscription_url":"https://api.github.com/repos/halide/Halide/subscription","commits_url":"https://api.github.com/repos/halide/Halide/commits{/sha}","git_commits_url":"https://api.github.com/repos/halide/Halide/git/commits{/sha}","comments_url":"https://api.github.com/repos/halide/Halide/comments{/number}","issue_comment_url":"https://api.github.com/repos/halide/Halide/issues/comments/{number}","contents_url":"https://api.github.com/repos/halide/Halide/contents/{+path}","compare_url":"https://api.github.com/repos/halide/Halide/compare/{base}...{head}","merges_url":"https://api.github.com/repos/halide/Halide/merges","archive_url":"https://api.github.com/repos/halide/Halide/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/halide/Halide/downloads","issues_url":"https://api.github.com/repos/halide/Halide/issues{/number}","pulls_url":"https://api.github.com/repos/halide/Halide/pulls{/number}","milestones_url":"https://api.github.com/repos/halide/Halide/milestones{/number}","notifications_url":"https://api.github.com/repos/halide/Halide/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/halide/Halide/labels{/name}","releases_url":"https://api.github.com/repos/halide/Halide/releases{/id}","created_at":"2012-07-31T04:05:25Z","updated_at":"2014-10-27T15:03:27Z","pushed_at":"2014-10-28T13:31:41Z","git_url":"git://github.com/halide/Halide.git","ssh_url":"git@github.com:halide/Halide.git","clone_url":"https://github.com/halide/Halide.git","svn_url":"https://github.com/halide/Halide","homepage":"http://halide-lang.org","size":111666,"stargazers_count":867,"watchers_count":867,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":138,"mirror_url":null,"open_issues_count":68,"forks":138,"open_issues":68,"watchers":867,"default_branch":"master"}},"base":{"label":"halide:master","ref":"master","sha":"69b0dd970bb251c0192dc26d3e5ce7204530b318","user":{"login":"halide","id":1548551,"avatar_url":"https://avatars.githubusercontent.com/u/1548551?v=2","gravatar_id":"","url":"https://api.github.com/users/halide","html_url":"https://github.com/halide","followers_url":"https://api.github.com/users/halide/followers","following_url":"https://api.github.com/users/halide/following{/other_user}","gists_url":"https://api.github.com/users/halide/gists{/gist_id}","starred_url":"https://api.github.com/users/halide/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/halide/subscriptions","organizations_url":"https://api.github.com/users/halide/orgs","repos_url":"https://api.github.com/users/halide/repos","events_url":"https://api.github.com/users/halide/events{/privacy}","received_events_url":"https://api.github.com/users/halide/received_events","type":"Organization","site_admin":false},"repo":{"id":5241312,"name":"Halide","full_name":"halide/Halide","owner":{"login":"halide","id":1548551,"avatar_url":"https://avatars.githubusercontent.com/u/1548551?v=2","gravatar_id":"","url":"https://api.github.com/users/halide","html_url":"https://github.com/halide","followers_url":"https://api.github.com/users/halide/followers","following_url":"https://api.github.com/users/halide/following{/other_user}","gists_url":"https://api.github.com/users/halide/gists{/gist_id}","starred_url":"https://api.github.com/users/halide/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/halide/subscriptions","organizations_url":"https://api.github.com/users/halide/orgs","repos_url":"https://api.github.com/users/halide/repos","events_url":"https://api.github.com/users/halide/events{/privacy}","received_events_url":"https://api.github.com/users/halide/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/halide/Halide","description":"a language for image processing and computational photography","fork":false,"url":"https://api.github.com/repos/halide/Halide","forks_url":"https://api.github.com/repos/halide/Halide/forks","keys_url":"https://api.github.com/repos/halide/Halide/keys{/key_id}","collaborators_url":"https://api.github.com/repos/halide/Halide/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/halide/Halide/teams","hooks_url":"https://api.github.com/repos/halide/Halide/hooks","issue_events_url":"https://api.github.com/repos/halide/Halide/issues/events{/number}","events_url":"https://api.github.com/repos/halide/Halide/events","assignees_url":"https://api.github.com/repos/halide/Halide/assignees{/user}","branches_url":"https://api.github.com/repos/halide/Halide/branches{/branch}","tags_url":"https://api.github.com/repos/halide/Halide/tags","blobs_url":"https://api.github.com/repos/halide/Halide/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/halide/Halide/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/halide/Halide/git/refs{/sha}","trees_url":"https://api.github.com/repos/halide/Halide/git/trees{/sha}","statuses_url":"https://api.github.com/repos/halide/Halide/statuses/{sha}","languages_url":"https://api.github.com/repos/halide/Halide/languages","stargazers_url":"https://api.github.com/repos/halide/Halide/stargazers","contributors_url":"https://api.github.com/repos/halide/Halide/contributors","subscribers_url":"https://api.github.com/repos/halide/Halide/subscribers","subscription_url":"https://api.github.com/repos/halide/Halide/subscription","commits_url":"https://api.github.com/repos/halide/Halide/commits{/sha}","git_commits_url":"https://api.github.com/repos/halide/Halide/git/commits{/sha}","comments_url":"https://api.github.com/repos/halide/Halide/comments{/number}","issue_comment_url":"https://api.github.com/repos/halide/Halide/issues/comments/{number}","contents_url":"https://api.github.com/repos/halide/Halide/contents/{+path}","compare_url":"https://api.github.com/repos/halide/Halide/compare/{base}...{head}","merges_url":"https://api.github.com/repos/halide/Halide/merges","archive_url":"https://api.github.com/repos/halide/Halide/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/halide/Halide/downloads","issues_url":"https://api.github.com/repos/halide/Halide/issues{/number}","pulls_url":"https://api.github.com/repos/halide/Halide/pulls{/number}","milestones_url":"https://api.github.com/repos/halide/Halide/milestones{/number}","notifications_url":"https://api.github.com/repos/halide/Halide/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/halide/Halide/labels{/name}","releases_url":"https://api.github.com/repos/halide/Halide/releases{/id}","created_at":"2012-07-31T04:05:25Z","updated_at":"2014-10-27T15:03:27Z","pushed_at":"2014-10-28T13:31:41Z","git_url":"git://github.com/halide/Halide.git","ssh_url":"git@github.com:halide/Halide.git","clone_url":"https://github.com/halide/Halide.git","svn_url":"https://github.com/halide/Halide","homepage":"http://halide-lang.org","size":111666,"stargazers_count":867,"watchers_count":867,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":138,"mirror_url":null,"open_issues_count":68,"forks":138,"open_issues":68,"watchers":867,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/halide/Halide/pulls/510"},"html":{"href":"https://github.com/halide/Halide/pull/510"},"issue":{"href":"https://api.github.com/repos/halide/Halide/issues/510"},"comments":{"href":"https://api.github.com/repos/halide/Halide/issues/510/comments"},"review_comments":{"href":"https://api.github.com/repos/halide/Halide/pulls/510/comments"},"review_comment":{"href":"https://api.github.com/repos/halide/Halide/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/halide/Halide/pulls/510/commits"},"statuses":{"href":"https://api.github.com/repos/halide/Halide/statuses/ccf0c4f56b55fb0188f64cc4a1419775f813fbaf"}}}}