bug-gnu-emacs
[Top][All Lists]
Advanced

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

bug#36370: 27.0.50; XFIXNAT called on negative numbers


From: Pip Cet
Subject: bug#36370: 27.0.50; XFIXNAT called on negative numbers
Date: Thu, 27 Jun 2019 19:56:17 +0000

On Thu, Jun 27, 2019 at 7:38 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
> >> if it were up to me we'd get rid of XFIXNAT entirely,
> >> and just use XFIXNUM. But old habits die hard....
> >
> > I actually think that would be best, so we're only disagreeing about
> > what the second-best solution is :-)
>
> Removing XFIXNAT would be outside the scope of this patch. However, if
> we're already fixing the code for some other reason and if the XFIXNATs
> are confusing that code, we might as well replace them with XFIXNUMs.
> The attached patch does that.

Cool!

> > My idea is to define eassume as follows:
> >
> > #define eassume(cond) (__builtin_constant_p (!(cond) == !(cond)) ?
> > ((cond) ? 0 : __builtin_unreachable ()) : 0)
>
> That would generate worse code in some cases, since after (say) "eassume
> (i >= 0); return i/2;" where i is a variable, GCC would not be able to
> optimize i/2 into i>>1 because GCC would not know that i is nonnegative.

I'm confused, and I'm not sure what you're saying.

int f (int i)
{
  eassume (i >= 0);
  printf("%d\n", i & 0x80000000);
  return 0;
}

The eassume tells GCC i is nonnegative, since (!(i >= 0) == !(i >= 0))
is indeed a constant. So the code generated is:

        xorl    %esi, %esi
        movl    $.LC0, %edi
        xorl    %eax, %eax
        call    printf

which is as it should be. For your example, it is:

        movl    %edi, %eax
        sarl    %eax
        ret

with the eassume and

        movl    %edi, %eax
        shrl    $31, %eax
        addl    %edi, %eax
        sarl    %eax
        ret

without.

> The main point of eassume (as opposed to eassert) is to enable
> optimizations like that.

Yes, indeed. I don't see how they are disabled by my proposed definition.

> Thanks for the review. In addition to the already-mentioned patches, I
> installed the attached and am closing the bug report.

Thanks!





reply via email to

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