On Fri, Jan 10, 2020 at 10:50:55AM +0100, Greg Kurz wrote: > On Fri, 10 Jan 2020 10:34:07 +0100 > Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote: > > > On 1/9/20 6:43 PM, Greg Kurz wrote: > > > On Thu, 9 Jan 2020 16:21:22 +0100 > > > Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote: > > > > > >> We only access these variables in RTAS_SYSPARM_SPLPAR_CHARACTERISTICS > > >> case, restrict their scope to avoid unnecessary initialization. > > >> > > > > > > I guess a decent compiler can be smart enough detect that the initialization > > > isn't needed outside of the RTAS_SYSPARM_SPLPAR_CHARACTERISTICS branch... > > > Anyway, reducing scope isn't bad. The only hitch I could see is that some > > > people do prefer to have all variables declared upfront, but there's a nested > > > param_val variable already so I guess it's okay. > > > > I don't want to outsmart compilers :) > > > > The MACHINE() macro is not a simple cast, it does object introspection > > with OBJECT_CHECK(), thus is not free. Since > > Sure, I understand the motivation in avoiding an unneeded call > to calling object_dynamic_cast_assert(). > > > object_dynamic_cast_assert() argument is not const, I'm not sure the > > compiler can remove the call. > > > > Not remove the call, but delay it to the branch that uses it, > ie. parameter == RTAS_SYSPARM_SPLPAR_CHARACTERISTICS. I think any performance consideration here is a red herring. This particular RTAS call is a handful-of-times-per-boot thing, and only AFAIK used by AIX guests. I'm in favour of the change on the grounds of code locality and readability. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
