Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-1187-g678636ba0d-fm-20230113.001-g678636ba
Mime-Version: 1.0
Message-Id: <a1d48420-862e-47e6-9435-462e23bb6be3@app.fastmail.com>
In-Reply-To: <Y8ggiF+7k0rViXcY@slm.duckdns.org>
References: <20230117164041.1207412-1-arnd@kernel.org>
 <99ea6e86d2594b40a6de96cc821c447b@AcuMS.aculab.com>
 <Y8ggiF+7k0rViXcY@slm.duckdns.org>
Date:   Wed, 18 Jan 2023 21:32:45 +0100
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Tejun Heo" <tj@kernel.org>
Cc:     "Arnd Bergmann" <arnd@kernel.org>,
        "Lai Jiangshan" <jiangshanlai@gmail.com>,
        "Richard Clark" <richard.xnu.clark@gmail.com>,
        =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        "Andrey Grodzovsky" <andrey.grodzovsky@amd.com>,
        "Tetsuo Handa" <penguin-kernel@i-love.sakura.ne.jp>,
        "linux-kernel@vger.kernel.org," <linux-kernel@vger.kernel.org>,
        "David Laight" <David.Laight@aculab.com>
Subject: Re: [PATCH] workqueue: fix enum type for gcc-13
Content-Type: text/plain
Precedence: bulk

On Wed, Jan 18, 2023, at 17:38, Tejun Heo wrote:
>> From: Arnd Bergmann
>> > Sent: 17 January 2023 16:41
>> > 
>> > In gcc-13, the WORK_STRUCT_WQ_DATA_MASK constant is a signed 64-bit
>> > type on 32-bit architectures because the enum definition has both
>> > negative numbers and numbers above LONG_MAX in it:
>> > 
>> ...
>> >  	/* convenience constants */
>> >  	WORK_STRUCT_FLAG_MASK	= (1UL << WORK_STRUCT_FLAG_BITS) - 1,
>> > -	WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
>> > +	WORK_STRUCT_WQ_DATA_MASK = (unsigned long)~WORK_STRUCT_FLAG_MASK,
>> >  	WORK_STRUCT_NO_POOL	= (unsigned long)WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT,
>
> I have a hard time understanding why gcc would change its behavior so that
> there's no way to compile the same code in a consistent manner across two
> adjacent compiler versions. The new behavior is fine but it makes no sense
> to introduce it like this. If at all possible, marking gcc13 broken sounds
> about right to me.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107405 has some more
information on the change. In short, the old behavior was a gcc
extension that was somewhat surprising and not well documented,
while the new behavior is consistent with C23 and C++ as well
as easier to understand: Any constant that is defined as part of
an enum now has the same type as the enum itself, even if it fits
within a shorter type. In a definition like

enum e {
   A = -1,
   B = -1u,
};

the enum type has to be compatible with 'long long' because
anything shorter would not fit both -1 and -1u (UINT_MAX).
A and B were both signed types to match the signedness of the
enum type, but A was actually a 32-bit integer since that is
sufficient, while B was also a 64-bit type since it exceeds
INT_MAX. Now they are both the same type.

I don't think there is a chance they will revert to the old behavior,
though we could try asking for an command line flag to warn about
cases where this changes code generation.

     Arnd