Re: [PATCH] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-15 Thread Catalin Marinas
On Mon, Sep 15, 2014 at 05:02:41PM +0100, Will Deacon wrote:
> On Mon, Sep 15, 2014 at 06:30:15AM +0100, beh...@converseincode.com wrote:
> > From: Mark Charlebois 
> > 
> > Remove '#' from immediate parameter in AARCH64 inline assembly in mmu.
> > 
> > This code now works with both gcc and clang.
> > 
> > Signed-off-by: Mark Charlebois 
> > Signed-off-by: Behan Webster 
> > ---
> >  arch/arm64/mm/mmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Acked-by: Will Deacon 
> 
> Thanks, Behan!
> 
> Catalin: please can you pick this up for 3.18? It's probably not worth me
> merging it as a fix, since there are other patches needed to get us building
> under clang anyway.

I agree, it will go in for 3.18 rather than 3.17.

-- 
Catalin
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-15 Thread Will Deacon
On Mon, Sep 15, 2014 at 06:30:15AM +0100, beh...@converseincode.com wrote:
> From: Mark Charlebois 
> 
> Remove '#' from immediate parameter in AARCH64 inline assembly in mmu.
> 
> This code now works with both gcc and clang.
> 
> Signed-off-by: Mark Charlebois 
> Signed-off-by: Behan Webster 
> ---
>  arch/arm64/mm/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Will Deacon 

Thanks, Behan!

Catalin: please can you pick this up for 3.18? It's probably not worth me
merging it as a fix, since there are other patches needed to get us building
under clang anyway.

Will

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index c555672..6894ef3 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -94,7 +94,7 @@ static int __init early_cachepolicy(char *p)
>*/
>   asm volatile(
>   "   mrs %0, mair_el1\n"
> - "   bfi %0, %1, #%2, #8\n"
> + "   bfi %0, %1, %2, #8\n"
>   "   msr mair_el1, %0\n"
>   "   isb\n"
>   : "=" (tmp)
> -- 
> 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/


Re: [PATCH] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-15 Thread Will Deacon
On Mon, Sep 15, 2014 at 06:30:15AM +0100, beh...@converseincode.com wrote:
 From: Mark Charlebois charl...@gmail.com
 
 Remove '#' from immediate parameter in AARCH64 inline assembly in mmu.
 
 This code now works with both gcc and clang.
 
 Signed-off-by: Mark Charlebois charl...@gmail.com
 Signed-off-by: Behan Webster beh...@converseincode.com
 ---
  arch/arm64/mm/mmu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Will Deacon will.dea...@arm.com

Thanks, Behan!

Catalin: please can you pick this up for 3.18? It's probably not worth me
merging it as a fix, since there are other patches needed to get us building
under clang anyway.

Will

 diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
 index c555672..6894ef3 100644
 --- a/arch/arm64/mm/mmu.c
 +++ b/arch/arm64/mm/mmu.c
 @@ -94,7 +94,7 @@ static int __init early_cachepolicy(char *p)
*/
   asm volatile(
  mrs %0, mair_el1\n
 -bfi %0, %1, #%2, #8\n
 +bfi %0, %1, %2, #8\n
  msr mair_el1, %0\n
  isb\n
   : =r (tmp)
 -- 
 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/


Re: [PATCH] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-15 Thread Catalin Marinas
On Mon, Sep 15, 2014 at 05:02:41PM +0100, Will Deacon wrote:
 On Mon, Sep 15, 2014 at 06:30:15AM +0100, beh...@converseincode.com wrote:
  From: Mark Charlebois charl...@gmail.com
  
  Remove '#' from immediate parameter in AARCH64 inline assembly in mmu.
  
  This code now works with both gcc and clang.
  
  Signed-off-by: Mark Charlebois charl...@gmail.com
  Signed-off-by: Behan Webster beh...@converseincode.com
  ---
   arch/arm64/mm/mmu.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 Acked-by: Will Deacon will.dea...@arm.com
 
 Thanks, Behan!
 
 Catalin: please can you pick this up for 3.18? It's probably not worth me
 merging it as a fix, since there are other patches needed to get us building
 under clang anyway.

I agree, it will go in for 3.18 rather than 3.17.

-- 
Catalin
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-10 Thread Ard Biesheuvel
On 10 September 2014 19:38, Will Deacon  wrote:
> On Mon, Sep 08, 2014 at 10:30:51AM +0100, Olof Johansson wrote:
>> On Fri, Sep 05, 2014 at 04:24:20PM -0700, beh...@converseincode.com wrote:
>> > From: Mark Charlebois 
>> >
>> > Fix variable types for 64-bit inline assembly.
>> >
>> > This patch now works with both gcc and clang.
>> >
>> > Signed-off-by: Mark Charlebois 
>> > Signed-off-by: Behan Webster 
>> > ---
>> >  arch/arm64/include/asm/arch_timer.h | 26 +++---
>> >  arch/arm64/include/asm/uaccess.h|  2 +-
>> >  arch/arm64/kernel/debug-monitors.c  |  8 
>> >  arch/arm64/kernel/perf_event.c  | 34 
>> > +-
>> >  arch/arm64/mm/mmu.c |  2 +-
>> >  5 files changed, 38 insertions(+), 34 deletions(-)
>> >
>> > diff --git a/arch/arm64/include/asm/arch_timer.h 
>> > b/arch/arm64/include/asm/arch_timer.h
>> > index 9400596..c1f87e0 100644
>> > --- a/arch/arm64/include/asm/arch_timer.h
>> > +++ b/arch/arm64/include/asm/arch_timer.h
>> > @@ -37,19 +37,23 @@ void arch_timer_reg_write_cp15(int access, enum 
>> > arch_timer_reg reg, u32 val)
>> > if (access == ARCH_TIMER_PHYS_ACCESS) {
>> > switch (reg) {
>> > case ARCH_TIMER_REG_CTRL:
>> > -   asm volatile("msr cntp_ctl_el0,  %0" : : "r" (val));
>> > +   asm volatile("msr cntp_ctl_el0,  %0"
>> > +   : : "r" ((u64)val));
>>
>> Ick. Care to elaborate in the patch description why this is needed with
>> LLVM? It's really messy and very annoying having to cast register values
>> every time they're passed in, instead of the compiler handling it for you.
>>
>> Is there a way to catch this with GCC? If not, I expect you to get broken
>> all the time on this by people who don't notice.
>
> Question to the clang people (Clangers?): what happens if the %0 above is
> rewritten as %x0 and the cast on val is dropped? I could stomach a change
> adding that, but it's still likely to regress without regular build testing.
>

I did a quick test with Clang, and indeed, it infers the type of
register from the size of the operand, but it also supports explicit
%x0 and %w0 casts.
So in this particular case, we could work around it in this way.

However, I think this uncovers a much more serious issue: while we
catch the problem here because msr simply does not support 32-bit
operands, most other instructions do, and we have no idea how the
Clang generated code deviates from what GCC produces. Or am I being
paranoid here?
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-10 Thread Will Deacon
On Mon, Sep 08, 2014 at 10:30:51AM +0100, Olof Johansson wrote:
> On Fri, Sep 05, 2014 at 04:24:20PM -0700, beh...@converseincode.com wrote:
> > From: Mark Charlebois 
> > 
> > Fix variable types for 64-bit inline assembly.
> > 
> > This patch now works with both gcc and clang.
> > 
> > Signed-off-by: Mark Charlebois 
> > Signed-off-by: Behan Webster 
> > ---
> >  arch/arm64/include/asm/arch_timer.h | 26 +++---
> >  arch/arm64/include/asm/uaccess.h|  2 +-
> >  arch/arm64/kernel/debug-monitors.c  |  8 
> >  arch/arm64/kernel/perf_event.c  | 34 +-
> >  arch/arm64/mm/mmu.c |  2 +-
> >  5 files changed, 38 insertions(+), 34 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/arch_timer.h 
> > b/arch/arm64/include/asm/arch_timer.h
> > index 9400596..c1f87e0 100644
> > --- a/arch/arm64/include/asm/arch_timer.h
> > +++ b/arch/arm64/include/asm/arch_timer.h
> > @@ -37,19 +37,23 @@ void arch_timer_reg_write_cp15(int access, enum 
> > arch_timer_reg reg, u32 val)
> > if (access == ARCH_TIMER_PHYS_ACCESS) {
> > switch (reg) {
> > case ARCH_TIMER_REG_CTRL:
> > -   asm volatile("msr cntp_ctl_el0,  %0" : : "r" (val));
> > +   asm volatile("msr cntp_ctl_el0,  %0"
> > +   : : "r" ((u64)val));
> 
> Ick. Care to elaborate in the patch description why this is needed with
> LLVM? It's really messy and very annoying having to cast register values
> every time they're passed in, instead of the compiler handling it for you.
> 
> Is there a way to catch this with GCC? If not, I expect you to get broken
> all the time on this by people who don't notice.

Question to the clang people (Clangers?): what happens if the %0 above is
rewritten as %x0 and the cast on val is dropped? I could stomach a change
adding that, but it's still likely to regress without regular build testing.

Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-10 Thread Olof Johansson
On Fri, Sep 05, 2014 at 04:24:20PM -0700, beh...@converseincode.com wrote:
> From: Mark Charlebois 
> 
> Fix variable types for 64-bit inline assembly.
> 
> This patch now works with both gcc and clang.
> 
> Signed-off-by: Mark Charlebois 
> Signed-off-by: Behan Webster 
> ---
>  arch/arm64/include/asm/arch_timer.h | 26 +++---
>  arch/arm64/include/asm/uaccess.h|  2 +-
>  arch/arm64/kernel/debug-monitors.c  |  8 
>  arch/arm64/kernel/perf_event.c  | 34 +-
>  arch/arm64/mm/mmu.c |  2 +-
>  5 files changed, 38 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arch_timer.h 
> b/arch/arm64/include/asm/arch_timer.h
> index 9400596..c1f87e0 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -37,19 +37,23 @@ void arch_timer_reg_write_cp15(int access, enum 
> arch_timer_reg reg, u32 val)
>   if (access == ARCH_TIMER_PHYS_ACCESS) {
>   switch (reg) {
>   case ARCH_TIMER_REG_CTRL:
> - asm volatile("msr cntp_ctl_el0,  %0" : : "r" (val));
> + asm volatile("msr cntp_ctl_el0,  %0"
> + : : "r" ((u64)val));

Ick. Care to elaborate in the patch description why this is needed with
LLVM? It's really messy and very annoying having to cast register values
every time they're passed in, instead of the compiler handling it for you.

Is there a way to catch this with GCC? If not, I expect you to get broken
all the time on this by people who don't notice.



-Olof
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-10 Thread Olof Johansson
On Fri, Sep 05, 2014 at 04:24:20PM -0700, beh...@converseincode.com wrote:
 From: Mark Charlebois charl...@gmail.com
 
 Fix variable types for 64-bit inline assembly.
 
 This patch now works with both gcc and clang.
 
 Signed-off-by: Mark Charlebois charl...@gmail.com
 Signed-off-by: Behan Webster beh...@converseincode.com
 ---
  arch/arm64/include/asm/arch_timer.h | 26 +++---
  arch/arm64/include/asm/uaccess.h|  2 +-
  arch/arm64/kernel/debug-monitors.c  |  8 
  arch/arm64/kernel/perf_event.c  | 34 +-
  arch/arm64/mm/mmu.c |  2 +-
  5 files changed, 38 insertions(+), 34 deletions(-)
 
 diff --git a/arch/arm64/include/asm/arch_timer.h 
 b/arch/arm64/include/asm/arch_timer.h
 index 9400596..c1f87e0 100644
 --- a/arch/arm64/include/asm/arch_timer.h
 +++ b/arch/arm64/include/asm/arch_timer.h
 @@ -37,19 +37,23 @@ void arch_timer_reg_write_cp15(int access, enum 
 arch_timer_reg reg, u32 val)
   if (access == ARCH_TIMER_PHYS_ACCESS) {
   switch (reg) {
   case ARCH_TIMER_REG_CTRL:
 - asm volatile(msr cntp_ctl_el0,  %0 : : r (val));
 + asm volatile(msr cntp_ctl_el0,  %0
 + : : r ((u64)val));

Ick. Care to elaborate in the patch description why this is needed with
LLVM? It's really messy and very annoying having to cast register values
every time they're passed in, instead of the compiler handling it for you.

Is there a way to catch this with GCC? If not, I expect you to get broken
all the time on this by people who don't notice.



-Olof
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-10 Thread Will Deacon
On Mon, Sep 08, 2014 at 10:30:51AM +0100, Olof Johansson wrote:
 On Fri, Sep 05, 2014 at 04:24:20PM -0700, beh...@converseincode.com wrote:
  From: Mark Charlebois charl...@gmail.com
  
  Fix variable types for 64-bit inline assembly.
  
  This patch now works with both gcc and clang.
  
  Signed-off-by: Mark Charlebois charl...@gmail.com
  Signed-off-by: Behan Webster beh...@converseincode.com
  ---
   arch/arm64/include/asm/arch_timer.h | 26 +++---
   arch/arm64/include/asm/uaccess.h|  2 +-
   arch/arm64/kernel/debug-monitors.c  |  8 
   arch/arm64/kernel/perf_event.c  | 34 +-
   arch/arm64/mm/mmu.c |  2 +-
   5 files changed, 38 insertions(+), 34 deletions(-)
  
  diff --git a/arch/arm64/include/asm/arch_timer.h 
  b/arch/arm64/include/asm/arch_timer.h
  index 9400596..c1f87e0 100644
  --- a/arch/arm64/include/asm/arch_timer.h
  +++ b/arch/arm64/include/asm/arch_timer.h
  @@ -37,19 +37,23 @@ void arch_timer_reg_write_cp15(int access, enum 
  arch_timer_reg reg, u32 val)
  if (access == ARCH_TIMER_PHYS_ACCESS) {
  switch (reg) {
  case ARCH_TIMER_REG_CTRL:
  -   asm volatile(msr cntp_ctl_el0,  %0 : : r (val));
  +   asm volatile(msr cntp_ctl_el0,  %0
  +   : : r ((u64)val));
 
 Ick. Care to elaborate in the patch description why this is needed with
 LLVM? It's really messy and very annoying having to cast register values
 every time they're passed in, instead of the compiler handling it for you.
 
 Is there a way to catch this with GCC? If not, I expect you to get broken
 all the time on this by people who don't notice.

Question to the clang people (Clangers?): what happens if the %0 above is
rewritten as %x0 and the cast on val is dropped? I could stomach a change
adding that, but it's still likely to regress without regular build testing.

Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-10 Thread Ard Biesheuvel
On 10 September 2014 19:38, Will Deacon will.dea...@arm.com wrote:
 On Mon, Sep 08, 2014 at 10:30:51AM +0100, Olof Johansson wrote:
 On Fri, Sep 05, 2014 at 04:24:20PM -0700, beh...@converseincode.com wrote:
  From: Mark Charlebois charl...@gmail.com
 
  Fix variable types for 64-bit inline assembly.
 
  This patch now works with both gcc and clang.
 
  Signed-off-by: Mark Charlebois charl...@gmail.com
  Signed-off-by: Behan Webster beh...@converseincode.com
  ---
   arch/arm64/include/asm/arch_timer.h | 26 +++---
   arch/arm64/include/asm/uaccess.h|  2 +-
   arch/arm64/kernel/debug-monitors.c  |  8 
   arch/arm64/kernel/perf_event.c  | 34 
  +-
   arch/arm64/mm/mmu.c |  2 +-
   5 files changed, 38 insertions(+), 34 deletions(-)
 
  diff --git a/arch/arm64/include/asm/arch_timer.h 
  b/arch/arm64/include/asm/arch_timer.h
  index 9400596..c1f87e0 100644
  --- a/arch/arm64/include/asm/arch_timer.h
  +++ b/arch/arm64/include/asm/arch_timer.h
  @@ -37,19 +37,23 @@ void arch_timer_reg_write_cp15(int access, enum 
  arch_timer_reg reg, u32 val)
  if (access == ARCH_TIMER_PHYS_ACCESS) {
  switch (reg) {
  case ARCH_TIMER_REG_CTRL:
  -   asm volatile(msr cntp_ctl_el0,  %0 : : r (val));
  +   asm volatile(msr cntp_ctl_el0,  %0
  +   : : r ((u64)val));

 Ick. Care to elaborate in the patch description why this is needed with
 LLVM? It's really messy and very annoying having to cast register values
 every time they're passed in, instead of the compiler handling it for you.

 Is there a way to catch this with GCC? If not, I expect you to get broken
 all the time on this by people who don't notice.

 Question to the clang people (Clangers?): what happens if the %0 above is
 rewritten as %x0 and the cast on val is dropped? I could stomach a change
 adding that, but it's still likely to regress without regular build testing.


I did a quick test with Clang, and indeed, it infers the type of
register from the size of the operand, but it also supports explicit
%x0 and %w0 casts.
So in this particular case, we could work around it in this way.

However, I think this uncovers a much more serious issue: while we
catch the problem here because msr simply does not support 32-bit
operands, most other instructions do, and we have no idea how the
Clang generated code deviates from what GCC produces. Or am I being
paranoid here?
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-09 Thread Will Deacon
On Mon, Sep 08, 2014 at 07:35:47PM +0100, Mark Charlebois wrote:
> When I compile
> 
> int main()
> {
> u64 foo, tmp;
> 
>// This fails for clang but not gcc
> asm volatile(
> "   mrs %0, mair_el1\n"
> "   bfi %0, %1, #%2, #8\n"
> "   msr mair_el1, %0\n"
> "   isb\n"
> : "=" (tmp)
> : "r" (foo), "i" (MT_NORMAL * 8));
> }
> 
> Clang fails and GCC generates:
> 
> 004004f0 :
>   4004f0: d538a208 mrs x8, mair_el1
>   4004f4: b3601d08 bfi x8, x8, #32, #8
>   4004f8: d518a208 msr mair_el1, x8
>   4004fc: d5033fdf isb
>   400500: 2a1f03e0 mov w0, wzr
>   400504: d65f03c0 ret

Ok, so we can just drop the '#' prefix as it probably shouldn't be there
anyway. Can you send a patch making just that change, please?

Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-09 Thread Will Deacon
On Mon, Sep 08, 2014 at 07:35:47PM +0100, Mark Charlebois wrote:
 When I compile
 
 int main()
 {
 u64 foo, tmp;
 
// This fails for clang but not gcc
 asm volatile(
mrs %0, mair_el1\n
bfi %0, %1, #%2, #8\n
msr mair_el1, %0\n
isb\n
 : =r (tmp)
 : r (foo), i (MT_NORMAL * 8));
 }
 
 Clang fails and GCC generates:
 
 004004f0 main:
   4004f0: d538a208 mrs x8, mair_el1
   4004f4: b3601d08 bfi x8, x8, #32, #8
   4004f8: d518a208 msr mair_el1, x8
   4004fc: d5033fdf isb
   400500: 2a1f03e0 mov w0, wzr
   400504: d65f03c0 ret

Ok, so we can just drop the '#' prefix as it probably shouldn't be there
anyway. Can you send a patch making just that change, please?

Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-08 Thread Mark Charlebois
When I compile

int main()
{
u64 foo, tmp;

// This works for both clang and gcc
asm volatile(
"   mrs %0, mair_el1\n"
"   bfi %0, %1, %2, #8\n"
"   msr mair_el1, %0\n"
"   isb\n"
: "=" (tmp)
: "r" (foo), "i" (MT_NORMAL * 8));
}

with clang I get:

004004f0 :
  4004f0: d538a208 mrs x8, mair_el1
  4004f4: b3601d08 bfi x8, x8, #32, #8
  4004f8: d518a208 msr mair_el1, x8
  4004fc: d5033fdf isb
  400500: 2a1f03e0 mov w0, wzr
  400504: d65f03c0 ret

When I compile it with GCC I get:

00400510 :
  400510: d10043ff sub sp, sp, #0x10
  400514: f94003e1 ldr x1, [sp]
  400518: d538a200 mrs x0, mair_el1
  40051c: b3601c20 bfi x0, x1, #32, #8
  400520: d518a200 msr mair_el1, x0
  400524: d5033fdf isb
  400528: f90007e0 str x0, [sp,#8]
  40052c: 910043ff add sp, sp, #0x10
  400530: d65f03c0 ret

When I compile

int main()
{
u64 foo, tmp;

   // This fails for clang but not gcc
asm volatile(
"   mrs %0, mair_el1\n"
"   bfi %0, %1, #%2, #8\n"
"   msr mair_el1, %0\n"
"   isb\n"
: "=" (tmp)
: "r" (foo), "i" (MT_NORMAL * 8));
}

Clang fails and GCC generates:

004004f0 :
  4004f0: d538a208 mrs x8, mair_el1
  4004f4: b3601d08 bfi x8, x8, #32, #8
  4004f8: d518a208 msr mair_el1, x8
  4004fc: d5033fdf isb
  400500: 2a1f03e0 mov w0, wzr
  400504: d65f03c0 ret

On Mon, Sep 8, 2014 at 3:53 AM, Will Deacon  wrote:
> On Sat, Sep 06, 2014 at 12:24:20AM +0100, beh...@converseincode.com wrote:
>> From: Mark Charlebois 
>>
>> Fix variable types for 64-bit inline assembly.
>>
>> This patch now works with both gcc and clang.
>
> Really? This looks like something the clang needs to do better on, as I
> really don't see people adding these casts to future code. They're ugly and
> redundant (or GCC).
>
> This hunk:
>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index c555672..6894ef3 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -94,7 +94,7 @@ static int __init early_cachepolicy(char *p)
>>*/
>>   asm volatile(
>>   "   mrs %0, mair_el1\n"
>> - "   bfi %0, %1, #%2, #8\n"
>> + "   bfi %0, %1, %2, #8\n"
>>   "   msr mair_el1, %0\n"
>>   "   isb\n"
>>   : "=" (tmp)
>
> also looks fishy. Does gas accept that without the '#' prefix?
>
> Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-08 Thread Will Deacon
On Sat, Sep 06, 2014 at 12:24:20AM +0100, beh...@converseincode.com wrote:
> From: Mark Charlebois 
> 
> Fix variable types for 64-bit inline assembly.
> 
> This patch now works with both gcc and clang.

Really? This looks like something the clang needs to do better on, as I
really don't see people adding these casts to future code. They're ugly and
redundant (or GCC).

This hunk:

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index c555672..6894ef3 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -94,7 +94,7 @@ static int __init early_cachepolicy(char *p)
>*/
>   asm volatile(
>   "   mrs %0, mair_el1\n"
> - "   bfi %0, %1, #%2, #8\n"
> + "   bfi %0, %1, %2, #8\n"
>   "   msr mair_el1, %0\n"
>   "   isb\n"
>   : "=" (tmp)

also looks fishy. Does gas accept that without the '#' prefix?

Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-08 Thread Will Deacon
On Sat, Sep 06, 2014 at 12:24:20AM +0100, beh...@converseincode.com wrote:
 From: Mark Charlebois charl...@gmail.com
 
 Fix variable types for 64-bit inline assembly.
 
 This patch now works with both gcc and clang.

Really? This looks like something the clang needs to do better on, as I
really don't see people adding these casts to future code. They're ugly and
redundant (or GCC).

This hunk:

 diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
 index c555672..6894ef3 100644
 --- a/arch/arm64/mm/mmu.c
 +++ b/arch/arm64/mm/mmu.c
 @@ -94,7 +94,7 @@ static int __init early_cachepolicy(char *p)
*/
   asm volatile(
  mrs %0, mair_el1\n
 -bfi %0, %1, #%2, #8\n
 +bfi %0, %1, %2, #8\n
  msr mair_el1, %0\n
  isb\n
   : =r (tmp)

also looks fishy. Does gas accept that without the '#' prefix?

Will
--
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] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang

2014-09-08 Thread Mark Charlebois
When I compile

int main()
{
u64 foo, tmp;

// This works for both clang and gcc
asm volatile(
   mrs %0, mair_el1\n
   bfi %0, %1, %2, #8\n
   msr mair_el1, %0\n
   isb\n
: =r (tmp)
: r (foo), i (MT_NORMAL * 8));
}

with clang I get:

004004f0 main:
  4004f0: d538a208 mrs x8, mair_el1
  4004f4: b3601d08 bfi x8, x8, #32, #8
  4004f8: d518a208 msr mair_el1, x8
  4004fc: d5033fdf isb
  400500: 2a1f03e0 mov w0, wzr
  400504: d65f03c0 ret

When I compile it with GCC I get:

00400510 main:
  400510: d10043ff sub sp, sp, #0x10
  400514: f94003e1 ldr x1, [sp]
  400518: d538a200 mrs x0, mair_el1
  40051c: b3601c20 bfi x0, x1, #32, #8
  400520: d518a200 msr mair_el1, x0
  400524: d5033fdf isb
  400528: f90007e0 str x0, [sp,#8]
  40052c: 910043ff add sp, sp, #0x10
  400530: d65f03c0 ret

When I compile

int main()
{
u64 foo, tmp;

   // This fails for clang but not gcc
asm volatile(
   mrs %0, mair_el1\n
   bfi %0, %1, #%2, #8\n
   msr mair_el1, %0\n
   isb\n
: =r (tmp)
: r (foo), i (MT_NORMAL * 8));
}

Clang fails and GCC generates:

004004f0 main:
  4004f0: d538a208 mrs x8, mair_el1
  4004f4: b3601d08 bfi x8, x8, #32, #8
  4004f8: d518a208 msr mair_el1, x8
  4004fc: d5033fdf isb
  400500: 2a1f03e0 mov w0, wzr
  400504: d65f03c0 ret

On Mon, Sep 8, 2014 at 3:53 AM, Will Deacon will.dea...@arm.com wrote:
 On Sat, Sep 06, 2014 at 12:24:20AM +0100, beh...@converseincode.com wrote:
 From: Mark Charlebois charl...@gmail.com

 Fix variable types for 64-bit inline assembly.

 This patch now works with both gcc and clang.

 Really? This looks like something the clang needs to do better on, as I
 really don't see people adding these casts to future code. They're ugly and
 redundant (or GCC).

 This hunk:

 diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
 index c555672..6894ef3 100644
 --- a/arch/arm64/mm/mmu.c
 +++ b/arch/arm64/mm/mmu.c
 @@ -94,7 +94,7 @@ static int __init early_cachepolicy(char *p)
*/
   asm volatile(
  mrs %0, mair_el1\n
 -bfi %0, %1, #%2, #8\n
 +bfi %0, %1, %2, #8\n
  msr mair_el1, %0\n
  isb\n
   : =r (tmp)

 also looks fishy. Does gas accept that without the '#' prefix?

 Will
--
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/