[v1] ipa: libipa: quantized: Enable `constexpr` operation
diff mbox series

Message ID 20260319080005.162571-1-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • [v1] ipa: libipa: quantized: Enable `constexpr` operation
Related show

Commit Message

Barnabás Pőcze March 19, 2026, 8 a.m. UTC
There is nothing inherently non-constexpr in the `Quantized` type. Whether
it can work in `constexpr` contexts depends on the traits type. There is
no reason to explicitly disallow `constexpr` operation. So mark all eligible
methods `constexpr`.

In addition, add some `static_assert()`s to the "quantized" test to check
constexpr operation.

For example, `FixedPointQTraits<...>::toFloat()` is `constexpr`, so this
enables the construction of `{U,}Q<...>` from the underlying quantized
value in `constexpr` contexts, which can be useful for example for
storing default values in e.g. `static constexpr` variables.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/libipa/quantized.h    | 32 +++++++++++++++++++-------------
 test/ipa/libipa/quantized.cpp | 15 +++++++++++++--
 2 files changed, 32 insertions(+), 15 deletions(-)

Comments

Kieran Bingham March 19, 2026, 12:46 p.m. UTC | #1
Quoting Barnabás Pőcze (2026-03-19 08:00:05)
> There is nothing inherently non-constexpr in the `Quantized` type. Whether
> it can work in `constexpr` contexts depends on the traits type. There is
> no reason to explicitly disallow `constexpr` operation. So mark all eligible
> methods `constexpr`.
> 
> In addition, add some `static_assert()`s to the "quantized" test to check
> constexpr operation.
> 
> For example, `FixedPointQTraits<...>::toFloat()` is `constexpr`, so this
> enables the construction of `{U,}Q<...>` from the underlying quantized
> value in `constexpr` contexts, which can be useful for example for
> storing default values in e.g. `static constexpr` variables.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Great! I think I remember trying to make parts constexpr, but you were
concerned about needing a specific C++ revision?

But I assume that doesn't impact here so:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/libipa/quantized.h    | 32 +++++++++++++++++++-------------
>  test/ipa/libipa/quantized.cpp | 15 +++++++++++++--
>  2 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h
> index 6bd196c7d..8d7b06a2d 100644
> --- a/src/ipa/libipa/quantized.h
> +++ b/src/ipa/libipa/quantized.h
> @@ -24,34 +24,40 @@ struct Quantized {
>         static_assert(std::is_arithmetic_v<QuantizedType>,
>                       "Quantized: QuantizedType must be arithmetic");
>  
> -       Quantized()
> +       constexpr Quantized()
>                 : Quantized(0.0f) {}
> -       Quantized(float x) { *this = x; }
> -       Quantized(QuantizedType x) { *this = x; }
>  
> -       Quantized &operator=(float x)
> +       constexpr Quantized(float x)
> +               : Quantized(Traits::fromFloat(x))
>         {
> -               quantized_ = Traits::fromFloat(x);
> -               value_ = Traits::toFloat(quantized_);
> +       }
> +
> +       constexpr Quantized(QuantizedType x)
> +               : quantized_(x), value_(Traits::toFloat(x))
> +       {
> +       }
> +
> +       constexpr Quantized &operator=(float x)
> +       {
> +               *this = Quantized(x);
>                 return *this;
>         }
>  
> -       Quantized &operator=(QuantizedType x)
> +       constexpr Quantized &operator=(QuantizedType x)
>         {
> -               value_ = Traits::toFloat(x);
> -               quantized_ = x;
> +               *this = Quantized(x);
>                 return *this;
>         }
>  
> -       float value() const { return value_; }
> -       QuantizedType quantized() const { return quantized_; }
> +       constexpr float value() const { return value_; }
> +       constexpr QuantizedType quantized() const { return quantized_; }
>  
> -       bool operator==(const Quantized &other) const
> +       constexpr bool operator==(const Quantized &other) const
>         {
>                 return quantized_ == other.quantized_;
>         }
>  
> -       bool operator!=(const Quantized &other) const
> +       constexpr bool operator!=(const Quantized &other) const
>         {
>                 return !(*this == other);
>         }
> diff --git a/test/ipa/libipa/quantized.cpp b/test/ipa/libipa/quantized.cpp
> index f138597f2..18655dac5 100644
> --- a/test/ipa/libipa/quantized.cpp
> +++ b/test/ipa/libipa/quantized.cpp
> @@ -25,7 +25,7 @@ struct BrightnessHueTraits {
>                 int quantized = std::lround(v * 128.0f);
>                 return std::clamp<int>(quantized, -128, 127);
>         }
> -       static float toFloat(QuantizedType v)
> +       static constexpr float toFloat(QuantizedType v)
>         {
>                 return static_cast<float>(v) / 128.0f;
>         }
> @@ -40,7 +40,7 @@ struct ContrastSaturationTraits {
>                 int quantized = std::lround(v * 128.0f);
>                 return std::clamp<int>(quantized, 0, 255);
>         }
> -       static float toFloat(QuantizedType v)
> +       static constexpr float toFloat(QuantizedType v)
>         {
>                 return static_cast<float>(v) / 128.0f;
>         }
> @@ -138,6 +138,17 @@ protected:
>                                 return TestFail;
>                 }
>  
> +               /* Test constexpr operation */
> +               {
> +                       constexpr BrightnessQ b1(uint8_t(1));
> +                       constexpr BrightnessQ b2(uint8_t(2));
> +
> +                       static_assert(b1.quantized() == 1);
> +                       static_assert(b2.quantized() == 2);
> +                       static_assert(b1 != b2);
> +                       static_assert(!(b1 == b2));
> +               }
> +
>                 return TestPass;
>         }
>  };
> -- 
> 2.53.0
>
Laurent Pinchart March 19, 2026, 4:22 p.m. UTC | #2
On Thu, Mar 19, 2026 at 09:00:05AM +0100, Barnabás Pőcze wrote:
> There is nothing inherently non-constexpr in the `Quantized` type. Whether
> it can work in `constexpr` contexts depends on the traits type. There is
> no reason to explicitly disallow `constexpr` operation. So mark all eligible
> methods `constexpr`.
> 
> In addition, add some `static_assert()`s to the "quantized" test to check
> constexpr operation.
> 
> For example, `FixedPointQTraits<...>::toFloat()` is `constexpr`, so this
> enables the construction of `{U,}Q<...>` from the underlying quantized
> value in `constexpr` contexts, which can be useful for example for
> storing default values in e.g. `static constexpr` variables.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/ipa/libipa/quantized.h    | 32 +++++++++++++++++++-------------
>  test/ipa/libipa/quantized.cpp | 15 +++++++++++++--
>  2 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h
> index 6bd196c7d..8d7b06a2d 100644
> --- a/src/ipa/libipa/quantized.h
> +++ b/src/ipa/libipa/quantized.h
> @@ -24,34 +24,40 @@ struct Quantized {
>  	static_assert(std::is_arithmetic_v<QuantizedType>,
>  		      "Quantized: QuantizedType must be arithmetic");
>  
> -	Quantized()
> +	constexpr Quantized()
>  		: Quantized(0.0f) {}
> -	Quantized(float x) { *this = x; }
> -	Quantized(QuantizedType x) { *this = x; }
>  
> -	Quantized &operator=(float x)
> +	constexpr Quantized(float x)
> +		: Quantized(Traits::fromFloat(x))
>  	{
> -		quantized_ = Traits::fromFloat(x);
> -		value_ = Traits::toFloat(quantized_);
> +	}
> +
> +	constexpr Quantized(QuantizedType x)
> +		: quantized_(x), value_(Traits::toFloat(x))
> +	{
> +	}
> +
> +	constexpr Quantized &operator=(float x)
> +	{
> +		*this = Quantized(x);
>  		return *this;
>  	}
>  
> -	Quantized &operator=(QuantizedType x)
> +	constexpr Quantized &operator=(QuantizedType x)
>  	{
> -		value_ = Traits::toFloat(x);
> -		quantized_ = x;
> +		*this = Quantized(x);
>  		return *this;
>  	}
>  
> -	float value() const { return value_; }
> -	QuantizedType quantized() const { return quantized_; }
> +	constexpr float value() const { return value_; }
> +	constexpr QuantizedType quantized() const { return quantized_; }
>  
> -	bool operator==(const Quantized &other) const
> +	constexpr bool operator==(const Quantized &other) const
>  	{
>  		return quantized_ == other.quantized_;
>  	}
>  
> -	bool operator!=(const Quantized &other) const
> +	constexpr bool operator!=(const Quantized &other) const
>  	{
>  		return !(*this == other);
>  	}
> diff --git a/test/ipa/libipa/quantized.cpp b/test/ipa/libipa/quantized.cpp
> index f138597f2..18655dac5 100644
> --- a/test/ipa/libipa/quantized.cpp
> +++ b/test/ipa/libipa/quantized.cpp
> @@ -25,7 +25,7 @@ struct BrightnessHueTraits {
>  		int quantized = std::lround(v * 128.0f);
>  		return std::clamp<int>(quantized, -128, 127);
>  	}
> -	static float toFloat(QuantizedType v)
> +	static constexpr float toFloat(QuantizedType v)
>  	{
>  		return static_cast<float>(v) / 128.0f;
>  	}
> @@ -40,7 +40,7 @@ struct ContrastSaturationTraits {
>  		int quantized = std::lround(v * 128.0f);
>  		return std::clamp<int>(quantized, 0, 255);
>  	}
> -	static float toFloat(QuantizedType v)
> +	static constexpr float toFloat(QuantizedType v)
>  	{
>  		return static_cast<float>(v) / 128.0f;
>  	}
> @@ -138,6 +138,17 @@ protected:
>  				return TestFail;
>  		}
>  
> +		/* Test constexpr operation */
> +		{
> +			constexpr BrightnessQ b1(uint8_t(1));
> +			constexpr BrightnessQ b2(uint8_t(2));
> +
> +			static_assert(b1.quantized() == 1);
> +			static_assert(b2.quantized() == 2);
> +			static_assert(b1 != b2);
> +			static_assert(!(b1 == b2));
> +		}
> +
>  		return TestPass;
>  	}
>  };

Patch
diff mbox series

diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h
index 6bd196c7d..8d7b06a2d 100644
--- a/src/ipa/libipa/quantized.h
+++ b/src/ipa/libipa/quantized.h
@@ -24,34 +24,40 @@  struct Quantized {
 	static_assert(std::is_arithmetic_v<QuantizedType>,
 		      "Quantized: QuantizedType must be arithmetic");
 
-	Quantized()
+	constexpr Quantized()
 		: Quantized(0.0f) {}
-	Quantized(float x) { *this = x; }
-	Quantized(QuantizedType x) { *this = x; }
 
-	Quantized &operator=(float x)
+	constexpr Quantized(float x)
+		: Quantized(Traits::fromFloat(x))
 	{
-		quantized_ = Traits::fromFloat(x);
-		value_ = Traits::toFloat(quantized_);
+	}
+
+	constexpr Quantized(QuantizedType x)
+		: quantized_(x), value_(Traits::toFloat(x))
+	{
+	}
+
+	constexpr Quantized &operator=(float x)
+	{
+		*this = Quantized(x);
 		return *this;
 	}
 
-	Quantized &operator=(QuantizedType x)
+	constexpr Quantized &operator=(QuantizedType x)
 	{
-		value_ = Traits::toFloat(x);
-		quantized_ = x;
+		*this = Quantized(x);
 		return *this;
 	}
 
-	float value() const { return value_; }
-	QuantizedType quantized() const { return quantized_; }
+	constexpr float value() const { return value_; }
+	constexpr QuantizedType quantized() const { return quantized_; }
 
-	bool operator==(const Quantized &other) const
+	constexpr bool operator==(const Quantized &other) const
 	{
 		return quantized_ == other.quantized_;
 	}
 
-	bool operator!=(const Quantized &other) const
+	constexpr bool operator!=(const Quantized &other) const
 	{
 		return !(*this == other);
 	}
diff --git a/test/ipa/libipa/quantized.cpp b/test/ipa/libipa/quantized.cpp
index f138597f2..18655dac5 100644
--- a/test/ipa/libipa/quantized.cpp
+++ b/test/ipa/libipa/quantized.cpp
@@ -25,7 +25,7 @@  struct BrightnessHueTraits {
 		int quantized = std::lround(v * 128.0f);
 		return std::clamp<int>(quantized, -128, 127);
 	}
-	static float toFloat(QuantizedType v)
+	static constexpr float toFloat(QuantizedType v)
 	{
 		return static_cast<float>(v) / 128.0f;
 	}
@@ -40,7 +40,7 @@  struct ContrastSaturationTraits {
 		int quantized = std::lround(v * 128.0f);
 		return std::clamp<int>(quantized, 0, 255);
 	}
-	static float toFloat(QuantizedType v)
+	static constexpr float toFloat(QuantizedType v)
 	{
 		return static_cast<float>(v) / 128.0f;
 	}
@@ -138,6 +138,17 @@  protected:
 				return TestFail;
 		}
 
+		/* Test constexpr operation */
+		{
+			constexpr BrightnessQ b1(uint8_t(1));
+			constexpr BrightnessQ b2(uint8_t(2));
+
+			static_assert(b1.quantized() == 1);
+			static_assert(b2.quantized() == 2);
+			static_assert(b1 != b2);
+			static_assert(!(b1 == b2));
+		}
+
 		return TestPass;
 	}
 };