From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com [IPv6:2607:f8b0:4864:20::22d]) by sourceware.org (Postfix) with ESMTPS id C88B13972492 for ; Fri, 27 Nov 2020 09:23:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C88B13972492 Received: by mail-oi1-x22d.google.com with SMTP id w15so5132135oie.13 for ; Fri, 27 Nov 2020 01:23:57 -0800 (PST) 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=xBvRJrM7yTscu79XUT1w6kRVTpUHB3rSYgUkCyjjGSI=; b=G/WUXICmDa3LupViBq9TE1ZtiZLn/PSnr/3BEyqj5+mx5VAS2bSCzeDYJmvLbyINdi fuYHvC4VMcTuTDBQitNeyEcOEdxAiouFzpMhRzXrAHpQLdpHnvPEBuCue2ops7fvD/nq vscwt5cUC7o0cfJ+JzI46PofR1qgHYy6GAJlVm0MPWXMXURug2mNL5GGjodBJ3YLTu0+ wA48Wl0GVdTUs3WLRGSHzAq3g+OHHFoNXglSoGPIGxdV7d5KYaOkmBPcxC3A2qiqx+G2 +6C5Qe4NWPUGCaDCBYh0VQE2S8zc9aeVlc3HBx8Ur6kDLJ/7CGADUOk2OvAMnEJQqCyS 5GGA== X-Gm-Message-State: AOAM533pdj6R/vaMof9FZFugbWAaFPsajWa2iky1rQrdHf9cUZOpgYT3 rjFJkMH54ha2ngOIOK+N3YZ/AZK+uh9+mECDH5ovkA== X-Google-Smtp-Source: ABdhPJwt+xwXPIsJgBSPy9H/u6nNuqlyaZCkEo3z04Pam8PjxHHMuujFhcHyLl2MhJl+RKFRbv7l6wA3ZCRyquYpRd8= X-Received: by 2002:aca:b355:: with SMTP id c82mr4624936oif.48.1606469037098; Fri, 27 Nov 2020 01:23:57 -0800 (PST) MIME-Version: 1.0 References: <4133f299-0b2b-b0de-6e36-51d59aecda0d@arm.com> <835de12f-56a2-4a57-8cac-cfc443bda5f5@arm.com> In-Reply-To: <835de12f-56a2-4a57-8cac-cfc443bda5f5@arm.com> From: Christophe Lyon Date: Fri, 27 Nov 2020 10:23:46 +0100 Message-ID: Subject: Re: [committed obvious][arm] Add test that was missing from old commit [PR91816] To: Stam Markianos-Wright Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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, 27 Nov 2020 09:23:59 -0000 On Fri, 27 Nov 2020 at 02:03, Stam Markianos-Wright wrote: > > On 26/11/2020 09:01, Christophe Lyon wrote: > > On Wed, 25 Nov 2020 at 14:24, Stam Markianos-Wright via Gcc-patches > > wrote: > >> > >> Hi all, > >> > >> A while back I submitted GCC10 commit: > >> > >> 44f77a6dea2f312ee1743f3dde465c1b8453ee13 > >> > >> for PR91816. > >> > >> Turns out I was an idiot and forgot to include the test in the actual > >> git commit, even my entire patch had been approved. > >> > >> Tested that the test still passes on a cross arm-none-eabi and also in a > >> Cortex A-15 bootstrap with no regressions. > >> > >> Submitting this as Obvious to gcc-11 and backporting to gcc-10. > >> > > > > Hi, > > > > This new test fails when forcing -mcpu=cortex-m3/4/5/7/33: > > FAIL: gcc.target/arm/pr91816.c scan-assembler-times beq\\t.L[0-9] 2 > > FAIL: gcc.target/arm/pr91816.c scan-assembler-times beq\\t.Lbcond[0-9] 1 > > FAIL: gcc.target/arm/pr91816.c scan-assembler-times bne\\t.L[0-9] 2 > > FAIL: gcc.target/arm/pr91816.c scan-assembler-times bne\\t.Lbcond[0-9] 1 > > > > I didn't check manually what is generated, can you have a look? > > > > Oh wow thank you for spotting this! > > It looks like the A class target that I had tested had a tendency to > emit a movw/movt pair, whereas these M class targets would emit a single > ldr. This resulted in an overall shorter jump for these targets that did > not trigger the new far-branch code. > > The test passes after... doubling it's own size: > > > > #define HW3 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 > #define HW4 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 > #define HW5 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 > +#define HW6 HW5 HW5 > > __attribute__((noinline,noclone)) void f1 (int a) > { > @@ -25,7 +26,7 @@ __attribute__((noinline,noclone)) void f2 (int a) > > __attribute__((noinline,noclone)) void f3 (int a) > { > - if (a) { HW5 } > + if (a) { HW6 } > } > > __attribute__((noinline,noclone)) void f4 (int a) > @@ -41,7 +42,7 @@ __attribute__((noinline,noclone)) void f5 (int a) > > __attribute__((noinline,noclone)) void f6 (int a) > { > - if (a == 1) { HW5 } > + if (a == 1) { HW6 } > } > > But this does effectively double the compilation time of an already > quite large test. Would that be ok? I guess that's OK for me. We can probably increase the timeout value if needed. > > Overall this is the edge case testing that the compiler behaves > correctly with a branch in huge compilation unit, so it would be nice to > have test coverage of it on as many targets as possible... but also > kinda rare. > > Hope this helps! > > Cheers, > Stam > > > > > Thanks, > > > > Christophe > > > > > > > > > >> Thanks, > >> Stam Markianos-Wright > >> > >> gcc/testsuite/ChangeLog: > >> PR target/91816 > >> * gcc.target/arm/pr91816.c: New test. >