ARM: teach __asmeq that r11 == fp and r12 == ip
PATCH v2 1/1 ARM: teach __asmeq that r11 == fp and r12 == ip
The __asmeq macro is used inside inline asm statements to ensure that register asm variables that explicitly specify a register are mapped correctly onto those registers when used in inline asm input and output constraints. However, the string based matching fails to take into account that 'fp' is often referred to as 'r11' and 'ip' is often referred to as 'r12', (e.g., by clang), causing false negatives.
Fix this by making __asmeq consider the ("fp","r11"), ("r11","fp"), ("ip","r12") and ("r12","ip") cases specifically.
---
v2:
Added r11 but not r10, as the latter is only referred to as 'sl' in objdump output, but GCC emitted inline asm substitutes the references with 'r10' instead. sp/lr/pc are unambiguous between GCC and clang.
Primary purpose of this patch is to fix the incompatibility between clang and GCC, but the side effect is that it relaxes the requirement imposed on the programmer to refer to a register by the same name as the compiler does. If that is seen as a substantial upside by some, we could indeed still decide to add r10, r13, r14 and r15 (for which no such incompatibility exists) as well. However, those are not currently used in __asmeq invocations anywhere under arch/arm
arch/arm/include/asm/compiler.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
arch/arm/include/asm/compiler.h +14 -1
@@ -8,8 +8,21 @@* This string is meant to be concatenated with the inline asm string and * will cause compilation to stop on mismatch. * (for details, see gcc PR 15089) +* For compatibility with clang, we have to specifically take the equivalence +* of 'r11' <-> 'fp' and 'r12' <-> 'ip' into account as well. */ -#define __asmeq (x, y) ".ifnc " x "," y " ; .err ; .endif\n\t" +#define __asmeq (x, y) \ + ".ifnc " x "," y "; " \ + ".ifnc " x y ",fpr11; " \ + ".ifnc " x y ",r11fp; " \ + ".ifnc " x y ",ipr12; " \ + ".ifnc " x y ",r12ip; " \ + ".err; " \ + ".endif; " \ + ".endif; " \ + ".endif; " \ + ".endif; " \ + ".endif\n\t"#endif /* __ASM_ARM_COMPILER_H */
Signature · 1 line
1.8.3.2
PATCH v2 1/1 ARM: teach __asmeq that r11 == fp and r12 == ip
Quoted 11 lines · Ard Biesheuvel
The
__asmeqmacro is used inside inline asm statements to ensure that register asm variables that explicitly specify a register are mapped correctly onto those registers when used in inline asm input and output constraints. However, the string based matching fails to take into account that 'fp' is often referred to as 'r11' and 'ip' is often referred to as 'r12', (e.g., by clang), causing false negatives.Fix this by making
__asmeqconsider the ("fp","r11"), ("r11","fp"), ("ip","r12") and ("r12","ip") cases specifically.Signed-off-by Ard Biesheuvel <[email protected]>
The method adopted is rather horrid in itself, but I guess there's no other way to deal with this... so I'm okay with the patch. (I won't say I'm happy with it!)
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 1/1 ARM: teach __asmeq that r11 == fp and r12 == ip
Quoted 8 lines · Russell King - ARM Linux
Quoted 7 lines · Ard Biesheuvel
The
__asmeqmacro is used inside inline asm statements to ensure that register asm variables that explicitly specify a register are mapped correctly onto those registers when used in inline asm input and output constraints. However, the string based matching fails to take into account that 'fp' is often referred to as 'r11' and 'ip' is often referred to as 'r12', (e.g., by clang), causing false negatives.
Quoted 9 lines
Quoted 4 lines
Fix this by making
__asmeqconsider the ("fp","r11"), ("r11","fp"), ("ip","r12") and ("r12","ip") cases specifically.Signed-off-by Ard Biesheuvel <[email protected]>The method adopted is rather horrid in itself, but I guess there's no other way to deal with this... so I'm okay with the patch. (I won't say I'm happy with it!)
How about we retain the old version unless __clang__ is defined?
PATCH v2 1/1 ARM: teach __asmeq that r11 == fp and r12 == ip
Quoted 13 lines · Ard Biesheuvel
Quoted 11 lines · Russell King - ARM Linux
Quoted 10 lines · Ard Biesheuvel
The
__asmeqmacro is used inside inline asm statements to ensure that register asm variables that explicitly specify a register are mapped correctly onto those registers when used in inline asm input and output constraints. However, the string based matching fails to take into account that 'fp' is often referred to as 'r11' and 'ip' is often referred to as 'r12', (e.g., by clang), causing false negatives.Fix this by making
__asmeqconsider the ("fp","r11"), ("r11","fp"), ("ip","r12") and ("r12","ip") cases specifically.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Ard Biesheuvel <[email protected]>The method adopted is rather horrid in itself, but I guess there's no other way to deal with this... so I'm okay with the patch. (I won't say I'm happy with it!)
How about we retain the old version unless
__clang__is defined?
I don't see much point - that only adds to the complexity with a #ifdef. The register names are equivalent in any compiler.
It's better that we avoid #ifdefs so we get better build coverage.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 1/1 ARM: teach __asmeq that r11 == fp and r12 == ip
In reply to Ard BiesheuvelQuoted 11 lines · Ard Biesheuvel
The
__asmeqmacro is used inside inline asm statements to ensure that register asm variables that explicitly specify a register are mapped correctly onto those registers when used in inline asm input and output constraints. However, the string based matching fails to take into account that 'fp' is often referred to as 'r11' and 'ip' is often referred to as 'r12', (e.g., by clang), causing false negatives.Fix this by making
__asmeqconsider the ("fp","r11"), ("r11","fp"), ("ip","r12") and ("r12","ip") cases specifically.Signed-off-by Ard Biesheuvel <[email protected]>
I'm satisfied. Kind of awkward, but it does what's needed, and solves the general problem rather than a specific instance of it in "bcm_kona_smc.c".
I tested it in my environment with "r12" and "ip" used in all combinations and can confirm they all work as desired.
Thanks for doing this Ard.
Quoted 45 lines
---
v2:
Added r11 but not r10, as the latter is only referred to as 'sl' in objdump output, but GCC emitted inline asm substitutes the references with 'r10' instead. sp/lr/pc are unambiguous between GCC and clang.Primary purpose of this patch is to fix the incompatibility between clang and GCC, but the side effect is that it relaxes the requirement imposed on the programmer to refer to a register by the same name as the compiler does. If that is seen as a substantial upside by some, we could indeed still decide to add r10, r13, r14 and r15 (for which no such incompatibility exists) as well. However, those are not currently used in
__asmeqinvocations anywhere underarch/armarch/arm/include/asm/compiler.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)arch/arm/include/asm/compiler.h +14
-1@@ -8,8 +8,21 @@* This string is meant to be concatenated with the inline asm string and * will cause compilation to stop on mismatch. * (for details, see gcc PR 15089) +* For compatibility with clang, we have to specifically take the equivalence +* of 'r11' <-> 'fp' and 'r12' <-> 'ip' into account as well. */ -#define __asmeq (x, y) ".ifnc " x "," y " ; .err ; .endif\n\t" +#define __asmeq (x, y) \ + ".ifnc " x "," y "; " \ + ".ifnc " x y ",fpr11; " \ + ".ifnc " x y ",r11fp; " \ + ".ifnc " x y ",ipr12; " \ + ".ifnc " x y ",r12ip; " \ + ".err; " \ + ".endif; " \ + ".endif; " \ + ".endif; " \ + ".endif; " \ + ".endif\n\t"#endif /* __ASM_ARM_COMPILER_H */
PATCH v2 1/1 ARM: teach __asmeq that r11 == fp and r12 == ip
In reply to Ard BiesheuvelQuoted 42 lines · Ard Biesheuvel
The
__asmeqmacro is used inside inline asm statements to ensure that register asm variables that explicitly specify a register are mapped correctly onto those registers when used in inline asm input and output constraints. However, the string based matching fails to take into account that 'fp' is often referred to as 'r11' and 'ip' is often referred to as 'r12', (e.g., by clang), causing false negatives.Fix this by making
__asmeqconsider the ("fp","r11"), ("r11","fp"), ("ip","r12") and ("r12","ip") cases specifically.Signed-off-by Ard Biesheuvel <[email protected]>---
v2:
Added r11 but not r10, as the latter is only referred to as 'sl' in objdump output, but GCC emitted inline asm substitutes the references with 'r10' instead. sp/lr/pc are unambiguous between GCC and clang.Primary purpose of this patch is to fix the incompatibility between clang and GCC, but the side effect is that it relaxes the requirement imposed on the programmer to refer to a register by the same name as the compiler does. If that is seen as a substantial upside by some, we could indeed still decide to add r10, r13, r14 and r15 (for which no such incompatibility exists) as well. However, those are not currently used in
__asmeqinvocations anywhere underarch/armarch/arm/include/asm/compiler.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)arch/arm/include/asm/compiler.h +4
-1@@ -8,8 +8,21 @@* This string is meant to be concatenated with the inline asm string and * will cause compilation to stop on mismatch. * (for details, see gcc PR 15089) +* For compatibility with clang, we have to specifically take the equivalence +* of 'r11' <-> 'fp' and 'r12' <-> 'ip' into account as well. */ -#define __asmeq (x, y) ".ifnc " x "," y " ; .err ; .endif\n\t" +#define __asmeq (x, y) \ + ".ifnc " x "," y "; " \
arch/arm/include/asm/compiler.h line 25
Quoted 10 lines
arch/arm/include/asm/compiler.h +10
-0+".ifnc " x y",fpr11; " \ +".ifnc " x y",r11fp; " \ +".ifnc " x y",ipr12; " \ +".ifnc " x y",r12ip; " \ +".err; " \ +".endif; " \ +".endif; " \ +".endif; " \ +".endif; " \ +".endif\n\t"
I share RMK's yuck reaction, but for lack of a better idea:
Nicolas