From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76465 invoked by alias); 25 Feb 2019 10:05:52 -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 76445 invoked by uid 89); 25 Feb 2019 10:05:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 25 Feb 2019 10:05:50 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 848F380D; Mon, 25 Feb 2019 02:05:48 -0800 (PST) Received: from [10.2.206.47] (e120808-lin.cambridge.arm.com [10.2.206.47]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 927DE3F703; Mon, 25 Feb 2019 02:05:47 -0800 (PST) Subject: Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341) To: Jakub Jelinek , Richard Earnshaw , Ramana Radhakrishnan , James Greenhalgh Cc: "gcc-patches@gcc.gnu.org" References: <20190223002051.GL7611@tucnak> From: Kyrill Tkachov Message-ID: <764e7bd6-a566-3b00-eaff-4b7914166464@foss.arm.com> Date: Mon, 25 Feb 2019 10:23:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190223002051.GL7611@tucnak> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2019-02/txt/msg01912.txt.bz2 Hi Jakub, On 2/23/19 12:20 AM, Jakub Jelinek wrote: > Hi! > > The testcase in the PR doesn't hoist any memory loads from the large > switch > before the switch on aarch64 and arm (unlike e.g. x86), because the > arm/aarch64 casesi patterns don't properly annotate the memory load > from the > jump table.  It is created by gen_* and in RTL directly one can't specify > the needed flags (MEM_READONLY_P and MEM_NOTRAP_P). > > Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and > aarch64-linux, ok for trunk? > Since you're changing Arm and Thumb-2-specific paths can you please make sure to bootstrap configurations --with-mode=arm and --with-mode=thumb. Otherwise the arm parts (and aarch64 FWIW) look ok to me. Most of the patch content is splitting the define_insn into a define_expand so that we can annotate the MEM with the right attributes. Thanks, Kyrill > 2019-02-23  Jakub Jelinek  > >         PR target/70341 >         * config/arm/arm.md (arm_casesi_internal): New define_expand.  > Rename >         old define_insn to ... >         (*arm_casesi_internal): ... this.  Add mode to LABEL_REFs. >         * config/arm/thumb2.md (thumb2_casesi_internal): New > define_expand. >         Rename old define_insn to ... >         (*thumb2_casesi_internal): ... this.  Add mode to LABEL_REFs. >         (thumb2_casesi_internal_pic): New define_expand. Rename old >         define_insn to ... >         (*thumb2_casesi_internal_pic): ... this.  Add mode to LABEL_REFs. >         * config/aarch64/aarch64.md (casesi): Create the casesi_dispatch >         MEM manually here, set MEM_READONLY_P and MEM_NOTRAP_P on it. > > --- gcc/config/arm/arm.md.jj    2019-02-18 20:48:32.643732307 +0100 > +++ gcc/config/arm/arm.md       2019-02-21 14:40:50.603452028 +0100 > @@ -8914,16 +8914,35 @@ (define_expand "casesi" > >  ;; The USE in this pattern is needed to tell flow analysis that this is >  ;; a CASESI insn.  It has no other purpose. > -(define_insn "arm_casesi_internal" > +(define_expand "arm_casesi_internal" > +  [(parallel [(set (pc) > +              (if_then_else > +               (leu (match_operand:SI 0 "s_register_operand") > +                    (match_operand:SI 1 "arm_rhs_operand")) > +               (match_dup 4) > +               (label_ref:SI (match_operand 3 "")))) > +             (clobber (reg:CC CC_REGNUM)) > +             (use (label_ref:SI (match_operand 2 "")))])] > +  "TARGET_ARM" > +{ > +  operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4)); > +  operands[4] = gen_rtx_PLUS (SImode, operands[4], > +                             gen_rtx_LABEL_REF (SImode, operands[2])); > +  operands[4] = gen_rtx_MEM (SImode, operands[4]); > +  MEM_READONLY_P (operands[4]) = 1; > +  MEM_NOTRAP_P (operands[4]) = 1; > +}) > + > +(define_insn "*arm_casesi_internal" >    [(parallel [(set (pc) >                 (if_then_else >                  (leu (match_operand:SI 0 "s_register_operand" "r") >                       (match_operand:SI 1 "arm_rhs_operand" "rI")) >                  (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4)) > -                                (label_ref (match_operand 2 "" "")))) > -               (label_ref (match_operand 3 "" "")))) > +                                (label_ref:SI (match_operand 2 "" "")))) > +               (label_ref:SI (match_operand 3 "" "")))) >                (clobber (reg:CC CC_REGNUM)) > -             (use (label_ref (match_dup 2)))])] > +             (use (label_ref:SI (match_dup 2)))])] >    "TARGET_ARM" >    "* >      if (flag_pic) > --- gcc/config/arm/thumb2.md.jj 2019-01-01 12:37:28.280792453 +0100 > +++ gcc/config/arm/thumb2.md    2019-02-21 15:00:26.811137210 +0100 > @@ -1079,17 +1079,37 @@ (define_insn "thumb2_zero_extendqisi2_v6 >     (set_attr "neg_pool_range" "*,250")] >  ) > > -(define_insn "thumb2_casesi_internal" > +(define_expand "thumb2_casesi_internal" > +  [(parallel [(set (pc) > +              (if_then_else > +               (leu (match_operand:SI 0 "s_register_operand") > +                    (match_operand:SI 1 "arm_rhs_operand")) > +               (match_dup 4) > +               (label_ref:SI (match_operand 3 "")))) > +             (clobber (reg:CC CC_REGNUM)) > +             (clobber (match_scratch:SI 5)) > +             (use (label_ref:SI (match_operand 2 "")))])] > +  "TARGET_THUMB2 && !flag_pic" > +{ > +  operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4)); > +  operands[4] = gen_rtx_PLUS (SImode, operands[4], > +                             gen_rtx_LABEL_REF (SImode, operands[2])); > +  operands[4] = gen_rtx_MEM (SImode, operands[4]); > +  MEM_READONLY_P (operands[4]) = 1; > +  MEM_NOTRAP_P (operands[4]) = 1; > +}) > + > +(define_insn "*thumb2_casesi_internal" >    [(parallel [(set (pc) >                 (if_then_else >                  (leu (match_operand:SI 0 "s_register_operand" "r") >                       (match_operand:SI 1 "arm_rhs_operand" "rI")) >                  (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4)) > -                                (label_ref (match_operand 2 "" "")))) > -               (label_ref (match_operand 3 "" "")))) > +                                (label_ref:SI (match_operand 2 "" "")))) > +               (label_ref:SI (match_operand 3 "" "")))) >                (clobber (reg:CC CC_REGNUM)) >                (clobber (match_scratch:SI 4 "=&r")) > -             (use (label_ref (match_dup 2)))])] > +             (use (label_ref:SI (match_dup 2)))])] >    "TARGET_THUMB2 && !flag_pic" >    "* return thumb2_output_casesi(operands);" >    [(set_attr "conds" "clob") > @@ -1097,18 +1117,39 @@ (define_insn "thumb2_casesi_internal" >     (set_attr "type" "multiple")] >  ) > > -(define_insn "thumb2_casesi_internal_pic" > +(define_expand "thumb2_casesi_internal_pic" > +  [(parallel [(set (pc) > +              (if_then_else > +               (leu (match_operand:SI 0 "s_register_operand") > +                    (match_operand:SI 1 "arm_rhs_operand")) > +               (match_dup 4) > +               (label_ref:SI (match_operand 3 "")))) > +             (clobber (reg:CC CC_REGNUM)) > +             (clobber (match_scratch:SI 5)) > +             (clobber (match_scratch:SI 6)) > +             (use (label_ref:SI (match_operand 2 "")))])] > +  "TARGET_THUMB2 && flag_pic" > +{ > +  operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4)); > +  operands[4] = gen_rtx_PLUS (SImode, operands[4], > +                             gen_rtx_LABEL_REF (SImode, operands[2])); > +  operands[4] = gen_rtx_MEM (SImode, operands[4]); > +  MEM_READONLY_P (operands[4]) = 1; > +  MEM_NOTRAP_P (operands[4]) = 1; > +}) > + > +(define_insn "*thumb2_casesi_internal_pic" >    [(parallel [(set (pc) >                 (if_then_else >                  (leu (match_operand:SI 0 "s_register_operand" "r") >                       (match_operand:SI 1 "arm_rhs_operand" "rI")) >                  (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4)) > -                                (label_ref (match_operand 2 "" "")))) > -               (label_ref (match_operand 3 "" "")))) > +                                (label_ref:SI (match_operand 2 "" "")))) > +               (label_ref:SI (match_operand 3 "" "")))) >                (clobber (reg:CC CC_REGNUM)) >                (clobber (match_scratch:SI 4 "=&r")) >                (clobber (match_scratch:SI 5 "=r")) > -             (use (label_ref (match_dup 2)))])] > +             (use (label_ref:SI (match_dup 2)))])] >    "TARGET_THUMB2 && flag_pic" >    "* return thumb2_output_casesi(operands);" >    [(set_attr "conds" "clob") > --- gcc/config/aarch64/aarch64.md.jj    2019-01-19 09:39:18.847831222 > +0100 > +++ gcc/config/aarch64/aarch64.md       2019-02-21 15:25:27.874532191 > +0100 > @@ -622,13 +622,27 @@ (define_expand "casesi" >                                      operands[0], operands[2], > operands[4])); > >      operands[2] = force_reg (DImode, gen_rtx_LABEL_REF (DImode, > operands[3])); > -    emit_jump_insn (gen_casesi_dispatch (operands[2], operands[0], > -                                        operands[3])); > +    operands[2] > +      = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, operands[2], operands[0]), > +                       UNSPEC_CASESI); > +    operands[2] = gen_rtx_MEM (DImode, operands[2]); > +    MEM_READONLY_P (operands[2]) = 1; > +    MEM_NOTRAP_P (operands[2]) = 1; > +    emit_jump_insn (gen_casesi_dispatch (operands[2], operands[3])); >      DONE; >    } >  ) > > -(define_insn "casesi_dispatch" > +(define_expand "casesi_dispatch" > +  [(parallel > +    [(set (pc) (match_operand:DI 0 "")) > +     (clobber (reg:CC CC_REGNUM)) > +     (clobber (match_scratch:DI 2)) > +     (clobber (match_scratch:DI 3)) > +     (use (label_ref:DI (match_operand 1 "")))])] > +  "") > + > +(define_insn "*casesi_dispatch" >    [(parallel >      [(set (pc) >            (mem:DI (unspec [(match_operand:DI 0 "register_operand" "r") > @@ -637,7 +651,7 @@ (define_insn "casesi_dispatch" >       (clobber (reg:CC CC_REGNUM)) >       (clobber (match_scratch:DI 3 "=r")) >       (clobber (match_scratch:DI 4 "=r")) > -     (use (label_ref (match_operand 2 "" "")))])] > +     (use (label_ref:DI (match_operand 2 "" "")))])] >    "" >    "* >    return aarch64_output_casesi (operands); > >         Jakub