[PATCH v2 0/5] riscv: Use __riscv_has_extension_{likely,unlikely}

[PATCH v2 0/5] riscv: Use __riscv_has_extension_{likely,unlikely}

[Vivian Wang]

There are about a dozen uses of asm goto in arch/riscv just to select
between two code paths with the alternative mechanism. Convert them to
the existing helpers __riscv_has_extension_{likely,unlikely}.

In each case, I have preserved the existing code's choice of asm goto
pattern while picking between "likely" and "unlikely", namely:

  ALTERNATIVE("j %l[no]", "nop", ...)   -> "likely"
  ALTERNATIVE("nop", "j %l[yes]", ...)  -> "unlikely"

Since the helpers are just implementations of these patterns, the
performance should be the same as before.

These patches are also available at:

https://github.com/dramforever/linux/tree/riscv/altn-helper/v2

---
Changes in v2:
- Cc'd authors who initially introduced the asm goto blocks
- Use existing __riscv_has_extension_{likely,unlikely} instead
- Remove bogus comment for Zbb being likely (checksum)
- Restructured patch to minimize diff (bitops, hweight, cmpxchg)
- Link to v1: https://lore.kernel.org/r/20250820-riscv-altn-helper-wip-v1-0-c3c626c1f7e6@iscas.ac.cn

---
Vivian Wang (5):
      riscv: pgtable: Use __riscv_has_extension_unlikely
      riscv: checksum: Use __riscv_has_extension_likely
      riscv: hweight: Use __riscv_has_extension_likely
      riscv: bitops: Use __riscv_has_extension_likely
      riscv: cmpxchg: Use __riscv_has_extension_likely

 arch/riscv/include/asm/arch_hweight.h | 24 ++++++----------
 arch/riscv/include/asm/bitops.h       | 32 ++++++---------------
 arch/riscv/include/asm/checksum.h     | 13 +++------
 arch/riscv/include/asm/cmpxchg.h      | 12 +++-----
 arch/riscv/include/asm/pgtable.h      | 15 +++++-----
 arch/riscv/lib/csum.c                 | 53 ++++++++---------------------------
 arch/riscv/mm/pgtable.c               | 22 +++++++--------
 7 files changed, 53 insertions(+), 118 deletions(-)
---
base-commit: 062b3e4a1f880f104a8d4b90b767788786aa7b78
change-id: 20250820-riscv-altn-helper-wip-00af3a552c37

Best regards,
-- 
Vivian "dramforever" Wang

[PATCH v2 1/5] riscv: pgtable: Use __riscv_has_extension_unlikely

[Vivian Wang]

Use __riscv_has_extension_unlikely() to check for RISCV_ISA_EXT_SVVPTC,
replacing the use of asm goto with ALTERNATIVE.

The "unlikely" variant is used to match the behavior of the original
implementation using ALTERNATIVE("nop", "j %l[svvptc]", ...).

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
 arch/riscv/include/asm/pgtable.h | 15 +++++++--------
 arch/riscv/mm/pgtable.c          | 22 ++++++++++------------
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 91697fbf1f9013005800f713797e4b6b1fc8d312..f37a0c3dab8a8c19e21743be743759724bb5fffd 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -495,8 +495,13 @@ static inline void update_mmu_cache_range(struct vm_fault *vmf,
 		struct vm_area_struct *vma, unsigned long address,
 		pte_t *ptep, unsigned int nr)
 {
-	asm goto(ALTERNATIVE("nop", "j %l[svvptc]", 0, RISCV_ISA_EXT_SVVPTC, 1)
-		 : : : : svvptc);
+	/*
+	 * Svvptc guarantees that the new valid pte will be visible within
+	 * a bounded timeframe, so when the uarch does not cache invalid
+	 * entries, we don't have to do anything.
+	 */
+	if (__riscv_has_extension_unlikely(0, RISCV_ISA_EXT_SVVPTC))
+		return;
 
 	/*
 	 * The kernel assumes that TLBs don't cache invalid entries, but
@@ -508,12 +513,6 @@ static inline void update_mmu_cache_range(struct vm_fault *vmf,
 	while (nr--)
 		local_flush_tlb_page(address + nr * PAGE_SIZE);
 
-svvptc:;
-	/*
-	 * Svvptc guarantees that the new valid pte will be visible within
-	 * a bounded timeframe, so when the uarch does not cache invalid
-	 * entries, we don't have to do anything.
-	 */
 }
 #define update_mmu_cache(vma, addr, ptep) \
 	update_mmu_cache_range(NULL, vma, addr, ptep, 1)
diff --git a/arch/riscv/mm/pgtable.c b/arch/riscv/mm/pgtable.c
index 8b6c0a112a8db4e91de54c3bd3bd527a605a6197..289ca6fa6b4de80d42287d28e266a0a8d3848cff 100644
--- a/arch/riscv/mm/pgtable.c
+++ b/arch/riscv/mm/pgtable.c
@@ -9,8 +9,16 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 			  unsigned long address, pte_t *ptep,
 			  pte_t entry, int dirty)
 {
-	asm goto(ALTERNATIVE("nop", "j %l[svvptc]", 0, RISCV_ISA_EXT_SVVPTC, 1)
-		 : : : : svvptc);
+	if (__riscv_has_extension_unlikely(0, RISCV_ISA_EXT_SVVPTC)) {
+		if (!pte_same(ptep_get(ptep), entry)) {
+			__set_pte_at(vma->vm_mm, ptep, entry);
+			/* Here only not svadu is impacted */
+			flush_tlb_page(vma, address);
+			return true;
+		}
+
+		return false;
+	}
 
 	if (!pte_same(ptep_get(ptep), entry))
 		__set_pte_at(vma->vm_mm, ptep, entry);
@@ -19,16 +27,6 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	 * the case that the PTE changed and the spurious fault case.
 	 */
 	return true;
-
-svvptc:
-	if (!pte_same(ptep_get(ptep), entry)) {
-		__set_pte_at(vma->vm_mm, ptep, entry);
-		/* Here only not svadu is impacted */
-		flush_tlb_page(vma, address);
-		return true;
-	}
-
-	return false;
 }
 
 int ptep_test_and_clear_young(struct vm_area_struct *vma,

-- 
2.50.1

[PATCH v2 2/5] riscv: checksum: Use __riscv_has_extension_likely

[Vivian Wang]

Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZBB,
replacing the use of asm goto with ALTERNATIVE.

The "likely" variant is used to match the behavior of the original
implementation using ALTERNATIVE("j %l[no_zbb]", "nop", ...).

While we're at it, also remove bogus comment about Zbb being likely
available. We have to choose between "likely" and "unlikely" due to
limitations of the asm goto feature, but that does not mean we should
put a bad comment on why we pick "likely" over "unlikely".

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
 arch/riscv/include/asm/checksum.h | 13 +++-------
 arch/riscv/lib/csum.c             | 53 +++++++++------------------------------
 2 files changed, 16 insertions(+), 50 deletions(-)

diff --git a/arch/riscv/include/asm/checksum.h b/arch/riscv/include/asm/checksum.h
index da378856f1d590e22271b90e803c7e55e8dd22e3..70eb50173fb6ab636f9e1534ce2ba58de5ee5c54 100644
--- a/arch/riscv/include/asm/checksum.h
+++ b/arch/riscv/include/asm/checksum.h
@@ -49,16 +49,11 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
 	 * ZBB only saves three instructions on 32-bit and five on 64-bit so not
 	 * worth checking if supported without Alternatives.
 	 */
-	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
+	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
+	    IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB) &&
+	    __riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB)) {
 		unsigned long fold_temp;
 
-		asm goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0,
-					      RISCV_ISA_EXT_ZBB, 1)
-		    :
-		    :
-		    :
-		    : no_zbb);
-
 		if (IS_ENABLED(CONFIG_32BIT)) {
 			asm(".option push				\n\
 			.option arch,+zbb				\n\
@@ -81,7 +76,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
 		}
 		return (__force __sum16)(csum >> 16);
 	}
-no_zbb:
+
 #ifndef CONFIG_32BIT
 	csum += ror64(csum, 32);
 	csum >>= 32;
diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c
index 9408f50ca59a8901f7cfbcf3297d1492172c6ea2..420e9eb93e8531bb988823e46f23b0bbb7ca0afb 100644
--- a/arch/riscv/lib/csum.c
+++ b/arch/riscv/lib/csum.c
@@ -40,20 +40,11 @@ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 	uproto = (__force unsigned int)htonl(proto);
 	sum += uproto;
 
-	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
+	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
+	    IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB) &&
+	    __riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB)) {
 		unsigned long fold_temp;
 
-		/*
-		 * Zbb is likely available when the kernel is compiled with Zbb
-		 * support, so nop when Zbb is available and jump when Zbb is
-		 * not available.
-		 */
-		asm goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0,
-					      RISCV_ISA_EXT_ZBB, 1)
-				  :
-				  :
-				  :
-				  : no_zbb);
 		asm(".option push					\n\
 		.option arch,+zbb					\n\
 			rori	%[fold_temp], %[sum], 32		\n\
@@ -66,7 +57,7 @@ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 		: [sum] "+r" (sum), [fold_temp] "=&r" (fold_temp));
 		return (__force __sum16)(sum >> 16);
 	}
-no_zbb:
+
 	sum += ror64(sum, 32);
 	sum >>= 32;
 	return csum_fold((__force __wsum)sum);
@@ -152,21 +143,11 @@ do_csum_with_alignment(const unsigned char *buff, int len)
 	csum = do_csum_common(ptr, end, data);
 
 #ifdef CC_HAS_ASM_GOTO_TIED_OUTPUT
-	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
+	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
+	    IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB) &&
+	    __riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB)) {
 		unsigned long fold_temp;
 
-		/*
-		 * Zbb is likely available when the kernel is compiled with Zbb
-		 * support, so nop when Zbb is available and jump when Zbb is
-		 * not available.
-		 */
-		asm goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0,
-					      RISCV_ISA_EXT_ZBB, 1)
-				  :
-				  :
-				  :
-				  : no_zbb);
-
 #ifdef CONFIG_32BIT
 		asm_goto_output(".option push			\n\
 		.option arch,+zbb				\n\
@@ -204,7 +185,7 @@ do_csum_with_alignment(const unsigned char *buff, int len)
 end:
 		return csum >> 16;
 	}
-no_zbb:
+
 #endif /* CC_HAS_ASM_GOTO_TIED_OUTPUT */
 #ifndef CONFIG_32BIT
 	csum += ror64(csum, 32);
@@ -234,21 +215,11 @@ do_csum_no_alignment(const unsigned char *buff, int len)
 	end = (const unsigned long *)(buff + len);
 	csum = do_csum_common(ptr, end, data);
 
-	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
+	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
+	    IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB) &&
+	    __riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB)) {
 		unsigned long fold_temp;
 
-		/*
-		 * Zbb is likely available when the kernel is compiled with Zbb
-		 * support, so nop when Zbb is available and jump when Zbb is
-		 * not available.
-		 */
-		asm goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0,
-					      RISCV_ISA_EXT_ZBB, 1)
-				  :
-				  :
-				  :
-				  : no_zbb);
-
 #ifdef CONFIG_32BIT
 		asm (".option push				\n\
 		.option arch,+zbb				\n\
@@ -274,7 +245,7 @@ do_csum_no_alignment(const unsigned char *buff, int len)
 #endif /* !CONFIG_32BIT */
 		return csum >> 16;
 	}
-no_zbb:
+
 #ifndef CONFIG_32BIT
 	csum += ror64(csum, 32);
 	csum >>= 32;

-- 
2.50.1

[PATCH v2 3/5] riscv: hweight: Use __riscv_has_extension_likely

[Vivian Wang]

Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZBB,
replacing the use of asm goto with ALTERNATIVE.

The "likely" variant is used to match the behavior of the original
implementation using ALTERNATIVE("j %l[legacy]", "nop", ...).

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
 arch/riscv/include/asm/arch_hweight.h | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/include/asm/arch_hweight.h b/arch/riscv/include/asm/arch_hweight.h
index 0e7cdbbec8efd3c293da2fa96a8c6d0a93faf56f..021bc671de299d604a33301e68c10cb1c28ad2c1 100644
--- a/arch/riscv/include/asm/arch_hweight.h
+++ b/arch/riscv/include/asm/arch_hweight.h
@@ -19,10 +19,10 @@
 
 static __always_inline unsigned int __arch_hweight32(unsigned int w)
 {
-#if defined(CONFIG_RISCV_ISA_ZBB) && defined(CONFIG_TOOLCHAIN_HAS_ZBB)
-	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
-				      RISCV_ISA_EXT_ZBB, 1)
-			  : : : : legacy);
+	if (!(IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
+	      IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB) &&
+	      __riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB)))
+		return __sw_hweight32(w);
 
 	asm (".option push\n"
 	     ".option arch,+zbb\n"
@@ -31,10 +31,6 @@ static __always_inline unsigned int __arch_hweight32(unsigned int w)
 	     : "=r" (w) : "r" (w) :);
 
 	return w;
-
-legacy:
-#endif
-	return __sw_hweight32(w);
 }
 
 static inline unsigned int __arch_hweight16(unsigned int w)
@@ -50,10 +46,10 @@ static inline unsigned int __arch_hweight8(unsigned int w)
 #if BITS_PER_LONG == 64
 static __always_inline unsigned long __arch_hweight64(__u64 w)
 {
-#if defined(CONFIG_RISCV_ISA_ZBB) && defined(CONFIG_TOOLCHAIN_HAS_ZBB)
-	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
-				      RISCV_ISA_EXT_ZBB, 1)
-			  : : : : legacy);
+	if (!(IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
+	      IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB) &&
+	      __riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB)))
+		return __sw_hweight64(w);
 
 	asm (".option push\n"
 	     ".option arch,+zbb\n"
@@ -62,10 +58,6 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
 	     : "=r" (w) : "r" (w) :);
 
 	return w;
-
-legacy:
-#endif
-	return __sw_hweight64(w);
 }
 #else /* BITS_PER_LONG == 64 */
 static inline unsigned long __arch_hweight64(__u64 w)

-- 
2.50.1

[PATCH v2 4/5] riscv: bitops: Use __riscv_has_extension_likely

[Vivian Wang]

Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZBB,
replacing the use of asm goto with ALTERNATIVE.

The "likely" variant is used to match the behavior of the original
implementation using ALTERNATIVE("j %l[legacy]", "nop", ...).

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
 arch/riscv/include/asm/bitops.h | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
index d59310f74c2ba70caeb7b9b0e9221882117583f5..f70ccc0c2ffb86a6fda3bc373504143d0c6a1093 100644
--- a/arch/riscv/include/asm/bitops.h
+++ b/arch/riscv/include/asm/bitops.h
@@ -47,9 +47,8 @@
 
 static __always_inline unsigned long variable__ffs(unsigned long word)
 {
-	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
-				      RISCV_ISA_EXT_ZBB, 1)
-			  : : : : legacy);
+	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
+		return generic___ffs(word);
 
 	asm volatile (".option push\n"
 		      ".option arch,+zbb\n"
@@ -58,9 +57,6 @@ static __always_inline unsigned long variable__ffs(unsigned long word)
 		      : "=r" (word) : "r" (word) :);
 
 	return word;
-
-legacy:
-	return generic___ffs(word);
 }
 
 /**
@@ -76,9 +72,8 @@ static __always_inline unsigned long variable__ffs(unsigned long word)
 
 static __always_inline unsigned long variable__fls(unsigned long word)
 {
-	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
-				      RISCV_ISA_EXT_ZBB, 1)
-			  : : : : legacy);
+	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
+		return generic___fls(word);
 
 	asm volatile (".option push\n"
 		      ".option arch,+zbb\n"
@@ -87,9 +82,6 @@ static __always_inline unsigned long variable__fls(unsigned long word)
 		      : "=r" (word) : "r" (word) :);
 
 	return BITS_PER_LONG - 1 - word;
-
-legacy:
-	return generic___fls(word);
 }
 
 /**
@@ -105,9 +97,8 @@ static __always_inline unsigned long variable__fls(unsigned long word)
 
 static __always_inline int variable_ffs(int x)
 {
-	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
-				      RISCV_ISA_EXT_ZBB, 1)
-			  : : : : legacy);
+	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
+		return generic_ffs(x);
 
 	if (!x)
 		return 0;
@@ -119,9 +110,6 @@ static __always_inline int variable_ffs(int x)
 		      : "=r" (x) : "r" (x) :);
 
 	return x + 1;
-
-legacy:
-	return generic_ffs(x);
 }
 
 /**
@@ -137,9 +125,8 @@ static __always_inline int variable_ffs(int x)
 
 static __always_inline int variable_fls(unsigned int x)
 {
-	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
-				      RISCV_ISA_EXT_ZBB, 1)
-			  : : : : legacy);
+	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
+		return generic_fls(x);
 
 	if (!x)
 		return 0;
@@ -151,9 +138,6 @@ static __always_inline int variable_fls(unsigned int x)
 		      : "=r" (x) : "r" (x) :);
 
 	return 32 - x;
-
-legacy:
-	return generic_fls(x);
 }
 
 /**

-- 
2.50.1

Re: [PATCH v2 4/5] riscv: bitops: Use __riscv_has_extension_likely

[Yury Norov]

On Thu, Aug 21, 2025 at 05:16:34PM +0800, Vivian Wang wrote:
> Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZBB,
> replacing the use of asm goto with ALTERNATIVE.
> 
> The "likely" variant is used to match the behavior of the original
> implementation using ALTERNATIVE("j %l[legacy]", "nop", ...).
> 
> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
> ---
>  arch/riscv/include/asm/bitops.h | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
> index d59310f74c2ba70caeb7b9b0e9221882117583f5..f70ccc0c2ffb86a6fda3bc373504143d0c6a1093 100644
> --- a/arch/riscv/include/asm/bitops.h
> +++ b/arch/riscv/include/asm/bitops.h
> @@ -47,9 +47,8 @@
>  
>  static __always_inline unsigned long variable__ffs(unsigned long word)
>  {
> -	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
> -				      RISCV_ISA_EXT_ZBB, 1)
> -			  : : : : legacy);
> +	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
> +		return generic___ffs(word);

So, on the previous round you spent quite a lot of time explaining how
'unlikely()' version is handy over '!likely()', and now use just the
latter. I feel really lost about how the code generation should look.

Can you please share bloat-o-meter report against this patch? Can you
also show an example of code generation before and after? Have you
tried the 'unlikely()` one? How the output looks?

>  	asm volatile (".option push\n"
>  		      ".option arch,+zbb\n"

Yeah, now the diff is much cleaner. Thanks.

Re: [PATCH v2 4/5] riscv: bitops: Use __riscv_has_extension_likely

[Vivian Wang]

On 8/21/25 22:44, Yury Norov wrote:

> On Thu, Aug 21, 2025 at 05:16:34PM +0800, Vivian Wang wrote:
>> Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZBB,
>> replacing the use of asm goto with ALTERNATIVE.
>>
>> The "likely" variant is used to match the behavior of the original
>> implementation using ALTERNATIVE("j %l[legacy]", "nop", ...).
>>
>> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
>> ---
>>  arch/riscv/include/asm/bitops.h | 32 ++++++++------------------------
>>  1 file changed, 8 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
>> index d59310f74c2ba70caeb7b9b0e9221882117583f5..f70ccc0c2ffb86a6fda3bc373504143d0c6a1093 100644
>> --- a/arch/riscv/include/asm/bitops.h
>> +++ b/arch/riscv/include/asm/bitops.h
>> @@ -47,9 +47,8 @@
>>  
>>  static __always_inline unsigned long variable__ffs(unsigned long word)
>>  {
>> -	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
>> -				      RISCV_ISA_EXT_ZBB, 1)
>> -			  : : : : legacy);
>> +	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
>> +		return generic___ffs(word);
> So, on the previous round you spent quite a lot of time explaining how
> 'unlikely()' version is handy over '!likely()', and now use just the
> latter. I feel really lost about how the code generation should look.

It's not "handy". The operative part is "has_extension", and both
functions return true if the extension is available and false if not.
Functionally:

    if (likely()) <-- equivalent --> if (unlikely())
    if (!likely()) <-- equivalent --> if (!unlikely())

Whereas:

    if (likely()) <-- **opposite of** --> if (!unlikely())
    if (unlikely()) <-- **opposite of** --> if (!likely())

All the asm goto dispatch stuff work like this:
static_branch_{likely,unlikely}, (arm64)
alternative_has_cap_{likely,unlikely},
__riscv_has_extension_{likely,unlikely}. Maybe it's confusing, but it
seems to be the convention.

And, codegen-wise:

ALTERNATIVE("j %l[no_alt]", "nop", ...) -> likely() ALTERNATIVE("nop",
"j %l[has_alt]", ...) -> unlikely()

Since the original code has the "likely" pattern, using "if (likely())"
preserves it. Whatever the codegen was, it's still the same.

> Can you please share bloat-o-meter report against this patch? Can you
> also show an example of code generation before and after? Have you
> tried the 'unlikely()` one? How the output looks?

Thanks for the tip on bloat-o-meter. I'll take a look tomorrow.

>>  	asm volatile (".option push\n"
>>  		      ".option arch,+zbb\n"
> Yeah, now the diff is much cleaner. Thanks.

This is why the condition at the top needed to be "!has_extension". So
the structure can be:

    if (!has_extension)
        return sw_version;

    multi_line
    zbb_version;

If I used "if (has_extension)" the code would need be

    if (has_extension) {
        multi_line
        zbb_version;
    } else {
        sw_version;
    }

And whether it was "likely" or "unlikely" had no bearing on the
structure of the code.

Vivian "dramforever" Wang

Re: [PATCH v2 4/5] riscv: bitops: Use __riscv_has_extension_likely

[Vivian Wang]

On 8/22/25 01:46, Vivian Wang wrote:

> [...]
>
> And, codegen-wise:
>
> ALTERNATIVE("j %l[no_alt]", "nop", ...) -> likely() ALTERNATIVE("nop",
> "j %l[has_alt]", ...) -> unlikely()
>
I messed up the formatting, should be:

    ALTERNATIVE("j %l[no_alt]", "nop", ...) -> likely()
    ALTERNATIVE("nop", "j %l[has_alt]", ...) -> unlikely()

Vivian "dramforever" Wang

Re: [PATCH v2 4/5] riscv: bitops: Use __riscv_has_extension_likely

[Yury Norov]

On Fri, Aug 22, 2025 at 01:46:19AM +0800, Vivian Wang wrote:
> On 8/21/25 22:44, Yury Norov wrote:
> 
> > On Thu, Aug 21, 2025 at 05:16:34PM +0800, Vivian Wang wrote:
> >> Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZBB,
> >> replacing the use of asm goto with ALTERNATIVE.
> >>
> >> The "likely" variant is used to match the behavior of the original
> >> implementation using ALTERNATIVE("j %l[legacy]", "nop", ...).
> >>
> >> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
> >> ---
> >>  arch/riscv/include/asm/bitops.h | 32 ++++++++------------------------
> >>  1 file changed, 8 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
> >> index d59310f74c2ba70caeb7b9b0e9221882117583f5..f70ccc0c2ffb86a6fda3bc373504143d0c6a1093 100644
> >> --- a/arch/riscv/include/asm/bitops.h
> >> +++ b/arch/riscv/include/asm/bitops.h
> >> @@ -47,9 +47,8 @@
> >>  
> >>  static __always_inline unsigned long variable__ffs(unsigned long word)
> >>  {
> >> -	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
> >> -				      RISCV_ISA_EXT_ZBB, 1)
> >> -			  : : : : legacy);
> >> +	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZBB))
> >> +		return generic___ffs(word);
> > So, on the previous round you spent quite a lot of time explaining how
> > 'unlikely()' version is handy over '!likely()', and now use just the
> > latter. I feel really lost about how the code generation should look.
> 
> It's not "handy". The operative part is "has_extension", and both
> functions return true if the extension is available and false if not.
> Functionally:
> 
>     if (likely()) <-- equivalent --> if (unlikely())
>     if (!likely()) <-- equivalent --> if (!unlikely())
> 
> Whereas:
> 
>     if (likely()) <-- **opposite of** --> if (!unlikely())
>     if (unlikely()) <-- **opposite of** --> if (!likely())
> 
> All the asm goto dispatch stuff work like this:
> static_branch_{likely,unlikely}, (arm64)
> alternative_has_cap_{likely,unlikely},
> __riscv_has_extension_{likely,unlikely}. Maybe it's confusing, but it
> seems to be the convention.
> 
> And, codegen-wise:
> 
> ALTERNATIVE("j %l[no_alt]", "nop", ...) -> likely() ALTERNATIVE("nop",
> "j %l[has_alt]", ...) -> unlikely()
> 
> Since the original code has the "likely" pattern, using "if (likely())"
> preserves it. Whatever the codegen was, it's still the same.
> 
> > Can you please share bloat-o-meter report against this patch? Can you
> > also show an example of code generation before and after? Have you
> > tried the 'unlikely()` one? How the output looks?
> 
> Thanks for the tip on bloat-o-meter. I'll take a look tomorrow.
> 
> >>  	asm volatile (".option push\n"
> >>  		      ".option arch,+zbb\n"
> > Yeah, now the diff is much cleaner. Thanks.
> 
> This is why the condition at the top needed to be "!has_extension". So
> the structure can be:
> 
>     if (!has_extension)
>         return sw_version;
> 
>     multi_line
>     zbb_version;
> 
> If I used "if (has_extension)" the code would need be
> 
>     if (has_extension) {
>         multi_line
>         zbb_version;
>     } else {
>         sw_version;
>     }
> 
> And whether it was "likely" or "unlikely" had no bearing on the
> structure of the code.

OK, I see. Sorry for confusion.

Acked-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>

Re: [PATCH v2 4/5] riscv: bitops: Use __riscv_has_extension_likely

[Vivian Wang]

On 8/22/25 01:46, Vivian Wang wrote:

> [...]
>> Can you please share bloat-o-meter report against this patch? Can you
>> also show an example of code generation before and after? Have you
>> tried the 'unlikely()` one? How the output looks?
> Thanks for the tip on bloat-o-meter. I'll take a look tomorrow.

That "tomorrow" took a while.

This is what it looks like, old being v6.17-rc1 and new being this patch
series.

It's not as identical as I had hoped originally, but I had went into
each plus and a few minuses and confirmed that the actual asm goto part
seems to have been recreated as expected. The rest of the differences
appear to be explainable by unpredictable factors in the compiler (GCC
14.3.0 in my case).

For example, bpf_lru_populate seems to have got worse register
allocation. It uses one more callee-saved register. Moreover, RISC-V
compressed instructions has shorter encodings when used with some
registers, so for example sd a1,32(s1) is encodable as 2 bytes, but sd
a1,32(s2) is only encodable as 4 bytes. This appears to explain the +16
in code size.

As far as I can tell, which is basically me staring at objdump and
seeing "yeah looks normal", all of these are caused by random factors
due to changes in how now we write the control structures:

add/remove: 0/0 grow/shrink: 14/24 up/down: 72/-234 (-162)
Function                                     old     new   delta
bpf_lru_populate                             450     466     +16
spi_nor_scan                                3506    3516     +10
wants_mount_setattr                          688     696      +8
regulator_irq_map_event_simple               202     208      +6
idling_boosts_thr_without_issues             198     204      +6
trie_lookup_elem                             704     708      +4
ethnl_set_tsconfig                          1694    1698      +4
dev_xdp_attach                              1142    1146      +4
add_mtd_device                              1468    1472      +4
xhci_count_num_new_endpoints.isra            104     106      +2
rtl_init_one                                4360    4362      +2
queued_read_lock_slowpath                    414     416      +2
osq_lock                                     262     264      +2
cpufreq_dbs_governor_start                   520     522      +2
thaw_super_locked                            622     620      -2
stop_machine_from_inactive_cpu               372     370      -2
objpool_init                                 962     960      -2
memweight                                    168     166      -2
irq_destroy_ipi                              248     246      -2
fat_fill_super                              3408    3406      -2
create_boot_cache                            292     290      -2
snd_soc_dapm_get_volsw                       588     584      -4
ip_rcv_core                                  770     766      -4
ip_mc_check_igmp                             736     732      -4
tmigr_quick_check                            224     218      -6
nvdimm_security_flags                        152     146      -6
inode_switch_wbs_work_fn                    1934    1928      -6
sd_uhs2_power_up                             176     168      -8
mmc_power_up.part                            402     394      -8
__alloc_bucket_spinlocks                     190     182      -8
__clk_hw_register_mux                        624     612     -12
bfq_bfqq_expire                              872     858     -14
perf_prepare_sample                         1810    1794     -16
wq_update_node_max_active                    308     288     -20
blk_mq_num_queues                             94      74     -20
register_pidns_sysctls                       248     226     -22
dw8250_setup_port                           1212    1182     -30
build_sched_domains                         4748    4716     -32
Total: Before=16029885, After=16029723, chg -0.00%

That's all I can figure out. I hope this is satisfactory, to anyone reading.

Vivian "dramforever" Wang

[PATCH v2 5/5] riscv: cmpxchg: Use __riscv_has_extension_likely

[Vivian Wang]

Use __riscv_has_extension_likely() to check for RISCV_ISA_EXT_ZAWRS,
replacing the use of asm goto with ALTERNATIVE.

The "likely" variant is used to match the behavior of the original
implementation using ALTERNATIVE("j %l[no_zawrs]", "nop", ...).

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
 arch/riscv/include/asm/cmpxchg.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 0b749e7102162477432f7cf9a34768fbdf2e8cc7..6a372ab9bcf68ba5eb712ad9d082ec66198b7160 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -370,9 +370,10 @@ static __always_inline void __cmpwait(volatile void *ptr,
 	u32 *__ptr32b;
 	ulong __s, __val, __mask;
 
-	asm goto(ALTERNATIVE("j %l[no_zawrs]", "nop",
-			     0, RISCV_ISA_EXT_ZAWRS, 1)
-		 : : : : no_zawrs);
+	if (!__riscv_has_extension_likely(0, RISCV_ISA_EXT_ZAWRS)) {
+		asm volatile(RISCV_PAUSE : : : "memory");
+		return;
+	}
 
 	switch (size) {
 	case 1:
@@ -434,11 +435,6 @@ static __always_inline void __cmpwait(volatile void *ptr,
 	default:
 		BUILD_BUG();
 	}
-
-	return;
-
-no_zawrs:
-	asm volatile(RISCV_PAUSE : : : "memory");
 }
 
 #define __cmpwait_relaxed(ptr, val) \

-- 
2.50.1