From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 814DA3858416; Fri, 30 Jun 2023 16:53:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 814DA3858416 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1688143998; bh=lKaDHDbbmHyZ4cjEiUSBUn61nkoO8bthtepWU1XzJvo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=HzVTC2Jm5eLg9KmcuIEptnhTf5c3G38XjLbNcUAdDFP5Vzos5GHecZNGCeGgcWbqC jhCMIVbOrL6Iif+pOvEXOrcKvRgKgjmXwZgvlgJ4Mzd5acmQ1755Eoy5PP446fCQcX 6Mc+eul7YH4/LTgeCXX4aKJiVy5k8xK+XhzGVp90= From: "gjl at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug target/110217] [avr] SREG: use BSET and BCLR instead of load/modify/write Date: Fri, 30 Jun 2023 16:53:18 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: target X-Bugzilla-Version: unknown X-Bugzilla-Keywords: missed-optimization X-Bugzilla-Severity: enhancement X-Bugzilla-Who: gjl at gcc dot gnu.org X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_severity Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D110217 Georg-Johann Lay changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement --- Comment #1 from Georg-Johann Lay --- The only case where this might make sense is for bit 7 (the I-flag), however the established coding style is to use cli() and sei() from AVR-LibC, cf. documentation of #include : https://www.nongnu.org/avr-libc/user-manual/group__avr__interrupts.html For more sophitsticated use cases there is even ATOMIC_BLOCK and friends provided by #include , cf: https://www.nongnu.org/avr-libc/user-manual/group__util__atomic.html This has the additional benefit of being more readable than bit manipulatio= ns.=20=20 Apart from that, the proposed patch won't work for indirect addressing, or = when the compiler is turning direct addresses to indirect addresses (using CSE e= tc, common subexpression elimination and similar strategies). Also the patch relies on insn combine which only runs when optimization is = on, thus any application which relies on that optimization will glitch at -O0. So I am inclined to "won't fix" this PR. Maybe you just missed avr/interrupt.h and / or util/atomic.h ? If you must not use AVR-LibC for some reason, then the next best approach i= s to use __builtin_avr_sei(), cf. AVR built-in functions: https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/AVR-Built-in-Functions.html Or implement it as static inline function that does __asm volatile ("sei" := :: "memory") if you are not allowed to use built-ins.=