On Fri, Jun 03, 2016 at 09:07:09AM -0600, Martin Sebor wrote:
> I'm not sure I understand your concern but at this point I would
> prefer to keep things as they are.  I like that the functionality

My concern is that if you declare that NULL is valid third argument for e.g.
__builtin_add_overflow, then unless we add some complicated wording into the
docs, we have to penalize the code we emit for e.g.
bool
foo (int a, int b, int *p)
{
  return __builtin_add_overflow (a, b, p);
}
While we could previously emit
        addl    %edi, %esi
        movl    %esi, (%rdx)
        seto    %al
        retq
you suddenly have to emit (and your patch doesn't do that):
        addl    %edi, %esi
        seto    %al
        testq   %rdx, %rdx
        je      .L1
        movl    %esi, (%rdx)
.L1:
        retq
because you can't at compile time prove if p is NULL (then you wouldn't
store the result) or not.

Trying to document that the third argument may be NULL, but only if it is
constant NULL pointer expression or something like that (what exactly?)
might not be very easily understandable and clear to users.

Which is why I've suggested just not allowing (like before) the third
argument to be NULL, and just add new 3 builtins for the test for overflow,
but don't store it anywhere.  They would just be folded early to the same
internal function.  And when adding the 3 new builtins, we can also choose
a different calling convention that would allow the C++98/C constant
expressions, by not having the third argument a pointer (we don't need to
dereference anything), but e.g. just any integer where we ignore the value
(well, evaluate for side-effects) and only care about the type.

        Jakub

Reply via email to