This is an archive of the discontinued LLVM Phabricator instance.

[X86] Improve selection of the mov instruction in FrameLowering
ClosedPublic

Authored by erikdesjardins on Dec 31 2021, 9:12 AM.

Details

Summary

MOV64ri results in a significantly longer encoding, and use of this
operator is fairly avoidable as we can always check the size of the
immediate we're using.

This is an updated version of D99045.

Co-authored-by: Simonas Kazlauskas <git@kazlauskas.me>

Diff Detail

Event Timeline

erikdesjardins created this revision.Dec 31 2021, 9:12 AM
erikdesjardins requested review of this revision.Dec 31 2021, 9:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 31 2021, 9:12 AM

restart build after adding parent rev

craig.topper added inline comments.Jan 2 2022, 10:59 PM
llvm/lib/Target/X86/X86FrameLowering.cpp
152

This is Is64Bit not IsLP64 right?

pengfei added inline comments.Jan 3 2022, 12:57 AM
llvm/lib/Target/X86/X86FrameLowering.cpp
155

What's the difference to ues it rather than MOV32ri?

craig.topper added inline comments.Jan 3 2022, 1:00 AM
llvm/lib/Target/X86/X86FrameLowering.cpp
155

MOV32ri64 has GR64 as a regclass but otherwise is the same encoding. MOV32ri would require a SUBREG_TO_REG to convert the register class.

IsLP64 -> Use64BitReg

llvm/lib/Target/X86/X86FrameLowering.cpp
152

It depends on the callsite--one uses 32bit when Is64Bit && !IsLP64. I agree IsLP64 is inaccurate though.

I'll rename to Use64BitReg for now, let me know if you have a better suggestion.

craig.topper accepted this revision.Jan 3 2022, 9:47 AM

LGTM

llvm/lib/Target/X86/X86FrameLowering.cpp
152

That works. I guess I didn't scroll down enough to notice that some callers use Is64Bit and some use LP64.

This revision is now accepted and ready to land.Jan 3 2022, 9:47 AM

Thanks!

I don't have commit access, please commit this (and D116420, which autogens the tests) on my behalf (Erik Desjardins <erikdesjardinspublic@gmail.com>).

Note that the go bindings tests are failing on CI, but I don't think these changes could cause that. (They also fail on D116420, which just autogens x86 tests.)
ninja check-all passes locally.

This revision was landed with ongoing or failed builds.Jan 3 2022, 11:13 AM
This revision was automatically updated to reflect the committed changes.