[llvm] [Scalarizer][DirectX] support structs return types (PR #111569)

Farzon Lotfi via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 20:17:17 PDT 2024


================
@@ -667,14 +686,26 @@ bool ScalarizerVisitor::splitBinary(Instruction &I, const Splitter &Split) {
 bool ScalarizerVisitor::isTriviallyScalarizable(Intrinsic::ID ID) {
   if (isTriviallyVectorizable(ID))
     return true;
+  // TODO: Move frexp to isTriviallyVectorizable.
+  // https://github.com/llvm/llvm-project/issues/112408
+  switch (ID) {
+  case Intrinsic::frexp:
+    return true;
+  }
   return Intrinsic::isTargetIntrinsic(ID) &&
          TTI->isTargetIntrinsicTriviallyScalarizable(ID);
 }
 
 /// If a call to a vector typed intrinsic function, split into a scalar call per
 /// element if possible for the intrinsic.
 bool ScalarizerVisitor::splitCall(CallInst &CI) {
-  std::optional<VectorSplit> VS = getVectorSplit(CI.getType());
+  Type *CallType = CI.getType();
+  bool AreAllMatchingVectors = isStructOfMatchingFixedVectors(CallType);
+  std::optional<VectorSplit> VS;
+  if (AreAllMatchingVectors)
+    VS = getVectorSplit(CallType->getContainedType(0));
----------------
farzonl wrote:

If I'm being honest  intrinsic behaviors determined by `isVectorIntrinsicWithOverloadTypeAtArg`,  `isVectorIntrinsicWithScalarOpAtArg` and `isTriviallyVectorizable` Is kind of awful.  Adding a new one of these to handle struct return types isn't ideal. For one it means every new intrinsic with struct returns has to get added to some obscure function to get properly scalarized and in the future vectorized. Maybe thats better because it makes scalarization and in the future vectorization more explicit? Personally not a fan.

I'm also not thrilled about having to add a new TTI  api. Its a bunch of boiler plate to express something very simple. Especially since the split double case doesn't need it because the returned struct of vectors are all the same type. 

Second, the fragility IMO is overstated. The code as it is captures the pattern that exist in the current set of llvm intrinsics that represent structs by their component parts.

Third i don't agree with this being a heuristic. That implies the behavior is  best guessing at what the next overload type should be when we are being very determinstic here.

Maybe should talk about  this a bit more. If we still want to do the compromise one area to discuss is  how we can do this without a TTI API for now. I'm thinking maybe the `isStructOfMatchingFixedVectors` api could also check element type and if they are all the same we  just use the existing code path of:
```cpp
 if (isVectorIntrinsicWithOverloadTypeAtArg(ID, -1))
    Tys.push_back(VS->SplitTy);
```
Which is the path splitdouble is currently taking.


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


More information about the llvm-commits mailing list