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

?

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

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