Re: [PATCH] arm64: LLVMLinux: Fix inline arm64 assembly for use with clang
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/