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

[David Blaikie]

On Thu, Feb 9, 2012 at 8:33 AM, Benjamin Kramer
<benny.kra at googlemail.com> wrote:
>
> 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 :)

Thanks (:

>>
>> ?
>>
>>>  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.

Ah, cunning. Sorry for the noise.

>
> - 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
>