[Mesa-dev] Mesa include guard style. (Was: [PATCH] i965/cfg: Remove redundant #pragma once.)
Ian Romanick
idr at freedesktop.org
Sun Mar 13 01:29:08 UTC 2016
On 03/11/2016 03:46 PM, Eric Anholt wrote:
> Ian Romanick <idr at freedesktop.org> writes:
>
>> On 03/10/2016 05:53 PM, Francisco Jerez wrote:
>>> Iago Toral <itoral at igalia.com> writes:
>>>
>>>> On Wed, 2016-03-09 at 19:04 -0800, Francisco Jerez wrote:
>>>>> Matt Turner <mattst88 at gmail.com> writes:
>>>>>
>>>>>> On Wed, Mar 9, 2016 at 1:37 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>>>>> Iago Toral <itoral at igalia.com> writes:
>>>>>>>
>>>>>>>> On Tue, 2016-03-08 at 17:42 -0800, Francisco Jerez wrote:
>>>>>>>>> brw_cfg.h already has include guards, remove the "#pragma once" which
>>>>>>>>> is redundant and non-standard.
>>>>>>>>
>>>>>>>> FWIW, I think using both #pragma once and include guards is a way to
>>>>>>>> keep portability while still getting the performance advantage of
>>>>>>>> #pragma once where it is supported.
>>>>>>>>
>>>>>>> It's highly unlikely to make any significant difference on any
>>>>>>> reasonably modern compiler. I cannot measure any change in compilation
>>>>>>> time locally from my cleanup.
>>>>>>>
>>>>>>>> Also it seems that we do the same thing in many other files...
>>>>>>>>
>>>>>>> Really? I'm not aware of any other file where we use both.
>>>>>>
>>>>>> There are quite a few in glsl/
>>>>>
>>>>> Heh, apparently you're right. Anyway it seems rather pointless to use
>>>>> '#pragma once' in a bunch of scattered header files with the expectation
>>>>> to gain some speed, the improvement from a single header file is so
>>>>> minuscule (if it will make any difference at all on a modern compiler
>>>>> and compilation workload, which I doubt) that we would have to use it
>>>>> universally in order to have the chance to measure any improvement.
>>>>>
>>>>> Can we please just decide for one of the include guard styles and use it
>>>>> consistently? Given that the majority of header files in the Mesa
>>>>> codebase use old-school define guards, that it's the only standard
>>>>> option, that it has well-defined semantics in presence of file copies
>>>>> and hardlinks, and that the performance argument against it is rather
>>>>> dubious (although I definitely find '#pragma once' prettier and more
>>>>> concise), I'd vote for using preprocessor define guards universally.
>>>>>
>>>>> What do other people think?
>>>>
>>>> I think we have to use define guards necessarily since #pragma once is
>>>> not standard even it it has wide support. So the question is whether we
>>>> want to use only define guards or define guards plus #pragma once. I am
>>>> fine with doing only define guards as you propose.
>>>
>>> *Shrug* I have the impression that the only real advantage of '#pragma
>>> once' is that you no longer need to do the ifndef/define dance, so I
>>> don't think I can see much benefit in doing both.
>>
>> Several compilers will cache the file name where '#pragma once' occurs
>> and never read that file again. A #include of a file previously seen
>> with '#pragma once' becomes a no-op. Since the file is never read, the
>> compiler avoids all the I/O and the parsing. That is true of MSVC and,
>> I thought, some versions of GCC. As Iago points out, some compilers
>> ignore the #pragma altogether. Since Mesa supports (or does it?) some
>> of these compilers, we have to have the ifdef/define/endif guards.
>
> Compilers have noticed that ifdef/define/endif is a thing and optimized
> it, anyway.
>
> https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
That's cool! I don't think GCC did that when I looked into this in
2010. It sounds like the #pragma actually breaks the GCC optimization,
so let's get rid of them all.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160312/3a8c3ebd/attachment.sig>
More information about the mesa-dev
mailing list