On Wed, Apr 12, 2017 at 11:16 PM, Jeff Law <l...@redhat.com> wrote: > On 04/07/2017 08:02 AM, Xi Ruoyao wrote: >> >> On 2017-04-06 11:12 -0600, Jeff Law wrote: >> >>> With the likely deprecation in mind, I've only done a cursory review of >>> the changes -- mostly to verify that they hit Cilk+ paths only. >> >> >>> What's the purpose behind changing when we set the in_lto_p flag? >> >> >> Without that change, GCC with my patch ICEed with _Cilk_spawn and >> -flto -O3 -fcilkplus since __cilkrts_stack_frame.ctx's type (array of void >> *) >> was not TYPE_STRUCTURAL_EQUALITY_P in lto stage. >> If this change is not proper, I'll work on modifying my patch to work >> without touching in_lto_p. > > It's certainly be preferable to not change in_lto_p-- unless Richi wants to > chime in on the safety of setting in_lto_p earlier. > > I'm not familiar enough with the LTO interactions to know if movement of > in_lto_p is safe.
It should be safe and even technically more correct given we now have various conditionals in the type building routines in tree.c that check in_lto_p and avoid setting TYPE_CANONICAL from them -- TYPE_CANONICAL is re-computed later, and for the builtin types we first zero TYPE_CANONICAL (see lto.c:read_cgraph_and_symbols). Now... I think those checks are somewhat wrong given that the middle-end _does_ create types later, hopefully not ones we use for alias purposes, but I'm not 100% sure. So it somewhat feels like a hack ;) So in theory the change is a good one. I'm still nervous at this stage. Did you verify LTO bootstrap still works with the patch? CCing Honza who fiddled with this last (and introduced all those in_lto_p checks). Thanks, Richard. > Jeff