From patchwork Thu May 25 17:55:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Georg-Johann Lay X-Patchwork-Id: 1786229 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gjlay.de header.i=@gjlay.de header.a=rsa-sha256 header.s=strato-dkim-0002 header.b=pRMftRt6; dkim=fail reason="signature verification failed" header.d=gjlay.de header.i=@gjlay.de header.a=ed25519-sha256 header.s=strato-dkim-0003 header.b=bRqIOyek; dkim-atps=neutral Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QRwh90MCvz20Q1 for ; Fri, 26 May 2023 03:55:36 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BE9F33857721 for ; Thu, 25 May 2023 17:55:33 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [81.169.146.218]) by sourceware.org (Postfix) with ESMTPS id E97123858D32 for ; Thu, 25 May 2023 17:55:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E97123858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gjlay.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=gjlay.de ARC-Seal: i=1; a=rsa-sha256; t=1685037316; cv=none; d=strato.com; s=strato-dkim-0002; b=LsJWanccf+3lnR2Uvx+50eMQAW81GD4FSS5bjucW6jTZnq91OYrl+sKfws+9kIHa1G AUXgROEnE/GUDj9npxBcaFoRAi3987kGpcSQr996ZEooLyt6ZsSVbDo/4Z6hJDInKTAt Wth9Ny3G+jGd8bnXxHzaZB2QLqpyTSNf4PqlEYHJB4Njj1F3KVPe7t7ctYv5LEDe8aKG +yC0OZdxD3br9BNVfVT60kGIIix+XhjHYVeCfCNTjQp02wwmXVLrYhJ6rCsSoq+/F1kQ GDEpbYB+dC6vF22mWQEiqhtMrzDJ99mlUIkdBEB3637celSeQ9HcRYvHE07q6tZT1aYp e2+g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1685037316; s=strato-dkim-0002; d=strato.com; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=xdcPhnnsy5MWp1gjZ0BM8kk1RElJOKy0W4Nd1jp1TGo=; b=fUXNl9f+lSxH0qKs/WTFTWRucOEiRtiws+FHRh22CRcdwe+52mtbecyH3zyXBu4O3x r83XwH5lwp0dKLpijoRh7pWMSZvxk6r1pBzUozxfrIUYjRfCjKsE8pDqmcBvEQf5F1Tv gFYZyjFowlaQXTXu50Azpp0di5umdZtTKw4JVyUPajCSZQdUvrBCRmYmVQlYemTo1gpZ TAj9LnI62rf6rf44hbal3KvGHp9P0JjwnU0XrRKL7A/XG86GGt5k9+UNNrsZKBXInzyY hVo1TIysGgRYH1+YmzB3iMmakpPc5pxjQO7ySstdHDNHcVTVLiZ5L+I+527PlmHRWhVX BOgA== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo00 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1685037316; s=strato-dkim-0002; d=gjlay.de; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=xdcPhnnsy5MWp1gjZ0BM8kk1RElJOKy0W4Nd1jp1TGo=; b=pRMftRt6iwZZWH3dJouE9g/hWPmK9TV/o0UJyxql9HCV1UDGEQQ8T6MtdMRi226x4p Db+63u4L6uvSqA/bXTqxkb5KFRFkXH6Y7JcPfQPa2n2WDnzMjNvT6d4sCxxMa/w6+F+E VeFJOfjKujDfIETMRxOqqlo5C5Fjpes5+CyIgTwjTmjkmSp+pRcdzQOtR3uLvLg3woi1 JjLE0JdN3LH6lhzOf9m8z47Kj/rKoR1TJVw380rahJoqkvKrBcGi5ZLMaXCen1AjL/FM AizwilQfvVYNzVIlHWcubgY+rvLXAm7tkmN3FNz+4BKJzHeTELfn6yJ+JpVb7joOD0DF y40Q== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1685037316; s=strato-dkim-0003; d=gjlay.de; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=xdcPhnnsy5MWp1gjZ0BM8kk1RElJOKy0W4Nd1jp1TGo=; b=bRqIOyek1nDpf03nYTGwSFfT8u8tcNJcad+hvdmlSNDn+EH7ENelkKgl69TA+ipYa4 Z6pQ0Gw5kaVs2LA08rDg== X-RZG-AUTH: ":LXoWVUeid/7A29J/hMvvT3koxZnKT7Qq0xotTetVnKkRmM69o2y+LiO3MutATA==" Received: from [192.168.2.102] by smtp.strato.de (RZmta 49.4.0 DYNA|AUTH) with ESMTPSA id z691f1z4PHtGuSP (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Thu, 25 May 2023 19:55:16 +0200 (CEST) Message-ID: <313b5291-80f1-01db-f74c-da7176853eb0@gjlay.de> Date: Thu, 25 May 2023 19:55:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: [avr,committed]: Implement PR104327 for avr Content-Language: en-US To: Richard Biener Cc: gcc-patches@gcc.gnu.org References: <72200c1e-a732-0aad-53ca-bcb895e028bb@gjlay.de> <86AC10B7-7919-4FC7-B36E-7CC2DABE444E@gmail.com> From: Georg-Johann Lay In-Reply-To: <86AC10B7-7919-4FC7-B36E-7CC2DABE444E@gmail.com> X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_PASS, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Am 25.05.23 um 17:07 schrieb Richard Biener: > > >> Am 25.05.2023 um 16:22 schrieb Georg-Johann Lay : >> >>  >> >>> Am 25.05.23 um 08:35 schrieb Richard Biener: >>>> On Wed, May 24, 2023 at 5:44 PM Georg-Johann Lay wrote: >>>> Am 24.05.23 um 11:38 schrieb Richard Biener: >>>>> On Tue, May 23, 2023 at 2:56 PM Georg-Johann Lay wrote: >>>>>> >>>>>> PR target/104327 not only affects s390 but also avr: >>>>>> The avr backend pre-sets some options depending on optimization level. >>>>>> The inliner then thinks that always_inline functions are not eligible >>>>>> for inlining and terminates with an error. >>>>>> >>>>>> Proposing the following patch that implements TARGET_CAN_INLINE_P. >>>>>> >>>>>> Ok to apply? >>>>>> >>>>>> Johann >>>>>> >>>>>> target/104327: Allow more inlining between different optimization levels. >>>>>> >>>>>> avr-common.cc introduces the following options that are set depending >>>>>> on optimization level: -mgas-isr-prologues, -mmain-is-OS-task and >>>>>> -fsplit-wide-types-early. The inliner thinks that different options >>>>>> disallow cross-optimization inlining, so provide can_inline_p. >>>>>> >>>>>> gcc/ >>>>>> PR target/104327 >>>>>> * config/avr/avr.cc (avr_can_inline_p): New static function. >>>>>> (TARGET_CAN_INLINE_P): Define to that function. >>>>>> diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc >>>>>> index 9fa50ca230d..55b48f63865 100644 >>>>>> --- a/gcc/config/avr/avr.cc >>>>>> +++ b/gcc/config/avr/avr.cc >>>>>> @@ -1018,6 +1018,22 @@ avr_no_gccisr_function_p (tree func) >>>>>> return avr_lookup_function_attribute1 (func, "no_gccisr"); >>>>>> } >>>>>> >>>>>> + >>>>>> +/* Implement `TARGET_CAN_INLINE_P'. */ >>>>>> +/* Some options like -mgas_isr_prologues depend on optimization level, >>>>>> + and the inliner might think that due to different options, inlining >>>>>> + is not permitted; see PR104327. */ >>>>>> + >>>>>> +static bool >>>>>> +avr_can_inline_p (tree /* caller */, tree callee) >>>>>> +{ >>>>>> + // For now, dont't allow to inline ISRs. If the user actually wants >>>>>> + // to inline ISR code, they have to turn the body of the ISR into an >>>>>> + // ordinary function. >>>>>> + >>>>>> + return ! avr_interrupt_function_p (callee); >>>>> >>>>> I'm not sure if AVR has ISA extensions but the above will likely break >>>>> things like >>>>> >>>>> void __attribute__((target("-mX"))) foo () { asm ("isa X opcode"); >>>>> stmt-that-generates-X-ISA; } >>>> >>>> This yields >>>> >>>> warning: target attribute is not supported on this machine [-Wattributes] >>> Ah, that's an interesting fact. So that indeed leaves >>> __attribute__((optimize(...))) >>> influencing the set of active target attributes via the generic option target >>> hooks like in your case the different defaults. >>>> avr has -mmcu= target options, but switching them in mid-air >>>> won't work because the file prologue might already be different >>>> and incompatible across different architectures. And I never >>>> saw any user requesting such a thing, and I can't imagine >>>> any reasonable use case... If the warning is not strong enough, >>>> may be it can be turned into an error, but -Wattributes is not >>>> specific enough for that. >>> Note the target attribute is then simply ignored. >>>>> void bar () >>>>> { >>>>> if (cpu-has-X) >>>>> foo (); >>>>> } >>>>> >>>>> if always-inlines are the concern you can use >>>>> >>>>> bool always_inline >>>>> = (DECL_DISREGARD_INLINE_LIMITS (callee) >>>>> && lookup_attribute ("always_inline", >>>>> DECL_ATTRIBUTES (callee))); >>>>> /* Do what the user says. */ >>>>> if (always_inline) >>>>> return true; >>>>> >>>>> return default_target_can_inline_p (caller, callee); >>>> >>>> The default implementation of can_inline_p worked fine for avr. >>>> As far as I understand, the new behavior is due to clean-up >>>> of global states for options? >>> I think the last change was r8-2658-g9b25e12d2d940a which >>> for targets without target attribute support made it more likely >>> to run into the default hook actually comparing the options. >>> Previously the "default" was oddly special-cased but you >>> could have still run into compares with two different set of >>> defaults when there's another "default" default. Say, compile >>> with -O2 and have one optimize(0) and one optimize(Os) >>> function it would compare the optimize(0) and optimize(Os) >>> set if they were distinct from the -O2 set. That probably never >>> happened for AVR. >>>> So I need to take into account inlining costs and decide on that >>>> whether it's preferred to inline a function or not? >>> No, the hook isn't about cost, it's about full incompatibility. So >>> if the different -m options that could be in effect for AVR in >>> a single TU for different functions never should prevent inlining >>> then simply make the hook return true. If there's a specific >>> option (that can differ from what specified on the compiler >>> command line!) that should, then you should compare the >>> setting of that option from the DECL_FUNCTION_SPECIFIC_TARGET >>> of the caller and the callee. >>> But as far as I can see simply returning true should be correct >>> for AVR, or like your patch handle interrupts differently (though >>> the -Winline diagnostic will tell the user there's a mismatch in >>> target options which might be confusing). >> >> Ok, simply "true" sounds reasonable. Is that change ok then? > > Yes. > > Richard Committed as https://gcc.gnu.org/r14-1245 Johann \f --- a/gcc/config/avr/avr.cc +++ b/gcc/config/avr/avr.cc @@ -1018,6 +1018,19 @@ avr_no_gccisr_function_p (tree func) return avr_lookup_function_attribute1 (func, "no_gccisr"); } + +/* Implement `TARGET_CAN_INLINE_P'. */ +/* Some options like -mgas_isr_prologues depend on optimization level, + and the inliner might think that due to different options, inlining + is not permitted; see PR104327. */ + +static bool +avr_can_inline_p (tree /* caller */, tree /* callee */) +{ + // No restrictions whatsoever. + return true; +} + /* Implement `TARGET_SET_CURRENT_FUNCTION'. */ /* Sanity cheching for above function attributes. */ @@ -14770,6 +14783,9 @@ avr_float_lib_compare_returns_bool (machine_mode mode, enum rtx_code) #undef TARGET_MD_ASM_ADJUST #define TARGET_MD_ASM_ADJUST avr_md_asm_adjust +#undef TARGET_CAN_INLINE_P +#define TARGET_CAN_INLINE_P avr_can_inline_p + struct gcc_target targetm = TARGET_INITIALIZER;