This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeTypes] Legalize vector rotate operations
ClosedPublic

Authored by rsanthir.quic on Oct 15 2020, 2:26 PM.

Details

Summary

Lower vector rotate operations as long as the legalization occurs outside of LegalizeVectorOps.

This fixes https://bugs.llvm.org/show_bug.cgi?id=47320

Diff Detail

Event Timeline

rsanthir.quic created this revision.Oct 15 2020, 2:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2020, 2:26 PM
rsanthir.quic requested review of this revision.Oct 15 2020, 2:26 PM
RKSimon added inline comments.Oct 16 2020, 5:10 AM
llvm/include/llvm/CodeGen/TargetLowering.h
4318

Maybe call it AllowVectorOps ?

Add missing doxygen param.

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
6293

clang-format this

rsanthir.quic marked an inline comment as done.

Changed the name or the param and added appropriate formatting

updated usage of param

craig.topper added inline comments.Oct 16 2020, 3:40 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
3517

Can you add a /*AllowVectorOps*/ comment to the true argument here and at the other call sites to help with readability?

rsanthir.quic marked an inline comment as done.

Added comment for readability

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
3517

Yes!

dmgreen added inline comments.
llvm/test/CodeGen/AArch64/expand-vector-rot.ll
2

Can you run update_llc_test_checks on this file.

RKSimon added inline comments.Oct 18 2020, 5:55 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
6293

You need to add the VT.isVector() check as well

rsanthir.quic marked 3 inline comments as done.

Ran update_llc_test_checks.py and added missing isVector check

RKSimon accepted this revision.Oct 19 2020, 1:32 PM

LGTM

This revision is now accepted and ready to land.Oct 19 2020, 1:32 PM

@dmgreen Are you happy for this to be committed?

dmgreen accepted this revision.Oct 22 2020, 5:27 AM

Oh yeah, sure. I'm happy if you were happy. Not crashing is certainly better than crashing.
LGTM.

@rsanthir.quic Are you OK to get this committed now?

Yes! If you could commit it for me that would be great! Thanks

This revision was automatically updated to reflect the committed changes.