[PATCH] D43457: [Evaluator] Improve evaluation of load/store

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 created this revision.
evgeny777 added reviewers: tejohnson, espindola, pcc.
Herald added a subscriber: mehdi_amini.
evgeny777 retitled this revision from "[Evaluator] Improve evaluation of bitcast" to "[Evaluator] Improve evaluation of load/store".

When storing pointers in struct/class constructor `clang` often generates IR like this:

  define internal void @_GLOBAL__sub_I_main.cpp()  {
    %1 = load i64, i64* bitcast (%struct.A** @gA to i64*), align 8
    store i64 %1, i64* bitcast (%struct.S* @s to i64*), align 8
    ret void
  }

which corresponds to following C++ code:

  extern A *gA;
  struct S {
    A* _a;
  } SObj(gA);

With https://reviews.llvm.org/D43077 it is now possible to import variables from different TU when using ThinLTO,
so it would be nice to optimize such static ctors out.


https://reviews.llvm.org/D43457

Files:
  lib/Transforms/Utils/Evaluator.cpp
  test/Transforms/GlobalOpt/evaluate-bitcast.ll


Index: test/Transforms/GlobalOpt/evaluate-bitcast.ll
===================================================================
--- test/Transforms/GlobalOpt/evaluate-bitcast.ll
+++ test/Transforms/GlobalOpt/evaluate-bitcast.ll
@@ -0,0 +1,28 @@
+; RUN: opt -globalopt -instcombine %s -S -o - | FileCheck %s
+
+; Static constructor should have been optimized out
+; CHECK:       i32 @main
+; CHECK-NEXT:     ret i32 69905
+; CHECK-NOT:   _GLOBAL__sub_I_main.cpp
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+%struct.S = type { %struct.A* }
+%struct.A = type { i64, i64 }
+
+ at s = internal local_unnamed_addr global %struct.S zeroinitializer, align 8
+ at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_main.cpp, i8* null }]
+ at gA = available_externally dso_local local_unnamed_addr global %struct.A* inttoptr (i64 69905 to %struct.A*), align 8
+
+define dso_local i32 @main() local_unnamed_addr {
+  %1 = load i64, i64* bitcast (%struct.S* @s to i64*), align 8
+  %2 = trunc i64 %1 to i32
+  ret i32 %2
+}
+
+define internal void @_GLOBAL__sub_I_main.cpp() section ".text.startup" {
+  %1 = load i64, i64* bitcast (%struct.A** @gA to i64*), align 8
+  store i64 %1, i64* bitcast (%struct.S* @s to i64*), align 8
+  ret void
+}
Index: lib/Transforms/Utils/Evaluator.cpp
===================================================================
--- lib/Transforms/Utils/Evaluator.cpp
+++ lib/Transforms/Utils/Evaluator.cpp
@@ -190,13 +190,24 @@
   }
 
   // Handle a constantexpr getelementptr.
-  if (ConstantExpr *CE = dyn_cast<ConstantExpr>(P))
+  if (ConstantExpr *CE = dyn_cast<ConstantExpr>(P)) {
     if (CE->getOpcode() == Instruction::GetElementPtr &&
         isa<GlobalVariable>(CE->getOperand(0))) {
       GlobalVariable *GV = cast<GlobalVariable>(CE->getOperand(0));
       if (GV->hasDefinitiveInitializer())
         return ConstantFoldLoadThroughGEPConstantExpr(GV->getInitializer(), CE);
+    } else if (CE->getOpcode() == Instruction::BitCast) {
+      GlobalVariable *GV = dyn_cast<GlobalVariable>(CE->getOperand(0));
+      if (GV && GV->hasDefinitiveInitializer()) {
+        // We should always load from a pointer
+        assert(P->getType()->isPointerTy());
+        auto *ResTy = P->getType()->getContainedType(0);
+        auto *I = GV->getInitializer();
+        if (I->getType()->isPointerTy() && ResTy->isIntegerTy())
+          return ConstantExpr::getPtrToInt(I, ResTy);
+      }
     }
+  }
 
   return nullptr;  // don't know how to evaluate.
 }
@@ -249,10 +260,15 @@
 
           Type *NewTy = cast<PointerType>(Ptr->getType())->getElementType();
 
+          auto TypeConversionSafe = [](Type *VTy, Type *Ty) {
+            return (VTy->isIntegerTy() && Ty->isPointerTy()) ||
+                   VTy->canLosslesslyBitCastTo(Ty);
+          };
           // In order to push the bitcast onto the stored value, a bitcast
           // from NewTy to Val's type must be legal.  If it's not, we can try
           // introspecting NewTy to find a legal conversion.
-          while (!Val->getType()->canLosslesslyBitCastTo(NewTy)) {
+          auto *VTy = Val->getType();
+          while (!TypeConversionSafe(VTy, NewTy)) {
             // If NewTy is a struct, we can convert the pointer to the struct
             // into a pointer to its first member.
             // FIXME: This could be extended to support arrays as well.
@@ -277,8 +293,9 @@
           }
 
           // If we found compatible types, go ahead and push the bitcast
-          // onto the stored value.
-          Val = ConstantExpr::getBitCast(Val, NewTy);
+          // onto the stored value or convert it from integer to pointer.
+          Val = VTy->isIntegerTy() ? ConstantExpr::getIntToPtr(Val, NewTy)
+                                   : ConstantExpr::getBitCast(Val, NewTy);
 
           DEBUG(dbgs() << "Evaluated bitcast: " << *Val << "\n");
         }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43457.134896.patch
Type: text/x-patch
Size: 3988 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180219/3e4c8243/attachment.bin>