On Wed, Jan 25, 2012 at 12:16 AM, Aldy Hernandez <al...@redhat.com> wrote:
> The problem here is that -flto cannot equate the instrumentation functions
> being generated with a user supplied version of the library functions. This
> would happen if the user tried to link a transactional program with libitm
> with -flto (as in -fwhole-program, etc).
>
> This is an easy problem to fix, just by changing DEF_TM_BUILTIN to set
> BOTH_P so the __builtin_ITM_* variants and _ITM_* are considered the same.
>
> In doing so, I noticed that there are a myriad of _ITM_* builtins that have
> incorrect prototypes as defined in the builtins*.def files. So using
> libitm/libitm.h fails miserably because of mismatches. The attached patch
> fixes all problems, and the PR.
>
> However...
>
> Looking at the generated code, I've noticed that LTO isn't inlining the TM
> builtins as one would expect. This is a two-fold problem:
>
> First, LTO streaming happens before the final TM lowering (tmmark pass), so
> there is nothing to inline. The instrumented function calls (_ITM_RU4, et
> al), aren't generated until after LTO.
Which means they are not available for inlining without LTO either ...
(unless you mark them to be always inlined, in which case the inline
transform, which happens first at local after-IPA passes, probably
still inlines it - but that should work at LTRANS stage as well).
> Second, it seems that by design, LTO prefers builtins to user-provided
> versions of them. In particular, lto_symtab_prevailing_decl() stipulates
> that builtins are their own prevailing decl. So even if we lowered TM
> before LTO streaming, user provided builtins wouldn't be preferred (and thus
> inlined) as we would expect into application code.
Hmm, so you say you have sth like
void *memcpy(void *dst, void *src, size_t n) { ...implementation... }
void foo()
{
memcpy (...);
}
and expect it to be inlined from the supplied body instead of using the
builtin expander?
I think we could make this work ... at least under a sort-of ODR, that
all bodies (from different TUs) and the builtin have the same behavior.
Mind to file an enhancement bug? Does it work without LTO?
> Though I don't think fixing LTO of instrumented TM code is 4.7 material, I
> thought I'd write this up for the record.
>
> With regards to the first problem, the reason we lower TM so late (in
> contrast to openmp) is because we were hoping to get good optimization of
> the memory operations before fudging them all up. So at some point we
> should decide what's the biggest gain, deeper optimization of the memory
> operations or the ability to inline the instrumentation. Or perhaps,
> whether we can simply move the LTO streaming point under certain conditions
> and get both.
I think that lowering at ipa_tm time should be good enough for scalar
code optimizations, but it will surely fuck up loop optimizations galore
(you might want to try).
Richard.
> With regards to the second problem, I suppose I should tackle #1 first, but
> I would be curious if the LTO experts could weigh in here.
>
> The attached patch fixes the ICE in the PR, though it won't do what the user
> ultimately wants to do, given the limitations described. Perhaps we could
> create another PR and tag it with an enhancement request.
>
> OK pending tests?
>
> Aldy
>
> p.s. Oh, and btw, does this all make sense?
