[ValueTracking] Fix usage of DataLayout::getTypeStoreSize()

[huihuiz]

Summary

DataLayout::getTypeStoreSize() returns TypeSize.

For cases where it can not be scalable vector (e.g., GlobalVariable),
explicitly call TypeSize::getFixedSize().

For cases where scalable property doesn't matter, (e.g., check for
zero-sized type), use TypeSize::isNonZero().

[efriedma]

llvm/lib/Analysis/ValueTracking.cpp

3499–3500

I'm a little uncomfortable with the way getKnownMinSize() is being used here; it's correct, but I don't really like leaving around a variable deceptively named "Size" in a big function like this.

Maybe it would make sense to add a method isNonZero() to TypeSize?

llvm/test/Transforms/MemCpyOpt/vscale.ll

5 ↗

(On Diff #251468)

We don't really need the text of the warning here...

I'm not really happy with the way the test here is exercising this codepath; if the code structure changes, it might end up covering nothing. Can you add a positive test that we detect <vscale x 4 x i32> zeroinitializer as a "bytewise" value?

[sdesmalen]

llvm/lib/Analysis/ValueTracking.cpp

3499–3500

That's a good suggestion, there are more instances where such a method would be useful.

[huihuiz]

Address review comments.

llvm/lib/Analysis/ValueTracking.cpp

3499–3500

Thanks Eli for the suggestion!
Get rid of "Size" in the update, and add TypeSize::isNonZero() to use here.

3499–3500

There are quite a few places, will post a nfc patch for refactoring.

llvm/test/Transforms/MemCpyOpt/vscale.ll

5 ↗

(On Diff #251468)

I am updating this test with more zero/non-zero array/struct to check for isBytewiseValue.
For vector, zero element is illegal. So only test for non-zero for vector. But single store instruction will help hit isBytewiseValue codepath.

[efriedma]

LGTM

llvm/include/llvm/Support/TypeSize.h

bool isScalable() const {
     return IsScalable;
   }
 
   // Returns true if the number of bits is a multiple of an 8-bit byte.
   bool isByteSized() const {
     return (MinSize & 7) == 0;
   }
 
+  // Returns true if the type size is non-zero.
+  bool isNonZero() const { return MinSize != 0; }
+
   // Casts to a uint64_t if this is a fixed-width size.
   //
   // This interface is deprecated and will be removed in a future version
   // of LLVM in favour of upgrading uses that rely on this implicit conversion
   // to uint64_t. Calls to functions that return a TypeSize should use the
   // proper interfaces to TypeSize.
   // In practice this is mostly calls to MVT/EVT::getSizeInBits().
   //

llvm/lib/Analysis/ValueTracking.cpp

     return V;
 
   LLVMContext &Ctx = V->getContext();
 
   // Undef don't care.
   auto *UndefInt8 = UndefValue::get(Type::getInt8Ty(Ctx));
   if (isa<UndefValue>(V))
     return UndefInt8;
 
-  const uint64_t Size = DL.getTypeStoreSize(V->getType());
-  if (!Size)
+  // Return Undef for zero-sized type.
+  if (!DL.getTypeStoreSize(V->getType()).isNonZero())
     return UndefInt8;
 
   Constant *C = dyn_cast<Constant>(V);
   if (!C) {
     // Conceptually, we could handle things like:
     //   %a = zext i8 %X to i16
     //   %b = shl i16 %a, 8
     //   %c = or i16 %a, %b

   ArrayType *ArrayTy;
   if (GV->getInitializer()->isNullValue()) {
     Type *GVTy = GV->getValueType();
     if ( (ArrayTy = dyn_cast<ArrayType>(GVTy)) ) {
       // A zeroinitializer for the array; there is no ConstantDataArray.
       Array = nullptr;
     } else {
       const DataLayout &DL = GV->getParent()->getDataLayout();
-      uint64_t SizeInBytes = DL.getTypeStoreSize(GVTy);
+      uint64_t SizeInBytes = DL.getTypeStoreSize(GVTy).getFixedSize();
       uint64_t Length = SizeInBytes / (ElementSize / 8);
       if (Length <= Offset)
         return false;
 
       Slice.Array = nullptr;
       Slice.Offset = 0;
       Slice.Length = Length - Offset;
       return true;

llvm/test/Transforms/MemCpyOpt/store-to-memset-is-nonzero-type.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S < %s -memcpyopt | FileCheck %s
+
+; Array
+
+define void @array_zero([0 x i8]* %p) {
+; CHECK-LABEL: @array_zero(
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast [0 x i8]* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 1 [[TMP1]], i8 undef, i64 0, i1 false)
+; CHECK-NEXT:    ret void
+;
+  store [0 x i8] zeroinitializer, [0 x i8]* %p
+  ret void
+}
+
+define void @array_nonzero([1 x i8]* %p) {
+; CHECK-LABEL: @array_nonzero(
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast [1 x i8]* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 1 [[TMP1]], i8 0, i64 1, i1 false)
+; CHECK-NEXT:    ret void
+;
+  store [1 x i8] zeroinitializer, [1 x i8]* %p
+  ret void
+}
+
+; Structure
+
+define void @struct_zero({ }* %p) {
+; CHECK-LABEL: @struct_zero(
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast {}* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 1 [[TMP1]], i8 undef, i64 0, i1 false)
+; CHECK-NEXT:    ret void
+;
+  store { } zeroinitializer, { }* %p
+  ret void
+}
+define void @struct_nonzero({ i8 }* %p) {
+; CHECK-LABEL: @struct_nonzero(
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast { i8 }* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 1 [[TMP1]], i8 0, i64 1, i1 false)
+; CHECK-NEXT:    ret void
+;
+  store { i8 } zeroinitializer, { i8 }* %p
+  ret void
+}
+
+; Vector
+
+; Test only non-zero vector. Zero element vector is illegal
+
+define void @vector_fixed_length_nonzero(<16 x i8>* %p) {
+; CHECK-LABEL: @vector_fixed_length_nonzero(
+; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr <16 x i8>, <16 x i8>* [[P:%.*]], i64 0
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr <16 x i8>, <16 x i8>* [[P]], i64 1
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <16 x i8>* [[TMP0]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 16 [[TMP1]], i8 0, i64 32, i1 false)
+; CHECK-NEXT:    ret void
+;
+  %tmp0 = getelementptr <16 x i8>, <16 x i8>* %p, i64 0
+  store <16 x i8> zeroinitializer, <16 x i8>* %tmp0
+  %tmp1 = getelementptr <16 x i8>, <16 x i8>* %p, i64 1
+  store <16 x i8> zeroinitializer, <16 x i8>* %tmp1
+  ret void
+}
+
+define void @vector_scalable_nonzero(<vscale x 4 x i32>* %p) {
+; CHECK-LABEL: @vector_scalable_nonzero(
+; CHECK-NEXT:    store <vscale x 4 x i32> zeroinitializer, <vscale x 4 x i32>* [[P:%.*]]
+; CHECK-NEXT:    ret void
+;
+  store <vscale x 4 x i32> zeroinitializer, <vscale x 4 x i32>* %p
+  ret void
+}