> On Dec 8, 2016, at 9:27 AM, Kurt Kennett <kurt.kenn...@microsoft.com> wrote:
> 
> This seems kind of silly.
> Why isn't this just const data?  This adds code and memory accesses that are 
> worthless and happen on every call to the function.
> 

K2,

I think this is an attempt to conform to the coding standard?

I do agree it is pointless to make it static if you are going to initialize it 
every time. Seems like it would have been better to make this a global 
variable. 

Given that a static local variable is very much like a global that is limited 
in scope it may make sense to relax the coding standard in this regard (I did 
not re-read the coding standard so hopefully this is not a bad assumption), or 
just require pre-initialized local static variables to be globals?


In this simple example you can see that a local static variable that is 
initialized ends up with code generation that is very close to a global 
variable. The name is not global and it is mangled with the function name. The 
other interesting tidbit is the compiler realized the static was const and 
marked it as such. 

~/work/Compiler>cat static.c
const unsigned char gMonthDays[] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 
30, 131 };

int test (int i)
{
  static unsigned char MonthDays[] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 
30, 31 };

  return MonthDays[i];
}
~/work/Compiler>clang -S -Os static.c
~/work/Compiler>cat static.S
        .section        __TEXT,__text,regular,pure_instructions
        .globl  _test
_test:                                  ## @test
        pushq   %rbp
        movq    %rsp, %rbp
        movslq  %edi, %rax
        leaq    _test.MonthDays(%rip), %rcx
        movzbl  (%rax,%rcx), %eax
        popq    %rbp
        retq

        .section        __TEXT,__const
        .globl  _gMonthDays             ## @gMonthDays
_gMonthDays:
        .ascii  "\037\034\037\036\037\036\037\037\036\037\036\203"

_test.MonthDays:                        ## @test.MonthDays
        .ascii  "\037\034\037\036\037\036\037\037\036\037\036\037"

Note: I had to make gMonthDays different than _test.MonthDays or 
_test.MonthDays gets optimized out. 

Thanks,

Andrew Fish


> K2
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan 
> Bi
> Sent: Thursday, December 8, 2016 2:54 AM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Subject: [edk2] [patch 2/8] FatPkg\EnhancedFatDxe: Initialize variable after 
> declaration
> 
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan...@intel.com>
> ---
> FatPkg/EnhancedFatDxe/Misc.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/FatPkg/EnhancedFatDxe/Misc.c b/FatPkg/EnhancedFatDxe/Misc.c 
> index f91759c..6ad688c 100644
> --- a/FatPkg/EnhancedFatDxe/Misc.c
> +++ b/FatPkg/EnhancedFatDxe/Misc.c
> @@ -696,15 +696,27 @@ Returns:
>   TRUE                  - The time is valid.
>   FALSE                 - The time is not valid.
> 
> --*/
> {
> -  static UINT8  MonthDays[] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 
> 31 };
> +  STATIC UINT8  MonthDays[12];
>   UINTN         Day;
>   BOOLEAN       ValidTime;
> 
>   ValidTime = TRUE;
> +  MonthDays[0] = 31;
> +  MonthDays[1] = 28;
> +  MonthDays[2] = 31;
> +  MonthDays[3] = 30;
> +  MonthDays[4] = 31;
> +  MonthDays[5] = 30;
> +  MonthDays[6] = 31;
> +  MonthDays[7] = 31;
> +  MonthDays[8] = 30;
> +  MonthDays[9] = 31;
> +  MonthDays[10] = 30;
> +  MonthDays[11] = 31;
> 
>   //
>   // Check the fields for range problems
>   // Fat can only support from 1980
>   //
> --
> 1.9.5.msysgit.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to