From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36280 invoked by alias); 2 Aug 2018 22:33:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 36265 invoked by uid 89); 2 Aug 2018 22:33:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=BAYES_00,BODY_8BITS,GARBLED_BODY,GIT_PATCH_2,KAM_ASCII_DIVIDERS,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 02 Aug 2018 22:33:53 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C546A307D866; Thu, 2 Aug 2018 22:33:51 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-6.rdu2.redhat.com [10.10.112.6]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5476510694C1; Thu, 2 Aug 2018 22:33:50 +0000 (UTC) Subject: Re: [2/5] C-SKY port: Backend implementation To: Sandra Loosemore , =?UTF-8?B?556/5LuZ5re8?= Cc: "gcc-patches@gcc.gnu.org" , Yunhai Shang References: <49d0a2c8-51a0-4a74-d015-0bf1c1098e38@codesourcery.com> <28cb3a6e-4594-3545-5236-c68784af6a57@codesourcery.com> <8a1b9bac-82dc-bb4f-e0a2-9a9b9cbea98a@redhat.com> <56004587-F43E-4004-B618-B819CF7A5E4A@c-sky.com> <37200996-f535-b070-4415-227d2dad794b@codesourcery.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: Date: Thu, 02 Aug 2018 22:33:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <37200996-f535-b070-4415-227d2dad794b@codesourcery.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00246.txt.bz2 On 07/27/2018 07:49 PM, Sandra Loosemore wrote: > On 07/26/2018 12:06 AM, 瞿仙淼 wrote: >> >> I wrote a case to reproduce this problem on C-SKY. C code is as follows: >> ----------------------------------------------------------------------- >> int e1, e2; >> >> void func (int a, int b, int c, int d, int f, int g) >> { >>    e1 = a > b ? f : g; >>    e2 = a > b ? c : d; >> >>    return; >> } >> ----------------------------------------------------------------------- >> >> compile to assembler with option “-O3 -S” : >> ----------------------------------------------------------------------- >> func: >>    cmplt a1, a0 >>    ld.w  t1, (sp, 0) >>    ld.w  t0, (sp, 4) >>    movt  t0, t1 >>    cmplt a1, a0 >>    movt  a3, a2 >>    lrw a1, e2 >>    lrw a2, e1 >>    st.w  a3, (a1, 0) >>    st.w  t0, (a2, 0) >>    rts >> ----------------------------------------------------------------------- >> There is an extra “cmplt a1, a0" in the above code without cse_cc. >> This situation mainly occurs when a relatively short branch jump is >> converted into a conditional execution instruction. And the CSE pass >> can not reduce the same conditional comparison instruction . Here is >> the rtx sequence after “cse2” pass. >> >> ----------------------------------------------------------------------- >> (insn 28 13 29 2 (set (reg:CC 33 c) >>          (gt:CC (reg/v:SI 77 [ a ]) >>              (reg/v:SI 78 [ b ]))) func.c:5 1099 {*cmpgtsi} >>       (nil)) >> (insn 29 28 30 2 (set (reg/v:SI 82 [ g ]) >>          (if_then_else:SI (eq (reg:CC 33 c) >>                  (const_int 0 [0])) >>              (reg/v:SI 82 [ g ]) >>              (reg/v:SI 81 [ f ]))) func.c:5 983 {movf} >>       (expr_list:REG_DEAD (reg/v:SI 81 [ f ]) >>          (expr_list:REG_DEAD (reg:CC 33 c) >>              (nil)))) >> (insn 30 29 31 2 (set (reg:CC 33 c) >>          (gt:CC (reg/v:SI 77 [ a ]) >>              (reg/v:SI 78 [ b ]))) func.c:5 1099 {*cmpgtsi} >>       (expr_list:REG_DEAD (reg/v:SI 78 [ b ]) >>          (expr_list:REG_DEAD (reg/v:SI 77 [ a ]) >>              (nil)))) >> (insn 31 30 18 2 (set (reg/v:SI 80 [ d ]) >>          (if_then_else:SI (eq (reg:CC 33 c) >>                  (const_int 0 [0])) >>              (reg/v:SI 80 [ d ]) >>              (reg/v:SI 79 [ c ]))) func.c:5 983 {movf} >>       (expr_list:REG_DEAD (reg/v:SI 79 [ c ]) >>          (expr_list:REG_DEAD (reg:CC 33 c) >>              (nil)))) >> ----------------------------------------------------------------------- >> >> It doesn't seem to check the same conditional comparison instruction >> .So I wrote this to solve this problem, but I am not sure if this is >> the best way : ) >> >> PS, the same conditional comparison instruction cannot be reduced with >> the latest version gcc with C-SKY because I just insert the “cse_cc” >> after “cse1”, when I insert after “cse2”, this problem can be solved >> very well. > > Thanks, this is very helpful.  I've verified this and I'm moving the > pass as you suggest, adding a more detailed comment to the source to > explain what the pass is for, and adding your test case to the > testsuite.  This will be included when I resubmit the patches to address > other review comments too. > > Jeff, does that adequately address your concerns about whether the pass > is useful? I think the pass is papering over problems elsewhere (see my most other reply on this thread). I do think it would be useful to take that code and create a test based on it. I suspect you'll want to verify multiple GT expressions prior to CSE2 and that after CSE2 you have a single GT expression. Presumably it'd be in the csky specific test. jeff