This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Alternate attempt to optimize sign-extended EXTRACT_VECTOR_ELT nodes.
AbandonedPublic

Authored by craig.topper on Feb 2 2021, 12:07 PM.

Details

Reviewers
frasercrmck
Summary

This is an alternate version of D95741 which defers the conversion
of extract_vector_elt to VMV_X_S until LegalDAG. This allows
it to be available to DAG combines for longer. To make this work
I've taught DAGCombiner to remove the SRA/SHL equivalent of
sext_inreg when the input to the pair is already sign extended.

If we decide to go this direction, I can split out the DAGCombiner
and Mips/atomic.ll change into a separate patch.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 2 2021, 12:07 PM
craig.topper requested review of this revision.Feb 2 2021, 12:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2021, 12:07 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
jrtc27 added inline comments.Feb 2 2021, 12:12 PM
llvm/test/CodeGen/Mips/atomic.ll
4300

Strange that this one didn't change

craig.topper added inline comments.Feb 2 2021, 12:42 PM
llvm/test/CodeGen/Mips/atomic.ll
4300

AtomicCmpSwap8 uses ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS which is expanded by SelectionDAGLegalize::ExpandNode to a sequence that includes an AssertSExt.

AtomicSwap8 just uses ISD::ATOMIC_SWAP and does not go through any expanding. A custom inserter creates ATOMIC_SWAP_I8_POSTRA and then that is expanded by MipsExpandPseudo.cpp. Only after that do the earlier shifts appear.

frasercrmck accepted this revision.Feb 3 2021, 3:17 AM

Cheers. I'm happy for you to split the DAGCombiner bits off into a separate patch. I can update mine as required.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
8371

Ah, excellent: so this is what I was missing. I'm surprised this wasn't already there.

This revision is now accepted and ready to land.Feb 3 2021, 3:17 AM
craig.topper abandoned this revision.Feb 4 2021, 1:46 PM

The DAGCombiner change was commited separately and D95741 has been reworked on top of it.