This is an archive of the discontinued LLVM Phabricator instance.

[X86][1/2] SUPPORT RAO-INT
ClosedPublic

Authored by pengfei on Oct 14 2022, 1:32 AM.

Details

Summary

For more details about these instructions, please refer to the latest ISE document: https://www.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html

Initial authored by Liu Chen (@LiuChen3)

Diff Detail

Event Timeline

pengfei created this revision.Oct 14 2022, 1:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 14 2022, 1:32 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
pengfei requested review of this revision.Oct 14 2022, 1:32 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 14 2022, 1:32 AM
pengfei updated this revision to Diff 468020.Oct 15 2022, 8:23 AM

Add atomic operations lowering for RAO-INT instructions.

pengfei retitled this revision from [X86][WIP] SUPPORT RAO-INT to [X86] SUPPORT RAO-INT.Oct 15 2022, 8:29 AM
pengfei edited the summary of this revision. (Show Details)
pengfei added reviewers: RKSimon, LuoYuanke, FreddyYe.
pengfei edited the summary of this revision. (Show Details)
pengfei added a subscriber: LiuChen3.
RKSimon added inline comments.Oct 15 2022, 10:08 AM
llvm/lib/Target/X86/X86ISelLowering.h
801 ↗(On Diff #468020)

very pedantic, but are these likely to get confused with ROR / RAND instructions? Would it be better to use a RAO_ prefix?

llvm/test/CodeGen/X86/atomic-instructions-32.ll
5 ↗(On Diff #468020)

Is the -O0 necessary?

craig.topper added inline comments.
llvm/lib/Target/X86/X86.td
259

Why do these require SSE2?

llvm/lib/Target/X86/X86ISelLowering.h
801 ↗(On Diff #468020)

Why not AADD etc to match the instruction names?

craig.topper added inline comments.Oct 15 2022, 10:38 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
31799 ↗(On Diff #468020)

This should probably be in a separate patch.

pengfei updated this revision to Diff 468049.Oct 15 2022, 8:07 PM
pengfei marked an inline comment as done.

Split atomic operations lowering into D136032.

pengfei added inline comments.Oct 15 2022, 8:09 PM
llvm/lib/Target/X86/X86.td
259

We need mfence instructions for strong orders. The mfence feature relies on SSE2.
I see your concern, we may need split these features from SSE2. Filed an issue https://github.com/llvm/llvm-project/issues/58388

llvm/lib/Target/X86/X86ISelLowering.h
801 ↗(On Diff #468020)

Good ideas, thanks!

llvm/test/CodeGen/X86/atomic-instructions-32.ll
5 ↗(On Diff #468020)

I missed that, thanks!

pengfei retitled this revision from [X86] SUPPORT RAO-INT to [X86][1/2] SUPPORT RAO-INT.Oct 15 2022, 8:09 PM
craig.topper added inline comments.Oct 15 2022, 8:39 PM
llvm/lib/Target/X86/X86.td
259

The lowering code for atomics could check SSE2 in addition to the RAO-INT feature. My primary concern was that the RAO-INT feature itself shouldn't require it.

Are there going to be intrinsics for RAO-INT?

pengfei added inline comments.Oct 15 2022, 8:51 PM
llvm/lib/Target/X86/X86.td
259

I don't think there will be a target supports RAO-INT but no SSE2. All I can think out is for Kernel build that disable vector registers intentionally.

Are there going to be intrinsics for RAO-INT?

This is a good question. In fact we wander whether providing intrinsics or not. The current preference is not. We don't want to add new intrinsics arbitrarily, there're burdens to maintenance and it's always easy to adding per to actual requests than removing afterwards.

Matt added a subscriber: Matt.Oct 19 2022, 5:13 PM
pengfei updated this revision to Diff 469642.Oct 21 2022, 9:03 AM

Add intrinsic support first.

FreddyYe added inline comments.Oct 26 2022, 10:54 PM
clang/docs/ReleaseNotes.rst
540

Add bullets for supported intrinsics.

clang/test/CodeGen/X86/raoint-builtins.c
2 ↗(On Diff #469642)

32 bit test coverage.

llvm/lib/Target/X86/X86InstrRAOINT.td
10

RAOINT

pengfei updated this revision to Diff 471040.Oct 27 2022, 12:37 AM

Address review comments; Change intrinsics name from *_si* to *_i*.

RKSimon accepted this revision.Oct 27 2022, 1:55 AM

LGTM

This revision is now accepted and ready to land.Oct 27 2022, 1:55 AM
This revision was landed with ongoing or failed builds.Oct 27 2022, 2:47 AM
This revision was automatically updated to reflect the committed changes.