[SimplifyLibCalls] Add initial support for non-8-bit bytes

The patch makes CharWidth argument of `getStringLength` mandatory
and ensures the correct values are passed in most cases.
This is *not* a complete support for unusual byte widths in
SimplifyLibCalls since `getConstantStringInfo` returns false for those.
The code guarded by `getConstantStringInfo` returning true is unchanged
because the changes are currently not testable.

Author: s-barannikov

llvm/include/llvm/Analysis/ValueTracking.h

@@ -394,7 +394,7 @@ struct ConstantDataArraySlice {
 /// with an appropriate offset.
 LLVM_ABI bool getConstantDataArrayInfo(const Value *V,
                                        ConstantDataArraySlice &Slice,
-                                       unsigned ElementSize,
+                                       unsigned ElementBitWidth,
                                        uint64_t Offset = 0);
 
 /// This function computes the length of a null-terminated C string pointed to
@@ -408,7 +408,7 @@ LLVM_ABI bool getConstantStringInfo(const Value *V, StringRef &Str,
 
 /// If we can compute the length of the string pointed to by the specified
 /// pointer, return 'len+1'.  If we can't, return 0.
-LLVM_ABI uint64_t GetStringLength(const Value *V, unsigned CharSize = 8);
+LLVM_ABI uint64_t getStringLength(const Value *V, unsigned CharWidth);
 
 /// This function returns call pointer argument that is considered the same by
 /// aliasing rules. You CAN'T use it to replace one value with another. If

llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h

@@ -260,8 +260,8 @@ class LibCallSimplifier {
   bool hasFloatVersion(const Module *M, StringRef FuncName);
 
   /// Shared code to optimize strlen+wcslen and strnlen+wcsnlen.
-  Value *optimizeStringLength(CallInst *CI, IRBuilderBase &B, unsigned CharSize,
-                              Value *Bound = nullptr);
+  Value *optimizeStringLength(CallInst *CI, IRBuilderBase &B,
+                              unsigned CharWidth, Value *Bound = nullptr);
 };
 } // End llvm namespace
 

llvm/lib/Analysis/MemoryBuiltins.cpp

@@ -379,7 +379,8 @@ llvm::getAllocSize(const CallBase *CB, const TargetLibraryInfo *TLI,
 
   // Handle strdup-like functions separately.
   if (FnData->AllocTy == StrDupLike) {
-    APInt Size(IntTyBits, GetStringLength(Mapper(CB->getArgOperand(0))));
+    APInt Size(IntTyBits, getStringLength(Mapper(CB->getArgOperand(0)),
+                                          DL.getByteWidth()));
     if (!Size)
       return std::nullopt;
 

llvm/lib/Analysis/ValueTracking.cpp

@@ -6357,17 +6357,14 @@ bool llvm::isGEPBasedOnPointerToString(const GEPOperator *GEP,
 }
 
 // If V refers to an initialized global constant, set Slice either to
-// its initializer if the size of its elements equals ElementSize, or,
-// for ElementSize == 8, to its representation as an array of unsiged
-// char. Return true on success.
-// Offset is in the unit "nr of ElementSize sized elements".
+// its initializer if the bit width of its elements equals ElementBitWidth,
+// or, for ElementBitWidth == CHAR_BIT, to its representation as an array
+// of unsigned char. Return true on success.
+// Offset is in the unit "nr of ElementBitWidth sized elements".
 bool llvm::getConstantDataArrayInfo(const Value *V,
                                     ConstantDataArraySlice &Slice,
-                                    unsigned ElementSize, uint64_t Offset) {
+                                    unsigned ElementBitWidth, uint64_t Offset) {
   assert(V && "V should not be null.");
-  assert((ElementSize % 8) == 0 &&
-         "ElementSize expected to be a multiple of the size of a byte.");
-  unsigned ElementSizeInBytes = ElementSize / 8;
 
   // Drill down into the pointer expression V, ignoring any intervening
   // casts, and determine the identity of the object it references along
@@ -6379,6 +6376,11 @@ bool llvm::getConstantDataArrayInfo(const Value *V,
     return false;
 
   const DataLayout &DL = GV->getDataLayout();
+  unsigned ByteWidth = DL.getByteWidth();
+  assert((ElementBitWidth % ByteWidth) == 0 &&
+         "ElementBitWidth is expected to be a multiple of the byte width");
+  unsigned ElementSizeInBytes = ElementBitWidth / ByteWidth;
+
   APInt Off(DL.getIndexTypeSizeInBits(V->getType()), 0);
 
   if (GV != V->stripAndAccumulateConstantOffsets(DL, Off,
@@ -6418,7 +6420,7 @@ bool llvm::getConstantDataArrayInfo(const Value *V,
   auto *Init = const_cast<Constant *>(GV->getInitializer());
   if (auto *ArrayInit = dyn_cast<ConstantDataArray>(Init)) {
     Type *InitElTy = ArrayInit->getElementType();
-    if (InitElTy->isIntegerTy(ElementSize)) {
+    if (InitElTy->isIntegerTy(ElementBitWidth)) {
       // If Init is an initializer for an array of the expected type
       // and size, use it as is.
       Array = ArrayInit;
@@ -6427,7 +6429,7 @@ bool llvm::getConstantDataArrayInfo(const Value *V,
   }
 
   if (!Array) {
-    if (ElementSize != 8)
+    if (ElementBitWidth != CHAR_BIT)
       // TODO: Handle conversions to larger integral types.
       return false;
 
@@ -6505,9 +6507,9 @@ bool llvm::getConstantStringInfo(const Value *V, StringRef &Str,
 
 /// If we can compute the length of the string pointed to by
 /// the specified pointer, return 'len+1'.  If we can't, return 0.
-static uint64_t GetStringLengthH(const Value *V,
-                                 SmallPtrSetImpl<const PHINode*> &PHIs,
-                                 unsigned CharSize) {
+static uint64_t getStringLength(const Value *V,
+                                SmallPtrSetImpl<const PHINode *> &PHIs,
+                                unsigned CharWidth) {
   // Look through noop bitcast instructions.
   V = V->stripPointerCasts();
 
@@ -6520,7 +6522,7 @@ static uint64_t GetStringLengthH(const Value *V,
     // If it was new, see if all the input strings are the same length.
     uint64_t LenSoFar = ~0ULL;
     for (Value *IncValue : PN->incoming_values()) {
-      uint64_t Len = GetStringLengthH(IncValue, PHIs, CharSize);
+      uint64_t Len = getStringLength(IncValue, PHIs, CharWidth);
       if (Len == 0) return 0; // Unknown length -> unknown.
 
       if (Len == ~0ULL) continue;
@@ -6536,9 +6538,9 @@ static uint64_t GetStringLengthH(const Value *V,
 
   // strlen(select(c,x,y)) -> strlen(x) ^ strlen(y)
   if (const SelectInst *SI = dyn_cast<SelectInst>(V)) {
-    uint64_t Len1 = GetStringLengthH(SI->getTrueValue(), PHIs, CharSize);
+    uint64_t Len1 = getStringLength(SI->getTrueValue(), PHIs, CharWidth);
     if (Len1 == 0) return 0;
-    uint64_t Len2 = GetStringLengthH(SI->getFalseValue(), PHIs, CharSize);
+    uint64_t Len2 = getStringLength(SI->getFalseValue(), PHIs, CharWidth);
     if (Len2 == 0) return 0;
     if (Len1 == ~0ULL) return Len2;
     if (Len2 == ~0ULL) return Len1;
@@ -6548,7 +6550,7 @@ static uint64_t GetStringLengthH(const Value *V,
 
   // Otherwise, see if we can read the string.
   ConstantDataArraySlice Slice;
-  if (!getConstantDataArrayInfo(V, Slice, CharSize))
+  if (!getConstantDataArrayInfo(V, Slice, CharWidth))
     return 0;
 
   if (Slice.Array == nullptr)
@@ -6570,12 +6572,12 @@ static uint64_t GetStringLengthH(const Value *V,
 
 /// If we can compute the length of the string pointed to by
 /// the specified pointer, return 'len+1'.  If we can't, return 0.
-uint64_t llvm::GetStringLength(const Value *V, unsigned CharSize) {
+uint64_t llvm::getStringLength(const Value *V, unsigned CharWidth) {
   if (!V->getType()->isPointerTy())
     return 0;
 
   SmallPtrSet<const PHINode*, 32> PHIs;
-  uint64_t Len = GetStringLengthH(V, PHIs, CharSize);
+  uint64_t Len = ::getStringLength(V, PHIs, CharWidth);
   // If Len is ~0ULL, we had an infinite phi cycle: this is dead code, so return
   // an empty string as a length.
   return Len == ~0ULL ? 1 : Len;

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

@@ -157,7 +157,8 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
   uint64_t Size = MemOpLength->getLimitedValue();
   assert(Size && "0-sized memory transferring should be removed already.");
 
-  if (Size > 8 || (Size&(Size-1)))
+  uint64_t MemOpWidth = Size * DL.getByteWidth();
+  if (MemOpWidth > 64 || (Size & (Size - 1)))
     return nullptr;  // If not 1/2/4/8 bytes, exit.
 
   // If it is an atomic and alignment is less than the size then we will
@@ -169,7 +170,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
       return nullptr;
 
   // Use an integer load+store unless we can find something better.
-  IntegerType* IntType = IntegerType::get(MI->getContext(), Size<<3);
+  IntegerType *IntType = IntegerType::get(MI->getContext(), MemOpWidth);
 
   // If the memcpy has metadata describing the members, see if we can get the
   // TBAA, scope and noalias tags describing our copy.
@@ -245,7 +246,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
   // Extract the length and alignment and fill if they are constant.
   ConstantInt *LenC = dyn_cast<ConstantInt>(MI->getLength());
   ConstantInt *FillC = dyn_cast<ConstantInt>(MI->getValue());
-  if (!LenC || !FillC || !FillC->getType()->isIntegerTy(8))
+  if (!LenC || !FillC || !FillC->getType()->isIntegerTy(DL.getByteWidth()))
     return nullptr;
   const uint64_t Len = LenC->getLimitedValue();
   assert(Len && "0-sized memory setting should be removed already.");
@@ -259,12 +260,13 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
     return nullptr;
 
   // memset(s,c,n) -> store s, c (for n=1,2,4,8)
-  if (Len <= 8 && isPowerOf2_32((uint32_t)Len)) {
+  uint64_t MemOpWidth = Len * DL.getByteWidth();
+  if (MemOpWidth <= 64 && isPowerOf2_32((uint32_t)Len)) {
     Value *Dest = MI->getDest();
 
     // Extract the fill value and store.
     Constant *FillVal = ConstantInt::get(
-        MI->getContext(), APInt::getSplat(Len * 8, FillC->getValue()));
+        MI->getContext(), APInt::getSplat(MemOpWidth, FillC->getValue()));
     StoreInst *S = Builder.CreateStore(FillVal, Dest, MI->isVolatile());
     S->copyMetadata(*MI, LLVMContext::MD_DIAssignID);
     auto replaceOpForAssignmentMarkers = [FillC, FillVal](auto *DbgAssign) {

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

@@ -3144,8 +3144,9 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
   if (MadeChange)
     return &GEP;
 
-  // Canonicalize constant GEPs to i8 type.
-  if (!GEPEltType->isIntegerTy(8) && GEP.hasAllConstantIndices()) {
+  // Canonicalize constant GEPs to byte type.
+  if (!GEPEltType->isIntegerTy(DL.getByteWidth()) &&
+      GEP.hasAllConstantIndices()) {
     APInt Offset(DL.getIndexTypeSizeInBits(GEPType), 0);
     if (GEP.accumulateConstantOffset(DL, Offset))
       return replaceInstUsesWith(