[llvm-commits] [llvm] r150174 - in /llvm/trunk: lib/Transforms/IPO/GlobalOpt.cpp test/Transforms/GlobalOpt/cxx-dtor.ll

[Benjamin Kramer]

On 09.02.2012, at 17:17, David Blaikie wrote:

> On Thu, Feb 9, 2012 at 6:26 AM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Thu Feb  9 08:26:06 2012
>> New Revision: 150174
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=150174&view=rev
>> Log:
>> GlobalOpt: Be more aggressive about elminating side-effect free static dtors.
>> 
>> GlobalOpt runs early in the pipeline (before inlining) and complex class
>> hierarchies often introduce bitcasts or GEPs which weren't optimized away.
>> Teach it to ignore side-effect free instructions instead of depending on
>> other passes to remove them.
>> 
>> Modified:
>>    llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
>>    llvm/trunk/test/Transforms/GlobalOpt/cxx-dtor.ll
>> 
>> Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=150174&r1=150173&r2=150174&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Thu Feb  9 08:26:06 2012
>> @@ -2753,7 +2753,8 @@
>>  /// destructor and can therefore be eliminated.
>>  /// Note that we assume that other optimization passes have already simplified
>>  /// the code so we only look for a function with a single basic block, where
>> -/// the only allowed instructions are 'ret' or 'call' to empty C++ dtor.
>> +/// the only allowed instructions side-effect free, 'ret' or 'call' to empty
>> +/// C++ dtor.
> 
> Is your revised comment missing an 'are' ("allowed instructions
>>> are<< side-effect free") though when I try to read that the
> "side-effect free" bit gets jumbled up with the following list, would
> it make more sense to say:
> 
> "the only allowed instructions are 'ret' or 'call to an empty C++
> dtor, both of which are side-effect free"

I tweaked it a bit in r150183, hopefully more readable now :)
> 
> ?
> 
>>  static bool cxxDtorIsEmpty(const Function &Fn,
>>                            SmallPtrSet<const Function *, 8> &CalledFunctions) {
>>   // FIXME: We could eliminate C++ destructors if they're readonly/readnone and
>> @@ -2786,9 +2787,9 @@
>>       if (!cxxDtorIsEmpty(*CalledFn, NewCalledFunctions))
>>         return false;
>>     } else if (isa<ReturnInst>(*I))
>> -      return true;
>> -    else
>> -      return false;
>> +      return true; // We're done.
>> +    else if (I->mayHaveSideEffects())
>> +      return false; // Destructor with side effects, bail.
> 
> a few else-after-returns (
> http://llvm.org/docs/CodingStandards.html#hl_else_after_return ) -
> though I realize you didn't write the original code, you were just
> improving it.

The elses are actually needed here, you can fall through the first if.

- Ben
> 
>>   }
>> 
>>   return false;
>> 
>> Modified: llvm/trunk/test/Transforms/GlobalOpt/cxx-dtor.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/cxx-dtor.ll?rev=150174&r1=150173&r2=150174&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/GlobalOpt/cxx-dtor.ll (original)
>> +++ llvm/trunk/test/Transforms/GlobalOpt/cxx-dtor.ll Thu Feb  9 08:26:06 2012
>> @@ -2,6 +2,7 @@
>> 
>>  %0 = type { i32, void ()* }
>>  %struct.A = type { i8 }
>> +%struct.B = type { }
>> 
>>  @a = global %struct.A zeroinitializer, align 1
>>  @__dso_handle = external global i8*
>> @@ -15,13 +16,14 @@
>>  }
>> 
>>  define linkonce_odr void @_ZN1AD1Ev(%struct.A* %this) nounwind align 2 {
>> -  call void @_ZN1AD2Ev(%struct.A* %this)
>> +  %t = bitcast %struct.A* %this to %struct.B*
>> +  call void @_ZN1BD1Ev(%struct.B* %t)
>>   ret void
>>  }
>> 
>>  declare i32 @__cxa_atexit(void (i8*)*, i8*, i8*)
>> 
>> -define linkonce_odr void @_ZN1AD2Ev(%struct.A* %this) nounwind align 2 {
>> +define linkonce_odr void @_ZN1BD1Ev(%struct.B* %this) nounwind align 2 {
>>   ret void
>>  }
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits