If we agree to use the same include guard define name for all CPU architecture, I re-sent out the v2 new patch. Please help to review it. Thanks.
Best regards, Gavin -----Original Message----- From: Kinney, Michael D <[email protected]> Sent: Saturday, July 1, 2023 12:59 AM To: Xue, Gavin <[email protected]>; [email protected]; Pedro Falcato <[email protected]> Cc: [email protected]; Warkentin, Andrei <[email protected]>; Wang, Yimin <[email protected]>; Sheng, Alan <[email protected]>; Kinney, Michael D <[email protected]> Subject: RE: [edk2-devel] [edk2 PATCH] MdePkg: Use same ProcessorBind symbol define for RISCV64 Using the same include guard define name is preferred. Why was anything other than that considered? Mike > -----Original Message----- > From: Xue, Gavin <[email protected]> > Sent: Friday, June 30, 2023 2:29 AM > To: Kinney, Michael D <[email protected]>; > [email protected]; Pedro Falcato <[email protected]> > Cc: [email protected]; Warkentin, Andrei > <[email protected]>; Wang, Yimin <[email protected]>; > Sheng, Alan <[email protected]> > 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 <[email protected]> > Sent: Wednesday, June 28, 2023 4:29 AM > To: [email protected]; Xue, Gavin <[email protected]>; Pedro > Falcato <[email protected]> > Cc: [email protected]; Warkentin, Andrei > <[email protected]>; Wang, Yimin <[email protected]>; > Sheng, Alan <[email protected]>; Kinney, Michael D > <[email protected]> > 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: [email protected] <[email protected]> On Behalf Of Xue, > Gavin > > Sent: Thursday, June 22, 2023 2:59 AM > > To: Pedro Falcato <[email protected]> > > Cc: [email protected]; [email protected]; Warkentin, Andrei > > <[email protected]>; Wang, Yimin <[email protected]>; > Sheng, > > Alan <[email protected]> > > 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 <[email protected]> > > Sent: Wednesday, June 21, 2023 10:16 PM > > To: Xue, Gavin <[email protected]> > > Cc: [email protected]; [email protected]; Warkentin, Andrei > > <[email protected]>; Wang, Yimin <[email protected]>; > Sheng, > > Alan <[email protected]> > > 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 <[email protected]> > 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 > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106617): https://edk2.groups.io/g/devel/message/106617 Mute This Topic: https://groups.io/mt/99567569/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
