Re: [XEN PATCH 3/4] xen/include: add pure and const attributes

From: Jan Beulich <jbeulich@suse.com>
To: Simone Ballarin <simone.ballarin@bugseng.com>
Cc: consulting@bugseng.com, sstabellini@kernel.org,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH 3/4] xen/include: add pure and const attributes
Date: Mon, 23 Oct 2023 17:33:37 +0200	[thread overview]
Message-ID: <e8bf9817-fd54-9bf4-4302-dcee682f9172@suse.com> (raw)
In-Reply-To: <0032186f-80c7-4dba-b46e-10d4a8e2a8cb@bugseng.com>

On 23.10.2023 17:23, Simone Ballarin wrote:
> On 23/10/23 15:34, Jan Beulich wrote:
>> On 18.10.2023 16:18, Simone Ballarin wrote:
>>> --- a/xen/include/xen/pdx.h
>>> +++ b/xen/include/xen/pdx.h
>>> @@ -164,7 +164,7 @@ static inline unsigned long pfn_to_pdx(unsigned long pfn)
>>>    * @param pdx Page index
>>>    * @return Obtained pfn after decompressing the pdx
>>>    */
>>> -static inline unsigned long pdx_to_pfn(unsigned long pdx)
>>> +static inline __attribute_pure__ unsigned long pdx_to_pfn(unsigned long pdx)
>>>   {
>>>       return (pdx & pfn_pdx_bottom_mask) |
>>>              ((pdx << pfn_pdx_hole_shift) & pfn_top_mask);
>>
>> Taking this as an example for what I've said above: The compiler can't
>> know that the globals used by the functions won't change value. Even
>> within Xen it is only by convention that these variables are assigned
>> their values during boot, and then aren't changed anymore. Which makes
>> me wonder: Did you check carefully that around the time the variables
>> have their values established, no calls to the functions exist (which
>> might then be subject to folding)?
> 
> There is no need to check that, the GCC documentation explicitly says:
> 
> However, functions declared with the pure attribute *can safely read any 
> non-volatile objects*, and modify the value of objects in a way that 
> does not affect their return value or the observable state of the program.

I did quote this same text in response to what Andrew has said, but I also
did note there that this needs to be taken with a grain of salt: The
compiler generally assumes a single-threaded environment, i.e. no changes
to globals behind the back of the code it is processing.

Jan