This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Scalarize gather/scatter on RV64 with Zve32* extension.
ClosedPublic

Authored by craig.topper on Jun 6 2022, 9:01 PM.

Details

Summary

i64 indices aren't supported on Zve32*. Scalarize gathers to prevent
generating illegal instructions.

Since InstCombine will aggressively canonicalize GEP indices to
pointer size, we're pretty much always going to have an i64 index.

Trying to predict when SelectionDAG will find a smaller index from
the TTI hook used by the ScalarizeMaskedMemIntrinPass seems fragile.
To optimize this we probably need an IR pass to rewrite it earlier.

Test RUN lines have also been added to make sure the strided load/store
optimization still works.

Diff Detail

Event Timeline

craig.topper created this revision.Jun 6 2022, 9:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 9:01 PM
craig.topper requested review of this revision.Jun 6 2022, 9:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 9:01 PM
reames accepted this revision.Jun 7 2022, 7:37 AM

LGTM. I'm fine with this landing, regardless of the answer to the following.

Having said that, given we really don't support zvl32b, should we bother? I'm tempted to simply document the lack of support instead, and defer this until someone cares. Having it half supported, half not seems worse than explicitly not supported.

This revision is now accepted and ready to land.Jun 7 2022, 7:37 AM

LGTM. I'm fine with this landing, regardless of the answer to the following.

Having said that, given we really don't support zvl32b, should we bother? I'm tempted to simply document the lack of support instead, and defer this until someone cares. Having it half supported, half not seems worse than explicitly not supported.

This patch allows Zve32f or Zve32x with Zvl64b or higher. Meaning elements limited to 32 bits and total vector size >= 64.

reames added a comment.Jun 7 2022, 7:52 AM

LGTM. I'm fine with this landing, regardless of the answer to the following.

Having said that, given we really don't support zvl32b, should we bother? I'm tempted to simply document the lack of support instead, and defer this until someone cares. Having it half supported, half not seems worse than explicitly not supported.

This patch allows Zve32f or Zve32x with Zvl64b or higher. Meaning elements limited to 32 bits and total vector size >= 64.

Ah, so I misunderstand the context.

Still LGTM.

The number of combinations we have here is absolutely ridiculous.

This revision was landed with ongoing or failed builds.Jun 7 2022, 8:18 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/CodeGen/RISCV/rvv/fixed-vector-strided-load-store.ll