From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tony Lindgren Subject: Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile Date: Tue, 4 Jan 2011 10:49:04 -0800 Message-ID: <20110104184904.GC7771@atomide.com> References: <1292649385-28771-1-git-send-email-sboyd@codeaurora.org> <1292875718-7980-1-git-send-email-sboyd@codeaurora.org> <1292875718-7980-2-git-send-email-sboyd@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Nicolas Pitre Cc: Stephen Boyd , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Arnd Bergmann , Daniel Walker List-Id: linux-arm-msm@vger.kernel.org * Nicolas Pitre [101220 13:38]: > On Mon, 20 Dec 2010, Stephen Boyd wrote: > > > Without marking the asm __dcc_getstatus() volatile my compiler > > decides it can cache the value of __ret in a register and then > > check the value of it continually in hvc_dcc_put_chars() (I had > > to replace get_wait/put_wait with 1 and fixup the branch > > otherwise my disassembler barfed on __dcc_(get|put)char). > > > > 00000000 : > > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > > 4: e3a0c000 mov ip, #0 ; 0x0 > > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > > c: ea000006 b 2c > > 10: e3530000 cmp r3, #0 ; 0x0 > > 14: 1afffffd bne 10 > > 18: e7d1000c ldrb r0, [r1, ip] > > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 20: 2afffffd bcs 1c > > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 28: e28cc001 add ip, ip, #1 ; 0x1 > > 2c: e15c0002 cmp ip, r2 > > 30: bafffff6 blt 10 > > 34: e1a00002 mov r0, r2 > > 38: e12fff1e bx lr > > > > As you can see, the value of the mrc is checked against > > DCC_STATUS_TX (bit 29) and then stored in r3 for later use. > > Marking the asm volatile produces the following: > > > > 00000000 : > > 0: e3a03000 mov r3, #0 ; 0x0 > > 4: ea000007 b 28 > > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > > c: e3100202 tst r0, #536870912 ; 0x20000000 > > 10: 1afffffc bne 8 > > 14: e7d10003 ldrb r0, [r1, r3] > > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 1c: 2afffffd bcs 18 > > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 24: e2833001 add r3, r3, #1 ; 0x1 > > 28: e1530002 cmp r3, r2 > > 2c: bafffff5 blt 8 > > 30: e1a00002 mov r0, r2 > > 34: e12fff1e bx lr > > > > which looks better and actually works. Mark all the inline > > assembly in this file as volatile since we don't want the > > compiler to optimize away these statements or move them around > > in any way. > > > > Cc: Tony Lindgren > > Cc: Arnd Bergmann > > Cc: Nicolas Pitre > > Cc: Daniel Walker > > Signed-off-by: Stephen Boyd > > Acked-by: Nicolas Pitre Acked-by: Tony Lindgren From mboxrd@z Thu Jan 1 00:00:00 1970 From: tony@atomide.com (Tony Lindgren) Date: Tue, 4 Jan 2011 10:49:04 -0800 Subject: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile In-Reply-To: References: <1292649385-28771-1-git-send-email-sboyd@codeaurora.org> <1292875718-7980-1-git-send-email-sboyd@codeaurora.org> <1292875718-7980-2-git-send-email-sboyd@codeaurora.org> Message-ID: <20110104184904.GC7771@atomide.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org * Nicolas Pitre [101220 13:38]: > On Mon, 20 Dec 2010, Stephen Boyd wrote: > > > Without marking the asm __dcc_getstatus() volatile my compiler > > decides it can cache the value of __ret in a register and then > > check the value of it continually in hvc_dcc_put_chars() (I had > > to replace get_wait/put_wait with 1 and fixup the branch > > otherwise my disassembler barfed on __dcc_(get|put)char). > > > > 00000000 : > > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > > 4: e3a0c000 mov ip, #0 ; 0x0 > > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > > c: ea000006 b 2c > > 10: e3530000 cmp r3, #0 ; 0x0 > > 14: 1afffffd bne 10 > > 18: e7d1000c ldrb r0, [r1, ip] > > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 20: 2afffffd bcs 1c > > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 28: e28cc001 add ip, ip, #1 ; 0x1 > > 2c: e15c0002 cmp ip, r2 > > 30: bafffff6 blt 10 > > 34: e1a00002 mov r0, r2 > > 38: e12fff1e bx lr > > > > As you can see, the value of the mrc is checked against > > DCC_STATUS_TX (bit 29) and then stored in r3 for later use. > > Marking the asm volatile produces the following: > > > > 00000000 : > > 0: e3a03000 mov r3, #0 ; 0x0 > > 4: ea000007 b 28 > > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > > c: e3100202 tst r0, #536870912 ; 0x20000000 > > 10: 1afffffc bne 8 > > 14: e7d10003 ldrb r0, [r1, r3] > > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 1c: 2afffffd bcs 18 > > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 24: e2833001 add r3, r3, #1 ; 0x1 > > 28: e1530002 cmp r3, r2 > > 2c: bafffff5 blt 8 > > 30: e1a00002 mov r0, r2 > > 34: e12fff1e bx lr > > > > which looks better and actually works. Mark all the inline > > assembly in this file as volatile since we don't want the > > compiler to optimize away these statements or move them around > > in any way. > > > > Cc: Tony Lindgren > > Cc: Arnd Bergmann > > Cc: Nicolas Pitre > > Cc: Daniel Walker > > Signed-off-by: Stephen Boyd > > Acked-by: Nicolas Pitre Acked-by: Tony Lindgren