Patchwork PR middle-end/51212: sorry out on -fgnu-tm + -fnon-call-exceptions

login
register
mail settings
Submitter Aldy Hernandez
Date Dec. 22, 2011, 7:47 p.m.
Message ID <4EF3894B.5050504@redhat.com>
Download mbox | patch
Permalink /patch/132894/
State New
Headers show

Comments

Aldy Hernandez - Dec. 22, 2011, 7:47 p.m.
The problem here is that with -fnon-call-exceptions, a memory 
dereference may trap, but when we instrument the store, we have lost the 
landing pad information.

One solution would be to move the EH information to the TM load/store 
instrumentation builtins, but that doesn't get us around the fact that 
libitm is not exception safe, and we have no mechanism for taking and 
exception (and propagating it) in the middle of a transaction.

Richard has suggested that another alternative could be to support the 
exception case for NULL, but non-null faulting memory references would 
still cause a crash.  In this case we would simply test for NULL at the 
start of the accessors and explicitly throw the exception.

And yet a third alternative, is to disable the -fgnu-tm and 
-fnon-call-exceptions combination.  I have implemented this one, as I'd 
rather have it not work, than work half-way.

Torvald, do you have any thoughts on the matter?

Attached patch for disabling the feature, if you both agree on this 
approach.
PR middle-end/51212
	* opts.c (finish_options): Call sorry on -fgnu-tm and
	-fnon-call-exception combination.
Richard Henderson - Dec. 22, 2011, 8:12 p.m.
On 12/22/2011 11:47 AM, Aldy Hernandez wrote:
> +    sorry ("non call exceptions and transactional memory are not supported");

Better worded as 

  transactional memory is not supported with non-call exceptions

Otherwise ok.


r~
Richard Guenther - Dec. 23, 2011, 9:31 a.m.
On Thu, Dec 22, 2011 at 8:47 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> The problem here is that with -fnon-call-exceptions, a memory dereference
> may trap, but when we instrument the store, we have lost the landing pad
> information.
>
> One solution would be to move the EH information to the TM load/store
> instrumentation builtins, but that doesn't get us around the fact that
> libitm is not exception safe, and we have no mechanism for taking and
> exception (and propagating it) in the middle of a transaction.
>
> Richard has suggested that another alternative could be to support the
> exception case for NULL, but non-null faulting memory references would still
> cause a crash.  In this case we would simply test for NULL at the start of
> the accessors and explicitly throw the exception.
>
> And yet a third alternative, is to disable the -fgnu-tm and
> -fnon-call-exceptions combination.  I have implemented this one, as I'd
> rather have it not work, than work half-way.
>
> Torvald, do you have any thoughts on the matter?
>
> Attached patch for disabling the feature, if you both agree on this
> approach.

I think this should be documented at the place -fgnu-tm is documented.

Richard.
Aldy Hernandez - Jan. 3, 2012, 2:10 p.m.
On 12/23/11 03:31, Richard Guenther wrote:
> On Thu, Dec 22, 2011 at 8:47 PM, Aldy Hernandez<aldyh@redhat.com>  wrote:
>> The problem here is that with -fnon-call-exceptions, a memory dereference
>> may trap, but when we instrument the store, we have lost the landing pad
>> information.
>>
>> One solution would be to move the EH information to the TM load/store
>> instrumentation builtins, but that doesn't get us around the fact that
>> libitm is not exception safe, and we have no mechanism for taking and
>> exception (and propagating it) in the middle of a transaction.
>>
>> Richard has suggested that another alternative could be to support the
>> exception case for NULL, but non-null faulting memory references would still
>> cause a crash.  In this case we would simply test for NULL at the start of
>> the accessors and explicitly throw the exception.
>>
>> And yet a third alternative, is to disable the -fgnu-tm and
>> -fnon-call-exceptions combination.  I have implemented this one, as I'd
>> rather have it not work, than work half-way.
>>
>> Torvald, do you have any thoughts on the matter?
>>
>> Attached patch for disabling the feature, if you both agree on this
>> approach.
>
> I think this should be documented at the place -fgnu-tm is documented.
>
> Richard.

I can certainly do this.  I am however, waiting for the final approval. 
  It wasn't clear whether that was an approval from Richard Henderson, 
or whether I should wait for an official ok.

OK for mainline?
Richard Henderson - Jan. 3, 2012, 8:33 p.m.
On 01/04/2012 01:10 AM, Aldy Hernandez wrote:
> I can certainly do this.  I am however, waiting for the final approval.  It wasn't clear whether that was an approval from Richard Henderson, or whether I should wait for an official ok.
> 
> OK for mainline?

Yes, it was approval.


r~

Patch

Index: testsuite/g++.dg/tm/pr51212.C
===================================================================
--- testsuite/g++.dg/tm/pr51212.C	(revision 0)
+++ testsuite/g++.dg/tm/pr51212.C	(revision 0)
@@ -0,0 +1,19 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm -fnon-call-exceptions" }
+
+struct S
+{
+  S ()
+  {
+  }
+};
+
+__attribute__ ((transaction_callable))
+void foo (int *p)
+{
+  S s;
+  if (*p)
+    ;
+}
+
+// { dg-message "sorry, unimplemented: non call exceptions and transactional memory" "-fnon-call-exceptions and -fgnu-tm together" { target *-*-* } 0 }
Index: opts.c
===================================================================
--- opts.c	(revision 182542)
+++ opts.c	(working copy)
@@ -663,6 +663,9 @@  finish_options (struct gcc_options *opts
       opts->x_flag_toplevel_reorder = 0;
     }
 
+  if (opts->x_flag_tm && opts->x_flag_non_call_exceptions)
+    sorry ("non call exceptions and transactional memory are not supported");
+
   /* -Wmissing-noreturn is alias for -Wsuggest-attribute=noreturn.  */
   if (opts->x_warn_missing_noreturn)
     opts->x_warn_suggest_attribute_noreturn = true;