[PATCH] Small cleanups to cse.c
Roger Sayle
roger@eyesopen.com
Sat Nov 25 21:00:00 GMT 2006
Hi Steven,
On Sat, 25 Nov 2006, Steven Bosscher wrote:
> On 11/25/06, Eric Botcazou <ebotcazou@libertysurf.fr> 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
if-then-else-if-then-else chains.
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))
gcc_unreachable ();
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.
Roger
--
More information about the Gcc-patches
mailing list