GH-100222: Redefine _Py_CODEUNIT as a union to clarify structure of c…

…ode unit. (GH-100223)

Include/cpython/code.h

@@ -16,21 +16,24 @@ extern "C" {
  * 2**32 - 1, rather than INT_MAX.
  */
 
-typedef uint16_t _Py_CODEUNIT;
-
-#ifdef WORDS_BIGENDIAN
-#  define _Py_OPCODE(word) ((word) >> 8)
-#  define _Py_OPARG(word) ((word) & 255)
-#  define _Py_MAKECODEUNIT(opcode, oparg) (((opcode)<<8)|(oparg))
-#else
-#  define _Py_OPCODE(word) ((word) & 255)
-#  define _Py_OPARG(word) ((word) >> 8)
-#  define _Py_MAKECODEUNIT(opcode, oparg) ((opcode)|((oparg)<<8))
-#endif
+typedef union {
+    uint16_t cache;
+    struct {
+        uint8_t opcode;
+        uint8_t oparg;
+    };
+} _Py_CODEUNIT;
+
+#define _Py_OPCODE(word) ((word).opcode)
+#define _Py_OPARG(word) ((word).oparg)
+
+static inline void
+_py_set_opocde(_Py_CODEUNIT *word, uint8_t opcode)
+{
+    word->opcode = opcode;
+}
 
-// Use "unsigned char" instead of "uint8_t" here to avoid illegal aliasing:
-#define _Py_SET_OPCODE(word, opcode) \
-    do { ((unsigned char *)&(word))[0] = (opcode); } while (0)
+#define _Py_SET_OPCODE(word, opcode) _py_set_opocde(&(word), opcode)
 
 typedef struct {
     PyObject *_co_code;

Include/internal/pycore_code.h

@@ -18,53 +18,53 @@ extern "C" {
 #define CACHE_ENTRIES(cache) (sizeof(cache)/sizeof(_Py_CODEUNIT))
 
 typedef struct {
-    _Py_CODEUNIT counter;
-    _Py_CODEUNIT index;
-    _Py_CODEUNIT module_keys_version[2];
-    _Py_CODEUNIT builtin_keys_version;
+    uint16_t counter;
+    uint16_t index;
+    uint16_t module_keys_version[2];
+    uint16_t builtin_keys_version;
 } _PyLoadGlobalCache;
 
 #define INLINE_CACHE_ENTRIES_LOAD_GLOBAL CACHE_ENTRIES(_PyLoadGlobalCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
+    uint16_t counter;
 } _PyBinaryOpCache;
 
 #define INLINE_CACHE_ENTRIES_BINARY_OP CACHE_ENTRIES(_PyBinaryOpCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
+    uint16_t counter;
 } _PyUnpackSequenceCache;
 
 #define INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE \
     CACHE_ENTRIES(_PyUnpackSequenceCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
-    _Py_CODEUNIT mask;
+    uint16_t counter;
+    uint16_t mask;
 } _PyCompareOpCache;
 
 #define INLINE_CACHE_ENTRIES_COMPARE_OP CACHE_ENTRIES(_PyCompareOpCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
-    _Py_CODEUNIT type_version[2];
-    _Py_CODEUNIT func_version;
+    uint16_t counter;
+    uint16_t type_version[2];
+    uint16_t func_version;
 } _PyBinarySubscrCache;
 
 #define INLINE_CACHE_ENTRIES_BINARY_SUBSCR CACHE_ENTRIES(_PyBinarySubscrCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
-    _Py_CODEUNIT version[2];
-    _Py_CODEUNIT index;
+    uint16_t counter;
+    uint16_t version[2];
+    uint16_t index;
 } _PyAttrCache;
 
 typedef struct {
-    _Py_CODEUNIT counter;
-    _Py_CODEUNIT type_version[2];
-    _Py_CODEUNIT keys_version[2];
-    _Py_CODEUNIT descr[4];
+    uint16_t counter;
+    uint16_t type_version[2];
+    uint16_t keys_version[2];
+    uint16_t descr[4];
 } _PyLoadMethodCache;
 
 
@@ -74,21 +74,21 @@ typedef struct {
 #define INLINE_CACHE_ENTRIES_STORE_ATTR CACHE_ENTRIES(_PyAttrCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
-    _Py_CODEUNIT func_version[2];
-    _Py_CODEUNIT min_args;
+    uint16_t counter;
+    uint16_t func_version[2];
+    uint16_t min_args;
 } _PyCallCache;
 
 #define INLINE_CACHE_ENTRIES_CALL CACHE_ENTRIES(_PyCallCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
+    uint16_t counter;
 } _PyStoreSubscrCache;
 
 #define INLINE_CACHE_ENTRIES_STORE_SUBSCR CACHE_ENTRIES(_PyStoreSubscrCache)
 
 typedef struct {
-    _Py_CODEUNIT counter;
+    uint16_t counter;
 } _PyForIterCache;
 
 #define INLINE_CACHE_ENTRIES_FOR_ITER CACHE_ENTRIES(_PyForIterCache)
@@ -409,7 +409,7 @@ write_location_entry_start(uint8_t *ptr, int code, int length)
 static inline uint16_t
 adaptive_counter_bits(int value, int backoff) {
     return (value << ADAPTIVE_BACKOFF_BITS) |
-           (backoff & ((1<<ADAPTIVE_BACKOFF_BITS)-1));
+        (backoff & ((1<<ADAPTIVE_BACKOFF_BITS)-1));
 }
 
 static inline uint16_t

Misc/NEWS.d/next/Core and Builtins/2022-12-13-16-05-18.gh-issue-100222.OVVvYe.rst

@@ -0,0 +1,2 @@
+Redefine the ``_Py_CODEUNIT`` typedef as a union to describe its layout to
+the C compiler, avoiding type punning and improving clarity.

Objects/codeobject.c

@@ -1520,9 +1520,10 @@ deopt_code(_Py_CODEUNIT *instructions, Py_ssize_t len)
         _Py_CODEUNIT instruction = instructions[i];
         int opcode = _PyOpcode_Deopt[_Py_OPCODE(instruction)];
         int caches = _PyOpcode_Caches[opcode];
-        instructions[i] = _Py_MAKECODEUNIT(opcode, _Py_OPARG(instruction));
+        instructions[i].opcode = opcode;
         while (caches--) {
-            instructions[++i] = _Py_MAKECODEUNIT(CACHE, 0);
+            instructions[++i].opcode = CACHE;
+            instructions[i].oparg = 0;
         }
     }
 }
@@ -1775,9 +1776,9 @@ code_richcompare(PyObject *self, PyObject *other, int op)
     for (int i = 0; i < Py_SIZE(co); i++) {
         _Py_CODEUNIT co_instr = _PyCode_CODE(co)[i];
         _Py_CODEUNIT cp_instr = _PyCode_CODE(cp)[i];
-        _Py_SET_OPCODE(co_instr, _PyOpcode_Deopt[_Py_OPCODE(co_instr)]);
-        _Py_SET_OPCODE(cp_instr, _PyOpcode_Deopt[_Py_OPCODE(cp_instr)]);
-        eq = co_instr == cp_instr;
+        co_instr.opcode = _PyOpcode_Deopt[_Py_OPCODE(co_instr)];
+        cp_instr.opcode =_PyOpcode_Deopt[_Py_OPCODE(cp_instr)];
+        eq = co_instr.cache == cp_instr.cache;
         if (!eq) {
             goto unequal;
         }

Python/ceval.c

@@ -864,7 +864,7 @@ GETITEM(PyObject *v, Py_ssize_t i) {
         STAT_INC(opcode, miss);                                  \
         STAT_INC((INSTNAME), miss);                              \
         /* The counter is always the first cache entry: */       \
-        if (ADAPTIVE_COUNTER_IS_ZERO(*next_instr)) {             \
+        if (ADAPTIVE_COUNTER_IS_ZERO(next_instr->cache)) {       \
             STAT_INC((INSTNAME), deopt);                         \
         }                                                        \
         else {                                                   \
@@ -1289,7 +1289,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
         }
         opcode = _PyOpcode_Deopt[opcode];
         if (_PyOpcode_Caches[opcode]) {
-            _Py_CODEUNIT *counter = &next_instr[1];
+            uint16_t *counter = &next_instr[1].cache;
             // The instruction is going to decrement the counter, so we need to
             // increment it here to make sure it doesn't try to specialize:
             if (!ADAPTIVE_COUNTER_IS_MAX(*counter)) {

Python/compile.c

@@ -263,22 +263,32 @@ write_instr(_Py_CODEUNIT *codestr, struct instr *instruction, int ilen)
     int caches = _PyOpcode_Caches[opcode];
     switch (ilen - caches) {
         case 4:
-            *codestr++ = _Py_MAKECODEUNIT(EXTENDED_ARG, (oparg >> 24) & 0xFF);
+            codestr->opcode = EXTENDED_ARG;
+            codestr->oparg = (oparg >> 24) & 0xFF;
+            codestr++;
             /* fall through */
         case 3:
-            *codestr++ = _Py_MAKECODEUNIT(EXTENDED_ARG, (oparg >> 16) & 0xFF);
+            codestr->opcode = EXTENDED_ARG;
+            codestr->oparg = (oparg >> 16) & 0xFF;
+            codestr++;
             /* fall through */
         case 2:
-            *codestr++ = _Py_MAKECODEUNIT(EXTENDED_ARG, (oparg >> 8) & 0xFF);
+            codestr->opcode = EXTENDED_ARG;
+            codestr->oparg = (oparg >> 8) & 0xFF;
+            codestr++;
             /* fall through */
         case 1:
-            *codestr++ = _Py_MAKECODEUNIT(opcode, oparg & 0xFF);
+            codestr->opcode = opcode;
+            codestr->oparg = oparg & 0xFF;
+            codestr++;
             break;
         default:
             Py_UNREACHABLE();
     }
     while (caches--) {
-        *codestr++ = _Py_MAKECODEUNIT(CACHE, 0);
+        codestr->opcode = CACHE;
+        codestr->oparg = 0;
+        codestr++;
     }
 }