[PATCH] D115646: [DAG][TLI][X86][ARM][AArch64] Add `isExtractSubvectorFree` / use it in `foldExtractSubvectorFromShuffleVector()`

[David Greene via Phabricator via llvm-commits]

greened added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:20795
+              // and that replacing one non-cheap subvector extract
+              // with two cheap ones is a win.
+  };
----------------
lebedev.ri wrote:
> greened wrote:
> > The values here make it seem like replacing a non-cheap operation with two cheap ones yields the same performance (1+1 = 2). contrary to the comment.  Is this intentional?
> Not only does it make seem so, that is exactly what the comment is saying.
It's not what the comment is saying.  The comment says "a win" which to my mind means, "performance improvement."  If the cost is the same, where's the improvement?  Maybe the comment is confusing me.  This is used to calculate `Budget` which I guess limits how many extracts we can do (so two at most?).  Perhaps the comment could be expanded to explain a bit more about what is going on.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12899
+  return isExtractSubvectorFree(ResVT, SrcVT, Index);
+}
+
----------------
lebedev.ri wrote:
> greened wrote:
> > So here, it seems like even if the extract is "free" (cost 0) it'll be counted as "cheap" (cost 1).
> > 
> > I wonder if it would be better to combine these functions into a single "getExtractSubvectorCost" target hook.
> > So here, it seems like even if the extract is "free" (cost 0) it'll be counted as "cheap" (cost 1).
> 
> Right. Because even free extractions can be done for cheap, if that makes sense.
> 
> > I wonder if it would be better to combine these functions into a single "getExtractSubvectorCost" target hook.
> 
> We could do that, there's already `*TTIImpl::getShuffleCost()`, the problem being, what will be the "cheap budget",
> i.e. how should the uses of `isExtractSubvectorCheap()` be adjusted?
>Right. Because even free extractions can be done for cheap, if that makes sense.

In some contexts I suppose.  A "free" extract is "cheap" if all you care about is whether an extract is "cheap."  A "free" extract is not "cheap" if you care about the relative cost of the two.  I could see the above code being very confusing in some circumstances.

> how should the uses of isExtractSubvectorCheap() be adjusted?

Not knowing all of the users of that function, I can't say.  It's probably context-dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115646