Address feedback

Author: diegorusso

Tools/jit/_stencils.py

@@ -209,13 +209,22 @@ def pad(self, alignment: int) -> None:
             self.disassembly.append(f"{offset:x}: {' '.join(['00'] * padding)}")
         self.body.extend([0] * padding)
 
-    def add_nop(self, alignment: int) -> None:
-        """Add a NOP if the offset is not aligned."""
+    def add_nops(self, nop: bytes, alignment: int) -> None:
+        """Add NOPs until there is alignment. Fail if it is not possible."""
         offset = len(self.body)
-        nop = b"\x1f\x20\x03\xD5"
-        if offset % alignment:
-            self.disassembly.append(f"{offset:x}: d503201f\t\t nop")
-            self.body.extend(nop)
+        nop_size = len(nop)
+
+        # Calculate the gap to the next multiple of alignment.
+        gap = -offset % alignment
+        if gap:
+            if gap % nop_size == 0:
+                count = gap // nop_size
+                self.body.extend(nop * count)
+            else:
+                raise ValueError(
+                    f"Cannot add nops of size '{nop_size}' to a body with "
+                    f"offset '{offset}' to align with '{alignment}'"
+                )
 
     def remove_jump(self) -> None:
         """Remove a zero-length continuation jump, if it exists."""
@@ -254,7 +263,6 @@ def remove_jump(self) -> None:
                 return
         if self.body[offset:] == jump:
             self.body = self.body[:offset]
-            self.disassembly = self.disassembly[:-2]
             self.holes.remove(hole)
 
 
@@ -275,10 +283,7 @@ class StencilGroup:
     _trampolines: set[int] = dataclasses.field(default_factory=set, init=False)
 
     def process_relocations(
-        self,
-        known_symbols: dict[str, int],
-        *,
-        alignment: int = 1,
+        self, known_symbols: dict[str, int], *, alignment: int = 1, nop: bytes = b""
     ) -> None:
         """Fix up all GOT and internal relocations for this stencil group."""
         for hole in self.code.holes.copy():
@@ -299,7 +304,7 @@ def process_relocations(
                 hole.addend = ordinal
                 hole.symbol = None
         self.code.remove_jump()
-        self.code.add_nop(alignment=alignment)
+        self.code.add_nops(nop=nop, alignment=alignment)
         self.data.pad(8)
         for stencil in [self.code, self.data]:
             for hole in stencil.holes:

Tools/jit/_targets.py

@@ -44,6 +44,15 @@ class _Target(typing.Generic[_S, _R]):
     verbose: bool = False
     known_symbols: dict[str, int] = dataclasses.field(default_factory=dict)
 
+    def _get_nop(self) -> bytes:
+        if re.fullmatch(r"aarch64-.*", self.triple):
+            nop = b"\x1f\x20\x03\xD5"
+        elif re.fullmatch(r"x86_64-.*|i686.*", self.triple):
+            nop = b"\x90"
+        else:
+            raise ValueError(f"NOP not defined for {self.triple}")
+        return nop
+
     def _compute_digest(self, out: pathlib.Path) -> str:
         hasher = hashlib.sha256()
         hasher.update(self.triple.encode())
@@ -172,7 +181,9 @@ async def _build_stencils(self) -> dict[str, _stencils.StencilGroup]:
         stencil_groups = {task.get_name(): task.result() for task in tasks}
         for stencil_group in stencil_groups.values():
             stencil_group.process_relocations(
-                known_symbols=self.known_symbols, alignment=self.alignment
+                known_symbols=self.known_symbols,
+                alignment=self.alignment,
+                nop=self._get_nop(),
             )
         return stencil_groups