This is an archive of the discontinued LLVM Phabricator instance.

[SVE][CodeGen] Replace uses of TypeSize comparison operators with calls to isKnownXY
ClosedPublic

Authored by david-arm on Oct 1 2020, 6:58 AM.

Details

Summary

In certain places in the code we can never end up in a situation where
we're mixing fixed width and scalable vector types. For example,
we can't have truncations and extends that change the lane count. In
such cases it's perfectly fine to replace the comparison operators
with calls to the equivalent TypeSize::isKnownXY functions. The answer
should always be known at compile time. Also, in other places such as
GenWidenVectorStores and GenWidenVectorLoads we know from the behaviour
of FindMemType that we can never choose a vector type with a different
scalable property.

In a couple of places I have used the EVT::bitsLT function instead, where it
probably makes sense to keep an assert that scalable properties match.

Diff Detail

Event Timeline

david-arm created this revision.Oct 1 2020, 6:58 AM
Herald added a project: Restricted Project. · View Herald Transcript
david-arm requested review of this revision.Oct 1 2020, 6:58 AM

I guess most of my comments relate to minimising direct uses of TypeSize.

llvm/include/llvm/CodeGen/BasicTTIImpl.h
949

Just a thought but if you get the EVT for Src then you could use bitsLT and then the comment is not needed as it'll assert as much.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
5031–5032

Perhaps worth breaking this out into separate blocks. e.g.

LdStVT = LDST->getMemoryVT();

//Don't convert between fixed length and scalable types.
if (LdStVT.isScalable() != MemVT.isScalable())
  return false;

if (LdStVT.bitsLT(MemVT))
......

What do you think?

11550

I got bored scrolling up but I believe this function is visitTRUNCATE and thus N0 has to have the same number of lanes as VT and thus bitsLT can be used?

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1825

Extends have to have the same number of lanes so I think this case can be converted to compare the size of the vector elements and thus use SrcVT.getScalarSizeInBits() * 2 < DestVT.getScalarSizeInBits().

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
674–675

My reading of this is that we're trying to create a scalar that's as big as the vector ValueVT. We don't support scalable scalar types and thus I'm wondering if the above assert can just use getFixedSizeInBits?

david-arm updated this revision to Diff 298567.Oct 16 2020, 1:54 AM
paulwalker-arm accepted this revision.Oct 16 2020, 4:42 AM
This revision is now accepted and ready to land.Oct 16 2020, 4:42 AM