Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-30 Thread Paolo Bonzini
On 29/10/20 23:12, David Laight wrote: >> https://godbolt.org/z/4dzPbM >> >> With -fno-strict-aliasing, the compiler reloads the pointer if you write >> to the start of what it points to, but not if you write to later >> elements. > I guess it assumes that global data doesn't overlap. Yeah,

RE: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread David Laight
From: Arvind Sankar > Sent: 29 October 2020 21:35 > > On Thu, Oct 29, 2020 at 09:41:13PM +0100, Thomas Gleixner wrote: > > On Thu, Oct 29 2020 at 17:59, Paolo Bonzini wrote: > > > On 29/10/20 17:56, Arvind Sankar wrote: > > >>> For those two just add: > > >>> struct apic *apic =

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Arvind Sankar
On Thu, Oct 29, 2020 at 09:41:13PM +0100, Thomas Gleixner wrote: > On Thu, Oct 29 2020 at 17:59, Paolo Bonzini wrote: > > On 29/10/20 17:56, Arvind Sankar wrote: > >>> For those two just add: > >>> struct apic *apic = x86_system_apic; > >>> before all the assignments. > >>> Less churn and much

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Thomas Gleixner
On Thu, Oct 29 2020 at 17:59, Paolo Bonzini wrote: > On 29/10/20 17:56, Arvind Sankar wrote: >>> For those two just add: >>> struct apic *apic = x86_system_apic; >>> before all the assignments. >>> Less churn and much better code. >>> >> Why would it be better code? >> > > I think he means

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Arvind Sankar
On Thu, Oct 29, 2020 at 05:59:54PM +0100, Paolo Bonzini wrote: > On 29/10/20 17:56, Arvind Sankar wrote: > >> For those two just add: > >>struct apic *apic = x86_system_apic; > >> before all the assignments. > >> Less churn and much better code. > >> > > Why would it be better code? > > > >

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Paolo Bonzini
On 29/10/20 17:56, Arvind Sankar wrote: >> For those two just add: >> struct apic *apic = x86_system_apic; >> before all the assignments. >> Less churn and much better code. >> > Why would it be better code? > I think he means the compiler produces better code, because it won't read the

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Arvind Sankar
On Thu, Oct 29, 2020 at 03:05:31PM +, David Laight wrote: > From: Arnd Bergmann > > Sent: 28 October 2020 21:21 > > > > From: Arnd Bergmann > > > > There are hundreds of warnings in a W=2 build about a local > > variable shadowing the global 'apic' definition: > > > >

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Thomas Gleixner
Arnd, On Thu, Oct 29 2020 at 10:51, Arnd Bergmann wrote: > On Thu, Oct 29, 2020 at 8:04 AM Paolo Bonzini wrote: >> On 28/10/20 22:20, Arnd Bergmann wrote: >> > Avoid this by renaming the global 'apic' variable to the more descriptive >> > 'x86_system_apic'. It was originally called 'genapic',

RE: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread David Laight
From: Arnd Bergmann > Sent: 29 October 2020 09:51 ... > I think ideally there would be no global variable, withall accesses > encapsulated in function calls, possibly using static_call() optimizations > if any of them are performance critical. There isn't really a massive difference between

RE: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread David Laight
From: Arnd Bergmann > Sent: 28 October 2020 21:21 > > From: Arnd Bergmann > > There are hundreds of warnings in a W=2 build about a local > variable shadowing the global 'apic' definition: > > arch/x86/kvm/lapic.h:149:65: warning: declaration of 'apic' shadows a global > declaration

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Arnd Bergmann
On Thu, Oct 29, 2020 at 8:04 AM Paolo Bonzini wrote: > > On 28/10/20 22:20, Arnd Bergmann wrote: > > Avoid this by renaming the global 'apic' variable to the more descriptive > > 'x86_system_apic'. It was originally called 'genapic', but both that > > and the current 'apic' seem to be a little

Re: [PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-29 Thread Paolo Bonzini
On 28/10/20 22:20, Arnd Bergmann wrote: > Avoid this by renaming the global 'apic' variable to the more descriptive > 'x86_system_apic'. It was originally called 'genapic', but both that > and the current 'apic' seem to be a little overly generic for a global > variable. The 'apic' affects only

[PATCH] [v2] x86: apic: avoid -Wshadow warning in header

2020-10-28 Thread Arnd Bergmann
From: Arnd Bergmann There are hundreds of warnings in a W=2 build about a local variable shadowing the global 'apic' definition: arch/x86/kvm/lapic.h:149:65: warning: declaration of 'apic' shadows a global declaration [-Wshadow] Avoid this by renaming the global 'apic' variable to the more