This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add patterns for using movss/movsd for atomic load/store of f32/64. Remove atomic fadd pseudos use isel patterns instead.
ClosedPublic

Authored by craig.topper on Apr 7 2019, 9:31 PM.

Details

Summary

This patch adds patterns for turning bitcasted atomic load/store into movss/sd.

It also removes the pseudo instructions for atomic RMW fadd. Instead just adding isel patterns for folding an atomic load into addss/sd. And relying on the new movss/sd store pattern to handle the write part.

This also makes the fadd patterns use VEX and EVEX instructions when AVX or AVX512F are enabled.

Event Timeline

craig.topper created this revision.Apr 7 2019, 9:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2019, 9:31 PM
RKSimon added inline comments.Apr 9 2019, 1:17 AM
llvm/test/CodeGen/X86/atomic-fp.ll
8

Maybe add SSE1 only tests?

147

Can we improve the i686 i64/f64 codegen at all?

craig.topper marked an inline comment as done.Apr 9 2019, 12:10 PM
craig.topper added inline comments.
llvm/test/CodeGen/X86/atomic-fp.ll
147

Yeah we need to add something like a v2i64 VEXTRACT_STORE node and turn the atomic store into that then select that to the MOVQ store. Maybe some more work depending on how the (v2i64 (scalar_to_vector (i64 (bitcast f64)))) that implies gets legalized and cleaned up

This only work if the atomic store isn't seq_cst. seq_cst either requires the locked cmpxchg8b that we have now or we need an mfence after the movq. gcc and icc both use the movq+mfence(or a locked or to stack if mfence isn't available) I think.

Add SSE1 test RUN lines.
Remove some FIXMEs and fix a comment X86InstrCompiler.td

Rebase after pre-commiting the SSE1 command lines

RKSimon accepted this revision.Apr 11 2019, 2:56 AM

LGTM

This revision is now accepted and ready to land.Apr 11 2019, 2:56 AM
This revision was automatically updated to reflect the committed changes.

I wonder if instead of matching bitcasts, implementing TODOs (hooks) form here https://reviews.llvm.org/D15471 was considered.