[{"id":1776363,"web_url":"http://patchwork.ozlabs.org/comment/1776363/","msgid":"<87k20kjltj.fsf@euler.schwinge.homeip.net>","list_archive_url":null,"date":"2017-09-27T13:46:48","subject":"Re: [PATCH] Pretty-print GOACC_REDUCTION arguments","submitter":{"id":6153,"url":"http://patchwork.ozlabs.org/api/people/6153/","name":"Thomas Schwinge","email":"thomas@codesourcery.com"},"content":"Hi!\n\nOn Mon, 25 Sep 2017 16:57:52 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:\n> currently for a GOACC_REDUCTION internal fn call we print:\n> ...\n>    sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0);\n> ...\n> \n> This patch adds a comment for some arguments explaining the meaning of \n> the argument:\n> ...\n>        sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*gang*/, 67 /*+*/, 0);\n> ...\n\nACK to the general idea.\n\nHowever, I note that for the \"CODE\" we currently *only* print the textual\nvariant (\"SETUP\") (just like we're also doing for a few other \"IFN_*\"),\nwhereas for \"LEVEL\" and \"OP\" you now print both.  Should these really be\ndifferent?  I think I actually do prefer the style you're using (print\nboth).  I would print the actual \"GOMP_DIM_GANG\" instead of \"gang\" etc.,\nto make it easier to see where these values are coming from.\n\n(I do see that for \"CODE\", we can easily use a suitable \"DEF\" macro with\nthe \"IFN_GOACC_REDUCTION_CODES\" define -- not currently possible with the\n\"GOMP_DIM_*\" constants.  That can of course be addressed later,\nseparately, if a Reviewer agrees to the proposed patch generally.)\n\n> OK for trunk, if testing is ok?\n\n> --- a/gcc/gimple-pretty-print.c\n> +++ b/gcc/gimple-pretty-print.c\n> @@ -765,6 +766,40 @@ dump_gimple_call_args (pretty_printer *buffer, gcall *gs, dump_flags_t flags)\n>        if (i)\n>  \tpp_string (buffer, \", \");\n>        dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);\n> +\n> +      if (gimple_call_internal_p (gs))\n> +\tswitch (gimple_call_internal_fn (gs))\n> +\t  {\n\nWill need to add a \"default\" case:\n\n    [...]/source-gcc/gcc/gimple-pretty-print.c:771:9: warning: enumeration value 'IFN_MASK_LOAD' not handled in switch [-Wswitch]\n      switch (gimple_call_internal_fn (gs))\n             ^\n    [followed by many more]\n\n> +\t  case IFN_GOACC_REDUCTION:\n> +\t    switch (i)\n> +\t      {\n> +\t      case 3:\n> +\t\tswitch (tree_to_uhwi (gimple_call_arg (gs, i)))\n\nSomething ;-) is wrong.  Running this on\nlibgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c, I run into:\n\n    during GIMPLE pass: omplower\n    dump file: reduction-1.c.006t.omplower\n    \n    In file included from source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c:9:0:\n    source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c: In function 'test_reductions':\n    source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction.h:20:7: internal compiler error: in tree_to_uhwi, at tree.c:6606\n           abort ();        \\\n           ^~~~~~~~\n    source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c:55:3: note: in expansion of macro 'check_reduction_op'\n       check_reduction_op (int, ^, 0, array[i], num_gangs (ng) num_workers (nw)\n       ^~~~~~~~~~~~~~~~~~\n\n    (gdb) bt\n    #0  fancy_abort (file=0x1659d70 \"[...]/source-gcc/gcc/tree.c\", line=6606, function=0x165e7f8 <tree_to_uhwi(tree_node const*)::__FUNCTION__> \"tree_to_uhwi\") at [...]/source-gcc/gcc/diagnostic.c:1487\n    #1  0x0000000000edf053 in tree_to_uhwi (t=<optimized out>) at [...]/source-gcc/gcc/tree.c:6606\n    #2  0x000000000096ca63 in dump_gimple_call_args (buffer=0x7fffffffc6c0, gs=0x7ffff66ad210, flags=0) at [...]/source-gcc/gcc/gimple-pretty-print.c:777\n    #3  0x000000000096f7f0 in dump_gimple_call (buffer=0x7fffffffc6c0, gs=0x7ffff66ad210, spc=20, flags=0) at [...]/source-gcc/gcc/gimple-pretty-print.c:946\n    [...]\n\nThis seems to be the \"LEVEL\" being \"-1\" -- probably meaning \"not yet\ndecided\"?  (Haven't looked that up.)\n\n> +\t\t  {\n> +\t\t  case GOMP_DIM_GANG:\n> +\t\t    pp_string (buffer, \" /*gang*/\");\n> +\t\t    break;\n> +\t\t  case GOMP_DIM_WORKER:\n> +\t\t    pp_string (buffer, \" /*worker*/\");\n> +\t\t    break;\n> +\t\t  case GOMP_DIM_VECTOR:\n> +\t\t    pp_string (buffer, \" /*vector*/\");\n> +\t\t    break;\n> +\t\t  default:\n> +\t\t    gcc_unreachable ();\n> +\t\t  }\n> +\t\tbreak;\n> +\t      case 4:\n> +\t\t{\n> +\t\t  enum tree_code rcode\n> +\t\t    = (enum tree_code)tree_to_uhwi (gimple_call_arg (gs, i));\n> +\t\t  pp_string (buffer, \" /*\");\n> +\t\t  pp_string (buffer, op_symbol_code (rcode));\n> +\t\t  pp_string (buffer, \"*/\");\n> +\t\t}\n> +\t\tbreak;\n\nI take it, there is no canned function for printing the textual\nrepresentation of the tree_code \"OP\" (\"case 4\").\n\n> +\t      }\n> +\t  }\n>      }\n>  \n>    if (gimple_call_va_arg_pack_p (gs))\n\n\nGrüße\n Thomas","headers":{"Return-Path":"<gcc-patches-return-463059-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-463059-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"BMu8OsBK\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y2Jz65wKHz9tXw\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 27 Sep 2017 23:47:13 +1000 (AEST)","(qmail 108536 invoked by alias); 27 Sep 2017 13:47:06 -0000","(qmail 108162 invoked by uid 89); 27 Sep 2017 13:47:05 -0000","from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131)\n\tby sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with\n\tESMTP; Wed, 27 Sep 2017 13:47:03 +0000","from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206]\n\thelo=SVR-ORW-FEM-02.mgc.mentorg.com)\tby relay1.mentorg.com\n\twith esmtps (TLSv1:ECDHE-RSA-AES256-SHA:256)\tid\n\t1dxCg0-0004Fz-LY from Thomas_Schwinge@mentor.com for\n\tgcc-patches@gcc.gnu.org; Wed, 27 Sep 2017 06:47:00 -0700","from tftp-cs (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com\n\t(147.34.96.168) with Microsoft SMTP Server id 14.3.224.2;\n\tWed, 27 Sep 2017 06:47:00 -0700","by tftp-cs (Postfix, from userid 49978)\tid B9C22C22BB;\n\tWed, 27 Sep 2017 06:46:59 -0700 (PDT)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender:from\n\t:to:cc:subject:in-reply-to:references:date:message-id\n\t:mime-version:content-type:content-transfer-encoding; q=dns; s=\n\tdefault; b=dVOdqG+lgjGOGP3+Ybw9KImMgAwUfeS5mujFs/NsQl1vD1s3+AXt8\n\tTkUBJxbGAK0F9VKU9cwOtIfjVKKlqdh3zC1iEJI9yTR4ERyNWxXZCgTFGM/iDRgs\n\tcPHb3dRG/QMRWxzb6FdC2hB9U/KUkGylfqcf6xTKZeax5Ma1T62udY=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender:from\n\t:to:cc:subject:in-reply-to:references:date:message-id\n\t:mime-version:content-type:content-transfer-encoding; s=default;\n\tbh=Bvr7Zt271CSzFB09sByHL8GqBpw=; b=BMu8OsBKoJl4XRbDM5ui68/C0lSk\n\t+ypJReTTE2re7q6oYTj1odNnUyQuVH4esZgbh1B84IFd1fNi/VWDFPpzn81BBt+a\n\tu0gw9IKyLBabZzIQz1l1tD5sF0rIn78d1HWMW/EvT9/zvTqUXZeKY2WBQbu8Efvq\n\tNDUpbx8RA7vtnuI=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-11.2 required=5.0 tests=AWL, BAYES_00,\n\tGIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE,\n\tSPF_PASS autolearn=ham version=3.3.2 spammy=Reviewer, canned,\n\t7656, H*r:PDT","X-HELO":"relay1.mentorg.com","From":"Thomas Schwinge <thomas@codesourcery.com>","To":"Tom de Vries <Tom_deVries@mentor.com>","CC":"GCC Patches <gcc-patches@gcc.gnu.org>","Subject":"Re: [PATCH] Pretty-print GOACC_REDUCTION arguments","In-Reply-To":"<b0855102-fb3e-ba64-a4a5-1f058fb7a38e@mentor.com>","References":"<b0855102-fb3e-ba64-a4a5-1f058fb7a38e@mentor.com>","User-Agent":"Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/25.2.1\n\t(x86_64-pc-linux-gnu)","Date":"Wed, 27 Sep 2017 15:46:48 +0200","Message-ID":"<87k20kjltj.fsf@euler.schwinge.homeip.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"quoted-printable"}},{"id":1778780,"web_url":"http://patchwork.ozlabs.org/comment/1778780/","msgid":"<7687fb9b-1e21-c237-4b8a-05874ac4ee94@mentor.com>","list_archive_url":null,"date":"2017-10-03T08:56:59","subject":"Re: [PATCH] Pretty-print GOACC_REDUCTION arguments","submitter":{"id":9225,"url":"http://patchwork.ozlabs.org/api/people/9225/","name":"Tom de Vries","email":"Tom_deVries@mentor.com"},"content":"On 09/27/2017 03:46 PM, Thomas Schwinge wrote:\n> Hi!\n> \n> On Mon, 25 Sep 2017 16:57:52 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:\n>> currently for a GOACC_REDUCTION internal fn call we print:\n>> ...\n>>     sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0);\n>> ...\n>>\n>> This patch adds a comment for some arguments explaining the meaning of\n>> the argument:\n>> ...\n>>         sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*gang*/, 67 /*+*/, 0);\n>> ...\n> \n> ACK to the general idea.\n> \n> However, I note that for the \"CODE\" we currently *only* print the textual\n> variant (\"SETUP\") (just like we're also doing for a few other \"IFN_*\"),\n> whereas for \"LEVEL\" and \"OP\" you now print both.  Should these really be\n> different?  I think I actually do prefer the style you're using (print\n> both).\n\nI chose the c-like syntax to make it easier to copy gimple to test-case \n( based on comment \nhttps://gcc.gnu.org/ml/gcc-patches/2015-09/msg02180.html ). But \nreflecting on it a bit more, I realized that it's an internal function \nand as such won't work anyway in test-cases, so I'm not sure how \nrelevant it is in this case.\n\n> I would print the actual \"GOMP_DIM_GANG\" instead of \"gang\" etc.,\n> to make it easier to see where these values are coming from.\n> \n\nDone.\n\n>> OK for trunk, if testing is ok?\n\n\n>> +\t  case IFN_GOACC_REDUCTION:\n>> +\t    switch (i)\n>> +\t      {\n>> +\t      case 3:\n>> +\t\tswitch (tree_to_uhwi (gimple_call_arg (gs, i)))\n> \n> Something ;-) is wrong.  Running this on\n> libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c, I run into:\n> \n>      during GIMPLE pass: omplower\n>      dump file: reduction-1.c.006t.omplower\n>      \n>      In file included from source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c:9:0:\n>      source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c: In function 'test_reductions':\n>      source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction.h:20:7: internal compiler error: in tree_to_uhwi, at tree.c:6606\n>             abort ();        \\\n\n> This seems to be the \"LEVEL\" being \"-1\" -- probably meaning \"not yet\n> decided\"?  (Haven't looked that up.)\n> \n\nIn execute_oacc_device_lower we find:\n...\n           case IFN_GOACC_REDUCTION:\n             /* Mark the function for SSA renaming.  */\n\t    mark_virtual_operands_for_renaming (cfun);\n\n             /* If the level is -1, this ended up being an unused \n\n                axis.  Handle as a default.  */\n             if (integer_minus_onep (gimple_call_arg (call, 3)))\n               default_goacc_reduction (call);\n             else\n               targetm.goacc.reduction (call);\n...\n\nI'm printing it now as GOMP_DIM_NONE.\n\nBootstrapped and reg-tested on x86_64.\n\nOK for trunk?\n\nThanks,\n- Tom\nPretty-print GOACC_REDUCTION arguments\n\nPrints\n  sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*GOMP_DIM_GANG*/, 67 /*+*/, 0);\ninstead of\n  sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0);\n\n2017-09-25  Tom de Vries  <tom@codesourcery.com>\n\n\t* gimple-pretty-print.c (dump_gimple_call_args): Pretty-print\n\tGOACC_REDUCTION arguments.\n\n---\n gcc/gimple-pretty-print.c | 42 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 42 insertions(+)\n\ndiff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c\nindex ed8e51c..33ca45d 100644\n--- a/gcc/gimple-pretty-print.c\n+++ b/gcc/gimple-pretty-print.c\n@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see\n #include \"stringpool.h\"\n #include \"attribs.h\"\n #include \"asan.h\"\n+#include \"gomp-constants.h\"\n \n #define INDENT(SPACE)\t\t\t\t\t\t\t\\\n   do { int i; for (i = 0; i < SPACE; i++) pp_space (buffer); } while (0)\n@@ -765,6 +766,47 @@ dump_gimple_call_args (pretty_printer *buffer, gcall *gs, dump_flags_t flags)\n       if (i)\n \tpp_string (buffer, \", \");\n       dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);\n+\n+      if (gimple_call_internal_p (gs))\n+\tswitch (gimple_call_internal_fn (gs))\n+\t  {\n+\t  case IFN_GOACC_REDUCTION:\n+\t    switch (i)\n+\t      {\n+\t      case 3:\n+\t\tswitch (tree_to_shwi (gimple_call_arg (gs, i)))\n+\t\t  {\n+\t\t  case GOMP_DIM_GANG:\n+\t\t    pp_string (buffer, \" /*GOMP_DIM_GANG*/\");\n+\t\t    break;\n+\t\t  case GOMP_DIM_WORKER:\n+\t\t    pp_string (buffer, \" /*GOMP_DIM_WORKER*/\");\n+\t\t    break;\n+\t\t  case GOMP_DIM_VECTOR:\n+\t\t    pp_string (buffer, \" /*GOMP_DIM_VECTOR*/\");\n+\t\t    break;\n+\t\t  case -1:\n+\t\t    pp_string (buffer, \" /*GOMP_DIM_NONE*/\");\n+\t\t    break;\n+\t\t  default:\n+\t\t    gcc_unreachable ();\n+\t\t  }\n+\t\tbreak;\n+\t      case 4:\n+\t\t{\n+\t\t  enum tree_code rcode\n+\t\t    = (enum tree_code)tree_to_shwi (gimple_call_arg (gs, i));\n+\t\t  pp_string (buffer, \" /*\");\n+\t\t  pp_string (buffer, op_symbol_code (rcode));\n+\t\t  pp_string (buffer, \"*/\");\n+\t\t}\n+\t\tbreak;\n+\t      default:\n+\t\tbreak;\n+\t      }\n+\t  default:\n+\t    break;\n+\t  }\n     }\n \n   if (gimple_call_va_arg_pack_p (gs))","headers":{"Return-Path":"<gcc-patches-return-463381-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-463381-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"PIU3ESUL\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y5tFx5SRBz9t3x\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue,  3 Oct 2017 19:57:25 +1100 (AEDT)","(qmail 78458 invoked by alias); 3 Oct 2017 08:57:16 -0000","(qmail 72120 invoked by uid 89); 3 Oct 2017 08:57:12 -0000","from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131)\n\tby sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with\n\tESMTP; Tue, 03 Oct 2017 08:57:10 +0000","from nat-ies.mentorg.com ([192.94.31.2]\n\thelo=SVR-IES-MBX-04.mgc.mentorg.com)\tby relay1.mentorg.com\n\twith esmtps (TLSv1.2:ECDHE-RSA-AES256-SHA384:256)\tid\n\t1dzJ0k-0000mf-NT from Tom_deVries@mentor.com ;\n\tTue, 03 Oct 2017 01:57:06 -0700","from [127.0.0.1] (137.202.0.87) by SVR-IES-MBX-04.mgc.mentorg.com\n\t(139.181.222.4) with Microsoft SMTP Server (TLS) id\n\t15.0.1263.5; Tue, 3 Oct 2017 09:57:02 +0100"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:subject:to:cc:references:from:message-id:date:mime-version\n\t:in-reply-to:content-type; q=dns; s=default; b=dY8JfA+39mJETs9ud\n\tOPNZEAjogsRRl2ecl618ZiA3+MFzhuam6e74pL8B2xaQ3Y9nvx5hs+zIK0TCGx13\n\t0vpc/JmSSt8ccSltI2t4Wkvt+ZddQNn2KCdZ7+F5q7GO1QjUUMZHJ1kZnE3t2sgy\n\t/qZG6xfKUgS94H3DT6XjnN6PJo=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:subject:to:cc:references:from:message-id:date:mime-version\n\t:in-reply-to:content-type; s=default; bh=3gEcWI7slzuIyImjj58+ayp\n\trNWI=; b=PIU3ESULKqOYRw2r7G6EaloQyp0UGp2XYt6HK714HmZWYnBWOANkFtu\n\t+brHOBObstewKDWXW3s6VmDG/gFxla4qnpNRY+onqfiuOBAFf5WoBpqgdDDp+ITt\n\t8oPqyfYLYTifSBwxr8u3HjcSUb9vZe52q2HTif0/axn7gcGrmSSA=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-24.6 required=5.0 tests=AWL, BAYES_00,\n\tGIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3,\n\tRCVD_IN_DNSWL_NONE, SPF_PASS,\n\tURIBL_RED autolearn=ham version=3.3.2 spammy=axis,\n\tD*mentor.com, ack","X-HELO":"relay1.mentorg.com","Subject":"Re: [PATCH] Pretty-print GOACC_REDUCTION arguments","To":"Thomas Schwinge <thomas@codesourcery.com>","CC":"GCC Patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>","References":"<b0855102-fb3e-ba64-a4a5-1f058fb7a38e@mentor.com>\n\t<87k20kjltj.fsf@euler.schwinge.homeip.net>","From":"Tom de Vries <Tom_deVries@mentor.com>","Message-ID":"<7687fb9b-1e21-c237-4b8a-05874ac4ee94@mentor.com>","Date":"Tue, 3 Oct 2017 10:56:59 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:52.0) Gecko/20100101 Thunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<87k20kjltj.fsf@euler.schwinge.homeip.net>","Content-Type":"multipart/mixed;\n\tboundary=\"------------029EEB089659BDA242484D56\"","X-ClientProxiedBy":"svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To\n\tSVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4)"}},{"id":1779909,"web_url":"http://patchwork.ozlabs.org/comment/1779909/","msgid":"<87mv577yal.fsf@hertz.schwinge.homeip.net>","list_archive_url":null,"date":"2017-10-04T15:00:50","subject":"Re: [PATCH] Pretty-print GOACC_REDUCTION arguments","submitter":{"id":6153,"url":"http://patchwork.ozlabs.org/api/people/6153/","name":"Thomas Schwinge","email":"thomas@codesourcery.com"},"content":"Hi Tom!\n\nOn Tue, 3 Oct 2017 10:56:59 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:\n> On 09/27/2017 03:46 PM, Thomas Schwinge wrote:\n> > On Mon, 25 Sep 2017 16:57:52 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:\n> >> currently for a GOACC_REDUCTION internal fn call we print:\n> >> ...\n> >>     sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0);\n> >> ...\n> >>\n> >> This patch adds a comment for some arguments explaining the meaning of\n> >> the argument:\n> >> ...\n> >>         sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*gang*/, 67 /*+*/, 0);\n> >> ...\n> > \n> > ACK to the general idea.\n> > \n> > However, I note that for the \"CODE\" we currently *only* print the textual\n> > variant (\"SETUP\") (just like we're also doing for a few other \"IFN_*\"),\n> > whereas for \"LEVEL\" and \"OP\" you now print both.  Should these really be\n> > different?  I think I actually do prefer the style you're using (print\n> > both).\n> \n> I chose the c-like syntax to make it easier to copy gimple to test-case \n> ( based on comment \n> https://gcc.gnu.org/ml/gcc-patches/2015-09/msg02180.html ). But \n> reflecting on it a bit more, I realized that it's an internal function \n> and as such won't work anyway in test-cases, so I'm not sure how \n> relevant it is in this case.\n\nThe way you're doing it still makes sense to me: we might (at some later\npoint...) read such dumps back in, using the GIMPLE front end or similar.\n(Clearly nothing to worry about right now.)\n\n\n> >> +\t  case IFN_GOACC_REDUCTION:\n> >> +\t    switch (i)\n> >> +\t      {\n> >> +\t      case 3:\n> >> +\t\tswitch (tree_to_uhwi (gimple_call_arg (gs, i)))\n> > \n> > Something ;-) is wrong.  Running this on\n> > libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c, I run into:\n> > \n> >      during GIMPLE pass: omplower\n> >      dump file: reduction-1.c.006t.omplower\n> >      \n> >      In file included from source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c:9:0:\n> >      source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c: In function 'test_reductions':\n> >      source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction.h:20:7: internal compiler error: in tree_to_uhwi, at tree.c:6606\n> >             abort ();        \\\n> \n> > This seems to be the \"LEVEL\" being \"-1\" -- probably meaning \"not yet\n> > decided\"?  (Haven't looked that up.)\n\nAs far as I can tell, initially all these \"LEVEL\" arguments are set to\n\"-1\".  See the \"place\" argument in the \"lower_oacc_reductions\" call in\n\"lower_oacc_head_tail\".  So, I understand this to mean \"not yet decided\",\nfrom here on, until some real level of parallelism gets set.\n\n> In execute_oacc_device_lower we find:\n> ...\n>            case IFN_GOACC_REDUCTION:\n>              /* Mark the function for SSA renaming.  */\n> \t    mark_virtual_operands_for_renaming (cfun);\n> \n>              /* If the level is -1, this ended up being an unused \n> \n>                 axis.  Handle as a default.  */\n>              if (integer_minus_onep (gimple_call_arg (call, 3)))\n>                default_goacc_reduction (call);\n>              else\n>                targetm.goacc.reduction (call);\n> ...\n> \n> I'm printing it now as GOMP_DIM_NONE.\n\nBut after this processing, there are no \"IFN_GOACC_REDUCTION\" anymore, so\nwe should rather print something like \"unknown\" instead of\n\"GOMP_DIM_NONE\" (which doesn't exist anyway).\n\n\nWith that changed, we're good as far as I'm concerned, thanks!  But I\ncan't formally approve, of course.  Reviewed-by: Thomas Schwinge\n<thomas@codesourcery.com> (See\n<http://mid.mail-archive.com/87tvzuk29t.fsf@euler.schwinge.homeip.net>.)\n\n\nGrüße\n Thomas\n\n\n> Bootstrapped and reg-tested on x86_64.\n> \n> OK for trunk?\n> \n> Thanks,\n> - Tom\n> Pretty-print GOACC_REDUCTION arguments\n> \n> Prints\n>   sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*GOMP_DIM_GANG*/, 67 /*+*/, 0);\n> instead of\n>   sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0);\n> \n> 2017-09-25  Tom de Vries  <tom@codesourcery.com>\n> \n> \t* gimple-pretty-print.c (dump_gimple_call_args): Pretty-print\n> \tGOACC_REDUCTION arguments.\n> \n> ---\n>  gcc/gimple-pretty-print.c | 42 ++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 42 insertions(+)\n> \n> diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c\n> index ed8e51c..33ca45d 100644\n> --- a/gcc/gimple-pretty-print.c\n> +++ b/gcc/gimple-pretty-print.c\n> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see\n>  #include \"stringpool.h\"\n>  #include \"attribs.h\"\n>  #include \"asan.h\"\n> +#include \"gomp-constants.h\"\n>  \n>  #define INDENT(SPACE)\t\t\t\t\t\t\t\\\n>    do { int i; for (i = 0; i < SPACE; i++) pp_space (buffer); } while (0)\n> @@ -765,6 +766,47 @@ dump_gimple_call_args (pretty_printer *buffer, gcall *gs, dump_flags_t flags)\n>        if (i)\n>  \tpp_string (buffer, \", \");\n>        dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);\n> +\n> +      if (gimple_call_internal_p (gs))\n> +\tswitch (gimple_call_internal_fn (gs))\n> +\t  {\n> +\t  case IFN_GOACC_REDUCTION:\n> +\t    switch (i)\n> +\t      {\n> +\t      case 3:\n> +\t\tswitch (tree_to_shwi (gimple_call_arg (gs, i)))\n> +\t\t  {\n> +\t\t  case GOMP_DIM_GANG:\n> +\t\t    pp_string (buffer, \" /*GOMP_DIM_GANG*/\");\n> +\t\t    break;\n> +\t\t  case GOMP_DIM_WORKER:\n> +\t\t    pp_string (buffer, \" /*GOMP_DIM_WORKER*/\");\n> +\t\t    break;\n> +\t\t  case GOMP_DIM_VECTOR:\n> +\t\t    pp_string (buffer, \" /*GOMP_DIM_VECTOR*/\");\n> +\t\t    break;\n> +\t\t  case -1:\n> +\t\t    pp_string (buffer, \" /*GOMP_DIM_NONE*/\");\n> +\t\t    break;\n> +\t\t  default:\n> +\t\t    gcc_unreachable ();\n> +\t\t  }\n> +\t\tbreak;\n> +\t      case 4:\n> +\t\t{\n> +\t\t  enum tree_code rcode\n> +\t\t    = (enum tree_code)tree_to_shwi (gimple_call_arg (gs, i));\n> +\t\t  pp_string (buffer, \" /*\");\n> +\t\t  pp_string (buffer, op_symbol_code (rcode));\n> +\t\t  pp_string (buffer, \"*/\");\n> +\t\t}\n> +\t\tbreak;\n> +\t      default:\n> +\t\tbreak;\n> +\t      }\n> +\t  default:\n> +\t    break;\n> +\t  }\n>      }\n>  \n>    if (gimple_call_va_arg_pack_p (gs))","headers":{"Return-Path":"<gcc-patches-return-463465-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-463465-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"YSdbyHmo\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y6fHM0GM4z9t1G\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu,  5 Oct 2017 02:01:14 +1100 (AEDT)","(qmail 84048 invoked by alias); 4 Oct 2017 15:01:05 -0000","(qmail 84026 invoked by uid 89); 4 Oct 2017 15:01:04 -0000","from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131)\n\tby sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with\n\tESMTP; Wed, 04 Oct 2017 15:01:01 +0000","from nat-ies.mentorg.com ([192.94.31.2]\n\thelo=svr-ies-mbx-01.mgc.mentorg.com)\tby relay1.mentorg.com\n\twith esmtps (TLSv1.2:ECDHE-RSA-AES256-SHA384:256)\tid\n\t1dzlAP-0002JJ-JU from Thomas_Schwinge@mentor.com ;\n\tWed, 04 Oct 2017 08:00:57 -0700","from hertz.schwinge.homeip.net (137.202.0.87) by\n\tsvr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) with Microsoft\n\tSMTP Server (TLS) id 15.0.1263.5; Wed, 4 Oct 2017 16:00:54 +0100"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender:from\n\t:to:cc:subject:in-reply-to:references:date:message-id\n\t:mime-version:content-type:content-transfer-encoding; q=dns; s=\n\tdefault; b=mnmTwz8GKUD2jLb7rUo69DEBL8sWv9X1UsEGSyeDJ+dEfRDMwjA8+\n\tE/bIBWGXIQL2laRrP8+xn/VHXP59Vqjdz9suh/P7T1axSHatrKUGoMRgcAR96ffi\n\t6DgubV34za6HmVnnZ3OfqVt5mLCubh9kdKZfD+tuTXg4k+FUXl9wms=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender:from\n\t:to:cc:subject:in-reply-to:references:date:message-id\n\t:mime-version:content-type:content-transfer-encoding; s=default;\n\tbh=s+/VKE2c1QdtK26EvHmxQjp7GbA=; b=YSdbyHmoSaLmWFa9pEXMyzYJETCe\n\tVDh/fg1dwS2zOKiEtBOD6KWII6i8EjRQDySM0rKYvn9EGiNRzBrxYhqcp4SmIMY7\n\t7tntJMn/OzOfumGV60enRzaCi9MU5njyHPPvJvPl3f1DNzQqqNR/wh3FfKSSu+QN\n\tJPJIWZS0y7W3qx4=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-24.5 required=5.0 tests=AWL, BAYES_00,\n\tGIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3,\n\tRCVD_IN_DNSWL_NONE, SPF_PASS,\n\tURIBL_RED autolearn=ham version=3.3.2 spammy=","X-HELO":"relay1.mentorg.com","From":"Thomas Schwinge <thomas@codesourcery.com>","To":"Tom de Vries <Tom_deVries@mentor.com>","CC":"GCC Patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>","Subject":"Re: [PATCH] Pretty-print GOACC_REDUCTION arguments","In-Reply-To":"<7687fb9b-1e21-c237-4b8a-05874ac4ee94@mentor.com>","References":"<b0855102-fb3e-ba64-a4a5-1f058fb7a38e@mentor.com>\n\t<87k20kjltj.fsf@euler.schwinge.homeip.net>\n\t<7687fb9b-1e21-c237-4b8a-05874ac4ee94@mentor.com>","User-Agent":"Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/24.5.1\n\t(x86_64-pc-linux-gnu)","Date":"Wed, 4 Oct 2017 17:00:50 +0200","Message-ID":"<87mv577yal.fsf@hertz.schwinge.homeip.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"quoted-printable","X-ClientProxiedBy":"svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To\n\tsvr-ies-mbx-01.mgc.mentorg.com (139.181.222.1)"}}]