[llvm] [LLVM][SelectionDAG] Reduce number of ComputeValueVTs variants. (PR #75614)

via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 15 06:59:03 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-selectiondag

Author: Paul Walker (paulwalker-arm)

<details>
<summary>Changes</summary>

This is another step in the direction of fixing the `Fixed(0) != Scalable(0)` bugbear, although whilst weird I don't believe it's causing us any real issues.

I'm not sure of the value of adding `TypeSize::getZero()`.  I prefer its explicitness when compared to `TypeSize()`, but it's subjective so am happy to take guidance.

---
Full diff: https://github.com/llvm/llvm-project/pull/75614.diff


6 Files Affected:

- (modified) llvm/include/llvm/CodeGen/Analysis.h (+15-20) 
- (modified) llvm/include/llvm/Support/TypeSize.h (+1) 
- (modified) llvm/lib/CodeGen/Analysis.cpp (+4-42) 
- (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+2-2) 
- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+2-2) 
- (modified) llvm/unittests/Support/TypeSizeTest.cpp (+3) 


``````````diff
diff --git a/llvm/include/llvm/CodeGen/Analysis.h b/llvm/include/llvm/CodeGen/Analysis.h
index 1c67fe2d003d90..6f7ed22b8ac718 100644
--- a/llvm/include/llvm/CodeGen/Analysis.h
+++ b/llvm/include/llvm/CodeGen/Analysis.h
@@ -62,36 +62,31 @@ inline unsigned ComputeLinearIndex(Type *Ty,
 /// If Offsets is non-null, it points to a vector to be filled in
 /// with the in-memory offsets of each of the individual values.
 ///
-void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL, Type *Ty,
-                     SmallVectorImpl<EVT> &ValueVTs,
-                     SmallVectorImpl<TypeSize> *Offsets,
-                     TypeSize StartingOffset);
-void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL, Type *Ty,
-                     SmallVectorImpl<EVT> &ValueVTs,
-                     SmallVectorImpl<TypeSize> *Offsets = nullptr,
-                     uint64_t StartingOffset = 0);
-void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL, Type *Ty,
-                     SmallVectorImpl<EVT> &ValueVTs,
-                     SmallVectorImpl<uint64_t> *FixedOffsets,
-                     uint64_t StartingOffset);
-
-/// Variant of ComputeValueVTs that also produces the memory VTs.
-void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL, Type *Ty,
-                     SmallVectorImpl<EVT> &ValueVTs,
-                     SmallVectorImpl<EVT> *MemVTs,
-                     SmallVectorImpl<TypeSize> *Offsets,
-                     TypeSize StartingOffset);
 void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL, Type *Ty,
                      SmallVectorImpl<EVT> &ValueVTs,
                      SmallVectorImpl<EVT> *MemVTs,
                      SmallVectorImpl<TypeSize> *Offsets = nullptr,
-                     uint64_t StartingOffset = 0);
+                     TypeSize StartingOffset = TypeSize::getZero());
 void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL, Type *Ty,
                      SmallVectorImpl<EVT> &ValueVTs,
                      SmallVectorImpl<EVT> *MemVTs,
                      SmallVectorImpl<uint64_t> *FixedOffsets,
                      uint64_t StartingOffset);
 
+/// Variant of ComputeValueVTs that don't produce memory VTs.
+inline void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
+                            Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
+                            SmallVectorImpl<TypeSize> *Offsets = nullptr,
+                            TypeSize StartingOffset = TypeSize::getZero()) {
+  ComputeValueVTs(TLI, DL, Ty, ValueVTs, nullptr, Offsets, StartingOffset);
+}
+inline void ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
+                            Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
+                            SmallVectorImpl<uint64_t> *FixedOffsets,
+                            uint64_t StartingOffset) {
+  ComputeValueVTs(TLI, DL, Ty, ValueVTs, nullptr, FixedOffsets, StartingOffset);
+}
+
 /// computeValueLLTs - Given an LLVM IR type, compute a sequence of
 /// LLTs that represent all the individual underlying
 /// non-aggregate types that comprise it.
diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index b00ebf9e8c454a..1b793b0eccf3c7 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -335,6 +335,7 @@ class TypeSize : public details::FixedOrScalableQuantity<TypeSize, uint64_t> {
   static constexpr TypeSize getScalable(ScalarTy MinimumSize) {
     return TypeSize(MinimumSize, true);
   }
+  static constexpr TypeSize getZero() { return TypeSize(0, false); }
 
   // All code for this class below this point is needed because of the
   // temporary implicit conversion to uint64_t. The operator overloads are
diff --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp
index 1994e6aec84b23..f763832360a82b 100644
--- a/llvm/lib/CodeGen/Analysis.cpp
+++ b/llvm/lib/CodeGen/Analysis.cpp
@@ -81,6 +81,8 @@ void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
                            SmallVectorImpl<EVT> *MemVTs,
                            SmallVectorImpl<TypeSize> *Offsets,
                            TypeSize StartingOffset) {
+  assert((Ty->isScalableTy() == StartingOffset.isScalable() ||
+         StartingOffset.isZero()) && "Offset/TypeSize mismatch!");
   // Given a struct type, recursively traverse the elements.
   if (StructType *STy = dyn_cast<StructType>(Ty)) {
     // If the Offsets aren't needed, don't query the struct layout. This allows
@@ -93,7 +95,7 @@ void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
          EI != EE; ++EI) {
       // Don't compute the element offset if we didn't get a StructLayout above.
       TypeSize EltOffset = SL ? SL->getElementOffset(EI - EB)
-                              : TypeSize::get(0, StartingOffset.isScalable());
+                              : TypeSize::getZero();
       ComputeValueVTs(TLI, DL, *EI, ValueVTs, MemVTs, Offsets,
                       StartingOffset + EltOffset);
     }
@@ -119,52 +121,12 @@ void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
     Offsets->push_back(StartingOffset);
 }
 
-void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
-                           Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
-                           SmallVectorImpl<TypeSize> *Offsets,
-                           TypeSize StartingOffset) {
-  return ComputeValueVTs(TLI, DL, Ty, ValueVTs, /*MemVTs=*/nullptr, Offsets,
-                         StartingOffset);
-}
-
-void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
-                           Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
-                           SmallVectorImpl<TypeSize> *Offsets,
-                           uint64_t StartingOffset) {
-  TypeSize Offset = TypeSize::get(StartingOffset, Ty->isScalableTy());
-  return ComputeValueVTs(TLI, DL, Ty, ValueVTs, Offsets, Offset);
-}
-
-void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
-                           Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
-                           SmallVectorImpl<uint64_t> *FixedOffsets,
-                           uint64_t StartingOffset) {
-  TypeSize Offset = TypeSize::get(StartingOffset, Ty->isScalableTy());
-  if (FixedOffsets) {
-    SmallVector<TypeSize, 4> Offsets;
-    ComputeValueVTs(TLI, DL, Ty, ValueVTs, &Offsets, Offset);
-    for (TypeSize Offset : Offsets)
-      FixedOffsets->push_back(Offset.getFixedValue());
-  } else {
-    ComputeValueVTs(TLI, DL, Ty, ValueVTs, nullptr, Offset);
-  }
-}
-
-void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
-                           Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
-                           SmallVectorImpl<EVT> *MemVTs,
-                           SmallVectorImpl<TypeSize> *Offsets,
-                           uint64_t StartingOffset) {
-  TypeSize Offset = TypeSize::get(StartingOffset, Ty->isScalableTy());
-  return ComputeValueVTs(TLI, DL, Ty, ValueVTs, MemVTs, Offsets, Offset);
-}
-
 void llvm::ComputeValueVTs(const TargetLowering &TLI, const DataLayout &DL,
                            Type *Ty, SmallVectorImpl<EVT> &ValueVTs,
                            SmallVectorImpl<EVT> *MemVTs,
                            SmallVectorImpl<uint64_t> *FixedOffsets,
                            uint64_t StartingOffset) {
-  TypeSize Offset = TypeSize::get(StartingOffset, Ty->isScalableTy());
+  TypeSize Offset = TypeSize::getFixed(StartingOffset);
   if (FixedOffsets) {
     SmallVector<TypeSize, 4> Offsets;
     ComputeValueVTs(TLI, DL, Ty, ValueVTs, MemVTs, &Offsets, Offset);
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 12ed4a82ee91a5..4506468e585a07 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4305,7 +4305,7 @@ void SelectionDAGBuilder::visitLoad(const LoadInst &I) {
   Type *Ty = I.getType();
   SmallVector<EVT, 4> ValueVTs, MemVTs;
   SmallVector<TypeSize, 4> Offsets;
-  ComputeValueVTs(TLI, DAG.getDataLayout(), Ty, ValueVTs, &MemVTs, &Offsets, 0);
+  ComputeValueVTs(TLI, DAG.getDataLayout(), Ty, ValueVTs, &MemVTs, &Offsets);
   unsigned NumValues = ValueVTs.size();
   if (NumValues == 0)
     return;
@@ -4473,7 +4473,7 @@ void SelectionDAGBuilder::visitStore(const StoreInst &I) {
   SmallVector<EVT, 4> ValueVTs, MemVTs;
   SmallVector<TypeSize, 4> Offsets;
   ComputeValueVTs(DAG.getTargetLoweringInfo(), DAG.getDataLayout(),
-                  SrcV->getType(), ValueVTs, &MemVTs, &Offsets, 0);
+                  SrcV->getType(), ValueVTs, &MemVTs, &Offsets);
   unsigned NumValues = ValueVTs.size();
   if (NumValues == 0)
     return;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index bb2a77daa60a76..42992910978c55 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -777,7 +777,7 @@ static Instruction *unpackLoadToAggregate(InstCombinerImpl &IC, LoadInst &LI) {
     auto *Zero = ConstantInt::get(IdxType, 0);
 
     Value *V = PoisonValue::get(T);
-    TypeSize Offset = TypeSize::get(0, ET->isScalableTy());
+    TypeSize Offset = TypeSize::getZero();
     for (uint64_t i = 0; i < NumElements; i++) {
       Value *Indices[2] = {
         Zero,
@@ -1302,7 +1302,7 @@ static bool unpackStoreToAggregate(InstCombinerImpl &IC, StoreInst &SI) {
     auto *IdxType = Type::getInt64Ty(T->getContext());
     auto *Zero = ConstantInt::get(IdxType, 0);
 
-    TypeSize Offset = TypeSize::get(0, AT->getElementType()->isScalableTy());
+    TypeSize Offset = TypeSize::getZero();
     for (uint64_t i = 0; i < NumElements; i++) {
       Value *Indices[2] = {
         Zero,
diff --git a/llvm/unittests/Support/TypeSizeTest.cpp b/llvm/unittests/Support/TypeSizeTest.cpp
index 503dc5d99b1823..34fe376989e7ba 100644
--- a/llvm/unittests/Support/TypeSizeTest.cpp
+++ b/llvm/unittests/Support/TypeSizeTest.cpp
@@ -82,9 +82,12 @@ static_assert(UINT64_C(2) * TSFixed32 == TypeSize::getFixed(64));
 static_assert(alignTo(TypeSize::getFixed(7), 8) == TypeSize::getFixed(8));
 
 static_assert(TypeSize() == TypeSize::getFixed(0));
+static_assert(TypeSize::getZero() == TypeSize::getFixed(0));
+static_assert(TypeSize::getZero() != TypeSize::getScalable(0));
 static_assert(TypeSize::getFixed(0) != TypeSize::getScalable(0));
 static_assert(TypeSize::getFixed(0).isZero());
 static_assert(TypeSize::getScalable(0).isZero());
+static_assert(TypeSize::getZero().isZero());
 static_assert(TypeSize::getFixed(0) ==
               (TypeSize::getFixed(4) - TypeSize::getFixed(4)));
 static_assert(TypeSize::getScalable(0) ==

``````````

</details>


https://github.com/llvm/llvm-project/pull/75614


More information about the llvm-commits mailing list