[PATCH] D74700: [IR] Remove temporary const operator created in Value::getPointerAlignment()

[Mikhail Lychkov via Phabricator via llvm-commits]

mlychkov created this revision.
mlychkov added reviewers: lebedev.ri, jdoerfert, nikic.
mlychkov created this object with edit policy "Administrators".
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

New temporary constant operator may be created in getPointerAlignment() function.
This instance is not used by any instruction. However later it may lead to errors during
code processing as not expected.
Remove this constant after alignment calculation as no more required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74700

Files:
  llvm/lib/IR/Value.cpp
  llvm/unittests/IR/ConstantsTest.cpp


Index: llvm/unittests/IR/ConstantsTest.cpp
===================================================================
--- llvm/unittests/IR/ConstantsTest.cpp
+++ llvm/unittests/IR/ConstantsTest.cpp
@@ -638,5 +638,33 @@
   EXPECT_FALSE(CP00U->isElementWiseEqual(CP00U0));
 }
 
+TEST(ConstantsTest, GlobalValueImmutabilityOnGetPtrAlignmentTest) {
+  LLVMContext Context;
+  SMDiagnostic Error;
+  std::unique_ptr<Module> M = parseAssemblyString(
+      "@gs = common global [8 x i32] zeroinitializer, align 8 "
+      "define void @f() { "
+      "entry: "
+      "  %0 = load i32, i32* getelementptr inbounds "
+      "         ([8 x i32], [8 x i32]* @gs, i64 0, i64 0), align 8 "
+      "  ret void "
+      "}",
+      Error, Context);
+
+  const Function *F = M->getFunction("f");
+  const BasicBlock &BB = F->getEntryBlock();
+  const Instruction &I = BB.front();
+  const Value *GEP = I.getOperand(0);
+  EXPECT_TRUE(isa<Constant>(GEP));
+
+  GlobalVariable *GV = M->getGlobalVariable("gs");
+  unsigned GVUsesNumBefore = GV->getNumUses();
+
+  MaybeAlign Align = GEP->getPointerAlignment(DataLayout(""));
+  (void)Align;
+
+  ASSERT_EQ(GVUsesNumBefore, GV->getNumUses());
+}
+
 }  // end anonymous namespace
 }  // end namespace llvm
Index: llvm/lib/IR/Value.cpp
===================================================================
--- llvm/lib/IR/Value.cpp
+++ llvm/lib/IR/Value.cpp
@@ -734,16 +734,23 @@
       return MaybeAlign(CI->getLimitedValue());
     }
   } else if (auto *CstPtr = dyn_cast<Constant>(this)) {
-    if (auto *CstInt = dyn_cast_or_null<ConstantInt>(ConstantExpr::getPtrToInt(
-            const_cast<Constant *>(CstPtr), DL.getIntPtrType(getType()),
-            /*OnlyIfReduced=*/true))) {
+    MaybeAlign Alignment(llvm::None);
+    // NOTE: getPtrToInt call may create extra constant operator
+    Constant *CstPTI = ConstantExpr::getPtrToInt(const_cast<Constant *>(CstPtr),
+                                                 DL.getIntPtrType(getType()),
+                                                 /*OnlyIfReduced=*/true);
+    if (auto *CstInt = dyn_cast_or_null<ConstantInt>(CstPTI)) {
       size_t TrailingZeros = CstInt->getValue().countTrailingZeros();
       // While the actual alignment may be large, elsewhere we have
-      // an arbitrary upper alignmet limit, so let's clamp to it.
-      return Align(TrailingZeros < Value::MaxAlignmentExponent
-                       ? uint64_t(1) << TrailingZeros
-                       : Value::MaximumAlignment);
+      // an arbitrary upper alignment limit, so let's clamp to it.
+      Alignment = Align(TrailingZeros < Value::MaxAlignmentExponent
+                            ? uint64_t(1) << TrailingZeros
+                            : Value::MaximumAlignment);
     }
+    // Remove unused ptrtoint operator if it has been created
+    if (CstPTI && !CstPTI->isConstantUsed())
+      CstPTI->destroyConstant();
+    return Alignment;
   }
   return llvm::None;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74700.244922.patch
Type: text/x-patch
Size: 2953 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200217/a29076af/attachment.bin>