This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Small cleanups to cse.c
- From: Roger Sayle <roger at eyesopen dot com>
- To: Steven Bosscher <stevenb dot gcc at gmail dot com>
- Cc: Eric Botcazou <ebotcazou at libertysurf dot fr>, <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 25 Nov 2006 12:32:31 -0700 (MST)
- Subject: Re: [PATCH] Small cleanups to cse.c
On Sat, 25 Nov 2006, Steven Bosscher wrote:
> On 11/25/06, Eric Botcazou <email@example.com> wrote:
> > > Actually, that looks like a mistake. I should have typed
> > >
> > > if (!any_condjump_p (insn))
> > > gcc_unreachable ();
> > >
> > > there, without the ENABLE_CHECKING.
> > Not the correct idiom either. :-)
> Well, not incorrect, just different. You'll find the form I suggested
> in a lot of places in GCC as well.
> > You probably simply want:
> > gcc_assert (any_condjump_p (insn));
> This would also work, but in this form we wouldn't check that insn has
> the valid form if gcc is configured with assert checking disabled.
> I prefer to err on the safe side and check this even with assert
> checking disabled ;-) But I don't feel strongly about it and I can
> go with your suggestion if that's what most other folks prefer.
I'm with Eric on this one and would prefer the more usual:
gcc_assert (any_condjump_p (insn));
This is precisely the intended use of gcc_assert, and anyone turning
off assertion checking presumably explicitly intends to disable this
form of internal consistency checking.
I did do a quick audit of the gcc/ directory to see if I could find
any instances of "if (foo) gcc_unreachable();" Although not exhaustive,
the vast majority of uses of gcc_unreachable are in switch statements
for handling invalid cases, or at the very end of long
I did find one instance in builtins.c:expand_builtin_int_roundingfn
(though as I said I wasn't exhaustive) that contained:
if (!validate_arglist (arglist, REAL_TYPE, VOID_TYPE))
which I think really should be just gcc_assert (or to match the
rest of builtins.c, it should just return NULL_RTX!). I'm sure
there may be legitimate counter examples, (perhaps involving
functions with side-effects) but I support Eric's view that the
"rule of thumb" is to always use gcc_assert.
Thanks in advance for making that change. This clean-up looks
good, BTW. It looks like this was the last remaining use of
USE_C_ALLOCA in the tree, no remaining backends define it, so
we should also remove the documentation from hostconfig.texi.