crypto: x86/sha256: Add parentheses around macros' single arguments

[PATCH] crypto: x86/sha256: Add parentheses around macros' single arguments

Posted by Fangrui Song 1 year, 5 months ago

The macros FOUR_ROUNDS_AND_SCHED and DO_4ROUNDS rely on an
unexpected/undocumented behavior of the GNU assembler, which might
change in the future
(https://sourceware.org/bugzilla/show_bug.cgi?id=32073).

    M (1) (2) // 1 arg !? Future: 2 args
    M 1 + 2   // 1 arg !? Future: 3 args

    M 1 2     // 2 args

Add parentheses around the single arguments to support future GNU
assembler and LLVM integrated assembler (when the IsOperator hack from
the following link is dropped).

Link: https://github.com/llvm/llvm-project/commit/055006475e22014b28a070db1bff41ca15f322f0
Signed-off-by: Fangrui Song <maskray@google.com>
---
 arch/x86/crypto/sha256-avx2-asm.S | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/crypto/sha256-avx2-asm.S b/arch/x86/crypto/sha256-avx2-asm.S
index 0ffb072be956..0bbec1c75cd0 100644
--- a/arch/x86/crypto/sha256-avx2-asm.S
+++ b/arch/x86/crypto/sha256-avx2-asm.S
@@ -592,22 +592,22 @@ SYM_TYPED_FUNC_START(sha256_transform_rorx)
 	leaq	K256+0*32(%rip), INP		## reuse INP as scratch reg
 	vpaddd	(INP, SRND), X0, XFER
 	vmovdqa XFER, 0*32+_XFER(%rsp, SRND)
-	FOUR_ROUNDS_AND_SCHED	_XFER + 0*32
+	FOUR_ROUNDS_AND_SCHED	(_XFER + 0*32)
 
 	leaq	K256+1*32(%rip), INP
 	vpaddd	(INP, SRND), X0, XFER
 	vmovdqa XFER, 1*32+_XFER(%rsp, SRND)
-	FOUR_ROUNDS_AND_SCHED	_XFER + 1*32
+	FOUR_ROUNDS_AND_SCHED	(_XFER + 1*32)
 
 	leaq	K256+2*32(%rip), INP
 	vpaddd	(INP, SRND), X0, XFER
 	vmovdqa XFER, 2*32+_XFER(%rsp, SRND)
-	FOUR_ROUNDS_AND_SCHED	_XFER + 2*32
+	FOUR_ROUNDS_AND_SCHED	(_XFER + 2*32)
 
 	leaq	K256+3*32(%rip), INP
 	vpaddd	(INP, SRND), X0, XFER
 	vmovdqa XFER, 3*32+_XFER(%rsp, SRND)
-	FOUR_ROUNDS_AND_SCHED	_XFER + 3*32
+	FOUR_ROUNDS_AND_SCHED	(_XFER + 3*32)
 
 	add	$4*32, SRND
 	cmp	$3*4*32, SRND
@@ -618,12 +618,12 @@ SYM_TYPED_FUNC_START(sha256_transform_rorx)
 	leaq	K256+0*32(%rip), INP
 	vpaddd	(INP, SRND), X0, XFER
 	vmovdqa XFER, 0*32+_XFER(%rsp, SRND)
-	DO_4ROUNDS	_XFER + 0*32
+	DO_4ROUNDS	(_XFER + 0*32)
 
 	leaq	K256+1*32(%rip), INP
 	vpaddd	(INP, SRND), X1, XFER
 	vmovdqa XFER, 1*32+_XFER(%rsp, SRND)
-	DO_4ROUNDS	_XFER + 1*32
+	DO_4ROUNDS	(_XFER + 1*32)
 	add	$2*32, SRND
 
 	vmovdqa	X2, X0
@@ -651,8 +651,8 @@ SYM_TYPED_FUNC_START(sha256_transform_rorx)
 	xor	SRND, SRND
 .align 16
 .Lloop3:
-	DO_4ROUNDS	 _XFER + 0*32 + 16
-	DO_4ROUNDS	 _XFER + 1*32 + 16
+	DO_4ROUNDS	(_XFER + 0*32 + 16)
+	DO_4ROUNDS	(_XFER + 1*32 + 16)
 	add	$2*32, SRND
 	cmp	$4*4*32, SRND
 	jb	.Lloop3
-- 
2.46.0.76.ge559c4bf1a-goog

Re: [PATCH] crypto: x86/sha256: Add parentheses around macros' single arguments

Posted by Herbert Xu 1 year, 5 months ago

On Tue, Aug 13, 2024 at 09:48:02PM -0700, Fangrui Song wrote:
> The macros FOUR_ROUNDS_AND_SCHED and DO_4ROUNDS rely on an
> unexpected/undocumented behavior of the GNU assembler, which might
> change in the future
> (https://sourceware.org/bugzilla/show_bug.cgi?id=32073).
> 
>     M (1) (2) // 1 arg !? Future: 2 args
>     M 1 + 2   // 1 arg !? Future: 3 args
> 
>     M 1 2     // 2 args
> 
> Add parentheses around the single arguments to support future GNU
> assembler and LLVM integrated assembler (when the IsOperator hack from
> the following link is dropped).
> 
> Link: https://github.com/llvm/llvm-project/commit/055006475e22014b28a070db1bff41ca15f322f0
> Signed-off-by: Fangrui Song <maskray@google.com>
> ---
>  arch/x86/crypto/sha256-avx2-asm.S | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: x86/sha256: Add parentheses around macros' single arguments

Posted by Jan Beulich 1 year, 5 months ago

On 14.08.2024 06:48, Fangrui Song wrote:
> The macros FOUR_ROUNDS_AND_SCHED and DO_4ROUNDS rely on an
> unexpected/undocumented behavior of the GNU assembler, which might
> change in the future
> (https://sourceware.org/bugzilla/show_bug.cgi?id=32073).
> 
>     M (1) (2) // 1 arg !? Future: 2 args
>     M 1 + 2   // 1 arg !? Future: 3 args
> 
>     M 1 2     // 2 args
> 
> Add parentheses around the single arguments to support future GNU
> assembler and LLVM integrated assembler (when the IsOperator hack from
> the following link is dropped).
> 
> Link: https://github.com/llvm/llvm-project/commit/055006475e22014b28a070db1bff41ca15f322f0
> Signed-off-by: Fangrui Song <maskray@google.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thank you for taking care of one of the many instances! That said,
upstream (binutils) plans now appear to be to continue to support
usages like the ones here, no matter that I'm not really happy about
that. Hence I'm uncertain whether that Clang hack you refer to can
actually be dropped any time soon.

Jan