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

[Daniil Fukalov via Phabricator via llvm-commits]

dfukalov added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringBase.cpp:1826-1828
+    if (LK.first == TypeScalarizeScalableVector)
+      return std::make_pair(InstructionCost::getInvalid(), MVT::getVT(Ty));
+
----------------
sdesmalen wrote:
> kmclaughlin wrote:
> > dfukalov wrote:
> > > My observation was that here less destructive for test is to just `Cost.setInvalid()` instead of return 'empty' invalid value. So the function continues to return the same numeric value but with `invalid` flag. It will create less impact on the current flow.
> > > 
> > > Most of operations with `InstructionCost` are not aware of invalid flag. I guess it might be be next separated step to stop loop here and just return invalid value and then gather all side effects of 'changed' cost numeric value.
> > > 
> > > Btw my D102915, D103407 and D103406 are preparation to return invalid cost flag from the function and to reduce impact of the change.
> > Hi @dfukalov, thanks for the suggestion. I have updated this to instead set the cost to Invalid where the kind is TypeScalarizeScalableVector for now.
> The value of Invalid is irrelevant when the Invalid flag is set. In fact, retrieving the value is not possible since `InstructionCost::getValue()` returns an `llvm::Optional`. Because there is nothing the code below can do to change the invalid cost to a valid cost, there's no reason not to break out of the loop early by returning `std::make_pair(InstructionCost::getInvalid(), ..)`.
Actually almost all operations on `InstructionCost` ignore invalid flag at the moment. I don't insist, just suggested to split the changes to smaller steps to reduce future side effects of a commit.


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

https://reviews.llvm.org/D102515