This is an archive of the discontinued LLVM Phabricator instance.

[X86] Lower used `(atomicrmw xor p, SignBit)` as `(atomicrmw add p, SignBit)`
ClosedPublic

Authored by goldstein.w.n on May 2 2023, 1:48 PM.

Details

Summary

(xor X, SignBit) == (add X, SignBit). For atomics whose result is
used, the add option is preferable because of the xadd instruction
which allows us to avoid either a CAS loop or a btc; setcc; shl.

Diff Detail

Unit TestsFailed

Event Timeline

goldstein.w.n created this revision.May 2 2023, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2023, 1:48 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
goldstein.w.n requested review of this revision.May 2 2023, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2023, 1:48 PM
RKSimon added inline comments.May 3 2023, 11:03 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
32257

While you're here - please can you add a m_MinSignedValue equivalent to the existing m_MaxSignedValue matcher?

RKSimon added inline comments.May 3 2023, 11:04 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
32257

Or just use m_SignMask?

goldstein.w.n marked an inline comment as done.

for m_SignMask()

llvm/lib/Target/X86/X86ISelLowering.cpp
32257

used m_SignMask. Can add m_MinSignedValue if you want, but prefer it in another patch (that I can then base this one on). LMK if you think it would be useful.

RKSimon accepted this revision.May 4 2023, 5:22 AM

LGTM

llvm/lib/Target/X86/X86ISelLowering.cpp
32257

m_SignMask is fine - in this case it matches the description better.

This revision is now accepted and ready to land.May 4 2023, 5:22 AM

Rebase with 32-bit tests coverage