This is an archive of the discontinued LLVM Phabricator instance.

AArch64: Change modeling of zero cycle zeroing.
ClosedPublic

Authored by MatzeB on Jun 28 2016, 6:23 PM.

Details

Summary

AArch64: Change modeling of zero cycle zeroing.

On CPUs with the zero cycle zeroing feature enabled "movi v.2d" should
be used to zero a vector register. This was previously done at
instruction selection time, however the register coalescer sometimes
widened multiple vregs to the Q width because of that leading to extra
spills. This patch leaves the decision on how to zero a register to the
AsmPrinter phase where it doesn't affect register allocation anymore.

This patch also sets isAsCheapAsAMove=1 on FMOVS0, FMOVD0.

This fixes http://llvm.org/PR27454, rdar://25866262

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 62161.Jun 28 2016, 6:23 PM
MatzeB retitled this revision from to AArch64: Change modeling of zero cycle zeroing..
MatzeB updated this object.
MatzeB added reviewers: mcrosier, t.p.northover.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.
rovka added a subscriber: rovka.Jun 28 2016, 11:59 PM
rovka added inline comments.
lib/Target/AArch64/AArch64AsmPrinter.cpp
432 ↗(On Diff #62161)

Shouldn't this be -AArch64::D0?

t.p.northover added inline comments.Jun 29 2016, 7:43 AM
lib/Target/AArch64/AArch64AsmPrinter.cpp
423–424 ↗(On Diff #62161)

I'm not sure I like the asymmetry here: ZCZ code knows and emits exactly what it wants, NoZCZ code gets passed its opcodes and operands. I'd be happy with either resolution though.

mcrosier edited edge metadata.Jun 29 2016, 8:22 AM

This looks reasonable to me..

I've asked @haicheng to review/investigate as well to make sure he's no longer seeing the spill code.

lib/Target/AArch64/AArch64AsmPrinter.cpp
432 ↗(On Diff #62161)

Yes, I believe @rovka is correct.

I've asked @haicheng to review/investigate as well to make sure he's no longer seeing the spill code.

I tried this patch and it is good. Thank you.

MatzeB marked an inline comment as done.Jun 29 2016, 1:45 PM
MatzeB added inline comments.
lib/Target/AArch64/AArch64AsmPrinter.cpp
432 ↗(On Diff #62161)

Indeed, good catch.

MatzeB updated this revision to Diff 62281.Jun 29 2016, 1:46 PM
MatzeB edited edge metadata.

Fix register translation, be "less smart" in the emit code to make the ZCZ/NonZCZ cases look more idiomatic/symmetric.

LGTM, but I'll defer to @t.p.northover for final approval.

lib/Target/AArch64/AArch64AsmPrinter.cpp
441 ↗(On Diff #62281)

Minor nit: the default case generally goes at the top of switch.

mcrosier accepted this revision.Jul 6 2016, 2:19 PM
mcrosier edited edge metadata.

This looks in reasonable shape, IMO. If Tim or others have concerns we can address them post-commit.

This revision is now accepted and ready to land.Jul 6 2016, 2:19 PM
This revision was automatically updated to reflect the committed changes.