This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: assertify cris


> Date: Sat, 30 Apr 2005 17:01:07 +0100
> From: Nathan Sidwell <nathan@codesourcery.com>

> I looked for a cris-sim.exp file in dejagnu/baseboards and couldn't
> see one.  I see there is a sim/cris directory as you say. I don't
> understand why I didn't get a cris-sim baseboard in my recent dejagnu
> checkout. I shall investigate.

The DejaGNU and sim (part of gdb but should be freed) projects
are not synched.  The master dejagnu repo is at
savannah.gnu.org, *NOT* sourceware.org, if that's what you refer
to by "checkout".  (People argue that the sourceware copy should
be deleted as it's confusing and out of date in many important
details.)

> > I'm not too keen on you changing the word "abort" to "die"
> > everywhere in comments.  It's highly spurious.  What's next, the
> > word "gets" because the function gets() is deprecated?
> maybe I'm being over keen.  What those comments really mean

Some of them, but definitely not all of them.

> is
> 'we'll violate an input constraint on OTHERROUTINE'.  'abort' is
> a specific mechanism for failure, one that won't necessarily occur.

I *want* it to occur; I *definitely do not* want silent failure
and incorrect code being generated, ever.  Besides, "abort" is a
generic term, which doesn't always refer to calling the function
abort.  You remove all occurrences of abort regardless of context.

> The other reason for changing is it makes my life easier grepping :)

Why not grep for the 'abort[ 	]*(' (your syntax may vary)?

> <haha-only-serious> 'gets' is not currently a problem in gcc, in
> that we don't have code using it :)

The point is missed, but I don't which know by how far, see above
<haha-only-serious>.

> > Don't these gcc_unreachable and gcc_assert get #defined out with
> > --disable-checking?  That would cause maintenance problems.
> 
> Correct, they are disabled, that's the whole point of
> --disable-checking.  If you want assert checking, then you should
> say --enable-checking=assert (or some more inclusive flag).  Notice
> that --enable-checking=release does NOT disable asserts.  There is
> a not insignificant cost with using asserts.

That cost of checking is insignificant in the backend (to
oppose, we'd need figures; I'll just point out that assembly
output, magnitudes more costly, is usually nearby this code).
The cost of silent miscompilation compared to ICE is by far
higher than the cost of checking in these parts.  I accept that
cost of checking in the backend on behalf of the CRIS port.

> The current --disable-checking behaviour is to remove the asserts,
> but leave the unreachables in place.

That matches my recollection and fear.  (I had hoped this had
changed; I'm a couple of weeks behind on mailing list reading.)

> > I'll take this on a test-run with cross to cris-axis-linux-gnu
> > and check it in myself when I find out the answer.
> 
> Many thanks!

...except I don't like the answer, so I'll keep the aborts.
Sorry, but thanks for your work.  Would it help your assertion
work if I'll change all abort calls in the CRIS port into
internal_error calls?

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]