Page MenuHomePhabricator

[Aarch64][SVE] Add DAG combine rules for gather loads and sext/zext

Authored by andwar on Nov 28 2019, 4:05 AM.



These changes allow us to support sign-extending gather loads with the existing intrinsics (i.e. @llvm.aarch64.sve.ld1.gather.*).

Diff Detail

Event Timeline

andwar created this revision.Nov 28 2019, 4:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 28 2019, 4:05 AM
sdesmalen added inline comments.Dec 2 2019, 5:53 AM

AArch64Subtarget *Subtarget isn't used by performSVEAndCombine, so you can remove this change.


The condition Src.hasOneUse() is not in the SVEAndCombine, so you probably want to add one there as well.
Also please make sure both cases have a unit test.

andwar updated this revision to Diff 232297.Dec 5 2019, 3:39 AM
andwar marked an inline comment as done.
  • Apply suggestions from @sdesmalen (e.g. add !Src.hasOneUse() in performANDCombine)
  • Removed a bunch of setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::nxv2i64, Legal), which are not needed for this patch
  • Simplified performSignExtendInRegCombine
  • Added patterns for sext_inreg (required for the new tests vvvv)
  • Added tests that verify that the new DAG Combine rules are not used when the result of gather load has multiple uses
andwar updated this revision to Diff 232365.Dec 5 2019, 9:06 AM

Remove a typo after rebase (uimm5s2 vs uim5s4)

sdesmalen accepted this revision.Dec 10 2019, 5:53 AM

Aside from two nits, LGTM!


nit: Are these asserts necessary?


nit: Can you make this a switch statement with a default that returns SDValue()?

This revision is now accepted and ready to land.Dec 10 2019, 5:53 AM
This revision was automatically updated to reflect the committed changes.