Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 17:08, Alex Elder wrote:
> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 14:11, Alex Elder wrote:
>>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 05:18, Behan Webster
wrote:
> From: Alex Elder
>
> My GCC-based build environment likes to call register r12 by the
> name "ip" in inline asm. Behan Webster informed me that his Clang-
> based build environment likes "r12" instead.
>
> Try to make them both happy.
>
> Signed-off-by: Alex Elder
> Signed-off-by: Behan Webster
> ---
> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
> b/arch/arm/mach-bcm/bcm_kona_smc.c
> index a55a7ec..3937bd5 100644
> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
> * request result appropriately. This result value is found in r0
> * when the "smc" request completes.
> */
> +#ifdef __clang__
> +#define R12"r12"
> +#else /* !__clang__ */
> +#define R12"ip"/* gcc calls r12 "ip" */
> +#endif /* !__clang__ */
Why not just use r12 for both?
>>>
>>> Yes, that would have been an obvious fix. But the
>>> assembler (in the GCC environment) doesn't accept that.
>>>
>>
>> Mine has no problems with it at all
>>
>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>>
>> and grepping for r12 under arch/arm suggests the same
>
> The use of "r12" is fine. But it's not just the assembler,
> I believe it also involves gcc.
>
> The problem is with the use of the __asmeq(x, y) macro.
>
Ah right. Apologies for assuming that you had missed something obvious here.
But __asmeq is not the toolchain, it is a local construct #define'd in
compiler.h
> If I assign the "ip" variable with "r12":
> register u32 ip asm("r12"); /* Also called ip */
>
> Then that's fine. However, this line then causes an error:
> __asmeq("%0", "r12")
>
> Apparently gcc uses register "ip" when it sees asm("r12"). So
> attempting to verify the desired register got used with __asmeq()
> causes a string mismatch--"ip" is not equal to "r12".
>
> So I could use:
>
> register u32 ip asm("r12"); /* Also called ip */
> ...
> __asmeq("%0", "ip")
>
> And that will build. But it's a little non-intuitive, and
> I suspect that clang might (rightfully) have a failure in
> this __asmeq() call.
>
In that case, I would strongly suggest fixing the __asmeq () macro
instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
The thing is, inline asm is a dodgy area to begin with in terms of
clang-to-gcc compatibility. On arm64, we have been seeing issues where
the width of the register -which is fixed on gcc- is selected based on
the size of that variable, i.e., an int32 gets a w# register and int64
gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
writes 8 bytes on GCC suddenly only writing 4 bytes when built with
clang.
If we also start using the preprocessor to conditionalise what is
emitted by inline asm, the waters get even murkier and it becomes even
harder to claim parity between the two.
--
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
> On 28 January 2015 at 05:18, Behan Webster wrote:
>> From: Alex Elder
>>
>> My GCC-based build environment likes to call register r12 by the
>> name "ip" in inline asm. Behan Webster informed me that his Clang-
>> based build environment likes "r12" instead.
>>
>> Try to make them both happy.
>>
>> Signed-off-by: Alex Elder
>> Signed-off-by: Behan Webster
>> ---
>> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>> index a55a7ec..3937bd5 100644
>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>> * request result appropriately. This result value is found in r0
>> * when the "smc" request completes.
>> */
>> +#ifdef __clang__
>> +#define R12"r12"
>> +#else /* !__clang__ */
>> +#define R12"ip"/* gcc calls r12 "ip" */
>> +#endif /* !__clang__ */
>
> Why not just use r12 for both?
Yes, that would have been an obvious fix. But the
assembler (in the GCC environment) doesn't accept that.
-Alex
>
>> static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
>> {
>> - register u32 ip asm("ip"); /* Also called r12 */
>> + register u32 ip asm(R12); /* Also called r12 */
>> register u32 r0 asm("r0");
>> register u32 r4 asm("r4");
>> register u32 r5 asm("r5");
>> @@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32
>> buffer_phys)
>>
>> asm volatile (
>> /* Make sure we got the registers we want */
>> - __asmeq("%0", "ip")
>> + __asmeq("%0", R12)
>> __asmeq("%1", "r0")
>> __asmeq("%2", "r4")
>> __asmeq("%3", "r5")
>> --
>> 1.9.1
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/15 12:11, Ard Biesheuvel wrote:
> On 28 January 2015 at 19:38, Ard Biesheuvel wrote:
>> On 28 January 2015 at 19:27, Alex Elder wrote:
>>> On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
On 28 January 2015 at 17:20, Ard Biesheuvel
wrote:
> On 28 January 2015 at 17:08, Alex Elder wrote:
>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 14:11, Alex Elder wrote:
On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
> On 28 January 2015 at 05:18, Behan Webster
> wrote:
>> From: Alex Elder
>>
>> My GCC-based build environment likes to call register r12 by the
>> name "ip" in inline asm. Behan Webster informed me that his Clang-
>> based build environment likes "r12" instead.
>>
>> Try to make them both happy.
>>
>> Signed-off-by: Alex Elder
>> Signed-off-by: Behan Webster
>> ---
>> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>> index a55a7ec..3937bd5 100644
>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>> * request result appropriately. This result value is found in r0
>> * when the "smc" request completes.
>> */
>> +#ifdef __clang__
>> +#define R12"r12"
>> +#else /* !__clang__ */
>> +#define R12"ip"/* gcc calls r12 "ip" */
>> +#endif /* !__clang__ */
> Why not just use r12 for both?
Yes, that would have been an obvious fix. But the
assembler (in the GCC environment) doesn't accept that.
>>> Mine has no problems with it at all
>>>
>>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp
>>> -
>>>
>>> and grepping for r12 under arch/arm suggests the same
>> The use of "r12" is fine. But it's not just the assembler,
>> I believe it also involves gcc.
>>
>> The problem is with the use of the __asmeq(x, y) macro.
>>
> Ah right. Apologies for assuming that you had missed something obvious
> here.
> But __asmeq is not the toolchain, it is a local construct #define'd in
> compiler.h
>
>> If I assign the "ip" variable with "r12":
>> register u32 ip asm("r12"); /* Also called ip */
>>
>> Then that's fine. However, this line then causes an error:
>> __asmeq("%0", "r12")
>>
>> Apparently gcc uses register "ip" when it sees asm("r12"). So
>> attempting to verify the desired register got used with __asmeq()
>> causes a string mismatch--"ip" is not equal to "r12".
>>
>> So I could use:
>>
>> register u32 ip asm("r12"); /* Also called ip */
>> ...
>> __asmeq("%0", "ip")
>>
>> And that will build. But it's a little non-intuitive, and
>> I suspect that clang might (rightfully) have a failure in
>> this __asmeq() call.
>>
> In that case, I would strongly suggest fixing the __asmeq () macro
> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>
> The thing is, inline asm is a dodgy area to begin with in terms of
> clang-to-gcc compatibility. On arm64, we have been seeing issues where
> the width of the register -which is fixed on gcc- is selected based on
> the size of that variable, i.e., an int32 gets a w# register and int64
> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
> clang.
>
> If we also start using the preprocessor to conditionalise what is
> emitted by inline asm, the waters get even murkier and it becomes even
> harder to claim parity between the two.
>
Something like this perhaps?
>>> So __asmeq() yields true if the register names (strings) are
>>> equal, or if one is "ip" and the other is "r12" (in either order).
>>>
>>> I can't comment on whether it's right in all build environments but
>>> this looks OK to me, to handle this special case.
>>>
>>> I would much rather you generate that patch. Is that OK?
>>>
>> Sure, I can cook up a patch if you guys can confirm that it fixes your
>> use case. (I tested GCC myself but I don't have clang installed)
>>
> Actually, if clang is guaranteed to emit the correct register name
> inside the inline asm for register asm variables used in input or
> output constraints, I think it makes sense to #define __asmeq as a nop
> if __clang__ is defined. (Note that __asmeq only exists to work around
> a specific GCC bug)
As far as I'm aware neither clang nor gcc will guarantee
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/15 11:38, Ard Biesheuvel wrote:
> On 28 January 2015 at 19:27, Alex Elder wrote:
>> On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 17:20, Ard Biesheuvel
>>> wrote:
On 28 January 2015 at 17:08, Alex Elder wrote:
> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 14:11, Alex Elder wrote:
>>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 05:18, Behan Webster
wrote:
> From: Alex Elder
>
> My GCC-based build environment likes to call register r12 by the
> name "ip" in inline asm. Behan Webster informed me that his Clang-
> based build environment likes "r12" instead.
>
> Try to make them both happy.
>
> Signed-off-by: Alex Elder
> Signed-off-by: Behan Webster
> ---
> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
> b/arch/arm/mach-bcm/bcm_kona_smc.c
> index a55a7ec..3937bd5 100644
> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
> * request result appropriately. This result value is found in r0
> * when the "smc" request completes.
> */
> +#ifdef __clang__
> +#define R12"r12"
> +#else /* !__clang__ */
> +#define R12"ip"/* gcc calls r12 "ip" */
> +#endif /* !__clang__ */
Why not just use r12 for both?
>>> Yes, that would have been an obvious fix. But the
>>> assembler (in the GCC environment) doesn't accept that.
>>>
>> Mine has no problems with it at all
>>
>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>>
>> and grepping for r12 under arch/arm suggests the same
> The use of "r12" is fine. But it's not just the assembler,
> I believe it also involves gcc.
>
> The problem is with the use of the __asmeq(x, y) macro.
>
Ah right. Apologies for assuming that you had missed something obvious
here.
But __asmeq is not the toolchain, it is a local construct #define'd in
compiler.h
> If I assign the "ip" variable with "r12":
> register u32 ip asm("r12"); /* Also called ip */
>
> Then that's fine. However, this line then causes an error:
> __asmeq("%0", "r12")
>
> Apparently gcc uses register "ip" when it sees asm("r12"). So
> attempting to verify the desired register got used with __asmeq()
> causes a string mismatch--"ip" is not equal to "r12".
>
> So I could use:
>
> register u32 ip asm("r12"); /* Also called ip */
> ...
> __asmeq("%0", "ip")
>
> And that will build. But it's a little non-intuitive, and
> I suspect that clang might (rightfully) have a failure in
> this __asmeq() call.
>
In that case, I would strongly suggest fixing the __asmeq () macro
instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
The thing is, inline asm is a dodgy area to begin with in terms of
clang-to-gcc compatibility. On arm64, we have been seeing issues where
the width of the register -which is fixed on gcc- is selected based on
the size of that variable, i.e., an int32 gets a w# register and int64
gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
writes 8 bytes on GCC suddenly only writing 4 bytes when built with
clang.
If we also start using the preprocessor to conditionalise what is
emitted by inline asm, the waters get even murkier and it becomes even
harder to claim parity between the two.
>>> Something like this perhaps?
>> So __asmeq() yields true if the register names (strings) are
>> equal, or if one is "ip" and the other is "r12" (in either order).
>>
>> I can't comment on whether it's right in all build environments but
>> this looks OK to me, to handle this special case.
>>
>> I would much rather you generate that patch. Is that OK?
>>
> Sure, I can cook up a patch if you guys can confirm that it fixes your
> use case. (I tested GCC myself but I don't have clang installed)
>
That appears to work with clang as well.
All in all a much better solution.
Thank you,
Behan
--
Behan Webster
beh...@converseincode.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 05:18, Behan Webster wrote:
> From: Alex Elder
>
> My GCC-based build environment likes to call register r12 by the
> name "ip" in inline asm. Behan Webster informed me that his Clang-
> based build environment likes "r12" instead.
>
> Try to make them both happy.
>
> Signed-off-by: Alex Elder
> Signed-off-by: Behan Webster
> ---
> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
> b/arch/arm/mach-bcm/bcm_kona_smc.c
> index a55a7ec..3937bd5 100644
> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
> * request result appropriately. This result value is found in r0
> * when the "smc" request completes.
> */
> +#ifdef __clang__
> +#define R12"r12"
> +#else /* !__clang__ */
> +#define R12"ip"/* gcc calls r12 "ip" */
> +#endif /* !__clang__ */
Why not just use r12 for both?
> static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
> {
> - register u32 ip asm("ip"); /* Also called r12 */
> + register u32 ip asm(R12); /* Also called r12 */
> register u32 r0 asm("r0");
> register u32 r4 asm("r4");
> register u32 r5 asm("r5");
> @@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32
> buffer_phys)
>
> asm volatile (
> /* Make sure we got the registers we want */
> - __asmeq("%0", "ip")
> + __asmeq("%0", R12)
> __asmeq("%1", "r0")
> __asmeq("%2", "r4")
> __asmeq("%3", "r5")
> --
> 1.9.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
> On 28 January 2015 at 14:11, Alex Elder wrote:
>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 05:18, Behan Webster
>>> wrote:
From: Alex Elder
My GCC-based build environment likes to call register r12 by the
name "ip" in inline asm. Behan Webster informed me that his Clang-
based build environment likes "r12" instead.
Try to make them both happy.
Signed-off-by: Alex Elder
Signed-off-by: Behan Webster
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the "smc" request completes.
*/
+#ifdef __clang__
+#define R12"r12"
+#else /* !__clang__ */
+#define R12"ip"/* gcc calls r12 "ip" */
+#endif /* !__clang__ */
>>>
>>> Why not just use r12 for both?
>>
>> Yes, that would have been an obvious fix. But the
>> assembler (in the GCC environment) doesn't accept that.
>>
>
> Mine has no problems with it at all
>
> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>
> and grepping for r12 under arch/arm suggests the same
The use of "r12" is fine. But it's not just the assembler,
I believe it also involves gcc.
The problem is with the use of the __asmeq(x, y) macro.
If I assign the "ip" variable with "r12":
register u32 ip asm("r12"); /* Also called ip */
Then that's fine. However, this line then causes an error:
__asmeq("%0", "r12")
Apparently gcc uses register "ip" when it sees asm("r12"). So
attempting to verify the desired register got used with __asmeq()
causes a string mismatch--"ip" is not equal to "r12".
So I could use:
register u32 ip asm("r12"); /* Also called ip */
...
__asmeq("%0", "ip")
And that will build. But it's a little non-intuitive, and
I suspect that clang might (rightfully) have a failure in
this __asmeq() call.
-Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/15 11:17, Ard Biesheuvel wrote:
> On 28 January 2015 at 17:20, Ard Biesheuvel wrote:
>> On 28 January 2015 at 17:08, Alex Elder wrote:
>>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 14:11, Alex Elder wrote:
> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 05:18, Behan Webster
>> wrote:
>>> From: Alex Elder
>>>
>>> My GCC-based build environment likes to call register r12 by the
>>> name "ip" in inline asm. Behan Webster informed me that his Clang-
>>> based build environment likes "r12" instead.
>>>
>>> Try to make them both happy.
>>>
>>> Signed-off-by: Alex Elder
>>> Signed-off-by: Behan Webster
>>> ---
>>> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
>>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>>> index a55a7ec..3937bd5 100644
>>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>>> * request result appropriately. This result value is found in r0
>>> * when the "smc" request completes.
>>> */
>>> +#ifdef __clang__
>>> +#define R12"r12"
>>> +#else /* !__clang__ */
>>> +#define R12"ip"/* gcc calls r12 "ip" */
>>> +#endif /* !__clang__ */
>> Why not just use r12 for both?
> Yes, that would have been an obvious fix. But the
> assembler (in the GCC environment) doesn't accept that.
>
Mine has no problems with it at all
$ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
and grepping for r12 under arch/arm suggests the same
>>> The use of "r12" is fine. But it's not just the assembler,
>>> I believe it also involves gcc.
>>>
>>> The problem is with the use of the __asmeq(x, y) macro.
>>>
>> Ah right. Apologies for assuming that you had missed something obvious here.
>> But __asmeq is not the toolchain, it is a local construct #define'd in
>> compiler.h
>>
>>> If I assign the "ip" variable with "r12":
>>> register u32 ip asm("r12"); /* Also called ip */
>>>
>>> Then that's fine. However, this line then causes an error:
>>> __asmeq("%0", "r12")
>>>
>>> Apparently gcc uses register "ip" when it sees asm("r12"). So
>>> attempting to verify the desired register got used with __asmeq()
>>> causes a string mismatch--"ip" is not equal to "r12".
>>>
>>> So I could use:
>>>
>>> register u32 ip asm("r12"); /* Also called ip */
>>> ...
>>> __asmeq("%0", "ip")
>>>
>>> And that will build. But it's a little non-intuitive, and
>>> I suspect that clang might (rightfully) have a failure in
>>> this __asmeq() call.
>>>
>> In that case, I would strongly suggest fixing the __asmeq () macro
>> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>>
>> The thing is, inline asm is a dodgy area to begin with in terms of
>> clang-to-gcc compatibility. On arm64, we have been seeing issues where
>> the width of the register -which is fixed on gcc- is selected based on
>> the size of that variable, i.e., an int32 gets a w# register and int64
>> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
>> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
>> clang.
>>
>> If we also start using the preprocessor to conditionalise what is
>> emitted by inline asm, the waters get even murkier and it becomes even
>> harder to claim parity between the two.
>>
> Something like this perhaps?
>
> >8--
> diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h
> index 8155db2f7fa1..f99c674b3751 100644
> --- a/arch/arm/include/asm/compiler.h
> +++ b/arch/arm/include/asm/compiler.h
> @@ -9,7 +9,8 @@
> * will cause compilation to stop on mismatch.
> * (for details, see gcc PR 15089)
> */
> -#define __asmeq(x, y) ".ifnc " x "," y " ; .err ; .endif\n\t"
> +#define __asmeq(x, y) ".ifnc " x "," y " ; .ifnc " x y ",ipr12 ; " \
> + ".ifnc " x y ",r12ip ; .err ; .endif ; .endif ; .endif\n\t"
>
>
> #endif /* __ASM_ARM_COMPILER_H */
> >8--
If that is acceptable, that's fine by me.
In principal none of us *want* to use #ifdefs.
Behan
--
Behan Webster
beh...@converseincode.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 19:27, Alex Elder wrote:
> On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 17:20, Ard Biesheuvel
>> wrote:
>>> On 28 January 2015 at 17:08, Alex Elder wrote:
On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
> On 28 January 2015 at 14:11, Alex Elder wrote:
>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 05:18, Behan Webster
>>> wrote:
From: Alex Elder
My GCC-based build environment likes to call register r12 by the
name "ip" in inline asm. Behan Webster informed me that his Clang-
based build environment likes "r12" instead.
Try to make them both happy.
Signed-off-by: Alex Elder
Signed-off-by: Behan Webster
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the "smc" request completes.
*/
+#ifdef __clang__
+#define R12"r12"
+#else /* !__clang__ */
+#define R12"ip"/* gcc calls r12 "ip" */
+#endif /* !__clang__ */
>>>
>>> Why not just use r12 for both?
>>
>> Yes, that would have been an obvious fix. But the
>> assembler (in the GCC environment) doesn't accept that.
>>
>
> Mine has no problems with it at all
>
> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>
> and grepping for r12 under arch/arm suggests the same
The use of "r12" is fine. But it's not just the assembler,
I believe it also involves gcc.
The problem is with the use of the __asmeq(x, y) macro.
>>>
>>> Ah right. Apologies for assuming that you had missed something obvious here.
>>> But __asmeq is not the toolchain, it is a local construct #define'd in
>>> compiler.h
>>>
If I assign the "ip" variable with "r12":
register u32 ip asm("r12"); /* Also called ip */
Then that's fine. However, this line then causes an error:
__asmeq("%0", "r12")
Apparently gcc uses register "ip" when it sees asm("r12"). So
attempting to verify the desired register got used with __asmeq()
causes a string mismatch--"ip" is not equal to "r12".
So I could use:
register u32 ip asm("r12"); /* Also called ip */
...
__asmeq("%0", "ip")
And that will build. But it's a little non-intuitive, and
I suspect that clang might (rightfully) have a failure in
this __asmeq() call.
>>>
>>> In that case, I would strongly suggest fixing the __asmeq () macro
>>> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>>>
>>> The thing is, inline asm is a dodgy area to begin with in terms of
>>> clang-to-gcc compatibility. On arm64, we have been seeing issues where
>>> the width of the register -which is fixed on gcc- is selected based on
>>> the size of that variable, i.e., an int32 gets a w# register and int64
>>> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
>>> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
>>> clang.
>>>
>>> If we also start using the preprocessor to conditionalise what is
>>> emitted by inline asm, the waters get even murkier and it becomes even
>>> harder to claim parity between the two.
>>>
>>
>> Something like this perhaps?
>
> So __asmeq() yields true if the register names (strings) are
> equal, or if one is "ip" and the other is "r12" (in either order).
>
> I can't comment on whether it's right in all build environments but
> this looks OK to me, to handle this special case.
>
> I would much rather you generate that patch. Is that OK?
>
Sure, I can cook up a patch if you guys can confirm that it fixes your
use case. (I tested GCC myself but I don't have clang installed)
--
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
> On 28 January 2015 at 17:20, Ard Biesheuvel wrote:
>> On 28 January 2015 at 17:08, Alex Elder wrote:
>>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 14:11, Alex Elder wrote:
> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 05:18, Behan Webster
>> wrote:
>>> From: Alex Elder
>>>
>>> My GCC-based build environment likes to call register r12 by the
>>> name "ip" in inline asm. Behan Webster informed me that his Clang-
>>> based build environment likes "r12" instead.
>>>
>>> Try to make them both happy.
>>>
>>> Signed-off-by: Alex Elder
>>> Signed-off-by: Behan Webster
>>> ---
>>> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
>>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>>> index a55a7ec..3937bd5 100644
>>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>>> * request result appropriately. This result value is found in r0
>>> * when the "smc" request completes.
>>> */
>>> +#ifdef __clang__
>>> +#define R12"r12"
>>> +#else /* !__clang__ */
>>> +#define R12"ip"/* gcc calls r12 "ip" */
>>> +#endif /* !__clang__ */
>>
>> Why not just use r12 for both?
>
> Yes, that would have been an obvious fix. But the
> assembler (in the GCC environment) doesn't accept that.
>
Mine has no problems with it at all
$ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
and grepping for r12 under arch/arm suggests the same
>>>
>>> The use of "r12" is fine. But it's not just the assembler,
>>> I believe it also involves gcc.
>>>
>>> The problem is with the use of the __asmeq(x, y) macro.
>>>
>>
>> Ah right. Apologies for assuming that you had missed something obvious here.
>> But __asmeq is not the toolchain, it is a local construct #define'd in
>> compiler.h
>>
>>> If I assign the "ip" variable with "r12":
>>> register u32 ip asm("r12"); /* Also called ip */
>>>
>>> Then that's fine. However, this line then causes an error:
>>> __asmeq("%0", "r12")
>>>
>>> Apparently gcc uses register "ip" when it sees asm("r12"). So
>>> attempting to verify the desired register got used with __asmeq()
>>> causes a string mismatch--"ip" is not equal to "r12".
>>>
>>> So I could use:
>>>
>>> register u32 ip asm("r12"); /* Also called ip */
>>> ...
>>> __asmeq("%0", "ip")
>>>
>>> And that will build. But it's a little non-intuitive, and
>>> I suspect that clang might (rightfully) have a failure in
>>> this __asmeq() call.
>>>
>>
>> In that case, I would strongly suggest fixing the __asmeq () macro
>> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>>
>> The thing is, inline asm is a dodgy area to begin with in terms of
>> clang-to-gcc compatibility. On arm64, we have been seeing issues where
>> the width of the register -which is fixed on gcc- is selected based on
>> the size of that variable, i.e., an int32 gets a w# register and int64
>> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
>> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
>> clang.
>>
>> If we also start using the preprocessor to conditionalise what is
>> emitted by inline asm, the waters get even murkier and it becomes even
>> harder to claim parity between the two.
>>
>
> Something like this perhaps?
So __asmeq() yields true if the register names (strings) are
equal, or if one is "ip" and the other is "r12" (in either order).
I can't comment on whether it's right in all build environments but
this looks OK to me, to handle this special case.
I would much rather you generate that patch. Is that OK?
-Alex
> >8--
> diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h
> index 8155db2f7fa1..f99c674b3751 100644
> --- a/arch/arm/include/asm/compiler.h
> +++ b/arch/arm/include/asm/compiler.h
> @@ -9,7 +9,8 @@
> * will cause compilation to stop on mismatch.
> * (for details, see gcc PR 15089)
> */
> -#define __asmeq(x, y) ".ifnc " x "," y " ; .err ; .endif\n\t"
> +#define __asmeq(x, y) ".ifnc " x "," y " ; .ifnc " x y ",ipr12 ; " \
> + ".ifnc " x y ",r12ip ; .err ; .endif ; .endif ; .endif\n\t"
>
>
> #endif /* __ASM_ARM_COMPILER_H */
> >8--
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 14:11, Alex Elder wrote:
> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 05:18, Behan Webster wrote:
>>> From: Alex Elder
>>>
>>> My GCC-based build environment likes to call register r12 by the
>>> name "ip" in inline asm. Behan Webster informed me that his Clang-
>>> based build environment likes "r12" instead.
>>>
>>> Try to make them both happy.
>>>
>>> Signed-off-by: Alex Elder
>>> Signed-off-by: Behan Webster
>>> ---
>>> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
>>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>>> index a55a7ec..3937bd5 100644
>>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>>> * request result appropriately. This result value is found in r0
>>> * when the "smc" request completes.
>>> */
>>> +#ifdef __clang__
>>> +#define R12"r12"
>>> +#else /* !__clang__ */
>>> +#define R12"ip"/* gcc calls r12 "ip" */
>>> +#endif /* !__clang__ */
>>
>> Why not just use r12 for both?
>
> Yes, that would have been an obvious fix. But the
> assembler (in the GCC environment) doesn't accept that.
>
Mine has no problems with it at all
$ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
and grepping for r12 under arch/arm suggests the same
--
Ard.
>>> static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
>>> {
>>> - register u32 ip asm("ip"); /* Also called r12 */
>>> + register u32 ip asm(R12); /* Also called r12 */
>>> register u32 r0 asm("r0");
>>> register u32 r4 asm("r4");
>>> register u32 r5 asm("r5");
>>> @@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32
>>> buffer_phys)
>>>
>>> asm volatile (
>>> /* Make sure we got the registers we want */
>>> - __asmeq("%0", "ip")
>>> + __asmeq("%0", R12)
>>> __asmeq("%1", "r0")
>>> __asmeq("%2", "r4")
>>> __asmeq("%3", "r5")
>>> --
>>> 1.9.1
>>>
>>>
>>> ___
>>> linux-arm-kernel mailing list
>>> linux-arm-ker...@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 17:20, Ard Biesheuvel wrote:
> On 28 January 2015 at 17:08, Alex Elder wrote:
>> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 14:11, Alex Elder wrote:
On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
> On 28 January 2015 at 05:18, Behan Webster
> wrote:
>> From: Alex Elder
>>
>> My GCC-based build environment likes to call register r12 by the
>> name "ip" in inline asm. Behan Webster informed me that his Clang-
>> based build environment likes "r12" instead.
>>
>> Try to make them both happy.
>>
>> Signed-off-by: Alex Elder
>> Signed-off-by: Behan Webster
>> ---
>> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>> index a55a7ec..3937bd5 100644
>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
>> * request result appropriately. This result value is found in r0
>> * when the "smc" request completes.
>> */
>> +#ifdef __clang__
>> +#define R12"r12"
>> +#else /* !__clang__ */
>> +#define R12"ip"/* gcc calls r12 "ip" */
>> +#endif /* !__clang__ */
>
> Why not just use r12 for both?
Yes, that would have been an obvious fix. But the
assembler (in the GCC environment) doesn't accept that.
>>>
>>> Mine has no problems with it at all
>>>
>>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>>>
>>> and grepping for r12 under arch/arm suggests the same
>>
>> The use of "r12" is fine. But it's not just the assembler,
>> I believe it also involves gcc.
>>
>> The problem is with the use of the __asmeq(x, y) macro.
>>
>
> Ah right. Apologies for assuming that you had missed something obvious here.
> But __asmeq is not the toolchain, it is a local construct #define'd in
> compiler.h
>
>> If I assign the "ip" variable with "r12":
>> register u32 ip asm("r12"); /* Also called ip */
>>
>> Then that's fine. However, this line then causes an error:
>> __asmeq("%0", "r12")
>>
>> Apparently gcc uses register "ip" when it sees asm("r12"). So
>> attempting to verify the desired register got used with __asmeq()
>> causes a string mismatch--"ip" is not equal to "r12".
>>
>> So I could use:
>>
>> register u32 ip asm("r12"); /* Also called ip */
>> ...
>> __asmeq("%0", "ip")
>>
>> And that will build. But it's a little non-intuitive, and
>> I suspect that clang might (rightfully) have a failure in
>> this __asmeq() call.
>>
>
> In that case, I would strongly suggest fixing the __asmeq () macro
> instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
>
> The thing is, inline asm is a dodgy area to begin with in terms of
> clang-to-gcc compatibility. On arm64, we have been seeing issues where
> the width of the register -which is fixed on gcc- is selected based on
> the size of that variable, i.e., an int32 gets a w# register and int64
> gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
> writes 8 bytes on GCC suddenly only writing 4 bytes when built with
> clang.
>
> If we also start using the preprocessor to conditionalise what is
> emitted by inline asm, the waters get even murkier and it becomes even
> harder to claim parity between the two.
>
Something like this perhaps?
>8--
diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h
index 8155db2f7fa1..f99c674b3751 100644
--- a/arch/arm/include/asm/compiler.h
+++ b/arch/arm/include/asm/compiler.h
@@ -9,7 +9,8 @@
* will cause compilation to stop on mismatch.
* (for details, see gcc PR 15089)
*/
-#define __asmeq(x, y) ".ifnc " x "," y " ; .err ; .endif\n\t"
+#define __asmeq(x, y) ".ifnc " x "," y " ; .ifnc " x y ",ipr12 ; " \
+ ".ifnc " x y ",r12ip ; .err ; .endif ; .endif ; .endif\n\t"
#endif /* __ASM_ARM_COMPILER_H */
>8--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 02:11 PM, Ard Biesheuvel wrote: > Actually, if clang is guaranteed to emit the correct register name > inside the inline asm for register asm variables used in input or > output constraints, I think it makes sense to #define __asmeq as a nop > if __clang__ is defined. (Note that __asmeq only exists to work around > a specific GCC bug) I agree completely. Behan, what do you think? -Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 19:38, Ard Biesheuvel wrote:
> On 28 January 2015 at 19:27, Alex Elder wrote:
>> On 01/28/2015 01:17 PM, Ard Biesheuvel wrote:
>>> On 28 January 2015 at 17:20, Ard Biesheuvel
>>> wrote:
On 28 January 2015 at 17:08, Alex Elder wrote:
> On 01/28/2015 10:17 AM, Ard Biesheuvel wrote:
>> On 28 January 2015 at 14:11, Alex Elder wrote:
>>> On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 05:18, Behan Webster
wrote:
> From: Alex Elder
>
> My GCC-based build environment likes to call register r12 by the
> name "ip" in inline asm. Behan Webster informed me that his Clang-
> based build environment likes "r12" instead.
>
> Try to make them both happy.
>
> Signed-off-by: Alex Elder
> Signed-off-by: Behan Webster
> ---
> arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
> b/arch/arm/mach-bcm/bcm_kona_smc.c
> index a55a7ec..3937bd5 100644
> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
> @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
> * request result appropriately. This result value is found in r0
> * when the "smc" request completes.
> */
> +#ifdef __clang__
> +#define R12"r12"
> +#else /* !__clang__ */
> +#define R12"ip"/* gcc calls r12 "ip" */
> +#endif /* !__clang__ */
Why not just use r12 for both?
>>>
>>> Yes, that would have been an obvious fix. But the
>>> assembler (in the GCC environment) doesn't accept that.
>>>
>>
>> Mine has no problems with it at all
>>
>> $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
>>
>> and grepping for r12 under arch/arm suggests the same
>
> The use of "r12" is fine. But it's not just the assembler,
> I believe it also involves gcc.
>
> The problem is with the use of the __asmeq(x, y) macro.
>
Ah right. Apologies for assuming that you had missed something obvious
here.
But __asmeq is not the toolchain, it is a local construct #define'd in
compiler.h
> If I assign the "ip" variable with "r12":
> register u32 ip asm("r12"); /* Also called ip */
>
> Then that's fine. However, this line then causes an error:
> __asmeq("%0", "r12")
>
> Apparently gcc uses register "ip" when it sees asm("r12"). So
> attempting to verify the desired register got used with __asmeq()
> causes a string mismatch--"ip" is not equal to "r12".
>
> So I could use:
>
> register u32 ip asm("r12"); /* Also called ip */
> ...
> __asmeq("%0", "ip")
>
> And that will build. But it's a little non-intuitive, and
> I suspect that clang might (rightfully) have a failure in
> this __asmeq() call.
>
In that case, I would strongly suggest fixing the __asmeq () macro
instead, and teach it that ("r12","ip") and ("ip","r12") are fine too.
The thing is, inline asm is a dodgy area to begin with in terms of
clang-to-gcc compatibility. On arm64, we have been seeing issues where
the width of the register -which is fixed on gcc- is selected based on
the size of that variable, i.e., an int32 gets a w# register and int64
gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that
writes 8 bytes on GCC suddenly only writing 4 bytes when built with
clang.
If we also start using the preprocessor to conditionalise what is
emitted by inline asm, the waters get even murkier and it becomes even
harder to claim parity between the two.
>>>
>>> Something like this perhaps?
>>
>> So __asmeq() yields true if the register names (strings) are
>> equal, or if one is "ip" and the other is "r12" (in either order).
>>
>> I can't comment on whether it's right in all build environments but
>> this looks OK to me, to handle this special case.
>>
>> I would much rather you generate that patch. Is that OK?
>>
>
> Sure, I can cook up a patch if you guys can confirm that it fixes your
> use case. (I tested GCC myself but I don't have clang installed)
>
Actually, if clang is guaranteed to emit the correct register name
inside the inline asm for register asm variables used in input or
output constraints, I think it makes sense to #define __asmeq as a nop
if __clang__ is defined. (Note that __asmeq only exists to work around
a specific GCC bug)
--
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote:
On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote:
From: Alex Elder el...@linaro.org
My GCC-based build environment likes to call register r12 by the
name ip in inline asm. Behan Webster informed me that his Clang-
based build environment likes r12 instead.
Try to make them both happy.
Signed-off-by: Alex Elder el...@linaro.org
Signed-off-by: Behan Webster beh...@converseincode.com
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the smc request completes.
*/
+#ifdef __clang__
+#define R12r12
+#else /* !__clang__ */
+#define R12ip/* gcc calls r12 ip */
+#endif /* !__clang__ */
Why not just use r12 for both?
Yes, that would have been an obvious fix. But the
assembler (in the GCC environment) doesn't accept that.
Mine has no problems with it at all
$ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp -
and grepping for r12 under arch/arm suggests the same
--
Ard.
static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
{
- register u32 ip asm(ip); /* Also called r12 */
+ register u32 ip asm(R12); /* Also called r12 */
register u32 r0 asm(r0);
register u32 r4 asm(r4);
register u32 r5 asm(r5);
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32
buffer_phys)
asm volatile (
/* Make sure we got the registers we want */
- __asmeq(%0, ip)
+ __asmeq(%0, R12)
__asmeq(%1, r0)
__asmeq(%2, r4)
__asmeq(%3, r5)
--
1.9.1
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? 8-- diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h index 8155db2f7fa1..f99c674b3751 100644 --- a/arch/arm/include/asm/compiler.h +++ b/arch/arm/include/asm/compiler.h @@ -9,7 +9,8 @@ * will cause compilation to stop on mismatch. * (for details, see gcc PR 15089) */ -#define __asmeq(x, y) .ifnc x , y ; .err ; .endif\n\t +#define __asmeq(x, y) .ifnc x , y ; .ifnc x y ,ipr12 ; \ + .ifnc x y ,r12ip ; .err ; .endif ; .endif ; .endif\n\t #endif /* __ASM_ARM_COMPILER_H */ 8-- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. -Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 01:17 PM, Ard Biesheuvel wrote: On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? So __asmeq() yields true if the register names (strings) are equal, or if one is ip and the other is r12 (in either order). I can't comment on whether it's right in all build environments but this looks OK to me, to handle this special case. I would much rather you generate that patch. Is that OK? -Alex 8-- diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h index 8155db2f7fa1..f99c674b3751 100644 --- a/arch/arm/include/asm/compiler.h +++ b/arch/arm/include/asm/compiler.h @@ -9,7 +9,8 @@ * will cause compilation to stop on mismatch. * (for details, see gcc PR 15089) */ -#define __asmeq(x, y) .ifnc x , y ; .err ; .endif\n\t +#define __asmeq(x, y) .ifnc x , y ; .ifnc x y ,ipr12 ; \ + .ifnc x y ,r12ip ; .err ; .endif ; .endif ; .endif\n\t #endif /* __ASM_ARM_COMPILER_H */ 8-- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/15 11:17, Ard Biesheuvel wrote: On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? 8-- diff --git a/arch/arm/include/asm/compiler.h b/arch/arm/include/asm/compiler.h index 8155db2f7fa1..f99c674b3751 100644 --- a/arch/arm/include/asm/compiler.h +++ b/arch/arm/include/asm/compiler.h @@ -9,7 +9,8 @@ * will cause compilation to stop on mismatch. * (for details, see gcc PR 15089) */ -#define __asmeq(x, y) .ifnc x , y ; .err ; .endif\n\t +#define __asmeq(x, y) .ifnc x , y ; .ifnc x y ,ipr12 ; \ + .ifnc x y ,r12ip ; .err ; .endif ; .endif ; .endif\n\t #endif /* __ASM_ARM_COMPILER_H */ 8-- If that is acceptable, that's fine by me. In principal none of us *want* to use #ifdefs. Behan -- Behan Webster beh...@converseincode.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 19:27, Alex Elder el...@linaro.org wrote: On 01/28/2015 01:17 PM, Ard Biesheuvel wrote: On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? So __asmeq() yields true if the register names (strings) are equal, or if one is ip and the other is r12 (in either order). I can't comment on whether it's right in all build environments but this looks OK to me, to handle this special case. I would much rather you generate that patch. Is that OK? Sure, I can cook up a patch if you guys can confirm that it fixes your use case. (I tested GCC myself but I don't have clang installed) -- Ard. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote:
From: Alex Elder el...@linaro.org
My GCC-based build environment likes to call register r12 by the
name ip in inline asm. Behan Webster informed me that his Clang-
based build environment likes r12 instead.
Try to make them both happy.
Signed-off-by: Alex Elder el...@linaro.org
Signed-off-by: Behan Webster beh...@converseincode.com
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the smc request completes.
*/
+#ifdef __clang__
+#define R12r12
+#else /* !__clang__ */
+#define R12ip/* gcc calls r12 ip */
+#endif /* !__clang__ */
Why not just use r12 for both?
static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
{
- register u32 ip asm(ip); /* Also called r12 */
+ register u32 ip asm(R12); /* Also called r12 */
register u32 r0 asm(r0);
register u32 r4 asm(r4);
register u32 r5 asm(r5);
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32
buffer_phys)
asm volatile (
/* Make sure we got the registers we want */
- __asmeq(%0, ip)
+ __asmeq(%0, R12)
__asmeq(%1, r0)
__asmeq(%2, r4)
__asmeq(%3, r5)
--
1.9.1
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/15 11:38, Ard Biesheuvel wrote: On 28 January 2015 at 19:27, Alex Elder el...@linaro.org wrote: On 01/28/2015 01:17 PM, Ard Biesheuvel wrote: On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? So __asmeq() yields true if the register names (strings) are equal, or if one is ip and the other is r12 (in either order). I can't comment on whether it's right in all build environments but this looks OK to me, to handle this special case. I would much rather you generate that patch. Is that OK? Sure, I can cook up a patch if you guys can confirm that it fixes your use case. (I tested GCC myself but I don't have clang installed) That appears to work with clang as well. All in all a much better solution. Thank you, Behan -- Behan Webster beh...@converseincode.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. -- Ard. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/15 12:11, Ard Biesheuvel wrote: On 28 January 2015 at 19:38, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 19:27, Alex Elder el...@linaro.org wrote: On 01/28/2015 01:17 PM, Ard Biesheuvel wrote: On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? So __asmeq() yields true if the register names (strings) are equal, or if one is ip and the other is r12 (in either order). I can't comment on whether it's right in all build environments but this looks OK to me, to handle this special case. I would much rather you generate that patch. Is that OK? Sure, I can cook up a patch if you guys can confirm that it fixes your use case. (I tested GCC myself but I don't have clang installed) Actually, if clang is guaranteed to emit the correct register name inside the inline asm for register asm variables used in input or output constraints, I think it makes sense to #define __asmeq as a nop if __clang__ is defined. (Note that __asmeq only exists to work around a specific GCC bug) As far as I'm aware neither clang nor gcc will guarantee this completely in all places where asmeq has been used. Register assignments are handled differently, and at different levels of the architecture between the 2 compilers. Certainly asmeq has caught these kinds of bad assumptions in a number of places in the kernel while we've ported to clang (like when naked functions are used, and the calling convention for -O2 is assumed). I personally would prefer code which doesn't rely on variables being in the correct registers. However I'm aware that in the case of the smc instruction there really isn't a choice, as that's
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 05:15 AM, Ard Biesheuvel wrote:
On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote:
From: Alex Elder el...@linaro.org
My GCC-based build environment likes to call register r12 by the
name ip in inline asm. Behan Webster informed me that his Clang-
based build environment likes r12 instead.
Try to make them both happy.
Signed-off-by: Alex Elder el...@linaro.org
Signed-off-by: Behan Webster beh...@converseincode.com
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c
b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the smc request completes.
*/
+#ifdef __clang__
+#define R12r12
+#else /* !__clang__ */
+#define R12ip/* gcc calls r12 ip */
+#endif /* !__clang__ */
Why not just use r12 for both?
Yes, that would have been an obvious fix. But the
assembler (in the GCC environment) doesn't accept that.
-Alex
static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
{
- register u32 ip asm(ip); /* Also called r12 */
+ register u32 ip asm(R12); /* Also called r12 */
register u32 r0 asm(r0);
register u32 r4 asm(r4);
register u32 r5 asm(r5);
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32
buffer_phys)
asm volatile (
/* Make sure we got the registers we want */
- __asmeq(%0, ip)
+ __asmeq(%0, R12)
__asmeq(%1, r0)
__asmeq(%2, r4)
__asmeq(%3, r5)
--
1.9.1
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 28 January 2015 at 19:38, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 19:27, Alex Elder el...@linaro.org wrote: On 01/28/2015 01:17 PM, Ard Biesheuvel wrote: On 28 January 2015 at 17:20, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 28 January 2015 at 17:08, Alex Elder el...@linaro.org wrote: On 01/28/2015 10:17 AM, Ard Biesheuvel wrote: On 28 January 2015 at 14:11, Alex Elder el...@linaro.org wrote: On 01/28/2015 05:15 AM, Ard Biesheuvel wrote: On 28 January 2015 at 05:18, Behan Webster beh...@converseincode.com wrote: From: Alex Elder el...@linaro.org My GCC-based build environment likes to call register r12 by the name ip in inline asm. Behan Webster informed me that his Clang- based build environment likes r12 instead. Try to make them both happy. Signed-off-by: Alex Elder el...@linaro.org Signed-off-by: Behan Webster beh...@converseincode.com --- arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c index a55a7ec..3937bd5 100644 --- a/arch/arm/mach-bcm/bcm_kona_smc.c +++ b/arch/arm/mach-bcm/bcm_kona_smc.c @@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void) * request result appropriately. This result value is found in r0 * when the smc request completes. */ +#ifdef __clang__ +#define R12r12 +#else /* !__clang__ */ +#define R12ip/* gcc calls r12 ip */ +#endif /* !__clang__ */ Why not just use r12 for both? Yes, that would have been an obvious fix. But the assembler (in the GCC environment) doesn't accept that. Mine has no problems with it at all $ echo 'mov r12, #0' | arm-linux-gnueabihf-gcc -c -x assembler-with-cpp - and grepping for r12 under arch/arm suggests the same The use of r12 is fine. But it's not just the assembler, I believe it also involves gcc. The problem is with the use of the __asmeq(x, y) macro. Ah right. Apologies for assuming that you had missed something obvious here. But __asmeq is not the toolchain, it is a local construct #define'd in compiler.h If I assign the ip variable with r12: register u32 ip asm(r12); /* Also called ip */ Then that's fine. However, this line then causes an error: __asmeq(%0, r12) Apparently gcc uses register ip when it sees asm(r12). So attempting to verify the desired register got used with __asmeq() causes a string mismatch--ip is not equal to r12. So I could use: register u32 ip asm(r12); /* Also called ip */ ... __asmeq(%0, ip) And that will build. But it's a little non-intuitive, and I suspect that clang might (rightfully) have a failure in this __asmeq() call. In that case, I would strongly suggest fixing the __asmeq () macro instead, and teach it that (r12,ip) and (ip,r12) are fine too. The thing is, inline asm is a dodgy area to begin with in terms of clang-to-gcc compatibility. On arm64, we have been seeing issues where the width of the register -which is fixed on gcc- is selected based on the size of that variable, i.e., an int32 gets a w# register and int64 gets a x# register. Imagine debugging that, e.g., a str %0, [xx] that writes 8 bytes on GCC suddenly only writing 4 bytes when built with clang. If we also start using the preprocessor to conditionalise what is emitted by inline asm, the waters get even murkier and it becomes even harder to claim parity between the two. Something like this perhaps? So __asmeq() yields true if the register names (strings) are equal, or if one is ip and the other is r12 (in either order). I can't comment on whether it's right in all build environments but this looks OK to me, to handle this special case. I would much rather you generate that patch. Is that OK? Sure, I can cook up a patch if you guys can confirm that it fixes your use case. (I tested GCC myself but I don't have clang installed) Actually, if clang is guaranteed to emit the correct register name inside the inline asm for register asm variables used in input or output constraints, I think it makes sense to #define __asmeq as a nop if __clang__ is defined. (Note that __asmeq only exists to work around a specific GCC bug) -- Ard. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] bcm: address clang inline asm incompatibility
On 01/28/2015 02:11 PM, Ard Biesheuvel wrote: Actually, if clang is guaranteed to emit the correct register name inside the inline asm for register asm variables used in input or output constraints, I think it makes sense to #define __asmeq as a nop if __clang__ is defined. (Note that __asmeq only exists to work around a specific GCC bug) I agree completely. Behan, what do you think? -Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] bcm: address clang inline asm incompatibility
From: Alex Elder
My GCC-based build environment likes to call register r12 by the
name "ip" in inline asm. Behan Webster informed me that his Clang-
based build environment likes "r12" instead.
Try to make them both happy.
Signed-off-by: Alex Elder
Signed-off-by: Behan Webster
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the "smc" request completes.
*/
+#ifdef __clang__
+#define R12"r12"
+#else /* !__clang__ */
+#define R12"ip"/* gcc calls r12 "ip" */
+#endif /* !__clang__ */
static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
{
- register u32 ip asm("ip"); /* Also called r12 */
+ register u32 ip asm(R12); /* Also called r12 */
register u32 r0 asm("r0");
register u32 r4 asm("r4");
register u32 r5 asm("r5");
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
asm volatile (
/* Make sure we got the registers we want */
- __asmeq("%0", "ip")
+ __asmeq("%0", R12)
__asmeq("%1", "r0")
__asmeq("%2", "r4")
__asmeq("%3", "r5")
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] bcm: address clang inline asm incompatibility
From: Alex Elder el...@linaro.org
My GCC-based build environment likes to call register r12 by the
name ip in inline asm. Behan Webster informed me that his Clang-
based build environment likes r12 instead.
Try to make them both happy.
Signed-off-by: Alex Elder el...@linaro.org
Signed-off-by: Behan Webster beh...@converseincode.com
---
arch/arm/mach-bcm/bcm_kona_smc.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ec..3937bd5 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -106,9 +106,14 @@ int __init bcm_kona_smc_init(void)
* request result appropriately. This result value is found in r0
* when the smc request completes.
*/
+#ifdef __clang__
+#define R12r12
+#else /* !__clang__ */
+#define R12ip/* gcc calls r12 ip */
+#endif /* !__clang__ */
static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
{
- register u32 ip asm(ip); /* Also called r12 */
+ register u32 ip asm(R12); /* Also called r12 */
register u32 r0 asm(r0);
register u32 r4 asm(r4);
register u32 r5 asm(r5);
@@ -120,7 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
asm volatile (
/* Make sure we got the registers we want */
- __asmeq(%0, ip)
+ __asmeq(%0, R12)
__asmeq(%1, r0)
__asmeq(%2, r4)
__asmeq(%3, r5)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
