Allow us to generate sext/zext masked loads which can access v4i8, v8i8 and v4i16 memory to produce v4i32, v8i16 and v4i32 respectively.
My little (endian) brain only really works in that mode, so I'm dubious about the big endian support here.
Paths
| Differential D68337
[ARM][MVE] Enable extending masked loads ClosedPublic Authored by samparker on Oct 2 2019, 7:24 AM.
Details Summary Allow us to generate sext/zext masked loads which can access v4i8, v8i8 and v4i16 memory to produce v4i32, v8i16 and v4i32 respectively. My little (endian) brain only really works in that mode, so I'm dubious about the big endian support here.
Diff Detail Event TimelineComment Actions Nice one! Looks very useful. I _think_ bigendian should be fine here, so long as we don't use the wrong type. There are a lot of other masked load tests in mve-masked-load.ll. I think we should add the same for widening loads and narrowing stores. There should be tests for things like align1 and different passthru values. We might want extra tests for odd types too, if we are making them legal through isLegalMaskedLoad.
Comment Actions Thanks for those points, I'll add loads more tests.
Comment Actions
samparker added a parent revision: D68400: [NFC][TTI] Add Alignment for isLegalMasked[Load/Store].Oct 4 2019, 1:50 AM
samparker added inline comments.
Comment Actions Now handling the a bitcast passthru value in LowerMLOAD. Corrected the half load addr values. Comment Actions Nice. I think this is looking good, just some details to sort out, like what to do about the target independent parts. We will presumably want to add the pre and post inc to these in the future too, which will probably bring up the same kinds of questions.
samparker added inline comments.
samparker added inline comments.
samparker added inline comments.
Comment Actions
Comment Actions @craig.topper This patch currently causes an isel failure for pr35443.ll when an v4i8 masked load is being zero extended into an v4i64. I know nothing about AVX, could you please advise whether this operation is supported or how to address the issue? Thanks.
Comment Actions
Its supported. We need to add isel patterns for all the extend flavors. Or we need to block masked loads in isVectorLoadExtDesirable on X86 for now. Comment Actions Thanks @craig.topper. I'll add the necessary changes into the X86 backend.
Comment Actions
Comment Actions LGTM. If Craig is happy with the rest.
This revision is now accepted and ready to land.Oct 16 2019, 5:49 AM Closed by commit rG39af8a3a3b66: [DAGCombine][ARM] Enable extending masked loads (authored by samparker). · Explain WhyOct 17 2019, 12:58 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 223172 lib/CodeGen/SelectionDAG/DAGCombiner.cpp
lib/Target/ARM/ARMInstrMVE.td
lib/Target/ARM/ARMTargetTransformInfo.cpp
test/CodeGen/Thumb2/LowOverheadLoops/mve-tail-data-types.ll
test/CodeGen/Thumb2/mve-masked-ldst.ll
test/CodeGen/Thumb2/mve-masked-load.ll
|
Is it true that whenever you have a legal extending load, you will also have the equivalent legal extending masked load? (For MVE we do, but is that true for all archs?)
Do we need to add an extra set of flags for this? Or is isVectorLoadExtDesirable good enough to handle these cases when there is an asymmetry?