[PATCH v4 5/8] riscv/runtime-const: Introduce runtime_const_mask_32()

[K Prateek Nayak]

Hello folks,

On 4/30/2026 3:17 PM, K Prateek Nayak wrote:
> Futex hash computation requires a mask operation with read-only after
> init data that will be converted to a runtime constant in the subsequent
> commit.
> 
> Introduce runtime_const_mask_32 to further optimize the mask operation
> in the futex hash computation hot path. GCC generates a:
> 
>   lui   a0, 0x12346       # upper; +0x800 then >>12 for correct rounding
>   addi  a0, a0, 0x678     # lower 12 bits
>   and   a1, a1, a0        # a1 = a1 & a0
> 
> pattern to tackle arbitrary 32-bit masks and the same was also suggested
> by Claude which is implemented here. The final (__ret & val) operation
> is intentionally placed outside of asm block to allow compilers to
> further optimize it if possible.
> 
> __runtime_fixup_ptr() already patches a "lui + addi" sequence which has
> been reused to patch the same sequence for __runtime_fixup_mask().
> 
> Assisted-by: Claude:claude-sonnet-4-5
> Signed-off-by: K Prateek Nayak <kprateek.nayak at amd.com>

Gentle ping! I believe this is only missing Acks for the RISC-V bits.
Any and all feedback are welcome. If you have concerns with Sashiko's
comments, I'll answer them below:

> ---
> changelog v3..v4:
> 
> o Reverted back to using __ret as the macro variable to prevent
>   collision with local varaibles at callsite. (Sashiko)
> 
> o Separated out the & operation to prevent any confusion with operator
>   precedence id "val" is an expression. (Sashiko)
> ---
>  arch/riscv/include/asm/runtime-const.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/runtime-const.h b/arch/riscv/include/asm/runtime-const.h
> index 1ce02605d2e43..684641cb0fe82 100644
> --- a/arch/riscv/include/asm/runtime-const.h
> +++ b/arch/riscv/include/asm/runtime-const.h
> @@ -159,6 +159,23 @@
>  	__ret;							\
>  })
>  
> +#define runtime_const_mask_32(val, sym)					\
> +({									\
> +	u32 __ret;							\

>> Could this local variable shadow a variable from the caller? If the caller
>> passes an expression for val that includes a variable named __ret, the
>> inner declaration will shadow it, causing the macro to silently ignore the
>> caller's value and unconditionally return the mask itself.

The "__ret" variable naming has been standard format for all the
runtime constant macros and any future usage with a "__ret" already
defined in the scope would fail build when compiled with -Werror for
variable redeclaration.


> +	asm_inline(".option push\n\t"					\
> +		".option norvc\n\t"					\
> +		"1:\t"							\
> +		"lui	%[__ret], %%hi(" RUNTIME_MAGIC ")\n\t"		\
> +		"addi	%[__ret],%[__ret], %%lo(" RUNTIME_MAGIC ")\n\t"	\

>> Does using addi here violate the RISC-V psABI for 32-bit types on RV64?

The immediate will be patched before the first usage and these only exist
as place holders here for code generation. Patching sequence will handle
it correctly and this is no different than a runtime_const_ptr() patching.

> +		".option pop\n\t"					\
> +		".pushsection runtime_mask_" #sym ",\"a\"\n\t"		\
> +		".long 1b - .\n\t"					\
> +		".popsection"						\
> +		: [__ret] "=r" (__ret));				\
> +	__ret &= val; /* Allow compiler to optimize & operation. */	\

>> Is it safe to expand val without parentheses here? If a complex expression
>> with lower precedence operators is passed as val, it might alter the
>> evaluation order and introduce unexpected runtime behavior.

&= has the lowest precedence with evaluation from right to left so this
shouldn't pose a problem but if required I can add a parenthesis around
the (val) and resend these bits if there are concerns still.

> +	__ret;								\
> +})
> +
>  #define runtime_const_init(type, sym) do {			\
>  	extern s32 __start_runtime_##type##_##sym[];		\
>  	extern s32 __stop_runtime_##type##_##sym[];		\
> @@ -262,6 +279,12 @@ static inline void __runtime_fixup_shift(void *where, unsigned long val)
>  	mutex_unlock(&text_mutex);
>  }
>  
> +static inline void __runtime_fixup_mask(void *where, unsigned long val)
> +{
> +	__runtime_fixup_32(where, where + 4, val);
> +	__runtime_fixup_caches(where, 2);
> +}
> +
>  static inline void runtime_const_fixup(void (*fn)(void *, unsigned long),
>  				       unsigned long val, s32 *start, s32 *end)
>  {

For the final review comment on the core futex bits:

>> Can evaluating the runtime constant immediately after initialization cause a
>> boot regression?

The runtime constants are patched before the first usage and the patching
function introduces sufficient barriers to ensure the following reads
don't see the placeholders and in-order commits will always ensure that
Read After Write hazards are dealt with correctly w.r.t. patching on
BSP.

-- 
Thanks and Regards,
Prateek