[GlobalOpt] Fix the load types when OptimizeGlobalAddressOfMalloc

[scui]

Summary

Currently, in OptimizeGlobalAddressOfMalloc, the transformation for global loads assumes that they have the same Type. With the support of ConstantExpr (https://reviews.llvm.org/D106589), this may not be true any more (as seen in the test case), and we miss the code to handle this, This is to fix that.

Hopefully, this is the problem that @saugustine reported in https://reviews.llvm.org/D106589. The test case I added here has the similar msg you have. Please verify. Thanks.

[efriedma]

llvm/lib/Transforms/IPO/GlobalOpt.cpp

1001

Why do you need to mess with the way you're iterating over the uses of LI? Calling ConstantExpr::getBitCast() doesn't mess with the use-list of LI.

1004

Is RepValue dead with this change?

[saugustine]

I need to test a little more extensively, but this does appear to fix the issue I had reported, which is good news. Thanks.

[scui]

llvm/lib/Transforms/IPO/GlobalOpt.cpp

1001

I guess I have a preference on for loop. I'll change it back.

1004

Will add code to make use of it like the original code.

[scui]

To address review comments

[efriedma]

LGTM

llvm/lib/Transforms/IPO/GlobalOpt.cpp

if (BitCastInst *BCI = dyn_cast<BitCastInst>(User)) {
       }
     } else {
       if (!TheBC)
         TheBC = new BitCastInst(NewGV, CI->getType(), "newgv", CI);
       User->replaceUsesOfWith(CI, TheBC);
     }
   }
 
-  Constant *RepValue = NewGV;
-  if (NewGV->getType() != GV->getValueType())
-    RepValue = ConstantExpr::getBitCast(RepValue, GV->getValueType());
+  SmallPtrSet<Constant *, 1> RepValues;
+  RepValues.insert(NewGV);
 
   // If there is a comparison against null, we will insert a global bool to
   // keep track of whether the global was initialized yet or not.
   GlobalVariable *InitBool =
     new GlobalVariable(Type::getInt1Ty(GV->getContext()), false,
                        GlobalValue::InternalLinkage,
                        ConstantInt::getFalse(GV->getContext()),
                        GV->getName()+".init", GV->getThreadLocalMode());
if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
                     SI->getSyncScopeID(), SI);
       SI->eraseFromParent();
       continue;
     }
 
     LoadInst *LI = cast<LoadInst>(U);
     while (!LI->use_empty()) {
       Use &LoadUse = *LI->use_begin();
       ICmpInst *ICI = dyn_cast<ICmpInst>(LoadUse.getUser());
       if (!ICI) {
-        LoadUse = RepValue;
+        auto *CE = ConstantExpr::getBitCast(NewGV, LI->getType());
+        RepValues.insert(CE);
+        LoadUse.set(CE);
         continue;
       }
 
       // Replace the cmp X, 0 with a use of the bool value.
       Value *LV = new LoadInst(InitBool->getValueType(), InitBool,
                                InitBool->getName() + ".val", false, Align(1),
                                LI->getOrdering(), LI->getSyncScopeID(), LI);
       InitBoolUsed = true;
OptimizeGlobalAddressOfMalloc(GlobalVariable *GV, CallInst *CI, Type *AllocTy,
 
   // Now the GV is dead, nuke it and the malloc..
   GV->eraseFromParent();
   CI->eraseFromParent();
 
   // To further other optimizations, loop over all users of NewGV and try to
   // constant prop them.  This will promote GEP instructions with constant
   // indices into GEP constant-exprs, which will allow global-opt to hack on it.
-  ConstantPropUsersOf(NewGV, DL, TLI);
-  if (RepValue != NewGV)
-    ConstantPropUsersOf(RepValue, DL, TLI);
+  for (auto *CE : RepValues)
+    ConstantPropUsersOf(CE, DL, TLI);
 
   return NewGV;
 }
 
 /// Scan the use-list of GV checking to make sure that there are no complex uses
 /// of GV.  We permit simple things like dereferencing the pointer, but not
 /// storing through the address, unless it is to the specified global.
 static bool

llvm/test/Transforms/GlobalOpt/2021-08-03-StoreOnceLoadMultiCasts.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -globalopt -S < %s | FileCheck %s
+; RUN: opt -passes=globalopt -S < %s | FileCheck %s
+
+@g = internal global i32* null, align 8
+
+define signext i32 @f() local_unnamed_addr {
+; CHECK-LABEL: @f(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    call void @f1()
+; CHECK-NEXT:    store i32 1, i32* @g.body, align 4
+; CHECK-NEXT:    call void @f1()
+; CHECK-NEXT:    store i8 2, i8* bitcast (i32* @g.body to i8*), align 4
+; CHECK-NEXT:    ret i32 1
+;
+entry:
+  %call = call i8* @malloc(i64 4)
+  %b = bitcast i8* %call to i32*
+  store i32* %b, i32** @g, align 8
+  call void @f1()
+  %0 = load i32*, i32** @g, align 8
+  store i32 1, i32* %0, align 4
+  call void @f1()
+  %1 = load i8*, i8** bitcast (i32** @g to i8**), align 8
+  store i8 2, i8* %1, align 4
+  ret i32 1
+}
+
+define signext i32 @main() {
+; CHECK-LABEL: @main(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CALL:%.*]] = call signext i32 @f()
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, i32* @g.body, align 4
+; CHECK-NEXT:    ret i32 [[TMP0]]
+;
+entry:
+  %call = call signext i32 @f()
+  %0 = load i32*, i32** @g, align 8
+  %1 = load i32, i32* %0, align 4
+  ret i32 %1
+}
+
+declare noalias align 16 i8* @malloc(i64)
+declare void @f1()