crypto: arm/sha256-neon - avoid ADRL pseudo instruction

PATCH Superseded Ard Biesheuvel 3 participants 7 messages Sep 15, 2020 – Sep 16, 2020

Subsystems (2)

Arm (32-bit) Crypto API

lore

Share & tools

lore

Ard Biesheuvel

author

2020-09-15 09:46 UTC #0 lore raw

PATCH 1/1 crypto: arm/sha256-neon - avoid ADRL pseudo instruction

The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.

So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.

Cc Nick Desaulniers <[email protected]>

Cc Stefan Agner <[email protected]>

Cc Peter Smith <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.

 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

arch/arm/crypto/sha256-armv4.pl +2 -2

index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32

@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
 	stmdb	sp!,{r4-r12,lr}
 
 	sub	$H,sp,#16*4+16
-	adrl	$Ktbl,K256
+	adr	$Ktbl,.Lsha256_block_data_order
+	add	$Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
 	bic	$H,$H,#15		@ align for 128-bit stores
 	mov	$t2,sp
 	mov	sp,$H			@ alloca

arch/arm/crypto/sha256-core.S_shipped +2 -2

index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32

@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
 	stmdb	sp!,{r4-r12,lr}
 
 	sub	r11,sp,#16*4+16
-	adrl	r14,K256
+	adr	r14,.Lsha256_block_data_order
+	add	r14,r14,#K256-.Lsha256_block_data_order
 	bic	r11,r11,#15		@ align for 128-bit stores
 	mov	r12,sp
 	mov	sp,r11			@ alloca

Signature · 1 line

2.17.1

Nick Desaulniers

2020-09-15 18:51 UTC #1 lore raw

Quoted 60 lines · Ard Biesheuvel

The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.
So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.
Cc Nick Desaulniers <[email protected]>
Cc Stefan Agner <[email protected]>
Cc Peter Smith <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.
 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/crypto/sha256-armv4.pl +2 -2
index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     $H,sp,#16*4+16
-       adrl    $Ktbl,K256
+       adr     $Ktbl,.Lsha256_block_data_order
+       add     $Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
        bic     $H,$H,#15               @ align for 128-bit stores
        mov     $t2,sp
        mov     sp,$H                   @ alloca
arch/arm/crypto/sha256-core.S_shipped +0 -1
index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32

arch/arm/crypto/sha256-core.S_shipped sha256_block_data_order_neon: · line 1888

Quoted 6 lines

arch/arm/crypto/sha256-core.S_shipped +2 -1

@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     r11,sp,#16*4+16
-       adrl    r14,K256
+       adr     r14,.Lsha256_block_data_order
+       add     r14,r14,#K256-.Lsha256_block_data_order

Hi Ard,
Thanks for the patch. With this patch applied:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1
-j71 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71
...
arch/arm/crypto/sha256-core.S:2038:2: error: out of range immediate fixup value
 add r14,r14,#K256-.Lsha256_block_data_order
 ^

Would the adr_l macro you wrote in
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#t
be helpful here?

Quoted 6 lines

arch/arm/crypto/sha256-core.S_shipped +0 -1

        bic     r11,r11,#15             @ align for 128-bit stores
        mov     r12,sp
        mov     sp,r11                  @ alloca
--

2.17.1

Signature · 2 lines

Thanks,
~Nick Desaulniers

Ard Biesheuvel

author

2020-09-15 21:33 UTC #2 lore raw

Quoted 80 lines · Nick Desaulniers

Quoted 60 lines · Ard Biesheuvel
The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.
So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.
Cc Nick Desaulniers <[email protected]>
Cc Stefan Agner <[email protected]>
Cc Peter Smith <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.
 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/crypto/sha256-armv4.pl +2 -2
index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     $H,sp,#16*4+16
-       adrl    $Ktbl,K256
+       adr     $Ktbl,.Lsha256_block_data_order
+       add     $Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
        bic     $H,$H,#15               @ align for 128-bit stores
        mov     $t2,sp
        mov     sp,$H                   @ alloca
arch/arm/crypto/sha256-core.S_shipped +0 -1
index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
arch/arm/crypto/sha256-core.S_shipped sha256_block_data_order_neon: · line 1888
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +2 -1
@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     r11,sp,#16*4+16
-       adrl    r14,K256
+       adr     r14,.Lsha256_block_data_order
+       add     r14,r14,#K256-.Lsha256_block_data_order
Hi Ard,
Thanks for the patch. With this patch applied:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1
-j71 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71
...
arch/arm/crypto/sha256-core.S:2038:2: error: out of range immediate fixup value
 add r14,r14,#K256-.Lsha256_block_data_order
 ^

Quoted 2 lines

:(

Strange. Could you change it to

sub r14,r14,#.Lsha256_block_data_order-K256

and try again?

If that does work, it means the Clang assembler does not update the instruction type for negative addends (add to sub in this case), which would be unfortunate, since it would be another functionality gap.

Quoted 15 lines

Would the adr_l macro you wrote in
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#t
be helpful here?
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +0 -1
        bic     r11,r11,#15             @ align for 128-bit stores
        mov     r12,sp
        mov     sp,r11                  @ alloca
--
2.17.1
--
Thanks,
~Nick Desaulniers

Nick Desaulniers

2020-09-15 23:56 UTC #3 lore raw

Quoted 87 lines · Ard Biesheuvel

Quoted 80 lines · Nick Desaulniers
Quoted 60 lines · Ard Biesheuvel
The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.
So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.
Cc Nick Desaulniers <[email protected]>
Cc Stefan Agner <[email protected]>
Cc Peter Smith <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.
 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/crypto/sha256-armv4.pl +2 -2
index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     $H,sp,#16*4+16
-       adrl    $Ktbl,K256
+       adr     $Ktbl,.Lsha256_block_data_order
+       add     $Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
        bic     $H,$H,#15               @ align for 128-bit stores
        mov     $t2,sp
        mov     sp,$H                   @ alloca
arch/arm/crypto/sha256-core.S_shipped +0 -1
index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
arch/arm/crypto/sha256-core.S_shipped sha256_block_data_order_neon: · line 1888
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +2 -1
@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     r11,sp,#16*4+16
-       adrl    r14,K256
+       adr     r14,.Lsha256_block_data_order
+       add     r14,r14,#K256-.Lsha256_block_data_order
Hi Ard,
Thanks for the patch. With this patch applied:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1
-j71 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71
...
arch/arm/crypto/sha256-core.S:2038:2: error: out of range immediate fixup value
 add r14,r14,#K256-.Lsha256_block_data_order
 ^
Quoted 2 lines
:(
Strange. Could you change it to

Quoted 7 lines

sub r14,r14,#.Lsha256_block_data_order-K256
and try again?
If that does work, it means the Clang assembler does not update the instruction type for negative addends (add to sub in this case), which would be unfortunate, since it would be another functionality gap.

Works. Can you describe the expected functionality a bit more, so we can come up with a bug report/test case? (an add with a negative operand should be converted to a sub with a positive operand, IIUC?)

Also, there's a similar adrl in arch/arm/crypto/sha512-core.S, err, is that generated?

Signature · 2 lines

Thanks,
~Nick Desaulniers

Ard Biesheuvel

author

2020-09-16 05:40 UTC #4 lore raw

Quoted 93 lines · Nick Desaulniers

Quoted 91 lines · Ard Biesheuvel
Quoted 80 lines · Nick Desaulniers
Quoted 60 lines · Ard Biesheuvel
The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.
So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.
Cc Nick Desaulniers <[email protected]>
Cc Stefan Agner <[email protected]>
Cc Peter Smith <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.
 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/crypto/sha256-armv4.pl +2 -2
index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     $H,sp,#16*4+16
-       adrl    $Ktbl,K256
+       adr     $Ktbl,.Lsha256_block_data_order
+       add     $Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
        bic     $H,$H,#15               @ align for 128-bit stores
        mov     $t2,sp
        mov     sp,$H                   @ alloca
arch/arm/crypto/sha256-core.S_shipped +0 -1
index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
arch/arm/crypto/sha256-core.S_shipped sha256_block_data_order_neon: · line 1888
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +2 -1
@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     r11,sp,#16*4+16
-       adrl    r14,K256
+       adr     r14,.Lsha256_block_data_order
+       add     r14,r14,#K256-.Lsha256_block_data_order
Hi Ard,
Thanks for the patch. With this patch applied:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1
-j71 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71
...
arch/arm/crypto/sha256-core.S:2038:2: error: out of range immediate fixup value
 add r14,r14,#K256-.Lsha256_block_data_order
 ^
Quoted 2 lines
:(
Strange. Could you change it to
sub r14,r14,#.Lsha256_block_data_order-K256
and try again?

Quoted 8 lines

Quoted 3 lines
If that does work, it means the Clang assembler does not update the instruction type for negative addends (add to sub in this case), which would be unfortunate, since it would be another functionality gap.
Works. Can you describe the expected functionality a bit more, so we can come up with a bug report/test case? (an add with a negative operand should be converted to a sub with a positive operand, IIUC?)

That is it, really. Not sure if this is laid out in a spec anywhere, although the ELF psABI for ARM covers some similar territory when it comes to turning add into sub instructions and vice versa, as well as manipulating the U bit of LDR instructions.

Quoted 2 lines

Also, there's a similar adrl in arch/arm/crypto/sha512-core.S, err, is that generated?

Indeed. I missed that one as it has been removed from the upstream OpenSSL version, but I'll add a fix there as well.

Stefan Agner

2020-09-16 07:45 UTC #5 lore raw

In reply to Ard Biesheuvel

Quoted 86 lines · Ard Biesheuvel

Quoted 80 lines · Nick Desaulniers
Quoted 60 lines · Ard Biesheuvel
The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.
So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.
Cc Nick Desaulniers <[email protected]>
Cc Stefan Agner <[email protected]>
Cc Peter Smith <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.
 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/crypto/sha256-armv4.pl +2 -2
index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     $H,sp,#16*4+16
-       adrl    $Ktbl,K256
+       adr     $Ktbl,.Lsha256_block_data_order
+       add     $Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
        bic     $H,$H,#15               @ align for 128-bit stores
        mov     $t2,sp
        mov     sp,$H                   @ alloca
arch/arm/crypto/sha256-core.S_shipped +0 -1
index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
arch/arm/crypto/sha256-core.S_shipped sha256_block_data_order_neon: · line 1888
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +2 -1
@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     r11,sp,#16*4+16
-       adrl    r14,K256
+       adr     r14,.Lsha256_block_data_order
+       add     r14,r14,#K256-.Lsha256_block_data_order
Hi Ard,
Thanks for the patch. With this patch applied:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1
-j71 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71
...
arch/arm/crypto/sha256-core.S:2038:2: error: out of range immediate fixup value
 add r14,r14,#K256-.Lsha256_block_data_order
 ^
Quoted 2 lines
:(
Strange. Could you change it to

Quoted 7 lines

sub r14,r14,#.Lsha256_block_data_order-K256
and try again?
If that does work, it means the Clang assembler does not update the instruction type for negative addends (add to sub in this case), which would be unfortunate, since it would be another functionality gap.

Hm interesting, I did not come across another instance where this was a problem.

In this particular case, is it guaranteed to be a subtraction? I guess then using sub for now would be fine...?

FWIW, we discussed possible solution also in this issue (mach-omap2/sleep34xx.S case is handled already):
https://github.com/ClangBuiltLinux/linux/issues/430

--
Stefan

Quoted 18 lines

Quoted 18 lines
Would the adr_l macro you wrote in
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#t
be helpful here?
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +0 -1
        bic     r11,r11,#15             @ align for 128-bit stores
        mov     r12,sp
        mov     sp,r11                  @ alloca
--
2.17.1
--
Thanks,
~Nick Desaulniers

Ard Biesheuvel

author

2020-09-16 09:59 UTC #6 lore raw

Quoted 96 lines · Stefan Agner

Quoted 86 lines · Ard Biesheuvel
Quoted 80 lines · Nick Desaulniers
Quoted 60 lines · Ard Biesheuvel
The ADRL pseudo instruction is not an architectural construct, but a convenience macro that was supported by the ARM proprietary assembler and adopted by binutils GAS as well, but only when assembling in 32-bit ARM mode. Therefore, it can only be used in assembler code that is known to assemble in ARM mode only, but as it turns out, the Clang assembler does not implement ADRL at all, and so it is better to get rid of it entirely.
So replace the ADRL instruction with a ADR instruction that refers to a nearer symbol, and apply the delta explicitly using an additional instruction.
Cc Nick Desaulniers <[email protected]>
Cc Stefan Agner <[email protected]>
Cc Peter Smith <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
I will leave it to the Clang folks to decide whether this needs to be backported and how far, but a Cc stable seems reasonable here.
 arch/arm/crypto/sha256-armv4.pl       | 4 ++--
 arch/arm/crypto/sha256-core.S_shipped | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/crypto/sha256-armv4.pl +2 -2
index 9f96ff48e4a8..8aeb2e82f915 100644
@@ -175,7 +175,6 @@ $code=<<___;
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
@@ -471,7 +470,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     $H,sp,#16*4+16
-       adrl    $Ktbl,K256
+       adr     $Ktbl,.Lsha256_block_data_order
+       add     $Ktbl,$Ktbl,#K256-.Lsha256_block_data_order
        bic     $H,$H,#15               @ align for 128-bit stores
        mov     $t2,sp
        mov     sp,$H                   @ alloca
arch/arm/crypto/sha256-core.S_shipped +0 -1
index ea04b2ab0c33..1861c4e8a5ba 100644
@@ -56,7 +56,6 @@
 #else
 .syntax unified
 # ifdef __thumb2__
-#  define adrl adr
 .thumb
 # else
 .code   32
arch/arm/crypto/sha256-core.S_shipped sha256_block_data_order_neon: · line 1888
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +2 -1
@@ -1885,7 +1884,8 @@ sha256_block_data_order_neon:
        stmdb   sp!,{r4-r12,lr}
 
        sub     r11,sp,#16*4+16
-       adrl    r14,K256
+       adr     r14,.Lsha256_block_data_order
+       add     r14,r14,#K256-.Lsha256_block_data_order
Hi Ard,
Thanks for the patch. With this patch applied:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1
-j71 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71
...
arch/arm/crypto/sha256-core.S:2038:2: error: out of range immediate fixup value
 add r14,r14,#K256-.Lsha256_block_data_order
 ^
Quoted 2 lines
:(
Strange. Could you change it to
Quoted 7 lines
sub r14,r14,#.Lsha256_block_data_order-K256
and try again?
If that does work, it means the Clang assembler does not update the instruction type for negative addends (add to sub in this case), which would be unfortunate, since it would be another functionality gap.

Quoted 6 lines

Hm interesting, I did not come across another instance where this was a problem.
In this particular case, is it guaranteed to be a subtraction? I guess then using sub for now would be fine...?

Yes for this code it is fine.

Quoted 25 lines

FWIW, we discussed possible solution also in this issue (mach-omap2/sleep34xx.S case is handled already):
https://github.com/ClangBuiltLinux/linux/issues/430
--
Stefan
Quoted 18 lines
Quoted 18 lines
Would the adr_l macro you wrote in
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#t
be helpful here?
Quoted 6 lines
arch/arm/crypto/sha256-core.S_shipped +0 -1
        bic     r11,r11,#15             @ align for 128-bit stores
        mov     r12,sp
        mov     sp,r11                  @ alloca
--
2.17.1
--
Thanks,
~Nick Desaulniers