math: Remove const attribute from totalorder* functions
Commit Message
Hi, Joseph,
On Wed, 04 Sep 2019, Joseph Myers wrote:
>This is OK, but it would be better to replace it with __attribute_pure__
>(the patch with that replacement is also OK, if it works).
Good point!
I made this change, tested and pushed the following patch.
Thanks!
Gabriel
From ab41100bab128fa98258aafbb0ab1622884cec4c Mon Sep 17 00:00:00 2001
From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>
Date: Wed, 4 Sep 2019 13:36:23 -0300
Subject: [PATCH] math: Replace const attribute with pure in totalorder*
functions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Since the commit
commit 42760d764649ad82f5fe45a26cbdf2c2500409f7
Author: Joseph Myers <joseph@codesourcery.com>
Date: Thu Aug 15 15:18:34 2019 +0000
Make totalorder and totalordermag functions take pointer arguments.
the test case math/test-totalorderl-ldbl-128ibm fails on every input
pair, when compiled with -O2, which is the case for glibc test suite.
Debugging showed that the test case is passing arguments incorrectly to
totalorderl. This can also be inferred by the fact that compiling the
test case with -O0 hides the bug.
The documentation for the const attribute in GCC manual reads:
Note that a function that has pointer arguments and examines the data
pointed to must not be declared const if the pointed-to data might
change between successive invocations of the function. In general,
since a function cannot distinguish data that might change from data
that cannot, const functions should never take pointer or, in C++,
reference arguments. Likewise, a function that calls a non-const
function usually must not be const itself.
Since the pointed-to data is likely to be changed by user code between
invocations of totalorder*, this patch removes the const attribute from
the declarations of all totalorder functions, replacing it with the pure
attribute, as suggested in the manual:
The pure attribute imposes similar but looser restrictions on a
function’s definition than the const attribute: pure allows the
function to read any non-volatile memory, even if it changes in
between successive invocations of the function.
Tested for powerpc64le and x86_64.
---
ChangeLog | 5 +++++
math/bits/mathcalls.h | 7 ++++---
2 files changed, 9 insertions(+), 3 deletions(-)
Comments
On Thu, 05 Sep 2019, Gabriel F. T. Gomes wrote:
>On Wed, 04 Sep 2019, Joseph Myers wrote:
>
>>This is OK, but it would be better to replace it with __attribute_pure__
~~~~~~~~~~~~~~~~~~
Oh! Only upon reading my own reply, I noticed the suggestion to use this
macro, instead of __attribute__ ((__pure__)). Should I go ahead and patch
it again...
>+__MATHDECL_1 (int, totalorder,, (const _Mdouble_ *__x,
>+ const _Mdouble_ *__y))
>+ __attribute__ ((__pure__));
... replacing '__attribute__ ((__pure__))' with '__attribute_pure__'?
> __MATHDECL_1 (int, totalordermag,, (const _Mdouble_ *__x,
> const _Mdouble_ *__y))
>- __attribute__ ((__const__));
>+ __attribute__ ((__pure__));
Likewise, here?
Or it doesn't matter?
Sorry for the oversight.
On Thu, 5 Sep 2019, Gabriel F. T. Gomes wrote:
> On Thu, 05 Sep 2019, Gabriel F. T. Gomes wrote:
>
> >On Wed, 04 Sep 2019, Joseph Myers wrote:
> >
> >>This is OK, but it would be better to replace it with __attribute_pure__
> ~~~~~~~~~~~~~~~~~~
> Oh! Only upon reading my own reply, I noticed the suggestion to use this
> macro, instead of __attribute__ ((__pure__)). Should I go ahead and patch
> it again...
>
> >+__MATHDECL_1 (int, totalorder,, (const _Mdouble_ *__x,
> >+ const _Mdouble_ *__y))
> >+ __attribute__ ((__pure__));
>
> ... replacing '__attribute__ ((__pure__))' with '__attribute_pure__'?
I think it should use __attribute_pure__, yes (although quite likely using
the older compilers for which they differ would show up other problems
with current glibc headers).
@@ -1,3 +1,8 @@
+2019-09-05 Gabriel F. T. Gomes <gabrielftg@linux.ibm.com>
+
+ * math/bits/mathcalls.h (totalorder, totalordermag): Replace
+ const attribute with pure attribute.
+
2019-09-04 Lukasz Majewski <lukma@denx.de>
* sysdeps/unix/sysv/linux/kernel-features.h
@@ -373,13 +373,14 @@ __MATHDECL_1 (int, canonicalize,, (_Mdouble_ *__cx, const _Mdouble_ *__x));
#if __GLIBC_USE (IEC_60559_BFP_EXT) || __MATH_DECLARING_FLOATN
/* Total order operation. */
-__MATHDECL_1 (int, totalorder,, (const _Mdouble_ *__x, const _Mdouble_ *__y))
- __attribute__ ((__const__));
+__MATHDECL_1 (int, totalorder,, (const _Mdouble_ *__x,
+ const _Mdouble_ *__y))
+ __attribute__ ((__pure__));
/* Total order operation on absolute values. */
__MATHDECL_1 (int, totalordermag,, (const _Mdouble_ *__x,
const _Mdouble_ *__y))
- __attribute__ ((__const__));
+ __attribute__ ((__pure__));
/* Get NaN payload. */
__MATHCALL (getpayload,, (const _Mdouble_ *__x));