Address comments about get_externals and trampoline helper

Author: savannahostrowski

PCbuild/get_external.py

@@ -11,31 +11,22 @@
 import zipfile
 
 
-def request_with_retry(request_func, *args, max_retries=7,
-                       err_msg='Request failed.', **kwargs):
-    """Make a request using request_func with exponential backoff"""
+def retrieve_with_retries(download_location, output_path, reporthook, max_retries=7):
+    """Download a file with retries."""
     for attempt in range(max_retries + 1):
         try:
-            resp = request_func(*args, **kwargs)
+            resp = urllib.request.urlretrieve(
+                download_location,
+                output_path,
+                reporthook,
+            )
         except (urllib.error.URLError, ConnectionError) as ex:
             if attempt == max_retries:
-                raise OSError(err_msg) from ex
+                raise OSError(f'Download from {download_location} failed.') from ex
             time.sleep(2.25**attempt)
         else:
             return resp
 
-
-def retrieve_with_retries(download_location, output_path, reporthook):
-    """Download a file with retries."""
-    return request_with_retry(
-        urllib.request.urlretrieve,
-        download_location,
-        output_path,
-        reporthook,
-        err_msg=f'Download from {download_location} failed.',
-    )
-
-
 def fetch_zip(commit_hash, zip_dir, *, org='python', binary=False, verbose=False):
     repo = 'cpython-bin-deps' if binary else 'cpython-source-deps'
     url = f'https://github.com/{org}/{repo}/archive/{commit_hash}.zip'

PCbuild/get_externals.bat

@@ -93,9 +93,9 @@ for %%b in (%binaries%) do (
     ) else (
         echo.Fetching %%b...
         if "%%b"=="llvm-20.1.8.0" (
-            %PYTHON% -E "%PCBUILD%\get_external.py" -r -O %ORG% -e "%EXTERNALS_DIR%" %%b
+            %PYTHON% -E "%PCBUILD%\get_external.py" --release --organization %ORG% --externals-dir "%EXTERNALS_DIR%" %%b
         ) else (
-            %PYTHON% -E "%PCBUILD%\get_external.py" -b -O %ORG% -e "%EXTERNALS_DIR%" %%b
+            %PYTHON% -E "%PCBUILD%\get_external.py" --binary --organization %ORG% --externals-dir "%EXTERNALS_DIR%" %%b
         )
     )
 )

Python/jit.c

@@ -461,6 +461,25 @@ void patch_x86_64_trampoline(unsigned char *location, int ordinal, jit_state *st
     #define DATA_ALIGN 1
 #endif
 
+// Get the trampoline memory location for a given symbol ordinal.
+static unsigned char *
+get_trampoline_slot(int ordinal, jit_state *state)
+{
+    const uint32_t symbol_mask = 1 << (ordinal % 32);
+    const uint32_t trampoline_mask = state->trampolines.mask[ordinal / 32];
+    assert(symbol_mask & trampoline_mask);
+
+     // Count the number of set bits in the trampoline mask lower than ordinal
+    int index = _Py_popcount32(trampoline_mask & (symbol_mask - 1));
+    for (int i = 0; i < ordinal / 32; i++) {
+        index += _Py_popcount32(state->trampolines.mask[i]);
+    }
+
+    unsigned char *trampoline = state->trampolines.mem + index * TRAMPOLINE_SIZE;
+    assert((size_t)(index + 1) * TRAMPOLINE_SIZE <= state->trampolines.size);
+    return trampoline;
+}
+
 // Generate and patch AArch64 trampolines. The symbols to jump to are stored
 // in the jit_stencils.h in the symbols_map.
 void
@@ -477,20 +496,8 @@ patch_aarch64_trampoline(unsigned char *location, int ordinal, jit_state *state)
         return;
     }
 
-    // Masking is done modulo 32 as the mask is stored as an array of uint32_t
-    const uint32_t symbol_mask = 1 << (ordinal % 32);
-    const uint32_t trampoline_mask = state->trampolines.mask[ordinal / 32];
-    assert(symbol_mask & trampoline_mask);
-
-    // Count the number of set bits in the trampoline mask lower than ordinal,
-    // this gives the index into the array of trampolines.
-    int index = _Py_popcount32(trampoline_mask & (symbol_mask - 1));
-    for (int i = 0; i < ordinal / 32; i++) {
-        index += _Py_popcount32(state->trampolines.mask[i]);
-    }
-
-    uint32_t *p = (uint32_t*)(state->trampolines.mem + index * TRAMPOLINE_SIZE);
-    assert((size_t)(index + 1) * TRAMPOLINE_SIZE <= state->trampolines.size);
+    // Out of range - need a trampoline
+    uint32_t *p = (uint32_t *)get_trampoline_slot(ordinal, state);
 
 
     /* Generate the trampoline
@@ -521,18 +528,7 @@ patch_x86_64_trampoline(unsigned char *location, int ordinal, jit_state *state)
     }
 
     // Out of range - need a trampoline
-    const uint32_t symbol_mask = 1 << (ordinal % 32);
-    const uint32_t trampoline_mask = state->trampolines.mask[ordinal / 32];
-    assert(symbol_mask & trampoline_mask);
-
-    // Count the number of set bits in the trampoline mask lower than ordinal
-    int index = _Py_popcount32(trampoline_mask & (symbol_mask - 1));
-    for (int i = 0; i < ordinal / 32; i++) {
-        index += _Py_popcount32(state->trampolines.mask[i]);
-    }
-
-    unsigned char *trampoline = state->trampolines.mem + index * TRAMPOLINE_SIZE;
-    assert((size_t)(index + 1) * TRAMPOLINE_SIZE <= state->trampolines.size);
+    unsigned char *trampoline = get_trampoline_slot(ordinal, state);
 
     /* Generate the trampoline (14 bytes, padded to 16):
        0: ff 25 00 00 00 00    jmp *(%rip)