diff mbox

PR51280 LTO/trans-mem ICE with TM builtins

Message ID 4F16F249.9010602@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Jan. 18, 2012, 4:24 p.m. UTC 
On 01/18/12 03:09, Richard Guenther wrote:
> On Tue, 17 Jan 2012, Aldy Hernandez wrote:
>
>>
>>>> What I have in mind is to abstract out the initialization of TM builtins
>>>> from
>>>> gtm-builtins.def (through its inclusion in builtins.def) into a separate
>>>> function that we can call either in c_define_builtins() from the C-ish
>>>> front-ends, or from lto_define_builtins (once we encounter a TM builtin
>>>> and
>>>> enable flag_tm appropriately).
>>>
>>> Hm?  They are included in builtins.def, that looks appropriate for
>>> middle-end builtins.  Thus they should be initialized by lto1, too.
>>> Are they not?
>>
>> Since flag_tm is NULL, when lto_define_builtins runs, the TM builtins do not
>> get initialized.  This is because DEF_TM_BUILTIN is predicated on flag_tm.
>
> Ok, so arrange that -fgnu-tm is enabled at link-time as soon as it was
> enabled in one TU.  You can do that alongside handling of OPT_fPIC, etc.
> in lto-wrapper.c:merge_and_complain.

The following patch fixes the problem.

OK?

>> It isn't until streamer_get_builtin_tree() that we realize the object file has
>> a TM builtin.
>
> I see.  Probably similar issues exist with -fopenmp and the GOMP
> builtins.

It is my understanding that openmp is lowered before LTO streaming, so 
this isn't a problem for it.  I manually tested something similar to the 
testcase in the patch below (but for openmp) and verified that openmp 
does not suffer from this problem.

Thanks for the suggestion.
PR lto/51280
	* lto-wrapper.c (run_gcc): Pass -fgnu_tm on.
	(merge_and_complain): Same.

Comments

Richard Biener Jan. 19, 2012, 9:13 a.m. UTC | #1 
On Wed, 18 Jan 2012, Aldy Hernandez wrote:

> On 01/18/12 03:09, Richard Guenther wrote:
> > On Tue, 17 Jan 2012, Aldy Hernandez wrote:
> > 
> > > 
> > > > > What I have in mind is to abstract out the initialization of TM
> > > > > builtins
> > > > > from
> > > > > gtm-builtins.def (through its inclusion in builtins.def) into a
> > > > > separate
> > > > > function that we can call either in c_define_builtins() from the C-ish
> > > > > front-ends, or from lto_define_builtins (once we encounter a TM
> > > > > builtin
> > > > > and
> > > > > enable flag_tm appropriately).
> > > > 
> > > > Hm?  They are included in builtins.def, that looks appropriate for
> > > > middle-end builtins.  Thus they should be initialized by lto1, too.
> > > > Are they not?
> > > 
> > > Since flag_tm is NULL, when lto_define_builtins runs, the TM builtins do
> > > not
> > > get initialized.  This is because DEF_TM_BUILTIN is predicated on flag_tm.
> > 
> > Ok, so arrange that -fgnu-tm is enabled at link-time as soon as it was
> > enabled in one TU.  You can do that alongside handling of OPT_fPIC, etc.
> > in lto-wrapper.c:merge_and_complain.
> 
> The following patch fixes the problem.
> 
> OK?

Ok.

Thanks,
Richard.
diff mbox

Patch

Index: testsuite/gcc.dg/lto/trans-mem-3_0.c
===================================================================
--- testsuite/gcc.dg/lto/trans-mem-3_0.c	(revision 0)
+++ testsuite/gcc.dg/lto/trans-mem-3_0.c	(revision 0)
@@ -0,0 +1,8 @@ 
+/* { dg-lto-options {{-flto}} } */
+/* { dg-lto-do link } */
+
+/* Test that we can build one object file with -fgnu-tm
+   (trans-mem-3_1.c), but do the final link of all objects without
+   -fgnu-tm.  */
+
+int i;
Index: testsuite/gcc.dg/lto/trans-mem-3_1.c
===================================================================
--- testsuite/gcc.dg/lto/trans-mem-3_1.c	(revision 0)
+++ testsuite/gcc.dg/lto/trans-mem-3_1.c	(revision 0)
@@ -0,0 +1,18 @@ 
+/* { dg-options "-fgnu-tm" } */
+
+extern int i;
+
+main()
+{
+  __transaction_atomic { i = 0; }
+}
+
+#define dummy(func)							\
+  __attribute__((noinline,noclone,used)) void func() { asm (""); }
+
+dummy(_ITM_beginTransaction)
+dummy(_ITM_commitTransaction)
+dummy(_ITM_WU4)
+dummy(_ITM_WU8)
+dummy(_ITM_registerTMCloneTable)
+dummy(_ITM_deregisterTMCloneTable)
Index: lto-wrapper.c
===================================================================
--- lto-wrapper.c	(revision 183244)
+++ lto-wrapper.c	(working copy)
@@ -403,6 +403,7 @@  merge_and_complain (struct cl_decoded_op
 	case OPT_fpie:
 	case OPT_fcommon:
 	case OPT_fexceptions:
+	case OPT_fgnu_tm:
 	  /* Do what the old LTO code did - collect exactly one option
 	     setting per OPT code, we pick the first we encounter.
 	     ???  This doesn't make too much sense, but when it doesn't
@@ -555,6 +556,7 @@  run_gcc (unsigned argc, char *argv[])
 	case OPT_fpie:
 	case OPT_fcommon:
 	case OPT_fexceptions:
+	case OPT_fgnu_tm:
 	  break;
 
 	default: