While for some of the functions there's locking involved, the acquiring
and releasing of a lock doesn't alter program state when comparing
"before" and "after" the function invocations. Furthermore without
(further) locking by callers, return values are stale anyway by the time
they can be evaluated. Hence both CSE and DCE are okay to occur for
invocations of these functions.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC for the (imo) ambiguous wording in documentation.

I further wonder whether functions trivially wrapping others without
de-referencing the rangeset pointer, like
rangeset_{add,remove,contains}_singleton(), wouldn't better be inline
ones. This would (afaict) eliminate the need for marking pure the one of
those altered here.

--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -52,7 +52,7 @@ void rangeset_limit(
 #define _RANGESETF_prettyprint_hex 0
 #define RANGESETF_prettyprint_hex  (1U << _RANGESETF_prettyprint_hex)
 
-bool_t __must_check rangeset_is_empty(
+bool __must_check __attribute_pure__ rangeset_is_empty(
     const struct rangeset *r);
 
 /* Add/claim/remove/query a numeric range. */
@@ -62,9 +62,9 @@ int __must_check rangeset_claim_range(st
                                       unsigned long *s);
 int __must_check rangeset_remove_range(
     struct rangeset *r, unsigned long s, unsigned long e);
-bool_t __must_check rangeset_contains_range(
+bool __must_check __attribute_pure__ rangeset_contains_range(
     struct rangeset *r, unsigned long s, unsigned long e);
-bool_t __must_check rangeset_overlaps_range(
+bool __must_check __attribute_pure__ rangeset_overlaps_range(
     struct rangeset *r, unsigned long s, unsigned long e);
 int rangeset_report_ranges(
     struct rangeset *r, unsigned long s, unsigned long e,
@@ -88,7 +88,7 @@ int __must_check rangeset_add_singleton(
     struct rangeset *r, unsigned long s);
 int __must_check rangeset_remove_singleton(
     struct rangeset *r, unsigned long s);
-bool_t __must_check rangeset_contains_singleton(
+bool __must_check __attribute_pure__ rangeset_contains_singleton(
     struct rangeset *r, unsigned long s);
 
 /* swap contents */

On Wed, Sep 28, 2022 at 02:12:30PM +0200, Jan Beulich wrote:
> While for some of the functions there's locking involved, the acquiring
> and releasing of a lock doesn't alter program state when comparing
> "before" and "after" the function invocations. Furthermore without
> (further) locking by callers, return values are stale anyway by the time
> they can be evaluated. Hence both CSE and DCE are okay to occur for
> invocations of these functions.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> RFC for the (imo) ambiguous wording in documentation.

What happens if the contents of the rangeset is modified outside of
the scope of the function.  Ie: would for example:

while ( !rangeset_is_empty(...) );

Become an infinite loop then, without taking into account that the
rangeset can be externally modified?

It's not clear to me whether the elimination of repeated calls take
into account that the observed variable can be externally modified?

Thanks, Roger.

On 28.09.2022 16:27, Roger Pau Monné wrote:
> On Wed, Sep 28, 2022 at 02:12:30PM +0200, Jan Beulich wrote:
>> While for some of the functions there's locking involved, the acquiring
>> and releasing of a lock doesn't alter program state when comparing
>> "before" and "after" the function invocations. Furthermore without
>> (further) locking by callers, return values are stale anyway by the time
>> they can be evaluated. Hence both CSE and DCE are okay to occur for
>> invocations of these functions.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> RFC for the (imo) ambiguous wording in documentation.
> 
> What happens if the contents of the rangeset is modified outside of
> the scope of the function.  Ie: would for example:
> 
> while ( !rangeset_is_empty(...) );
> 
> Become an infinite loop then, without taking into account that the
> rangeset can be externally modified?
> 
> It's not clear to me whether the elimination of repeated calls take
> into account that the observed variable can be externally modified?

Hmm, right, this actually matches the feof() counter-example that the
gcc doc has. So I guess I need to withdraw the patch and we will need
to live with these functions not being possible to eliminate even in
cases where it would be okay (and helpful).

Jan