Skip to content

[VFABI] Add support for vector functions that return struct types #119000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Dec 18, 2024

Conversation

MacDue
Copy link
Member

@MacDue MacDue commented Dec 6, 2024

This patch updates the VFABIDemangler to support vector functions that return struct types. For example, a vector variant of sincos that returns a vector of sine values and a vector of cosine values within a struct.

This patch also adds some helpers for vectorizing types (including struct types). Some of these are used in the VFABIDemangler, and others will be used in subsequent patches, so this patch simply adds tests for them.

This patch updates the `VFABIDemangler` to support vector functions that
return struct types. For example, a vector variant of `sincos` that
returns a vector of sine values and a vector of cosine values within a
struct.

This patch also adds some helpers in `llvm/IR/CallWideningUtils.h` for
widening call return types. Some of these are used in the
`VFABIDemangler`, and others will be used in subsequent patches, so this
patch simply adds tests for them.
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-analysis

Author: Benjamin Maxwell (MacDue)

Changes

This patch updates the VFABIDemangler to support vector functions that return struct types. For example, a vector variant of sincos that returns a vector of sine values and a vector of cosine values within a struct.

This patch also adds some helpers in llvm/IR/CallWideningUtils.h for widening call return types. Some of these are used in the VFABIDemangler, and others will be used in subsequent patches, so this patch simply adds tests for them.


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

9 Files Affected:

  • (modified) llvm/include/llvm/Analysis/VectorUtils.h (+1-13)
  • (added) llvm/include/llvm/IR/CallWideningUtils.h (+44)
  • (added) llvm/include/llvm/IR/VectorUtils.h (+32)
  • (modified) llvm/lib/IR/CMakeLists.txt (+1)
  • (added) llvm/lib/IR/CallWideningUtils.cpp (+73)
  • (modified) llvm/lib/IR/VFABIDemangler.cpp (+11-7)
  • (modified) llvm/unittests/IR/CMakeLists.txt (+1)
  • (added) llvm/unittests/IR/CallWideningUtilsTest.cpp (+149)
  • (modified) llvm/unittests/IR/VFABIDemanglerTest.cpp (+67)
diff --git a/llvm/include/llvm/Analysis/VectorUtils.h b/llvm/include/llvm/Analysis/VectorUtils.h
index c1016dd7bdddbd..5433231a1018e9 100644
--- a/llvm/include/llvm/Analysis/VectorUtils.h
+++ b/llvm/include/llvm/Analysis/VectorUtils.h
@@ -18,6 +18,7 @@
 #include "llvm/Analysis/LoopAccessAnalysis.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/VFABIDemangler.h"
+#include "llvm/IR/VectorUtils.h"
 #include "llvm/Support/CheckedArithmetic.h"
 
 namespace llvm {
@@ -127,19 +128,6 @@ namespace Intrinsic {
 typedef unsigned ID;
 }
 
-/// A helper function for converting Scalar types to vector types. If
-/// the incoming type is void, we return void. If the EC represents a
-/// scalar, we return the scalar type.
-inline Type *ToVectorTy(Type *Scalar, ElementCount EC) {
-  if (Scalar->isVoidTy() || Scalar->isMetadataTy() || EC.isScalar())
-    return Scalar;
-  return VectorType::get(Scalar, EC);
-}
-
-inline Type *ToVectorTy(Type *Scalar, unsigned VF) {
-  return ToVectorTy(Scalar, ElementCount::getFixed(VF));
-}
-
 /// Identify if the intrinsic is trivially vectorizable.
 /// This method returns true if the intrinsic's argument types are all scalars
 /// for the scalar form of the intrinsic and all vectors (or scalars handled by
diff --git a/llvm/include/llvm/IR/CallWideningUtils.h b/llvm/include/llvm/IR/CallWideningUtils.h
new file mode 100644
index 00000000000000..de51c8f6c6ba1f
--- /dev/null
+++ b/llvm/include/llvm/IR/CallWideningUtils.h
@@ -0,0 +1,44 @@
+//===---- CallWideningUtils.h - Utils for widening scalar to vector calls --==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_IR_CALLWIDENINGUTILS_H
+#define LLVM_IR_CALLWIDENINGUTILS_H
+
+#include "llvm/IR/DerivedTypes.h"
+
+namespace llvm {
+
+/// A helper for converting to wider (vector) types. For scalar types, this is
+/// equivalent to calling `ToVectorTy`. For struct types, this returns a new
+/// struct where each element type has been widened to a vector type. Note: Only
+/// unpacked literal struct types are supported.
+Type *ToWideTy(Type *Ty, ElementCount EC);
+
+/// A helper for converting wide types to narrow (non-vector) types. For vector
+/// types, this is equivalent to calling .getScalarType(). For struct types,
+/// this returns a new struct where each element type has been converted to a
+/// scalar type. Note: Only unpacked literal struct types are supported.
+Type *ToNarrowTy(Type *Ty);
+
+/// Returns the types contained in `Ty`. For struct types, it returns the
+/// elements, all other types are returned directly.
+SmallVector<Type *, 2> getContainedTypes(Type *Ty);
+
+/// Returns true if `Ty` is a vector type or a struct of vector types where all
+/// vector types share the same VF.
+bool isWideTy(Type *Ty);
+
+/// Returns the vectorization factor for a widened type.
+inline ElementCount getWideTypeVF(Type *Ty) {
+  assert(isWideTy(Ty) && "expected widened type");
+  return cast<VectorType>(getContainedTypes(Ty).front())->getElementCount();
+}
+
+} // namespace llvm
+
+#endif
diff --git a/llvm/include/llvm/IR/VectorUtils.h b/llvm/include/llvm/IR/VectorUtils.h
new file mode 100644
index 00000000000000..a2e34a02e08a67
--- /dev/null
+++ b/llvm/include/llvm/IR/VectorUtils.h
@@ -0,0 +1,32 @@
+//===----------- VectorUtils.h -  Vector type utility functions -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_IR_VECTORUTILS_H
+#define LLVM_IR_VECTORUTILS_H
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/DerivedTypes.h"
+
+namespace llvm {
+
+/// A helper function for converting Scalar types to vector types. If
+/// the incoming type is void, we return void. If the EC represents a
+/// scalar, we return the scalar type.
+inline Type *ToVectorTy(Type *Scalar, ElementCount EC) {
+  if (Scalar->isVoidTy() || Scalar->isMetadataTy() || EC.isScalar())
+    return Scalar;
+  return VectorType::get(Scalar, EC);
+}
+
+inline Type *ToVectorTy(Type *Scalar, unsigned VF) {
+  return ToVectorTy(Scalar, ElementCount::getFixed(VF));
+}
+
+} // namespace llvm
+
+#endif
diff --git a/llvm/lib/IR/CMakeLists.txt b/llvm/lib/IR/CMakeLists.txt
index 544f4ea9223d0e..874ee8c4795a96 100644
--- a/llvm/lib/IR/CMakeLists.txt
+++ b/llvm/lib/IR/CMakeLists.txt
@@ -6,6 +6,7 @@ add_llvm_component_library(LLVMCore
   AutoUpgrade.cpp
   BasicBlock.cpp
   BuiltinGCs.cpp
+  CallWideningUtils.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantFPRange.cpp
diff --git a/llvm/lib/IR/CallWideningUtils.cpp b/llvm/lib/IR/CallWideningUtils.cpp
new file mode 100644
index 00000000000000..ec0bc6e3baa463
--- /dev/null
+++ b/llvm/lib/IR/CallWideningUtils.cpp
@@ -0,0 +1,73 @@
+//===----------- VectorUtils.cpp - Vector type utility functions ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/CallWideningUtils.h"
+#include "llvm/ADT/SmallVectorExtras.h"
+#include "llvm/IR/VectorUtils.h"
+
+using namespace llvm;
+
+/// A helper for converting to wider (vector) types. For scalar types, this is
+/// equivalent to calling `ToVectorTy`. For struct types, this returns a new
+/// struct where each element type has been widened to a vector type. Note: Only
+/// unpacked literal struct types are supported.
+Type *llvm::ToWideTy(Type *Ty, ElementCount EC) {
+  if (EC.isScalar())
+    return Ty;
+  auto *StructTy = dyn_cast<StructType>(Ty);
+  if (!StructTy)
+    return ToVectorTy(Ty, EC);
+  assert(StructTy->isLiteral() && !StructTy->isPacked() &&
+         "expected unpacked struct literal");
+  return StructType::get(
+      Ty->getContext(),
+      map_to_vector(StructTy->elements(), [&](Type *ElTy) -> Type * {
+        return VectorType::get(ElTy, EC);
+      }));
+}
+
+/// A helper for converting wide types to narrow (non-vector) types. For vector
+/// types, this is equivalent to calling .getScalarType(). For struct types,
+/// this returns a new struct where each element type has been converted to a
+/// scalar type. Note: Only unpacked literal struct types are supported.
+Type *llvm::ToNarrowTy(Type *Ty) {
+  auto *StructTy = dyn_cast<StructType>(Ty);
+  if (!StructTy)
+    return Ty->getScalarType();
+  assert(StructTy->isLiteral() && !StructTy->isPacked() &&
+         "expected unpacked struct literal");
+  return StructType::get(
+      Ty->getContext(),
+      map_to_vector(StructTy->elements(), [](Type *ElTy) -> Type * {
+        return ElTy->getScalarType();
+      }));
+}
+
+/// Returns the types contained in `Ty`. For struct types, it returns the
+/// elements, all other types are returned directly.
+SmallVector<Type *, 2> llvm::getContainedTypes(Type *Ty) {
+  auto *StructTy = dyn_cast<StructType>(Ty);
+  if (StructTy)
+    return to_vector<2>(StructTy->elements());
+  return {Ty};
+}
+
+/// Returns true if `Ty` is a vector type or a struct of vector types where all
+/// vector types share the same VF.
+bool llvm::isWideTy(Type *Ty) {
+  auto *StructTy = dyn_cast<StructType>(Ty);
+  if (StructTy && (!StructTy->isLiteral() || StructTy->isPacked()))
+    return false;
+  auto ContainedTys = getContainedTypes(Ty);
+  if (ContainedTys.empty() || !ContainedTys.front()->isVectorTy())
+    return false;
+  ElementCount VF = cast<VectorType>(ContainedTys.front())->getElementCount();
+  return all_of(ContainedTys, [&](Type *Ty) {
+    return Ty->isVectorTy() && cast<VectorType>(Ty)->getElementCount() == VF;
+  });
+}
diff --git a/llvm/lib/IR/VFABIDemangler.cpp b/llvm/lib/IR/VFABIDemangler.cpp
index 897583084bf38c..19c922c8bf035b 100644
--- a/llvm/lib/IR/VFABIDemangler.cpp
+++ b/llvm/lib/IR/VFABIDemangler.cpp
@@ -10,6 +10,7 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/IR/CallWideningUtils.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -346,12 +347,15 @@ getScalableECFromSignature(const FunctionType *Signature, const VFISAKind ISA,
   // Also check the return type if not void.
   Type *RetTy = Signature->getReturnType();
   if (!RetTy->isVoidTy()) {
-    std::optional<ElementCount> ReturnEC = getElementCountForTy(ISA, RetTy);
-    // If we have an unknown scalar element type we can't find a reasonable VF.
-    if (!ReturnEC)
-      return std::nullopt;
-    if (ElementCount::isKnownLT(*ReturnEC, MinEC))
-      MinEC = *ReturnEC;
+    for (Type *RetTy : getContainedTypes(RetTy)) {
+      std::optional<ElementCount> ReturnEC = getElementCountForTy(ISA, RetTy);
+      // If we have an unknown scalar element type we can't find a reasonable
+      // VF.
+      if (!ReturnEC)
+        return std::nullopt;
+      if (ElementCount::isKnownLT(*ReturnEC, MinEC))
+        MinEC = *ReturnEC;
+    }
   }
 
   // The SVE Vector function call ABI bases the VF on the widest element types
@@ -566,7 +570,7 @@ FunctionType *VFABI::createFunctionType(const VFInfo &Info,
 
   auto *RetTy = ScalarFTy->getReturnType();
   if (!RetTy->isVoidTy())
-    RetTy = VectorType::get(RetTy, VF);
+    RetTy = ToWideTy(RetTy, VF);
   return FunctionType::get(RetTy, VecTypes, false);
 }
 
diff --git a/llvm/unittests/IR/CMakeLists.txt b/llvm/unittests/IR/CMakeLists.txt
index ed93ee547d2231..c4cc1bd78403a8 100644
--- a/llvm/unittests/IR/CMakeLists.txt
+++ b/llvm/unittests/IR/CMakeLists.txt
@@ -15,6 +15,7 @@ add_llvm_unittest(IRTests
   AttributesTest.cpp
   BasicBlockTest.cpp
   BasicBlockDbgInfoTest.cpp
+  CallWideningUtilsTest.cpp
   CFGBuilder.cpp
   ConstantFPRangeTest.cpp
   ConstantRangeTest.cpp
diff --git a/llvm/unittests/IR/CallWideningUtilsTest.cpp b/llvm/unittests/IR/CallWideningUtilsTest.cpp
new file mode 100644
index 00000000000000..939806212eee6a
--- /dev/null
+++ b/llvm/unittests/IR/CallWideningUtilsTest.cpp
@@ -0,0 +1,149 @@
+//===------- CallWideningUtilsTest.cpp - Call widening utils tests --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/CallWideningUtils.h"
+#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/LLVMContext.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class CallWideningUtilsTest : public ::testing::Test {};
+
+TEST(CallWideningUtilsTest, TestToWideTy) {
+  LLVMContext C;
+
+  Type *ITy = Type::getInt32Ty(C);
+  Type *FTy = Type::getFloatTy(C);
+  Type *HomogeneousStructTy = StructType::get(FTy, FTy, FTy);
+  Type *MixedStructTy = StructType::get(FTy, ITy);
+  Type *VoidTy = Type::getVoidTy(C);
+
+  for (ElementCount VF :
+       {ElementCount::getFixed(4), ElementCount::getScalable(2)}) {
+    Type *IntVec = ToWideTy(ITy, VF);
+    EXPECT_TRUE(isa<VectorType>(IntVec));
+    EXPECT_EQ(IntVec, VectorType::get(ITy, VF));
+
+    Type *FloatVec = ToWideTy(FTy, VF);
+    EXPECT_TRUE(isa<VectorType>(FloatVec));
+    EXPECT_EQ(FloatVec, VectorType::get(FTy, VF));
+
+    Type *WideHomogeneousStructTy = ToWideTy(HomogeneousStructTy, VF);
+    EXPECT_TRUE(isa<StructType>(WideHomogeneousStructTy));
+    EXPECT_TRUE(
+        cast<StructType>(WideHomogeneousStructTy)->containsHomogeneousTypes());
+    EXPECT_TRUE(cast<StructType>(WideHomogeneousStructTy)->getNumElements() ==
+                3);
+    EXPECT_TRUE(cast<StructType>(WideHomogeneousStructTy)->getElementType(0) ==
+                VectorType::get(FTy, VF));
+
+    Type *WideMixedStructTy = ToWideTy(MixedStructTy, VF);
+    EXPECT_TRUE(isa<StructType>(WideMixedStructTy));
+    EXPECT_TRUE(cast<StructType>(WideMixedStructTy)->getNumElements() == 2);
+    EXPECT_TRUE(cast<StructType>(WideMixedStructTy)->getElementType(0) ==
+                VectorType::get(FTy, VF));
+    EXPECT_TRUE(cast<StructType>(WideMixedStructTy)->getElementType(1) ==
+                VectorType::get(ITy, VF));
+
+    EXPECT_EQ(ToWideTy(VoidTy, VF), VoidTy);
+  }
+
+  ElementCount ScalarVF = ElementCount::getFixed(1);
+  for (Type *Ty : {ITy, FTy, HomogeneousStructTy, MixedStructTy, VoidTy}) {
+    EXPECT_EQ(ToWideTy(Ty, ScalarVF), Ty);
+  }
+}
+
+TEST(CallWideningUtilsTest, TestToNarrowTy) {
+  LLVMContext C;
+
+  Type *ITy = Type::getInt32Ty(C);
+  Type *FTy = Type::getFloatTy(C);
+  Type *HomogeneousStructTy = StructType::get(FTy, FTy, FTy);
+  Type *MixedStructTy = StructType::get(FTy, ITy);
+  Type *VoidTy = Type::getVoidTy(C);
+
+  for (ElementCount VF : {ElementCount::getFixed(1), ElementCount::getFixed(4),
+                          ElementCount::getScalable(2)}) {
+    for (Type *Ty : {ITy, FTy, HomogeneousStructTy, MixedStructTy, VoidTy}) {
+      // ToNarrowTy should be the inverse of ToWideTy.
+      EXPECT_EQ(ToNarrowTy(ToWideTy(Ty, VF)), Ty);
+    };
+  }
+}
+
+TEST(CallWideningUtilsTest, TestGetContainedTypes) {
+  LLVMContext C;
+
+  Type *ITy = Type::getInt32Ty(C);
+  Type *FTy = Type::getFloatTy(C);
+  Type *HomogeneousStructTy = StructType::get(FTy, FTy, FTy);
+  Type *MixedStructTy = StructType::get(FTy, ITy);
+  Type *VoidTy = Type::getVoidTy(C);
+
+  EXPECT_EQ(getContainedTypes(ITy), SmallVector<Type *>({ITy}));
+  EXPECT_EQ(getContainedTypes(FTy), SmallVector<Type *>({FTy}));
+  EXPECT_EQ(getContainedTypes(VoidTy), SmallVector<Type *>({VoidTy}));
+  EXPECT_EQ(getContainedTypes(HomogeneousStructTy),
+            SmallVector<Type *>({FTy, FTy, FTy}));
+  EXPECT_EQ(getContainedTypes(MixedStructTy), SmallVector<Type *>({FTy, ITy}));
+}
+
+TEST(CallWideningUtilsTest, TestIsWideTy) {
+  LLVMContext C;
+
+  Type *ITy = Type::getInt32Ty(C);
+  Type *FTy = Type::getFloatTy(C);
+  Type *NarrowStruct = StructType::get(FTy, ITy);
+  Type *VoidTy = Type::getVoidTy(C);
+
+  EXPECT_FALSE(isWideTy(ITy));
+  EXPECT_FALSE(isWideTy(NarrowStruct));
+  EXPECT_FALSE(isWideTy(VoidTy));
+
+  ElementCount VF = ElementCount::getFixed(4);
+  EXPECT_TRUE(isWideTy(ToWideTy(ITy, VF)));
+  EXPECT_TRUE(isWideTy(ToWideTy(NarrowStruct, VF)));
+
+  Type *MixedVFStruct =
+      StructType::get(VectorType::get(ITy, ElementCount::getFixed(2)),
+                      VectorType::get(ITy, ElementCount::getFixed(4)));
+  EXPECT_FALSE(isWideTy(MixedVFStruct));
+
+  // Currently only literals types are considered wide.
+  Type *NamedWideStruct = StructType::create("Named", VectorType::get(ITy, VF),
+                                             VectorType::get(ITy, VF));
+  EXPECT_FALSE(isWideTy(NamedWideStruct));
+
+  // Currently only unpacked types are considered wide.
+  Type *PackedWideStruct = StructType::get(
+      C, ArrayRef<Type *>{VectorType::get(ITy, VF), VectorType::get(ITy, VF)},
+      /*isPacked=*/true);
+  EXPECT_FALSE(isWideTy(PackedWideStruct));
+}
+
+TEST(CallWideningUtilsTest, TestGetWideTypeVF) {
+  LLVMContext C;
+
+  Type *ITy = Type::getInt32Ty(C);
+  Type *FTy = Type::getFloatTy(C);
+  Type *HomogeneousStructTy = StructType::get(FTy, FTy, FTy);
+  Type *MixedStructTy = StructType::get(FTy, ITy);
+
+  for (ElementCount VF :
+       {ElementCount::getFixed(4), ElementCount::getScalable(2)}) {
+    for (Type *Ty : {ITy, FTy, HomogeneousStructTy, MixedStructTy}) {
+      EXPECT_EQ(getWideTypeVF(ToWideTy(Ty, VF)), VF);
+    };
+  }
+}
+
+} // namespace
diff --git a/llvm/unittests/IR/VFABIDemanglerTest.cpp b/llvm/unittests/IR/VFABIDemanglerTest.cpp
index 07bff16df49335..896cd48ad11d6d 100644
--- a/llvm/unittests/IR/VFABIDemanglerTest.cpp
+++ b/llvm/unittests/IR/VFABIDemanglerTest.cpp
@@ -753,6 +753,73 @@ TEST_F(VFABIParserTest, ParseVoidReturnTypeSVE) {
   EXPECT_EQ(VectorName, "vector_foo");
 }
 
+TEST_F(VFABIParserTest, ParseWideStructReturnTypeSVE) {
+  EXPECT_TRUE(
+      invokeParser("_ZGVsMxv_foo(vector_foo)", "{double, double}(float)"));
+  EXPECT_EQ(ISA, VFISAKind::SVE);
+  EXPECT_TRUE(isMasked());
+  FunctionType *FTy = FunctionType::get(
+      StructType::get(
+          VectorType::get(Type::getDoubleTy(Ctx), ElementCount::getScalable(2)),
+          VectorType::get(Type::getDoubleTy(Ctx),
+                          ElementCount::getScalable(2))),
+      {
+          VectorType::get(Type::getFloatTy(Ctx), ElementCount::getScalable(2)),
+          VectorType::get(Type::getInt1Ty(Ctx), ElementCount::getScalable(2)),
+      },
+      false);
+  EXPECT_EQ(getFunctionType(), FTy);
+  EXPECT_EQ(Parameters.size(), 2U);
+  EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
+  EXPECT_EQ(Parameters[1], VFParameter({1, VFParamKind::GlobalPredicate}));
+  EXPECT_EQ(VF, ElementCount::getScalable(2));
+  EXPECT_EQ(ScalarName, "foo");
+  EXPECT_EQ(VectorName, "vector_foo");
+}
+
+TEST_F(VFABIParserTest, ParseWideStructMixedReturnTypeSVE) {
+  EXPECT_TRUE(invokeParser("_ZGVsMxv_foo(vector_foo)", "{float, i64}(float)"));
+  EXPECT_EQ(ISA, VFISAKind::SVE);
+  EXPECT_TRUE(isMasked());
+  FunctionType *FTy = FunctionType::get(
+      StructType::get(
+          VectorType::get(Type::getFloatTy(Ctx), ElementCount::getScalable(2)),
+          VectorType::get(Type::getInt64Ty(Ctx), ElementCount::getScalable(2))),
+      {
+          VectorType::get(Type::getFloatTy(Ctx), ElementCount::getScalable(2)),
+          VectorType::get(Type::getInt1Ty(Ctx), ElementCount::getScalable(2)),
+      },
+      false);
+  EXPECT_EQ(getFunctionType(), FTy);
+  EXPECT_EQ(Parameters.size(), 2U);
+  EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
+  EXPECT_EQ(Parameters[1], VFParameter({1, VFParamKind::GlobalPredicate}));
+  EXPECT_EQ(VF, ElementCount::getScalable(2));
+  EXPECT_EQ(ScalarName, "foo");
+  EXPECT_EQ(VectorName, "vector_foo");
+}
+
+TEST_F(VFABIParserTest, ParseWideStructReturnTypeNEON) {
+  EXPECT_TRUE(
+      invokeParser("_ZGVnN2v_foo(vector_foo)", "{float, float}(float)"));
+  EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
+  EXPECT_FALSE(isMasked());
+  FunctionType *FTy = FunctionType::get(
+      StructType::get(
+          VectorType::get(Type::getFloatTy(Ctx), ElementCount::getFixed(2)),
+          VectorType::get(Type::getFloatTy(Ctx), ElementCount::getFixed(2))),
+      {
+          VectorType::get(Type::getFloatTy(Ctx), ElementCount::getFixed(2)),
+      },
+      false);
+  EXPECT_EQ(getFunctionType(), FTy);
+  EXPECT_EQ(Parameters.size(), 1U);
+  EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
+  EXPECT_EQ(VF, ElementCount::getFixed(2));
+  EXPECT_EQ(ScalarName, "foo");
+  EXPECT_EQ(VectorName, "vector_foo");
+}
+
 // Make sure we reject unsupported parameter types.
 TEST_F(VFABIParserTest, ParseUnsupportedElementTypeSVE) {
   EXPECT_FALSE(invokeParser("_ZGVsMxv_foo(vector_foo)", "void(i128)"));

MacDue added a commit to MacDue/llvm-project that referenced this pull request Dec 9, 2024
This is a split-off from llvm#109833 and only adds code relating to checking
if a struct-returning call can be vectorized.

This initial patch only allows the case where all users of the struct
return are `extractvalue` operations that can be widened.

```
%call = tail call { float, float } @foo(float %in_val) #0
%extract_a = extractvalue { float, float } %call, 0
%extract_b = extractvalue { float, float } %call, 1
```

Note: The tests require the VFABI changes from llvm#119000 to pass.
Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this only used by transforms or analysis also? It might be better placed as either transform or analysis utils, instead of in IR?

@MacDue
Copy link
Member Author

MacDue commented Dec 12, 2024

Is this only used by transforms or analysis also? It might be better placed as either transform or analysis utils, instead of in IR?

It's a circular dependency to place the helpers in transform or analysis (and use them in the VFABI) as both of those depend on core.

Copy link
Contributor

@david-arm david-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for taking so long to leave these comments. For some reason I thought I'd already left them!

/// equivalent to calling `ToVectorTy`. For struct types, this returns a new
/// struct where each element type has been widened to a vector type. Note: Only
/// unpacked literal struct types are supported.
Type *llvm::ToWideTy(Type *Ty, ElementCount EC) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth adding a note that for now the only strategy we support is one where we widen the individual struct members, but in theory it could be extended to other strategies, for example a struct { i32, i32 } with a given VF could return a vector of VF x 2 elements.

Copy link
Member Author

@MacDue MacDue Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear on how other strategies could work in practice here. If you widened struct { i32, i32 } to <4 x i32> where that's meant to represent the interleaved access of two structs, you hit the problem that now there's no defined mapping from <4 x i32> back to a narrow type. It could have come from a struct or a scalar i32.

Copy link
Contributor

@david-arm david-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking pretty good now to me now - thanks for addressing the comments. I just have a few more comments ...

Copy link
Contributor

@david-arm david-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! This seems reasonable to me, but please wait a day to see if @fhahn is happy with the new names.

@MacDue
Copy link
Member Author

MacDue commented Dec 17, 2024

I intend to land this in the morrow if there's no further comments 🙂

@paulwalker-arm
Copy link
Collaborator

paulwalker-arm commented Dec 17, 2024

Bike-shedding I know plus I've not had anything to do with this PR so don't intend to hold it up but is it possible to come up with a better name than VectorUtils?

The name is already in use and from what I can see the functionality of the new one has little to do with vectors other than VectorType. Perhaps VectorTypeUtils? This assumes there's good reason why these helpers cannot be part of DerivedTypes.h?

@MacDue
Copy link
Member Author

MacDue commented Dec 17, 2024

Bike-shedding I know plus I've not had anything to do with this PR so don't intend to hold it up but is it possible to come up with a better name than VectorUtils?

The name is already in use and from what I can see the functionality of the new one has little to do with vectors other than VectorType. Perhaps VectorTypeUtils? This assumes there's good reason why these helpers cannot be part of DerivedTypes.h?

I've moved the helpers to VectorTypeUtils 👍 I don't think these fit within DerivedTypes.h, which contains class definitions for types rather than free-floating helpers. Also, from a general compile-time perspective smaller more-focused headers are probably better.

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. It's probably better to keep it as separate header, to avoid including it in many places that don't need it

@MacDue MacDue merged commit 1ee740a into llvm:main Dec 18, 2024
8 checks passed
@MacDue
Copy link
Member Author

MacDue commented Dec 18, 2024

Thanks for the reviews :)

@MacDue MacDue deleted the vfabi_wide_structs branch December 18, 2024 09:47
MacDue added a commit to MacDue/llvm-project that referenced this pull request Dec 18, 2024
This is a split-off from llvm#109833 and only adds code relating to checking
if a struct-returning call can be vectorized.

This initial patch only allows the case where all users of the struct
return are `extractvalue` operations that can be widened.

```
%call = tail call { float, float } @foo(float %in_val) #0
%extract_a = extractvalue { float, float } %call, 0
%extract_b = extractvalue { float, float } %call, 1
```

Note: The tests require the VFABI changes from llvm#119000 to pass.
MacDue added a commit to MacDue/llvm-project that referenced this pull request Dec 19, 2024
This is a split-off from llvm#109833 and only adds code relating to checking
if a struct-returning call can be vectorized.

This initial patch only allows the case where all users of the struct
return are `extractvalue` operations that can be widened.

```
%call = tail call { float, float } @foo(float %in_val) #0
%extract_a = extractvalue { float, float } %call, 0
%extract_b = extractvalue { float, float } %call, 1
```

Note: The tests require the VFABI changes from llvm#119000 to pass.
MacDue added a commit to MacDue/llvm-project that referenced this pull request Jan 6, 2025
This is a split-off from llvm#109833 and only adds code relating to checking
if a struct-returning call can be vectorized.

This initial patch only allows the case where all users of the struct
return are `extractvalue` operations that can be widened.

```
%call = tail call { float, float } @foo(float %in_val) #0
%extract_a = extractvalue { float, float } %call, 0
%extract_b = extractvalue { float, float } %call, 1
```

Note: The tests require the VFABI changes from llvm#119000 to pass.
MacDue added a commit that referenced this pull request Jan 9, 2025
This is a split-off from #109833 and only adds code relating to checking
if a struct-returning call can be vectorized.

This initial patch only allows the case where all users of the struct
return are `extractvalue` operations that can be widened.

```
%call = tail call { float, float } @foo(float %in_val)
%extract_a = extractvalue { float, float } %call, 0
%extract_b = extractvalue { float, float } %call, 1
```

Note: The tests require the VFABI changes from #119000 to pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants