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: [RFA][PATCH] Isolate erroneous paths optimization


On Wed, Nov 6, 2013 at 4:08 PM, Ian Lance Taylor <iant@google.com> wrote:
> On Wed, Nov 6, 2013 at 6:29 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Wed, Nov 6, 2013 at 3:24 PM, Ian Lance Taylor <iant@google.com> wrote:
>>> On Tue, Nov 5, 2013 at 10:50 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>> On Tue, 5 Nov 2013, Ian Lance Taylor wrote:
>>>>
>>>>> This patch actually breaks the Go testsuite.  In Go dereferencing a
>>>>> nil pointer is well-defined: it causes panic that can be caught.  This
>>>>> breaks a test for that functionality by changing the panic to a
>>>>> builtin_trap.
>>>>>
>>>>> That's not a big deal; I'll just disable this optimization in the Go
>>>>> frontend.
>>>>
>>>>
>>>> Shouldn't go use -fno-delete-null-pointer-checks by default then? That
>>>> should disable this optimization and others that rely on the same idea.
>>>
>>> No, that is a different optimization with different properties.  The
>>> -fdelete-null-pointer-checks optimization assumes that if you write
>>>     x = *p;
>>>     if (p == NULL) { printf ("NULL\n"); }
>>> that the test p == NULL can not succeed.  In Go, that is true.  If p
>>> is NULL the *p will cause a panic and ordinary code execution will not
>>> proceed.
>>>
>>> The recent -fisolate-erroneous-paths optimization will change code
>>> like this:
>>>     if (p == NULL) { printf ("NULL\n"); }
>>>     x = *p;
>>> into code like this:
>>>     if (p == NULL) { printf ("NULL\n"); __builtin_trap (); }
>>>     x = *p;
>>> That is, it will insert a trap rather than dereferencing the pointer
>>> known to be NULL.  This doesn't work for Go because we really do want
>>> the panic, not the __builtin_trap.  This optimization would work fine
>>> for Go if there were a way to explicitly call the panic function
>>> rather than calling trap, but that would be a frontend-dependent
>>> aspect to a middle-end optimization.
>>
>> But then you have -fnon-call-exceptions enabled?
>
> Yes (go_langhook_init_options_struct in go/go-lang.c).
>
>> Where obviously
>> -fisolate-erroneous-paths also shouldn't apply?
>
> I guess that's not entirely obvious to me.  I'm not sure that
> -fnon-call-exceptions means that all trapping instructions must be
> executed.

No, I don't think it means that.  But I think that you cannot transform

foo ()
{
  *0 = 1;
}

to __builtin_trap as you can catch the trap via an exception handler
in a caller of foo, no?  The question is whether we may change
one kind of "trap" for another (which we'd do in this case).  Also
__builtin_trap () may expand to a abort () call IIRC if the target
doesn't have a suitable instruction that traps.

Richard.

> Ian


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