On Tue, 17 Jan 2012, Aldy Hernandez wrote: > On 12/20/11 03:43, Richard Guenther wrote: > > On Mon, 19 Dec 2011, Patrick Marlier wrote: > > > > > On 12/16/2011 03:54 AM, Richard Guenther wrote: > > > > On Thu, 15 Dec 2011, Patrick Marlier wrote: > > > > > > > > > In PR51280, LTO does ICE because the object file uses TM builtin but > > > > > the > > > > > TM is > > > > > not enabled. > > > > > In the patch, it displays a error message if the builtin is not > > > > > defined > > > > > and > > > > > due to TM. > > > > > I moved is_tm_builtin() from calls.c to trans-mem.c. I splitted it > > > > > into 2 > > > > > functions is_tm_builtin/is_tm_builtin_code. In is_tm_builtin_code, I > > > > > added > > > > > some missing builtins (TM_START, TM_GETTMCLONE_SAFE, TM_MALLOC, > > > > > TM_CALLOC, > > > > > TM_FREE). Finally, I declared them into tree.h to be usable in calls.c > > > > > and > > > > > tree-streamer-in.c. > > > > > > > > > > Bootstrapped and LTO/TM regtested on Linux/i686. > > > > > (If ok, please commit it. Thanks.) > > > > > > > > No - why should this matter? All of TM should be lowered to a point > > > > where only target specific code should be needed. > > > > > > > > Richard. > > > > > > Thanks Richard. > > > > > > In lto file, there is GIMPLE_TRANSACTION statement and a builtin call > > > (__builtin_ITM_commitTransaction) to delimit the end of the transaction > > > region. The transaction is not yet instrumented. So all of TM are not > > > lowered. > > > > > > I guess this could be also added even if we should always break at the > > > missing > > > _ITM_commitTransaction builtin declaration. > > > > > > Index: gimple-streamer-in.c > > > =================================================================== > > > --- gimple-streamer-in.c (revision 182487) > > > +++ gimple-streamer-in.c (working copy) > > > @@ -234,6 +234,9 @@ input_gimple_stmt (struct lto_input_block *ib, str > > > break; > > > > > > case GIMPLE_TRANSACTION: > > > + if (!flag_tm) > > > + error_at (gimple_location (stmt), > > > + "use of transactional memory without support enabled"); > > > gimple_transaction_set_label (stmt, stream_read_tree (ib, > > > data_in)); > > > break; > > > > > > > > > It seems a bit out of my scope of my GCC knowledge so I guess I will let > > > GCC > > > guys solve this in a proper way. > > > > I'd say we should stream the new IL elements and enable TM at link-time > > once we encounter such IL element (similar to how we enable exceptions > > when one TU contains EH regions). > > Ok, I was finally able to reproduce with the new testcase Patrick provided. > I'm back on the saddle on this one. > > Richi, I can certainly do something similar here, but let me see if we're on > the same wavelength before I commit many keystrokes. > > 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? > We may also have to add a hook to initialize target dependent TM builtins (for > example, ix86_init_tm_builtins on x86). There is one, targetm.builtin_decl - the targets should simply initialize them on-demand (which is for what that target-hook is designed for). > Is this acceptable or did you have something else in mind? Not sure I fully understood the issue with the existing gtm-builtins.def setup. Richard.
