[PATCH] D135263: [ValueTracking][SimplifyLibCalls] Fix bug in getConstantDataArrayInfo for wchar_t

[Bjorn Pettersson via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG01e1f3297151: [ValueTracking][SimplifyLibCalls] Fix bug in getConstantDataArrayInfo for… (authored by bjope).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135263/new/

https://reviews.llvm.org/D135263

Files:
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/test/Transforms/InstCombine/wcslen-1.ll


Index: llvm/test/Transforms/InstCombine/wcslen-1.ll
===================================================================
--- llvm/test/Transforms/InstCombine/wcslen-1.ll
+++ llvm/test/Transforms/InstCombine/wcslen-1.ll
@@ -217,10 +217,9 @@
 @ws = constant [10 x i32] [i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0]
 
 ; Fold wcslen(ws + 2) => 7.
-; FIXME: This fold is faulty, result should be 7 not 1.
 define i64 @fold_wcslen_1() {
 ; CHECK-LABEL: @fold_wcslen_1(
-; CHECK-NEXT:    ret i64 1
+; CHECK-NEXT:    ret i64 7
 ;
   %p = getelementptr inbounds [10 x i32], ptr @ws, i64 0, i64 2
   %len = tail call i64 @wcslen(ptr %p)
@@ -229,11 +228,11 @@
 
 ; Should not crash on this, and no optimization expected (idea is to get into
 ; llvm::getConstantDataArrayInfo looking for an array with 32-bit elements but
-; with an offset that isn't a multiple of the element size).  FIXME: Looks a
-; bit weird. Don't think we should return 6 here.
+; with an offset that isn't a multiple of the element size).
 define i64 @no_fold_wcslen_1() {
 ; CHECK-LABEL: @no_fold_wcslen_1(
-; CHECK-NEXT:    ret i64 6
+; CHECK-NEXT:    %len = tail call i64 @wcslen(ptr nonnull getelementptr inbounds ([15 x i8], ptr @ws, i64 0, i64 3))
+; CHECK-NEXT:    ret i64 %len
 ;
   %p = getelementptr [15 x i8], ptr @ws, i64 0, i64 3
   %len = tail call i64 @wcslen(ptr %p)
Index: llvm/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -4232,10 +4232,14 @@
 // 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".
 bool llvm::getConstantDataArrayInfo(const Value *V,
                                     ConstantDataArraySlice &Slice,
                                     unsigned ElementSize, uint64_t Offset) {
-  assert(V);
+  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
@@ -4259,15 +4263,19 @@
     // Fail if the constant offset is excessive.
     return false;
 
-  Offset += StartIdx;
+  // Off/StartIdx is in the unit of bytes. So we need to convert to number of
+  // elements. Simply bail out if that isn't possible.
+  if ((StartIdx % ElementSizeInBytes) != 0)
+    return false;
 
+  Offset += StartIdx / ElementSizeInBytes;
   ConstantDataArray *Array = nullptr;
   ArrayType *ArrayTy = nullptr;
 
   if (GV->getInitializer()->isNullValue()) {
     Type *GVTy = GV->getValueType();
     uint64_t SizeInBytes = DL.getTypeStoreSize(GVTy).getFixedSize();
-    uint64_t Length = SizeInBytes / (ElementSize / 8);
+    uint64_t Length = SizeInBytes / ElementSizeInBytes;
 
     Slice.Array = nullptr;
     Slice.Offset = 0;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135263.466062.patch
Type: text/x-patch
Size: 3145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221007/0e1aadb2/attachment-0001.bin>