[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: Please focus this issue on documenting changes, and open new issues for further optimizations. -- ___ Python tracker ___ ___

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Demur Rumed added the comment: PREDICT could be optimized by having a HAS_ARG check on the constant op to decide whether we assign oparg = OPARG(word) -- ___ Python tracker ___

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: Why not using sizeof(unsigned short) in the macro, rtbaer than 2? -- ___ Python tracker ___ ___ Pyth

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: Opps, sorry. Fixed now. Thank you Berker for fast response. -- ___ Python tracker ___ ___ Python-b

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Roundup Robot added the comment: New changeset b26d07812a8e by Serhiy Storchaka in branch 'default': Fixed the use of _Py_IS_ALIGNED (issue #27097). https://hg.python.org/cpython/rev/b26d07812a8e -- ___ Python tracker

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Berker Peksag added the comment: Debug builds are broken after eda3716d6425: http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.x%203.x/builds/4332/steps/compile/logs/stdio -- nosy: +berker.peksag ___ Python tracker

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Changes by Serhiy Storchaka : -- status: open -> closed ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https:/

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Changes by Serhiy Storchaka : -- assignee: -> serhiy.storchaka resolution: -> fixed stage: patch review -> resolved ___ Python tracker ___ _

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Roundup Robot added the comment: New changeset eda3716d6425 by Serhiy Storchaka in branch 'default': Issue #27097: Python interpreter is now about 7% faster due to optimized https://hg.python.org/cpython/rev/eda3716d6425 -- nosy: +python-dev ___ Pytho

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: ushort3.patch LGTM. You might also add (copy) the added assertions in PyCode_New*(). -- ___ Python tracker ___

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: Updated patch addresses Victor's comments. -- Added file: http://bugs.python.org/file42990/ushort3.patch ___ Python tracker ___ __

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: Demur: "This patch [ ushort.patch ] seeks to be a minimal change to achieve the desired aligned/batched memory read behavior of NEXTOP/NEXTARG." We are concerned by undefined behaviours in CPython for portability reasons. Using a "unsigned short*" pointer avoi

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: Antti Haapala: > However what I'd prefer here as the size of the type is important, to use > `uint16_t` or typedef instead of just `unsigned short`, which is a statemeent > that the value must be at least "2 bytes wide". I'm not aware of a platform where sizeo

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: ushort2.patch LGTM, but I added many small comments on the review ;-) -- ___ Python tracker ___ ___

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: This is separate issue (see issue17884). -- ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Antti Haapala added the comment: Casting the pointer is OK, explicitly typing the pointer is even better; then the arithmetic is even clearer and it is easy to add offsets in words! However what I'd prefer here as the size of the type is important, to use `uint16_t` or typedef instead of just

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: Hmm, don't know why my patch was not uploaded first time. It contained exactly what Victor said. Yes, we need to multiply or divide by 2 when convert from pointer difference to byte offset, but this doesn't affect performance. I think the next step is the ch

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Demur Rumed added the comment: There's some difficulty in changing next_instr to an unsigned short* unless pointer arithmetic converts it back to unsigned char*. Code relies on f_lasti to be a byte index more than it relies on f_lasti to be -1 at first. Should this change include converting ju

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: > +#define NEXTOPARG() (oparg = *(unsigned short*)next_instr, opcode = > OPOF(oparg), oparg = ARGOF(oparg), next_instr += 2) I dislike this approach. I would prefer to use the unsigned short* type for next_instr, and put an assertion when next_instr is fir

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

STINNER Victor added the comment: > I'm pretty sure this is undefined behavior. Are you talking about the exact C implementation, or the risk of unaligned memory accesses? I suggest to add an assertion to ensure that the pointer is aligned to 16-bits. -- nosy: +haypo

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Demur Rumed added the comment: There is no strict aliasing issues because aliasing is explicitly allowed for char buffers. The only issue is unaligned memory reads, but allocations are well aligned, so there'd have to be explicit work to allocate & then put code at an uneven offset. CPython ne

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: I hope following patch is free from undefined behavior. -- ___ Python tracker ___ ___ Python-bugs-

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Adrian Wielgosik added the comment: I'm pretty sure this is undefined behavior. Here's how similar code was fixed in Chromium: https://codereview.chromium.org/9117014/patch/1/2 -- nosy: +Adrian Wielgosik ___ Python tracker

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: Using Raymond's microbenchmark from issue25823: $ ./python.exe exercise_oparg.py Unpatched: 0.8331978429996525 Patched:0.7600522060020012 Pybench exposes 7% speed up (detailed report is attached). -- Added file: http://bugs.python.org/file42966/

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Serhiy Storchaka added the comment: What if change the type of next_instr to unsigned short*? This would guarantee that the read is aligned since the start of code data is aligned. I have small doubt about PREDICT(). I doubt that the compiler is smart enough to infer that *next_instr and *(uns

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Mark Dickinson added the comment: > has undefined behaviour, according to the C standards. I retract this; I'm no longer convinced this is true. Sorry for the noise. -- ___ Python tracker _

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Mark Dickinson added the comment: This assignment: oparg = *(unsigned short*)next_instr has undefined behaviour, according to the C standards. I'm not sure it's a good idea to introduce this into the core. -- nosy: +mark.dickinson ___ Python tr

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Changes by Serhiy Storchaka : -- stage: -> patch review Added file: http://bugs.python.org/file42964/ushort.patch ___ Python tracker ___

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

Changes by Serhiy Storchaka : -- nosy: +serhiy.storchaka ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https:

[issue27097] ceval: Wordcode follow up, explicit unsigned short read

New submission from Demur Rumed: Attached is a patch based off #26647 which converts ceval to read opcode/oparg by casting next_instr to an unsigned short. I don't believe we need to complicate the code with checking the pointer alignment of the instruction stream; some effort must be gone thr