Re: [PATCH 7/9] ENABLE_CHECKING refactoring: middle-end, LTO FE

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Oct 6, 2015 at 2:46 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 10/06/2015 01:39 AM, Mikhail Maltsev wrote:
>>
>>   void verify_insn_chain (void);
>> +static inline void checking_verify_insn_chain ();
>>   static void fixup_fallthru_exit_predecessor (void);
>>   static int can_delete_note_p (const rtx_note *);
>>   static int can_delete_label_p (const rtx_code_label *);
>
> [...]
>>
>> @@ -3990,6 +3987,16 @@ verify_insn_chain (void)
>>
>>     gcc_assert (insn_cnt1 == insn_cnt2);
>>   }
>> +
>> +/* Perform checks, if they were requested by corresponding flag.  */
>> +
>> +static inline void
>> +checking_verify_insn_chain ()
>> +{
>> +  if (flag_checking)
>> +    verify_insn_chain ();
>> +}
>> +
>
>
> There are many new such small inline functions, and I don't think they buy
> us much over just writing out the pattern where they are called. Also, just
> defined the function before its first use rather than writing a forward
> declaration.
>
>> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
>> index 3b8d594..7771514 100644
>> --- a/gcc/tree-ssa-alias.c
>> +++ b/gcc/tree-ssa-alias.c
>> @@ -1443,11 +1443,8 @@ refs_may_alias_p_1 (ao_ref *ref1, ao_ref *ref2,
>> bool tbaa_p)
>>                                       tbaa_p);
>>
>>    /* We really do not want to end up here, but returning true is safe.
>> */
>> -#ifdef ENABLE_CHECKING
>> -  gcc_unreachable ();
>> -#else
>> +  gcc_checking_assert (false);
>>    return true;
>> -#endif
>>  }
>
>
> I think the consensus has been to avoid assert (false), so this would be
>
> if (checking)
>   gcc_unreachable ();
>
> but maybe we really just want to do the gcc_unreachable unconditionally.

I'm fine with unconditional gcc_unreachable - the code is old enough now
that we shouldn't hit this anymore.

Richard.

>
> Bernd