[llvm-branch-commits] [llvm] [IRBuilder] Add getByteTy and use it in CreatePtrAdd (PR #106539)

[Sergei Barannikov via llvm-branch-commits]

https://github.com/s-barannikov created https://github.com/llvm/llvm-project/pull/106539

The change requires DataLayout instance to be available, which, in turn,
requires insertion point to be set. In-tree tests detected only one case
when the function was called without setting an insertion point, it was
changed to create a constant expression directly.

>From 2a28c47e53009b0ab5506deae34176d56b95e8c2 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Thu, 22 Aug 2024 15:10:58 +0300
Subject: [PATCH] [IRBuilder] Add getByteTy and use it in CreatePtrAdd

The change requires DataLayout instance to be available, which, in turn,
requires insertion point to be set. In-tree tests detected only one case
when the function was called without setting an insertion point, it was
changed to create a constant expression directly.
---
 llvm/include/llvm/IR/IRBuilder.h              | 10 +++++++--
 .../Instrumentation/SanitizerCoverage.cpp     |  5 ++---
 llvm/unittests/IR/IRBuilderTest.cpp           | 22 +++++++++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 0dbcbc0b2cb76f..9b7566e8d7b587 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -504,6 +504,12 @@ class IRBuilderBase {
   // Type creation methods
   //===--------------------------------------------------------------------===//
 
+  /// Fetch the type representing a byte.
+  IntegerType *getByteTy() {
+    const DataLayout &DL = BB->getDataLayout();
+    return Type::getIntNTy(Context, DL.getByteWidth());
+  }
+
   /// Fetch the type representing a single bit
   IntegerType *getInt1Ty() {
     return Type::getInt1Ty(Context);
@@ -1995,12 +2001,12 @@ class IRBuilderBase {
 
   Value *CreatePtrAdd(Value *Ptr, Value *Offset, const Twine &Name = "",
                       GEPNoWrapFlags NW = GEPNoWrapFlags::none()) {
-    return CreateGEP(getInt8Ty(), Ptr, Offset, Name, NW);
+    return CreateGEP(getByteTy(), Ptr, Offset, Name, NW);
   }
 
   Value *CreateInBoundsPtrAdd(Value *Ptr, Value *Offset,
                               const Twine &Name = "") {
-    return CreateGEP(getInt8Ty(), Ptr, Offset, Name,
+    return CreateGEP(getByteTy(), Ptr, Offset, Name,
                      GEPNoWrapFlags::inBounds());
   }
 
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index 6a89cee9aaf6cc..e6094edae96b36 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -328,14 +328,13 @@ ModuleSanitizerCoverage::CreateSecStartEnd(Module &M, const char *Section,
       new GlobalVariable(M, Ty, false, Linkage, nullptr,
                          getSectionEnd(Section));
   SecEnd->setVisibility(GlobalValue::HiddenVisibility);
-  IRBuilder<> IRB(M.getContext());
   if (!TargetTriple.isOSBinFormatCOFF())
     return std::make_pair(SecStart, SecEnd);
 
   // Account for the fact that on windows-msvc __start_* symbols actually
   // point to a uint64_t before the start of the array.
-  auto GEP =
-      IRB.CreatePtrAdd(SecStart, ConstantInt::get(IntptrTy, sizeof(uint64_t)));
+  Constant *GEP = ConstantExpr::getGetElementPtr(
+      Int8Ty, SecStart, ConstantInt::get(IntptrTy, sizeof(uint64_t)));
   return std::make_pair(GEP, SecEnd);
 }
 
diff --git a/llvm/unittests/IR/IRBuilderTest.cpp b/llvm/unittests/IR/IRBuilderTest.cpp
index 9a4d0afbb2d1bc..7cca52a4c7fb47 100644
--- a/llvm/unittests/IR/IRBuilderTest.cpp
+++ b/llvm/unittests/IR/IRBuilderTest.cpp
@@ -516,6 +516,14 @@ TEST_F(IRBuilderTest, DataLayout) {
   EXPECT_FALSE(M->getDataLayout().isLegalInteger(32));
 }
 
+TEST_F(IRBuilderTest, GetByteTy) {
+  IRBuilder<> Builder(BB);
+
+  EXPECT_TRUE(Builder.getByteTy()->isIntegerTy(8));
+  M->setDataLayout("b:32");
+  EXPECT_TRUE(Builder.getByteTy()->isIntegerTy(32));
+}
+
 TEST_F(IRBuilderTest, GetIntTy) {
   IRBuilder<> Builder(BB);
   IntegerType *Ty1 = Builder.getInt1Ty();
@@ -527,6 +535,20 @@ TEST_F(IRBuilderTest, GetIntTy) {
   EXPECT_EQ(IntPtrTy, IntegerType::get(Ctx, IntPtrBitSize));
 }
 
+TEST_F(IRBuilderTest, CreatePtrAdd) {
+  IRBuilder<> Builder(BB);
+
+  M->setDataLayout("b:16-p:32:32");
+  Value *V = Builder.CreatePtrAdd(GV, ConstantInt::get(Ctx, APInt(32, 42)));
+  ASSERT_TRUE(isa<GEPOperator>(V));
+  EXPECT_TRUE(cast<GEPOperator>(V)->getResultElementType()->isIntegerTy(16));
+
+  M->setDataLayout("b:32-p:64:32");
+  V = Builder.CreateInBoundsPtrAdd(GV, ConstantInt::get(Ctx, APInt(64, 42)));
+  ASSERT_TRUE(isa<GEPOperator>(V));
+  EXPECT_TRUE(cast<GEPOperator>(V)->getResultElementType()->isIntegerTy(32));
+}
+
 TEST_F(IRBuilderTest, UnaryOperators) {
   IRBuilder<NoFolder> Builder(BB);
   Value *V = Builder.CreateLoad(GV->getValueType(), GV);