From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2] x86/current: Provide additional information to optimise get_cpu_info() Date: Mon, 15 Sep 2014 09:16:24 +0100 Message-ID: <5416A058.9040305@citrix.com> References: <54048379020000780002F761@mail.emea.novell.com> <1409585245-32325-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Marcin Cieslak , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 13/09/2014 17:10, Marcin Cieslak wrote: >>> Andrew Cooper wrote: >> Exactly as with c/s d55c5eefe "x86: use compiler visible "add" instead of >> inline assembly "or" in get_cpu_info()", this is achieved by providing more >> information to the compiler. >> >> This causes a net drop of almost 4K of .text >> >> Signed-off-by: Andrew Cooper >> CC: Jan Beulich >> >> --- >> v2: Less speculation about generated code in the comment >> --- >> xen/include/asm-x86/current.h | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h >> index 2081015..b95fd79 100644 >> --- a/xen/include/asm-x86/current.h >> +++ b/xen/include/asm-x86/current.h >> @@ -25,9 +25,9 @@ struct cpu_info { >> >> static inline struct cpu_info *get_cpu_info(void) >> { >> - unsigned long tos; >> - __asm__ ( "and %%rsp,%0" : "=r" (tos) : "0" (~(STACK_SIZE-1)) ); >> - return (struct cpu_info *)(tos + STACK_SIZE) - 1; >> + register unsigned long sp asm("rsp"); >> + >> + return (struct cpu_info *)((sp & ~(STACK_SIZE-1)) + STACK_SIZE) - 1; >> } > Hello, it seems to me that the above code fails on me with clang 3.4 > on FreeBSD-CURRENT: > > xen/include/asm/current.h:30:33: > error: variable 'sp' is uninitialized when used here [-Werror,-Wuninitialized] That is rather unfortunate for clang. The stack pointer has an initialised and perfectly good value anywhere this function can be used. > Reverting df0ae94fd56d5f9c64089364efecb1793442360b helps. > > There is a workaround suggested inhttp://llvm.org/devmtg/2014-02/slides/moller-llvmlinux.pdf: > > diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h > index b95fd79..e133d9d 100644 > --- a/xen/include/asm-x86/current.h > +++ b/xen/include/asm-x86/current.h > @@ -26,6 +26,7 @@ struct cpu_info { > static inline struct cpu_info *get_cpu_info(void) > { > register unsigned long sp asm("rsp"); > + asm("" : "=r" (sp)); > > return (struct cpu_info *)((sp & ~(STACK_SIZE-1)) + STACK_SIZE) - 1; > } > > That silences the warning (not sure it it works). It functions under GCC as well, but undoes some (but not all of) the improvements introduced as a result of df0ae94f. It would probably be acceptable in a suitable #ifdef, along with comment why this seemingly redundant statement is present. ~Andrew