From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [85.215.255.22]) by sourceware.org (Postfix) with ESMTPS id 775913858D28; Wed, 24 May 2023 15:44:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 775913858D28 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=1684943061; cv=none; d=strato.com; s=strato-dkim-0002; b=Wr/TbLhbJofBKS1noQdTpxmaxXhWOpCDSJLCRDK5zyiYsqqiSMlkMF7OLbJ0WDoEOF ZFx6LsLPkRM0mBeSMpd50fFMLrG91rEdk1dDuxxeGYWOJ8R8i3ig0v7AC7lL21yt7JXE UdRL6ZxRZ7VqFov78X3JmXDEp8I9W4jkzo65P+3USeRy7pGwT/H6pzuEL1nTXjxHX7rF BFKGWQX9abJ3CTtNtMTy+fXgQMPQ9yrDMTAxXA355vsHOXIB60WIgzUsPNFjoG4o+akH zBTjB+CqXhJCLIOiFORi+p6/x2HD1RvqS88np6/w7urjgnA7SQgrZyyKy50S++umEDhG zqrA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1684943061; 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=ZSABejma540pu+RozTeN0bnfrrJK3Ny4gyAa2ceyCas=; b=bUv1OIMmbJUj6tjGVo66DcDhhrhZ3PwBCPRVI01rkhLomi73ivKJOLwPqGzvGND+zX Iq1sGwayaWl2lCgxojWo+p6bt8XAiF4IABCFhnyoogUV7WBLA3phK4sTTTujXFXN3Vdu cau4pi97fD7dwd1H/wrm2EEPg3s/clVB+FszXI2ZPKgstZxNWp/BzZqbimmvcLa+gTiJ 22X1GuLY6wJ4fT3oXzlAqnXFHHdt7YmSyOQFar1SVg8G0ZU/WechJxXZLUPA0FVvzMUf YKVR6Ziq/erXk0RiNPT1Hgsvu7yDh4cCIlXyDQ3QvKbqKL+B/gn6JFY8Jjwi18OHyAx/ er2g== 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=1684943061; 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=ZSABejma540pu+RozTeN0bnfrrJK3Ny4gyAa2ceyCas=; b=ZzFAH5Y+dEtKbV5sofgYR0UBmU+ovA14C1Xm63iUZhga/R39HHWvZ7SLI3RxFAJGjZ fcDR60BOB+8IZqYsr6g1CW4gZCSqXdrsMv0q+K2QM8LcHxcYyrHoMC1dJ6l1E4rrggVB tTn5dAhW0vDP9MWFuXYviiPCDWrOlMVBzR5xeVKCJ2CmDBiREwTqXQQEmGd+DoN49k9P oPOiWm0UuMYwNurk8IjTLLrFBYiT31C1N7sPrttK2o98dvarW1Ul/365h/5EnPTrP1/2 w52nCGGDsFqf3EpO8wwEPlVO/cD+FO9ujkMT4H6GSY8VcRwKMas+31QEUOBzxe0XZxiH 2dCg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1684943061; 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=ZSABejma540pu+RozTeN0bnfrrJK3Ny4gyAa2ceyCas=; b=IfhytIVvhMUZg8BCWdKhAVRMDMi7qDvLjjUf2gVjRPFv8CNq1ILARpnqmi3jB2CR0s USJOWVWXufT3pm20KXBw== 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 z691f1z4OFiLmxC (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Wed, 24 May 2023 17:44:21 +0200 (CEST) Message-ID: Date: Wed, 24 May 2023 17:44:20 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [patch]: Implement PR104327 for avr Content-Language: en-US To: Richard Biener Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org References: <7a3552f0-ac4d-754f-a7ba-cba3ff9a4a41@gjlay.de> <6ecf31c6-1fa2-330d-6744-2a1d013fd530@gjlay.de> From: Georg-Johann Lay In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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] 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. > 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? So I need to take into account inlining costs and decide on that whether it's preferred to inline a function or not? Johann >> +} >> + >> /* Implement `TARGET_SET_CURRENT_FUNCTION'. */ >> /* Sanity cheching for above function attributes. */ >> >> @@ -14713,6 +14729,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;