Implement RFC 17: Remove log2_int.

Reexports of `amaranth.utils` functions are removed from
`amaranth._utils` to avoid a circular import issue (for `deprecated`).
Since this is a private module, this should not be a problem.

amaranth/_utils.py

@@ -6,11 +6,9 @@ import re
 from collections import OrderedDict
 from collections.abc import Iterable
 
-from .utils import *
 
-
-__all__ = ["flatten", "union" , "log2_int", "bits_for", "memoize", "final", "deprecated",
-           "get_linter_options", "get_linter_option"]
+__all__ = ["flatten", "union", "memoize", "final", "deprecated", "get_linter_options",
+           "get_linter_option"]
 
 
 def flatten(i):

amaranth/back/rtlil.py

@@ -4,7 +4,8 @@ from contextlib import contextmanager
 import warnings
 import re
 
-from .._utils import bits_for, flatten
+from .._utils import flatten
+from ..utils import bits_for
 from ..hdl import ast, ir, mem, xfrm, _repr
 from ..lib import wiring
 

amaranth/hdl/ast.py

@@ -10,6 +10,7 @@ from itertools import chain
 
 from ._repr import *
 from .. import tracer
+from ..utils import *
 from .._utils import *
 from .._utils import _ignore_deprecated
 from .._unused import *

amaranth/hdl/dsl.py

@@ -5,7 +5,8 @@ from enum import Enum
 import warnings
 import sys
 
-from .._utils import flatten, bits_for
+from .._utils import flatten
+from ..utils import bits_for
 from .. import tracer
 from .ast import *
 from .ir import *

amaranth/lib/fifo.py

@@ -4,7 +4,7 @@ import warnings
 
 from .. import *
 from ..asserts import *
-from .._utils import log2_int
+from ..utils import ceil_log2
 from .coding import GrayEncoder, GrayDecoder
 from .cdc import FFSynchronizer, AsyncFFSynchronizer
 
@@ -353,13 +353,12 @@ class AsyncFIFO(Elaboratable, FIFOInterface):
 
     def __init__(self, *, width, depth, r_domain="read", w_domain="write", exact_depth=False):
         if depth != 0:
-            try:
-                depth_bits = log2_int(depth, need_pow2=exact_depth)
-                depth = 1 << depth_bits
-            except ValueError:
+            depth_bits = ceil_log2(depth)
+            if exact_depth and depth != 1 << depth_bits:
                 raise ValueError("AsyncFIFO only supports depths that are powers of 2; requested "
                                  "exact depth {} is not"
                                  .format(depth)) from None
+            depth = 1 << depth_bits
         else:
             depth_bits = 0
         super().__init__(width=width, depth=depth)
@@ -530,13 +529,12 @@ class AsyncFIFOBuffered(Elaboratable, FIFOInterface):
 
     def __init__(self, *, width, depth, r_domain="read", w_domain="write", exact_depth=False):
         if depth != 0:
-            try:
-                depth_bits = log2_int(max(0, depth - 1), need_pow2=exact_depth)
-                depth = (1 << depth_bits) + 1
-            except ValueError:
+            depth_bits = ceil_log2(max(0, depth - 1))
+            if exact_depth and depth != (1 << depth_bits) + 1:
                 raise ValueError("AsyncFIFOBuffered only supports depths that are one higher "
                                  "than powers of 2; requested exact depth {} is not"
                                  .format(depth)) from None
+            depth = (1 << depth_bits) + 1
         super().__init__(width=width, depth=depth)
 
         self.r_rst = Signal()

amaranth/utils.py

@@ -1,7 +1,35 @@
-__all__ = ["log2_int", "bits_for"]
+import operator
+
+from ._utils import deprecated
+
+__all__ = ["ceil_log2", "exact_log2", "log2_int", "bits_for"]
 
 
+def ceil_log2(n):
+    """Returns the integer log2 of the smallest power-of-2 greater than or equal to `n`.
+
+    Raises a `ValueError` for negative inputs."""
+    n = operator.index(n)
+    if n < 0:
+        raise ValueError("{n} is negative")
+    if n == 0:
+        return 0
+    return (n - 1).bit_length()
+
+
+def exact_log2(n):
+    """Returns the integer log2 of `n`, which must be an exact power of two.
+
+    Raises a `ValueError` if `n` is not a power of two."""
+    n = operator.index(n)
+    if n <= 0 or (n & (n - 1)):
+        raise ValueError("{n} is not a power of 2")
+    return (n - 1).bit_length()
+
+
+@deprecated("instead of `log2_int(n, True)`, use `exact_log2(n)`; instead of `log2_int(n, False)` use `ceil_log2(n)`")
 def log2_int(n, need_pow2=True):
+    n = operator.index(n)
     if n == 0:
         return 0
     r = (n - 1).bit_length()
@@ -11,11 +39,12 @@ def log2_int(n, need_pow2=True):
 
 
 def bits_for(n, require_sign_bit=False):
+    n = operator.index(n)
     if n > 0:
-        r = log2_int(n + 1, False)
+        r = ceil_log2(n + 1)
     else:
         require_sign_bit = True
-        r = log2_int(-n, False)
+        r = ceil_log2(-n)
     if require_sign_bit:
         r += 1
     return r

docs/changes.rst

@@ -10,13 +10,31 @@ Version 0.5 (unreleased)
 The Migen compatibility layer has been removed.
 
 
+Migrating from version 0.4
+--------------------------
+
+Apply the following changes to code written against Amaranth 0.4 to migrate it to version 0.5:
+
+* Update uses of :func:`amaranth.utils.log2_int(need_pow2=False)` to :func:`amaranth.utils.ceil_log2`
+* Update uses of :func:`amaranth.utils.log2_int(need_pow2=True)` to :func:`amaranth.utils.exact_log2`
+
+
+Implemented RFCs
+----------------
+
+.. _RFC 17: https://amaranth-lang.org/rfcs/0017-remove-log2-int.html
+
+* `RFC 17`_: Remove ``log2_int``
+
+
 Language changes
 ----------------
 
 .. currentmodule:: amaranth.hdl
 
-* Deprecated: argument `run_script=` in :meth:`BuildPlan.execute_local`
-* Added: `class:ast.Slice` objects have been made const-castable.
+* Added: :class:`ast.Slice` objects have been made const-castable.
+* Added: :func:`amaranth.utils.ceil_log2`, :func:`amaranth.utils.exact_log2`. (`RFC 17`_)
+* Deprecated: :func:`amaranth.utils.log2_int`. (`RFC 17`_)
 * Removed: (deprecated in 0.4) :meth:`Const.normalize`. (`RFC 5`_)
 * Removed: (deprecated in 0.4) :class:`ast.Sample`, :class:`ast.Past`, :class:`ast.Stable`, :class:`ast.Rose`, :class:`ast.Fell`.
 
@@ -39,6 +57,7 @@ Platform integration changes
 * Added: :meth:`BuildPlan.execute_local_docker`.
 * Added: :meth:`BuildPlan.extract`.
 * Added: ``build.sh``  begins with ``#!/bin/sh``.
+* Deprecated: argument `run_script=` in :meth:`BuildPlan.execute_local`
 * Removed: (deprecated in 0.4) :mod:`vendor.intel`, :mod:`vendor.lattice_ecp5`, :mod:`vendor.lattice_ice40`, :mod:`vendor.lattice_machxo2_3l`, :mod:`vendor.quicklogic`, :mod:`vendor.xilinx`. (`RFC 18`_)
 
 

tests/test_utils.py

@@ -0,0 +1,75 @@
+import unittest
+
+from amaranth.utils import *
+from amaranth._utils import _ignore_deprecated
+
+
+class Log2TestCase(unittest.TestCase):
+    def test_ceil_log2(self):
+        self.assertEqual(ceil_log2(0), 0)
+        self.assertEqual(ceil_log2(1), 0)
+        self.assertEqual(ceil_log2(2), 1)
+        self.assertEqual(ceil_log2(3), 2)
+        self.assertEqual(ceil_log2(4), 2)
+        self.assertEqual(ceil_log2(5), 3)
+        self.assertEqual(ceil_log2(8), 3)
+        self.assertEqual(ceil_log2(9), 4)
+        with self.assertRaises(TypeError):
+            ceil_log2(1.5)
+        with self.assertRaises(ValueError):
+            ceil_log2(-1)
+
+    def test_exact_log2(self):
+        self.assertEqual(exact_log2(1), 0)
+        self.assertEqual(exact_log2(2), 1)
+        self.assertEqual(exact_log2(4), 2)
+        self.assertEqual(exact_log2(8), 3)
+        for val in [-1, 0, 3, 5, 6, 7, 9]:
+            with self.assertRaises(ValueError):
+                exact_log2(val)
+        with self.assertRaises(TypeError):
+            exact_log2(1.5)
+
+    @_ignore_deprecated
+    def test_log2_int(self):
+        self.assertEqual(log2_int(1), 0)
+        self.assertEqual(log2_int(2), 1)
+        self.assertEqual(log2_int(4), 2)
+        self.assertEqual(log2_int(8), 3)
+        for val in [-1, 3, 5, 6, 7, 9]:
+            with self.assertRaises(ValueError):
+                log2_int(val)
+        with self.assertRaises(TypeError):
+            log2_int(1.5)
+        self.assertEqual(log2_int(0, False), 0)
+        self.assertEqual(log2_int(1, False), 0)
+        self.assertEqual(log2_int(2, False), 1)
+        self.assertEqual(log2_int(3, False), 2)
+        self.assertEqual(log2_int(4, False), 2)
+        self.assertEqual(log2_int(5, False), 3)
+        self.assertEqual(log2_int(8, False), 3)
+        self.assertEqual(log2_int(9, False), 4)
+
+    def test_bits_for(self):
+        self.assertEqual(bits_for(-4), 3)
+        self.assertEqual(bits_for(-3), 3)
+        self.assertEqual(bits_for(-2), 2)
+        self.assertEqual(bits_for(-1), 1)
+        self.assertEqual(bits_for(0), 1)
+        self.assertEqual(bits_for(1), 1)
+        self.assertEqual(bits_for(2), 2)
+        self.assertEqual(bits_for(3), 2)
+        self.assertEqual(bits_for(4), 3)
+        self.assertEqual(bits_for(5), 3)
+        self.assertEqual(bits_for(-4, True), 3)
+        self.assertEqual(bits_for(-3, True), 3)
+        self.assertEqual(bits_for(-2, True), 2)
+        self.assertEqual(bits_for(-1, True), 1)
+        self.assertEqual(bits_for(0, True), 1)
+        self.assertEqual(bits_for(1, True), 2)
+        self.assertEqual(bits_for(2, True), 3)
+        self.assertEqual(bits_for(3, True), 3)
+        self.assertEqual(bits_for(4, True), 4)
+        self.assertEqual(bits_for(5, True), 4)
+        with self.assertRaises(TypeError):
+            bits_for(1.5)