PR lto/51698 and the state of LTO + transactional memory

[Aldy Hernandez <aldyh at redhat dot com>]

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.

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.

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.

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?

Attachment: curr
Description: Text document