Re: [edk2-devel] [edk2 PATCH] MdePkg: Use same ProcessorBind symbol define for RISCV64

["Michael D Kinney" <michael.d.kinney@intel.com>]

Using the same include guard define name is preferred.

Why was anything other than that considered?

Mike

> -----Original Message-----
> From: Xue, Gavin <gavin.xue@intel.com>
> Sent: Friday, June 30, 2023 2:29 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; Pedro Falcato <pedro.falcato@gmail.com>
> Cc: sunilvl@ventanamicro.com; Warkentin, Andrei
> <andrei.warkentin@intel.com>; Wang, Yimin <yimin.wang@intel.com>;
> Sheng, Alan <alan.sheng@intel.com>
> Subject: RE: [edk2-devel] [edk2 PATCH] MdePkg: Use same ProcessorBind
> symbol define for RISCV64
> 
> Hi Mike,
> 
> Thanks for your comments.
> I haven't seen specific error message when using the same include guard
> name:
> __PROCESSOR_BIND_H__ .
> 
> For short-term, I think RISC-V also could use same guard name with
> AArch64/Arm/Ebc/Ia32/X64
> CPU architecture, which also keep code alignment.
> How about your comment? Thanks.
> 
> Best regards,
> Gavin
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, June 28, 2023 4:29 AM
> To: devel@edk2.groups.io; Xue, Gavin <gavin.xue@intel.com>; Pedro
> Falcato <pedro.falcato@gmail.com>
> Cc: sunilvl@ventanamicro.com; Warkentin, Andrei
> <andrei.warkentin@intel.com>; Wang, Yimin <yimin.wang@intel.com>;
> Sheng, Alan <alan.sheng@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [edk2 PATCH] MdePkg: Use same ProcessorBind
> symbol define for RISCV64
> 
> It is better if we can use the same include guard names, but is not
> strictly required for builds to work.
> 
> What is the specific error message seen when using the same include
> guard
> names as other CPU types?
> 
> Include guards have 2 elements work discussing:
> * Use of define names that start with '_' or '__' are reserved either
> by the
>   ANSI C spec or for compilers.  The historical use by EDK II code to
> start
>   include guards with '_' could cause potential conflicts with some
> compilers
>   and may need to be addressed everywhere.
> 
> * Modern compilers support #pragma once that provides the same feature
> and
>   may actually have some build performance benefits.  This is a better
> long term
>   direction to remove the misuse of '_' and '__' and avoid potential
> collisions
>   with ANSI C or compilers.  It also reduces the number of defines in
> an EDK II
>   build.
> 
> 	https://en.wikipedia.org/wiki/Pragma_once
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Xue,
> Gavin
> > Sent: Thursday, June 22, 2023 2:59 AM
> > To: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: devel@edk2.groups.io; sunilvl@ventanamicro.com; Warkentin, Andrei
> > <andrei.warkentin@intel.com>; Wang, Yimin <yimin.wang@intel.com>;
> Sheng,
> > Alan <alan.sheng@intel.com>
> > Subject: Re: [edk2-devel] [edk2 PATCH] MdePkg: Use same ProcessorBind
> > symbol define for RISCV64
> >
> > Hi Pedro,
> >
> > Thanks for your feedback.
> >
> > The sample code what I listed in last mail is from/owned by another
> team,
> > and I didn't find other special #ifndef case for RSIC-V building so
> far.
> > RISC-V is an new processor architecture in edk2 implementation, in
> our
> > internal BIOS code, there are many similar common code for edk2 and
> Windows
> > app (for simulation).
> > It's better if we can reuse existing code (mostly are from x86) and
> > minimize modifications as much as possible. So I think use same guard
> name
> > is make sense.
> > How about your comments? Thanks.
> >
> > Best regards,
> > Gavin
> >
> > -----Original Message-----
> > From: Pedro Falcato <pedro.falcato@gmail.com>
> > Sent: Wednesday, June 21, 2023 10:16 PM
> > To: Xue, Gavin <gavin.xue@intel.com>
> > Cc: devel@edk2.groups.io; sunilvl@ventanamicro.com; Warkentin, Andrei
> > <andrei.warkentin@intel.com>; Wang, Yimin <yimin.wang@intel.com>;
> Sheng,
> > Alan <alan.sheng@intel.com>
> > Subject: Re: [edk2-devel] [edk2 PATCH] MdePkg: Use same ProcessorBind
> > symbol define for RISCV64
> >
> > On Fri, Jun 16, 2023 at 4:52 PM Xue, Gavin <gavin.xue@intel.com>
> wrote:
> > >
> > > Hi Sunil/Pedro,
> > >
> > > 1. As you know, ProcessorBind.h file of CPU Architecture file
> declares
> > sets of base types for edk2 code compiling.
> > > So data type in edk2 code doesn't rely on specific compiler (msvc,
> gcc
> > etc.), which is a good design.
> > >
> > > But in practice, for the purpose of reuse, some code can be built
> with
> > edk2, and also can be built to a standalone application (e.g. Win
> App).
> > > Just like below code piece:
> > > ===========
> > > #ifndef __WRAPPER_BASE_TYPES_H__
> > > #define __WRAPPER_BASE_TYPES_H__
> > >
> > > //
> > > // To avoid definition conflict during EDK2 build, it must include
> > > // ProcessorBind.h before xxx.h
> > > //
> > > #ifndef __PROCESSOR_BIND_H__
> > >
> > > #include <stdint.h>
> > > typedef uint8_t  UINT8;
> > > ==========
> > >
> > > In this case, if this is a edk2 build, the code will refer to data
> types
> > from ProcessorBind.h, otherwise, it will refer to stdint.h from
> compiler.
> > >
> > > 2. Regarding the guard name, it's same __PROCESSOR_BIND_H__ macro
> in
> > AArch64/Arm/Ebc/Ia32/X64, but it is PROCESSOR_BIND_H_
> > > in RiscV64 and LoongArh64. For above code, if we build BIOS for
> RISCV64,
> > it will try to include stdint.h due to different guard name.
> > >
> > > I am not sure if we can use same guard name to keep code alignment,
> or
> > give some comments. Thanks.
> >
> > Hi,
> > Hmm, interesting problem. Have you tried to #ifndef with some other
> > define? Like, I don't know, MAX_UINTN or EFIAPI?
> >
> > --
> > Pedro
> >
> >
> > 
> >