Skip to content

Commit

Permalink
Merge pull request #6090 from hanno-arm/fix_bnmul_arm_v7a
Browse files Browse the repository at this point in the history 
Remove encoding width suffix from Arm bignum assembly
  • Loading branch information
@daverodgman
daverodgman authored Aug 18, 2022
2 parents 8c9a0ae + 03f7a6e commit 92cd864
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
18 changes: 9 additions & 9 deletions library/bn_mul.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -743,10 +743,10 @@

#define MULADDC_X1_CORE \
".p2align 2 \n\t" \
"ldr.w %[a], [%[in]], #4 \n\t" \
"ldr.w %[b], [%[acc]] \n\t" \
"ldr %[a], [%[in]], #4 \n\t" \
"ldr %[b], [%[acc]] \n\t" \
"umaal %[b], %[carry], %[scalar], %[a] \n\t" \
"str.w %[b], [%[acc]], #4 \n\t"
"str %[b], [%[acc]], #4 \n\t"

#define MULADDC_X1_STOP \
: [a] "=&r" (tmp_a), \
Expand Down Expand Up @@ -777,14 +777,14 @@
* 2 cycles, while subsequent loads/stores are single-cycle. */
#define MULADDC_X2_CORE \
".p2align 2 \n\t" \
"ldr.w %[a0], [%[in]], #+8 \n\t" \
"ldr.w %[b0], [%[acc]], #+8 \n\t" \
"ldr.w %[a1], [%[in], #-4] \n\t" \
"ldr.w %[b1], [%[acc], #-4] \n\t" \
"ldr %[a0], [%[in]], #+8 \n\t" \
"ldr %[b0], [%[acc]], #+8 \n\t" \
"ldr %[a1], [%[in], #-4] \n\t" \
"ldr %[b1], [%[acc], #-4] \n\t" \
"umaal %[b0], %[carry], %[scalar], %[a0] \n\t" \
"umaal %[b1], %[carry], %[scalar], %[a1] \n\t" \
"str.w %[b0], [%[acc], #-8] \n\t" \
"str.w %[b1], [%[acc], #-4] \n\t"
"str %[b0], [%[acc], #-8] \n\t" \
"str %[b1], [%[acc], #-4] \n\t"

#define MULADDC_X2_STOP \
: [a0] "=&r" (tmp_a0), \
Expand Down
16 changes: 11 additions & 5 deletions tests/scripts/all.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2948,6 +2948,7 @@ component_build_armcc () {
scripts/config.py baremetal
# armc[56] don't support SHA-512 intrinsics
scripts/config.py unset MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT
scripts/config.py set MBEDTLS_HAVE_ASM

make CC="$ARMC5_CC" AR="$ARMC5_AR" WARNING_CFLAGS='--strict --c99' lib

Expand All @@ -2956,20 +2957,25 @@ component_build_armcc () {

make clean

# Compile with -O1 since some Arm inline assembly is disabled for -O0.

# ARM Compiler 6 - Target ARMv7-A
armc6_build_test "--target=arm-arm-none-eabi -march=armv7-a"
armc6_build_test "-O1 --target=arm-arm-none-eabi -march=armv7-a"

# ARM Compiler 6 - Target ARMv7-M
armc6_build_test "--target=arm-arm-none-eabi -march=armv7-m"
armc6_build_test "-O1 --target=arm-arm-none-eabi -march=armv7-m"

# ARM Compiler 6 - Target ARMv7-M+DSP
armc6_build_test "-O1 --target=arm-arm-none-eabi -march=armv7-m+dsp"

# ARM Compiler 6 - Target ARMv8-A - AArch32
armc6_build_test "--target=arm-arm-none-eabi -march=armv8.2-a"
armc6_build_test "-O1 --target=arm-arm-none-eabi -march=armv8.2-a"

# ARM Compiler 6 - Target ARMv8-M
armc6_build_test "--target=arm-arm-none-eabi -march=armv8-m.main"
armc6_build_test "-O1 --target=arm-arm-none-eabi -march=armv8-m.main"

# ARM Compiler 6 - Target ARMv8.2-A - AArch64
armc6_build_test "--target=aarch64-arm-none-eabi -march=armv8.2-a+crypto"
armc6_build_test "-O1 --target=aarch64-arm-none-eabi -march=armv8.2-a+crypto"
}

component_test_tls13_only () {
Expand Down

5 comments on commit 92cd864

@peter-toft-greve
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is golden! Fixes armv7l build errors that I have with master <3
However a question to @[daverodgman - why only use -O1 in tests/scripts/all.sh? Personally I would test with -O2 or ... actually -O3 :)

@peter-toft-greve
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THANX @daverodgman

@peter-toft-greve
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 vote from me :)

@gilles-peskine-arm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However a question to @[daverodgman - why only use -O1 in tests/scripts/all.sh? Personally I would test with -O2 or ... actually -O3 :)

Since we're not testing these builds it makes sense to optimize for compilation speed. Just not -O0 because we disable some of the inline assembly if __OPTIMIZE__ is unset.

When we finally add testing for arm builds back to the CI, we should switch those builds to high optimization since that can reveal bugs such as not declaring clobbers properly.

@peter-toft-greve
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good comment @gilles-peskine-arm

Please sign in to comment.