On Mon, Sep 14, 2015 at 8:41 PM, Jeff Law <l...@redhat.com> wrote: > On 09/14/2015 04:11 AM, Richard Biener wrote: >> >> On Wed, Sep 9, 2015 at 11:07 PM, Jeff Law <l...@redhat.com> wrote: >>> >>> On 08/31/2015 05:30 AM, Richard Biener wrote: >>>> >>>> >>>> On Mon, Aug 31, 2015 at 7:49 AM, Mikhail Maltsev <malts...@gmail.com> >>>> wrote: >>>>> >>>>> >>>>> Hi, all! >>>>> >>>>> This patch removes some conditional compilation from GCC. In this patch >>>>> I >>>>> define >>>>> a macro CHECKING_P, which is equal to 1 when ENABLE_CHECKING is defined >>>>> and 0 >>>>> otherwise. The reason for using a new name is the following: currently >>>>> in >>>>> GCC >>>>> there are many places where ENABLE_CHECKING is checked using #ifdef, >>>>> and >>>>> it is a >>>>> common way to do such checks (though #if would also work and is used in >>>>> several >>>>> places). If we change it in such way that it is always defined, >>>>> accidentally >>>>> using "#ifdef" instead of "#if" will lead to subtle errors: some >>>>> expensive >>>>> checks intended only for development stage will be enabled in release >>>>> build and >>>>> cause performance degradation. >>>>> >>>>> This patch removes all uses of ENABLE_CHECKING (I also tried poisoning >>>>> this >>>>> identifier in system.h, and the build succeeded, but I don't know how >>>>> will this >>>>> affect, e.g. merging feature branches, so I think such decisions should >>>>> be made >>>>> by maintainers). >>>> >>>> >>>> >>>> I think we want to keep ENABLE_CHECKING for macro use and for some >>>> selected >>>> cases. >>> >>> >>> Can you outline which cases you want to keep? My general feeling is to >>> avoid conditionally compiled code as much as we can. >> >> >> I guess I was merely looking for the patch to be split up to see the >> motivation >> of a always-defined CHECKING_P macro. > > Ah. I suspect the motivation is that was the easiest way forward and > roughly what was initially suggested. > > > With the suggestion to have >> >> a runtime flag_checking variable I wonder if there is any real code that >> is guarded by ENABLE_CHECKING now that can't use flag_checking > > I wouldn't expect to see any in the .c files. There'll likely be > stragglers (like tree/rtl checking). But even flushing all the stuff out of > the .c files is a major win. > > Do you have a preference on whether or not to make flag_checking a pre, post > or integrated patch? I can see arguments for any of those three choices.
I'd like to see it as part of the patch, even if the initial implementation would be a static const int flag_checking = CHECKING_P; in flags.h so we can avoid chaging all uses twice. It requires some more careful review of what uses are changed to flag_checking and which one to CHECKING_P of course which means it would make sense to do it as a pre patch handling the obvious cases first. And then adding -fchecking "properly" wouldn't be too hard anyway. Thanks, Richard. > jeff >
