> On Thu, May 4, 2017 at 11:04 AM, Jan Hubicka <hubi...@ucw.cz> wrote: > >> > > >> >Sure, I'm not questioning the patch, just wondering if we shouldn't > >> >improve > >> >store-merging further (we want to do it anyway for e.g. bitop adjacent > >> >operations etc.). > >> > >> We definitely want to do that. It should also 'nicely' merge with bswap > >> for gathering the load side of a piecewise memory to memory copy. > > > > The code we produce now in .optimized is: > > <bb 8> [15.35%]: > > # DEBUG this => _42 > > MEM[(struct inline_summary *)_42].estimated_self_stack_size = 0; > > MEM[(struct inline_summary *)_42].self_size = 0; > > _44 = &MEM[(struct inline_summary *)_42].self_time; > > # DEBUG this => _44 > > # DEBUG sig => 0 > > # DEBUG exp => 0 > > MEM[(struct sreal *)_42 + 16B].m_sig = 0; > > MEM[(struct sreal *)_42 + 16B].m_exp = 0; > > sreal::normalize (_44); > > # DEBUG this => NULL > > # DEBUG sig => NULL > > # DEBUG exp => NULL > > MEM[(struct inline_summary *)_42].min_size = 0; > > MEM[(struct inline_summary *)_42].inlinable = 0; > > MEM[(struct inline_summary *)_42].contains_cilk_spawn = 0; > > MEM[(struct inline_summary *)_42].single_caller = 0; > > MEM[(struct inline_summary *)_42].fp_expressions = 0; > > MEM[(struct inline_summary *)_42].estimated_stack_size = 0; > > MEM[(struct inline_summary *)_42].stack_frame_offset = 0; > > It should handle at least the bitfields here (inlinable, contains_cilk_spawn, > single_caller and fp_expression). Ah, I guess it doesn't because the > padding isn't initialized and it doesn't want to touch it. Well, just a > guess. > > inline_summary is also very badly laid out now with lots of padding.
Yep, I am aware it does not make sense now after reorganization. I plan to break it into inliner independent part and inliner hints and then I will fix layout ;) Honza > > > _45 = &MEM[(struct inline_summary *)_42].time; > > # DEBUG this => _45 > > # DEBUG sig => 0 > > # DEBUG exp => 0 > > MEM[(struct sreal *)_42 + 56B].m_sig = 0; > > MEM[(struct sreal *)_42 + 56B].m_exp = 0; > > sreal::normalize (_45); > > # DEBUG this => NULL > > # DEBUG sig => NULL > > # DEBUG exp => NULL > > MEM[(struct inline_summary *)_42].size = 0; > > MEM[(void *)_42 + 80B] = 0; > > MEM[(void *)_42 + 88B] = 0; > > MEM[(void *)_42 + 96B] = 0; > > MEM[(struct predicate * *)_42 + 104B] = 0; > > MEM[(void *)_42 + 112B] = 0; > > MEM[(void *)_42 + 120B] = 0; > > goto <bb 10>; [100.00%] > > > > so indeed it is not quite pretty at all. Even the bitfields are split > > and there is offlined call to normlize zero in sreal. > > I wonder why inliner does not see htis as obviously good inlining > > oppurtunity. > > I will look into that problem. For store merging it would indeed be > > very nice to improve this as well. > > > > Honza > >> > >> Richard. > >> > >> > > >> > Jakub