[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