[llvm-commits] cxa_guard elimination

[Nick Lewycky]

Eli Friedman wrote:
> On Sat, Jan 7, 2012 at 6:48 PM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>> This patch implements some really basic elimination of calls to __cxa_guard.
>> More advanced cases can be added on request. Patch attached, please review!
>>
>> There's one thing I need to draw to a reviewer's attention. I make the
>> assumption that the code in the 'not-yet-initialized' path is indeed
>> initializing. That is to say that I assume that this pseudo-code:
>>
>>   *global = x;
>>   if (expr) {
>>     if (cxa_guard_acquire(global_guard)) {
>>       *global = 0;
>>       cxa_guard_release(global_guard);
>>     }
>>   }
>>   use(global);
>>
>> represents undefined behavior because global is initialized before the call
>> to __cxa_guard_acquire, and the guard is protecting a normal store, not the
>> first store.
>>
>> The Itanium C++ ABI never states that guard_acquire/release must be used for
>> *initializing*, so this patch arguably can miscompile. However, it's
>> generally understood that this is for initialization (indeed, the section
>> heading is "Once-time initialization API") so I'm hoping we can agree that
>> this assumption is valid and ask users doing other things with
>> cxa_guard_acquire/release to pass -fno-builtins.
>
> I think I agree with your assumption for the global associated with
> the guard.  You can't make that assumption for all globals, though.
> Consider something like the following:
>
> static int x;
> struct C { C() { x = 10; } };
> int f() { x = 20; static C dummy; return x; }
>
> As far as I can tell, with a bit of inlining etc. we end up with code
> which looks exactly like your bad pattern.

Clever! Thanks for the testcase.

That's unfortunate, but repairable. Given that we already check that all 
accesses come from within the same function, we'll need to show that no 
access is made to the global before the acquire is reached (and don't 
forget recursive calls).

Regrettably, it means that we'll probably need a domtree (or else do 
potentially-expensive CFG analysis ourselves). I'll see whether it still 
fits in -simplify-libcalls when I'm done.

Nick