From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 11214385B835 for ; Mon, 30 Mar 2020 04:00:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 11214385B835 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 02U3X3TT105847 for ; Mon, 30 Mar 2020 00:00:06 -0400 Received: from e06smtp04.uk.ibm.com (e06smtp04.uk.ibm.com [195.75.94.100]) by mx0b-001b2d01.pphosted.com with ESMTP id 3022qfmt8j-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 30 Mar 2020 00:00:06 -0400 Received: from localhost by e06smtp04.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 30 Mar 2020 04:59:54 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197) by e06smtp04.uk.ibm.com (192.168.101.134) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Mon, 30 Mar 2020 04:59:51 +0100 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 02U400ll46792764 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 30 Mar 2020 04:00:00 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8DC77AE055; Mon, 30 Mar 2020 04:00:00 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 80083AE051; Mon, 30 Mar 2020 03:59:58 +0000 (GMT) Received: from luoxhus-MacBook-Pro.local (unknown [9.197.235.18]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 30 Mar 2020 03:59:58 +0000 (GMT) Subject: Re: [PATCH] rs6000: Don't split constant oprator when add, move to temp register for future optimization To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, wschmidt@linux.ibm.com, guojiufu@linux.ibm.com, linkw@gcc.gnu.org References: <20200326100643.32487-1-luoxhu@linux.ibm.com> <20200327143331.GD22482@gate.crashing.org> From: luoxhu Date: Mon, 30 Mar 2020 11:59:57 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200327143331.GD22482@gate.crashing.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 20033003-0016-0000-0000-000002FAB42C X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 20033003-0017-0000-0000-0000335E6B74 Message-Id: <388143a3-7889-0476-abd7-d2e9b667e5f4@linux.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138, 18.0.645 definitions=2020-03-29_10:2020-03-27, 2020-03-29 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 lowpriorityscore=0 spamscore=0 bulkscore=0 adultscore=0 priorityscore=1501 malwarescore=0 mlxlogscore=999 mlxscore=0 phishscore=0 suspectscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2003300032 X-Spam-Status: No, score=-23.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Mar 2020 04:00:10 -0000 On 2020/3/27 22:33, Segher Boessenkool wrote: > Hi! > > On Thu, Mar 26, 2020 at 05:06:43AM -0500, luoxhu@linux.ibm.com wrote: >> Remove split code from add3 to allow a later pass to split. >> This allows later logic to hoist out constant load in add instructions. >> In loop, lis+ori could be hoisted out to improve performance compared with >> previous addis+addi (About 15% on typical case), weak point is >> one more register is used and one more instruction is generated. i.e.: >> >> addis 3,3,0x8765 >> addi 3,3,0x4321 >> >> => >> >> lis 9,0x8765 >> ori 9,9,0x4321 >> add 3,3,9 > > What does it do overall? Say, to SPEC. What does it do to execution > time, and what does it do to binary size? > > Do we want something later in the RTL pipeline to make "addi"s etc. again? > >> 2020-03-26 Xiong Hu Luo >> >> * config/rs6000/rs6000.md (add3): Remove split code, move constant >> to temp register before add. > > This should not be indented, so just: > * config/rs6000/rs6000.md (add3): Remove split code, move constant > to temp register before add. > > We have six separate add3 patterns (three of those are in rs6000.md, > too). You can write something like > (add3 for SDI): > to show which iterator (or mode) this one is for. This is helpful with > any or or the like, even if there (currently) is only one > pattern you could mean. > Thanks, it is necessary to re-enable split add as some later RTL passes like final will still need generate addis+addi (case: g++.dg/opt/thunk1.C ). Update the patch as below: [PATCH] rs6000: Don't split constant operator add before reload, move to temp register for future optimization Don't split code from add3 for SDI to allow a later pass to split. This allows later logic to hoist out constant load in add instructions. In loop, lis+ori could be hoisted out to improve performance compared with previous addis+addi (About 15% on typical case), weak point is one more register is used and one more instruction is generated. i.e.: addis 3,3,0x8765 addi 3,3,0x4321 => lis 9,0x8765 ori 9,9,0x4321 add 3,3,9 No obvious performance and binary size change to SPEC2017. gcc/ChangeLog: 2020-03-30 Xiong Hu Luo * config/rs6000/rs6000.md (add3 for SDI): Don't split before reload, move constant to temp register for add. gcc/testsuite/ChangeLog: 2020-03-26 Xiong Hu Luo * gcc.target/powerpc/add-const.c: New. --- gcc/config/rs6000/rs6000.md | 51 +++++++++----------- gcc/testsuite/gcc.target/powerpc/add-const.c | 18 +++++++ 2 files changed, 42 insertions(+), 27 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/add-const.c diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index ad88b6783af..76af3d5b1d9 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -1729,34 +1729,31 @@ (define_expand "add3" if (CONST_INT_P (operands[2]) && !add_operand (operands[2], mode)) { - rtx tmp = ((!can_create_pseudo_p () - || rtx_equal_p (operands[0], operands[1])) - ? operands[0] : gen_reg_rtx (mode)); - - /* Adding a constant to r0 is not a valid insn, so use a different - strategy in that case. */ - if (reg_or_subregno (operands[1]) == 0 || reg_or_subregno (tmp) == 0) - { - if (operands[0] == operands[1]) - FAIL; - rs6000_emit_move (operands[0], operands[2], mode); - emit_insn (gen_add3 (operands[0], operands[1], operands[0])); - DONE; - } - - HOST_WIDE_INT val = INTVAL (operands[2]); - HOST_WIDE_INT low = ((val & 0xffff) ^ 0x8000) - 0x8000; - HOST_WIDE_INT rest = trunc_int_for_mode (val - low, mode); - - if (mode == DImode && !satisfies_constraint_L (GEN_INT (rest))) - FAIL; + if (can_create_pseudo_p ()) + { + rtx tmp = rtx_equal_p (operands[0], operands[1]) + ? operands[0] : gen_reg_rtx (mode); - /* The ordering here is important for the prolog expander. - When space is allocated from the stack, adding 'low' first may - produce a temporary deallocation (which would be bad). */ - emit_insn (gen_add3 (tmp, operands[1], GEN_INT (rest))); - emit_insn (gen_add3 (operands[0], tmp, GEN_INT (low))); - DONE; + rs6000_emit_move (tmp, operands[2], mode); + emit_insn (gen_add3 (operands[0], operands[1], tmp)); + DONE; + } + else + { + HOST_WIDE_INT val = INTVAL (operands[2]); + HOST_WIDE_INT low = ((val & 0xffff) ^ 0x8000) - 0x8000; + HOST_WIDE_INT rest = trunc_int_for_mode (val - low, mode); + + if (mode == DImode && !satisfies_constraint_L (GEN_INT (rest))) + FAIL; + + /* The ordering here is important for the prolog expander. + When space is allocated from the stack, adding 'low' first may + produce a temporary deallocation (which would be bad). */ + emit_insn (gen_add3 (operands[0], operands[1], GEN_INT (rest))); + emit_insn (gen_add3 (operands[0], operands[0], GEN_INT (low))); + DONE; + } } }) diff --git a/gcc/testsuite/gcc.target/powerpc/add-const.c b/gcc/testsuite/gcc.target/powerpc/add-const.c new file mode 100644 index 00000000000..e1007247b32 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/add-const.c @@ -0,0 +1,18 @@ +/* { dg-do compile { target { lp64 } } } */ +/* { dg-options "-O3 -fno-unroll-loops" } */ + +/* Ensure the lis,ori are generated, which indicates they have + been hoisted outside of the loop. */ + +typedef unsigned long ulong; +ulong +foo (ulong n, ulong h) +{ + int i; + for (i = 0; i < n; i++) + h = ((h + 8) | h) + 0x87654321; + return h; +} + +/* { dg-final { scan-assembler-times {\mlis\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mori\M} 1 } } */ -- 2.21.0.777.g83232e3864