[llvm-commits] patch: CXAGuardElimination pass.

Nick Lewycky nicholas at mxc.ca
Mon May 25 15:33:34 PDT 2009


Duncan Sands wrote:
> Hi Nick,
> 
>> +    CallInst *A = dyn_cast<CallInst>(I);
>> +    if (!A) continue;
>> +
>> +    // TODO: __cxa_guard_acquire returns a value, then makes a conditional
>> +    // branch off of it. We need to handle this.
> 
> the acquire function may be being passed as a parameter to some other
> function rather than being called.

Right. Well, that should never happen in a real program so if it ever 
does then how about we abort the transform across the whole module.

>> +      if (I->mayReadFromMemory() || I->mayWriteToMemory())
> 
> Maybe you should use mayHaveSideEffects rather than mayWriteToMemory,
> since a "const" function can still throw an exception.  On the other
> hand, since such a function doesn't read or write memory it shouldn't
> need to be wrapped in guards anyway.

Good idea. Done.

>> +BasicBlock *CXAGuardElimination::getUniqueSuccessor(TerminatorInst *TI,
>> +                                                    CallInst *Acquire) {
>> +  if (TI->getNumSuccessors() == 1)
>> +    return TI->getSuccessor(0);
> 
> What about this:
> 
> bb1: acquire
>       br bbr
> bb2: acquire
>       br bbr
> bbr: release
> 
> Aren't you going to eliminate the first acquire and the release, but
> not the second acquire?

No, it'll eliminate the second acquire and the release, but not the 
first acquire. Note that there's code to detect this case in 
deadGuardElim (see the comments).

As far as I can tell that's valid behaviour for this transform so long 
as we don't mind removing infinite loops (note that these infinite loops 
can't be deliberately constructed by the user code, the calls to 
__cxa_guard are always emitted by the compiler...).

>> +  addPass(PM, createCXAGuardEliminationPass());  // Remove __cxa_guard_* calls.
> 
> Are you going to add this to llvm-gcc too?

Yes.

>> +  This pass deletes dead calls to __cxa_guard_acquire and __cxa_guard_release
>> +  which are part of the Itanium C++ ABI. T hese are used to prevent a function
> 
> T hese -> These

That line is gone.

Updated patch attached. Thanks for the review!

Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cxaguardelim-2.patch
Type: text/x-diff
Size: 13074 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090525/f7760236/attachment.patch>


More information about the llvm-commits mailing list