From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x244.google.com (mail-oi1-x244.google.com [IPv6:2607:f8b0:4864:20::244]) by sourceware.org (Postfix) with ESMTPS id 81393393C860 for ; Fri, 28 Aug 2020 13:25:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 81393393C860 Received: by mail-oi1-x244.google.com with SMTP id u24so771348oic.7 for ; Fri, 28 Aug 2020 06:25:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GbshFyqzjrR87U5ToX4jVneY2smd/d93Z0sBOk4ymDw=; b=RgUx/0wrFahE/1Tzneeq6AozSxY7JtYMu5CfA1wt6FitZwTgghK4ZbJNdJ5Lo88KKE JqBgQu4naLxJ4zku2syL9YsKb0BOzJroXn+Or2R5C14azQIOgRdo+zOUT1rOYbjmQ3rC Mj+ZWEUaRdj2sVytOJM2dhREMYyvZk/ysO7dhfEXwsswos9tbunrSaIcTDHDtiiT1yBe l92UXDUF7606TDU/LdUoXpHwm1jB72GpqYZ2vOonY1fW5yLxdizhdEnEzdlm0Qktuf9c cnE2JkrcfMJeMKRkMskiFfljzhnzsmOv3UEeJlu8rsRwiCjKNlZTVyiLl4QV01p0UDwP QDHQ== X-Gm-Message-State: AOAM530hEEC7+dHME0Yfnqdvfv7/1/HGB3EUYNbsKgq13R02DM17uYp4 MPRgaC970TLItqOy6WGM0H5JpJcApBsTwiyomjkK9g== X-Google-Smtp-Source: ABdhPJxBX82z7AKFohKECs2m1wzxygGv9vhCHPXtQGicxWnxNUwtjmbcRh+StTlMkyxP0yRbpal0a4R5E0Wa7p7pOf4= X-Received: by 2002:a05:6808:913:: with SMTP id w19mr871232oih.48.1598621104721; Fri, 28 Aug 2020 06:25:04 -0700 (PDT) MIME-Version: 1.0 References: <1598534847-8544-1-git-send-email-christophe.lyon@linaro.org> In-Reply-To: From: Christophe Lyon Date: Fri, 28 Aug 2020 15:24:53 +0200 Message-ID: Subject: Re: [PATCH] arm: Fix switch tables for thumb-1 with -mpure-code [PR96768] To: Richard Earnshaw Cc: gcc Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: Fri, 28 Aug 2020 13:25:06 -0000 On Fri, 28 Aug 2020 at 14:00, Richard Earnshaw wrote: > > On 27/08/2020 14:27, Christophe Lyon via Gcc-patches wrote: > > In comment 14 from PR94538, it was suggested to switch off jump tables > > on thumb-1 cores when using -mpure-code, like we already do for thumb-2. > > > > This is what this patch does, and also restores the previous value of > > CASE_VECTOR_PC_RELATIVE since it was not the right way of handling > > this. > > > > It also adds a new test, since the existing no-casesi.c did not catch > > this problem. > > > > Tested by running the whole testsuite with -mpure-code -mcpu=cortex-m0 > > -mfloat-abi=soft, no regression and the new test passes (and fails > > without the fix). > > > > 2020-08-27 Christophe Lyon > > > > gcc/ > > * config/arm/arm.h (CASE_VECTOR_PC_RELATIVE): Remove condition on > > -mpure-code. > > * config/arm/thumb1.md (tablejump): Disable when -mpure-code. > > > > gcc/testsuite/ > > * gcc.target/arm/pure-code/pr96768.c: New test. > > --- > > gcc/config/arm/arm.h | 5 ++--- > > gcc/config/arm/thumb1.md | 2 +- > > gcc/testsuite/gcc.target/arm/pure-code/pr96768.c | 21 +++++++++++++++++++++ > > 3 files changed, 24 insertions(+), 4 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/arm/pure-code/pr96768.c > > > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > > index 3887c51..7d43721 100644 > > --- a/gcc/config/arm/arm.h > > +++ b/gcc/config/arm/arm.h > > @@ -1975,10 +1975,9 @@ enum arm_auto_incmodes > > for the index in the tablejump instruction. */ > > #define CASE_VECTOR_MODE Pmode > > > > -#define CASE_VECTOR_PC_RELATIVE ((TARGET_THUMB2 \ > > +#define CASE_VECTOR_PC_RELATIVE (TARGET_THUMB2 \ > > || (TARGET_THUMB1 \ > > - && (optimize_size || flag_pic))) \ > > - && (!target_pure_code)) > > + && (optimize_size || flag_pic))) > > > > > > #define CASE_VECTOR_SHORTEN_MODE(min, max, body) \ > > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > > index f0129db..602039e 100644 > > --- a/gcc/config/arm/thumb1.md > > +++ b/gcc/config/arm/thumb1.md > > @@ -2012,7 +2012,7 @@ (define_insn "*epilogue_insns" > > (define_expand "tablejump" > > [(parallel [(set (pc) (match_operand:SI 0 "register_operand")) > > (use (label_ref (match_operand 1 "" "")))])] > > - "TARGET_THUMB1" > > + "TARGET_THUMB1 && !target_pure_code" > > " > > if (flag_pic) > > { > > diff --git a/gcc/testsuite/gcc.target/arm/pure-code/pr96768.c b/gcc/testsuite/gcc.target/arm/pure-code/pr96768.c > > new file mode 100644 > > index 0000000..fd4cad5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/arm/pure-code/pr96768.c > > @@ -0,0 +1,21 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-mpure-code" } */ > > + > > +int f2 (int x, int y) > > +{ > > + switch (x) > > + { > > + case 0: return y + 0; > > + case 1: return y + 1; > > + case 2: return y + 2; > > + case 3: return y + 3; > > + case 4: return y + 4; > > + case 5: return y + 5; > > + } > > + return y; > > +} > > + > > +/* We do not want any load from literal pool, but accept loads from r7 > > + (frame pointer, used at -O0). */ > > +/* { dg-final { scan-assembler-not "ldr\tr\\d+, \\\[r\[0-689\]+\\\]" } } */ > > +/* { dg-final { scan-assembler "text,\"0x20000006\"" } } */ > > > > Having switch tables is only a problem if they are embedded in the .text > segment. But the case you show in the PR has them in the .rodata > segment, so is this really necessary? > Well, in the original PR94538, comment #14, Wilco said this was the best option. > R.