This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Use Py_SIZE(x) instead of x->ob_size
Type: behavior Stage: resolved
Components: Interpreter Core, Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: facundobatista, loewis, mark.dickinson, rhettinger, skrah, tim.peters, trex58, vstinner
Priority: normal Keywords:

Created on 2016-08-10 09:51 by trex58, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Messages (12)
msg272324 - (view) Author: REIX Tony (trex58) Date: 2016-08-10 09:51
I'm porting Python 2.7.* on AIX.

I am now using a version 13.1.3.2 of XLC.
I was previously using version 12.1.0.14 .
It appears that this new compiler shows a potential issue in Python v2 code. This issue seems to appear when -O2 is used.

I was porting Python 2.7.11 with -O2 instead of -O0 when I got it.

The issue appears when running tests involving power() with Decimals where the x_sub() routine is used.

The following Python code shows the issue:
  from decimal import *
  c = Context(7, ROUND_UP)
  d = c.power(Decimal(0.7), Decimal(3.4))
  print d
Good result is:  0.2973948 .
The bug returns: 2.440099 

However, I have about 22 errors when running:
./python Lib/test/regrtest.py -v test_decimal
It is random in 64bit and quite constant in 32bit.

The issue deals with using x->ob-size.
There is a macro:
Include/object.h:#define Py_SIZE(ob)  (((PyVarObject*)(ob))->ob_size)

The issue appears in last lines of Objects/longobject.c:x_sub() routine, when changing the sign:   z->ob_size = -(z->ob_size);

I have looked at version 2.7.12 and 2.7.10 of Python: they contain the same code as in 2.7.11.
And ->ob_size is used about 45 times, mainly in Objects/longobject.c. However, Py_SIZE(...) is used ten times more (475).

- Looking at version 3.5.1 of Python, I see that ->ob_size is now used only once, in: Objects/object.c .
And Py_SIZE(...) is now used 616 times.

And I also see that the Python 2.7.11-12 x_sub() code:
      if (sign < 0)
        z->ob_size = -(z->ob_size);
      return long_normalize(z);
has been replaced in Python 3.5.1 by:
      if (sign < 0) {
        _PyLong_Negate(&z);
        if (z == NULL)
            return NULL;
      }
      return long_normalize(z);

  /* If a freshly-allocated int is already shared, it must
      be a small integer, so negating it must go to PyLong_FromLong */
  Py_LOCAL_INLINE(void)
  _PyLong_Negate(PyLongObject **x_p)
  {
     PyLongObject *x;

     x = (PyLongObject *)*x_p;
     if (Py_REFCNT(x) == 1) {
         Py_SIZE(x) = -Py_SIZE(x);
         return;
     }

     *x_p = (PyLongObject *)PyLong_FromLong(-MEDIUM_VALUE(x));
     Py_DECREF(x);
 }


So, it looks to me that Python v2 version should do the same that Python v3 has done: replace x->ob-size by Py_SIZE(x) everywhere, and improve the code of x_sub() by using some _PyLong_Negate() routine or macro.
msg272414 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-08-11 07:53
I'm not clear on how non-use of the macro is causing your observed failures or whether that is due to a compiler bug.  That said, I don't see any downside to using Py_SIZE everywhere it is applicable.
msg272419 - (view) Author: REIX Tony (trex58) Date: 2016-08-11 08:14
Hi Raymond

I've got several email exchanges with the IBM XLC expert. From his own study of my issue, his conclusion is that this kind of Python v2 coding is not ANSI-aliasing safe. It seems that there is a standard that requires C code to NOT do some kinds of coding so that any C compiler optimizer can do its best.

The issue was not there with XLC v12.1.0.14 and -O2.
It appeared with XLC v13.1.3.2 and -O2 since XLC v13 optimizer is more agressive.

About GCC, I've not experimented yet with it for now (will do later today I hope), but the impact should be the same according to the optimizer level and improvements.

Here is what IBMer Steven said:

"I found the problem.
It is not a problem with the compiler, but a problem with the source code/option set.
It is an ansi aliasing violation. I'll try to provide as much detail as I can to explain it.

At line 2512 of Objects/longobject.c, we have the following code:

if (sign < 0)
z->ob_size = -(z->ob_size);
return long_normalize(z);

Note that we use z->ob_size to access size, and the type of z is "PyLongObject *".
This value is loaded in long_normalization.
After we inline this function call, the compiler moves the load done in long_normalization above the if statement (past the store that writes to it), which is why we ends up with the wrong sign.

Now the question is why does the compiler think that this is legal ?

In long_normalize, the size is obtained using a macro Py_SIZE(v) (line 47).
This macro expands to:

(((PyVarObject*)(v))->ob_size)

Notice that the pointer is cast to something of type PyVarObject*.
PyVarObject and PyLongObject are not compatible types, and, because ansi aliasing is assumed, the compiler believes they do not reference the same memory. Therefore it is safe to move.

A simple solution is to use "-qalias=noansi" when compiling. That will work, but could also hurt performance.

The other solution is to use either Py_SIZE all of the time to access the memory or never.
Do not mix and match. This will require some code changes.
I'll leave it to you to figure out how to handle it, but my guess is that Py_SIZE is supposed to always be used.
The comments in "object.h" lines 11-17 include this phrase "they must be accessed through special macros and functions only."
msg272421 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-08-11 08:29
In 2.7 we use -fno-strict-aliasing and -fwrapv for gcc. I think it is probably required to use the equivalent options for xlc.

These are examples of things that have been cleaned up in 3.x. 2.7 actually relies on these options.
msg272429 - (view) Author: REIX Tony (trex58) Date: 2016-08-11 09:29
Thanks a lot Stefan, that should completely explain my issues.

-fno-strict-aliasing -fwrapv for gcc

So, that means that you would get better performance if you applied on Python v2.7 what Python v3.5 did about Py_SIZE(x) .
However, there are probably other places where the aliasing issue still appears in v2.7 .

Hummm I'll use -qalias=noansi with XLC and see what happens.
msg272432 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-08-11 09:46
On Thu, Aug 11, 2016 at 09:29:44AM +0000, REIX Tony wrote:
> -fno-strict-aliasing -fwrapv for gcc
> 
> So, that means that you would get better performance if you applied on Python v2.7 what Python v3.5 did about Py_SIZE(x) .
> However, there are probably other places where the aliasing issue still appears in v2.7 .

I doubt that you'll see any measurable performance difference. The main
inefficiencies in Python aren't that low-level: They are in the interpreter
loop.

> Hummm I'll use -qalias=noansi with XLC and see what happens.

Yes, and also use -fwrapv or similar.  Otherwise you might get other issues
that are hard to track down.
msg272458 - (view) Author: REIX Tony (trex58) Date: 2016-08-11 15:36
With XLC v13 -O2, using -qalias=noansi for building Objects/longobject.o only and not for all the other .o files did fix the 10 more failed tests I see with -O2 compared to -O0 (7-8 failed tests).
So, ANSI-aliasing in Objects/longobject.c is the issue.

About -fwrapv , I have to find an equivalent for XLC.

I've given a first try with GCC 4.8.4 . I've got about 44 failed tests compared to 7-8 with XLC. To be improved.
msg272934 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-08-17 13:18
I suggest to close this issue.

Aliasing issues are complex and simply cannot be fixed in Python 2. The root issue has been fixed in Python 3, it required to change the main C structures of Python objects.

The fix for your issue to already known, use -qalias=noansi with XLC.
msg272936 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-08-17 13:19
I mean that fixing ob->ob_size in Objects/longobject.c is not enough. You should also fix C structures and fix all other C files in the code base... It's too late for such major refactoring in Python 2.
msg272940 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-08-17 13:30
Agreed, the changes are too big for 2.7.
msg272942 - (view) Author: REIX Tony (trex58) Date: 2016-08-17 13:43
OK.

However, compiling ONLY the file Objects/longobject.c with -qalias=noansi did fix the issue on AIX. That could be the same on Linux.

I haven't tried to use Py_SIZE() in all places where it should be used. Now trying to figure out why GCC behaves worst than XLC.

Anyway, I have a satisfactory work-around.
Thanks for your help !
msg272944 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-08-17 14:03
> However, compiling ONLY the file Objects/longobject.c with -qalias=noansi did fix the issue on AIX. That could be the same on Linux.

Nobody asked to only set the compiler file to only this file.

As we said, the issue is sprayed in all the C code base. The whole Python 2 project has aliasing issues. Extension modules have same the issue.
History
Date User Action Args
2022-04-11 14:58:34adminsetgithub: 71912
2016-08-17 14:03:30vstinnersetmessages: + msg272944
2016-08-17 13:43:01trex58setmessages: + msg272942
2016-08-17 13:30:01skrahsetstatus: open -> closed
resolution: wont fix
messages: + msg272940

stage: resolved
2016-08-17 13:19:47vstinnersetmessages: + msg272936
2016-08-17 13:18:54vstinnersetnosy: + vstinner
messages: + msg272934
2016-08-11 15:36:12trex58setmessages: + msg272458
2016-08-11 09:46:26skrahsetmessages: + msg272432
2016-08-11 09:29:43trex58setmessages: + msg272429
2016-08-11 08:29:55skrahsetmessages: + msg272421
2016-08-11 08:28:12rhettingersetnosy: + tim.peters, loewis
2016-08-11 08:14:03trex58setmessages: + msg272419
2016-08-11 07:53:40rhettingersetmessages: + msg272414
2016-08-10 09:55:12SilentGhostsetnosy: + rhettinger, facundobatista, mark.dickinson, skrah
components: + Interpreter Core
2016-08-10 09:51:37trex58create