[PATCH] D102515: [CostModel] Return an invalid cost for memory ops with unsupported types

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen requested changes to this revision.
sdesmalen added a comment.
This revision now requires changes to proceed.

Hi @kmclaughlin, I'm requesting changes to this patch because after a closer look I believe we can/should rely on the CodeGenerator's information on how to legalize a vector type in order to know how to cost it. The `[AArch64]TargetTransformInfo::get*MemoryCost` implementations actually already do this. For example, a `<vscale x 8 x i32>` for SVE  will need splitting into 2 x `<vscale x 4 x i32>`, and so the operation cost would be 2 x the cost of a memory operation on a legal type. For scalable vector types that need scalarization, we know the cost is Invalid, because the code-generator is unable to handle these types. In practice, this requires:

- Fixing `getTypeConversion` to return `TypeScalarizeScalableVector` when a scalable vector type cannot be legalized by widening/splitting.
- Returning an Invalid cost from `getTypeLegalizationCost` when the method of legalization is `TypeScalarizeScalableVector`.
- In the AArch64TTI methods that return the cost of memory operations, we already call `getTypeLegalizationCost`, so all that needs doing is returning Invalid.

The method `isElementTypeLegalForScalableVector` is more something to add in D102253 <https://reviews.llvm.org/D102253> where it is called by the LV //before// it has picked a VF, to know if there is //any// VF which can handle these types. If so, then it must be possible to legalize the types by splitting or widening and so the CostModel must be able to cost them.



================
Comment at: llvm/lib/CodeGen/TargetLoweringBase.cpp:998
       if (VT.getVectorElementCount() == ElementCount::getScalable(1))
-        report_fatal_error("Cannot legalize this scalable vector");
+        return LegalizeKind(TypeScalarizeScalableVector, EltVT);
       return LegalizeKind(TypeSplitVector,
----------------
kmclaughlin wrote:
> david-arm wrote:
> > Hi @kmclaughlin is it possible to add a test for this? For example, just another test for <vscale x 1 x i128>.
> Hi @david-arm, I've changed the `@store_nxvi128` test to use `vscale x 1 x i128`
This comment will become redundant when you address my other comment, but I found that when removing this change, the tests still passed because the calls to `isElementTypeLegalForScalableVector` return before calling the `getTypeLegalizationCost` code (which ends up in this function).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102515/new/

https://reviews.llvm.org/D102515