[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Preview: portable dumper

From: Daniel Colascione
Subject: Re: Preview: portable dumper
Date: Tue, 20 Feb 2018 10:14:18 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 02/20/2018 10:09 AM, Robert Pluim wrote:
Daniel Colascione <address@hidden> writes:

On 02/20/2018 09:28 AM, Paul Eggert wrote:
On 02/20/2018 09:19 AM, Daniel Colascione wrote:
Do you get an assertion failure if you enable assertion checking?
If we're seeing this kind of problem from the compiler expectation
alone, I'll definitely revert that change.

In my experience, eassume (and 'assume') shouldn't be used for
complicated expressions, as GCC is too-easily confused by
them. eassume should be used only for expressions where the
assumption really does help the compiler, either by generating
significantly-better code or by pacifying a false alarm.

I'd hoped things had gotten better, and some local testing seemed to
suggest it was safe to use these days. Apparently it's not, which is a
shame. It'd be nice, but not urgent, to track down which specific
assumptions caused what I think must be miscompilations so we could
report them upstream.

The guilty change is:

--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -607,7 +607,7 @@ dump_set_have_current_referrer (struct dump_context *ctx, 
bool have)
  static void
  DUMP_CLEAR_REFERRER (struct dump_context *ctx)
-  eassert (ctx->have_current_referrer);
+  eassume (ctx->have_current_referrer);
    dump_set_have_current_referrer (ctx, false);
    if (dump_tracking_referrers_p (ctx))
      ctx->current_referrer = Qnil;

If I revert just that hunk, the build succeeds.

Wow: thanks for tracking that down.

What on earth? I mean, we're instructing the compiler to assume something that's both trivial and true. What GCC version is this? If it's recent, I think it's probably worth reporting.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]