This is an archive of the discontinued LLVM Phabricator instance.

[X86] Support -march=raptorlake, meteorlake
ClosedPublic

Authored by FreddyYe on Oct 13 2022, 8:19 PM.

Diff Detail

Event Timeline

FreddyYe created this revision.Oct 13 2022, 8:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 13 2022, 8:19 PM
FreddyYe requested review of this revision.Oct 13 2022, 8:19 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptOct 13 2022, 8:19 PM
Herald added subscribers: llvm-commits, Restricted Project, cfe-commits. · View Herald Transcript
craig.topper added inline comments.
clang/test/CodeGen/target-builtin-noerror.c
131

I think this list was once in alphabetical order

compiler-rt/lib/builtins/cpu_model.c
110 ↗(On Diff #467672)

The raptorlake patch also puts a constant here. What is the correct order? Can you please rebase one patch on top of the other or merge them into a single patch.

RKSimon added inline comments.Oct 14 2022, 1:26 AM
llvm/lib/Target/X86/X86.td
1445

This should be with the alderlake/raptorlake defs below (and use ADL model/features/tuning)

RKSimon added a comment.EditedOct 14 2022, 2:02 AM
This comment has been deleted.
FreddyYe updated this revision to Diff 468387.Oct 17 2022, 6:35 PM

Merge raptorlake patch and address comments.

FreddyYe marked 3 inline comments as done.Oct 17 2022, 6:37 PM

THX for review! Gcc is also recently reviewing related patches. So to align with the compiler-rt and libgcc, let's wait for their land first.

Please can you update the summary now that raptorlake + meteorlake are in the same patch?

clang/test/Preprocessor/predefined-arch-macros.c
2245

(pedantic) Probably better to put these after the alderlake tests so its easier to find?

FreddyYe retitled this revision from [X86] Support -march=meteorlake to [X86] Support -march=raptorlake, meteorlake.Oct 18 2022, 5:31 AM
Matt added a subscriber: Matt.Oct 19 2022, 5:01 PM
RKSimon added inline comments.Oct 20 2022, 7:35 AM
clang/test/Driver/x86-march.c
128

Move these after alderlake instead of the old atom cores?

skan added inline comments.Oct 20 2022, 6:57 PM
compiler-rt/lib/builtins/cpu_model.c
110 ↗(On Diff #468387)

typo ? (ZHAOXIN_FAM7H_LUJIAZUI)

FreddyYe marked an inline comment as done.Oct 20 2022, 7:06 PM
FreddyYe added inline comments.
compiler-rt/lib/builtins/cpu_model.c
110 ↗(On Diff #468387)

I think not. That is to keep aligned with libgcc for compatibility by Craig's catch on raptorlake patch. See https://github.com/gcc-mirror/gcc/blob/master/gcc/common/config/i386/i386-cpuinfo.h#L94

We know gcc is also recently modify here. To wait for their landing and then aligning with them, I'll convert to draft for this patch.

FreddyYe retitled this revision from [X86] Support -march=raptorlake, meteorlake to [WIP][X86] Support -march=raptorlake, meteorlake.Oct 20 2022, 7:07 PM
MaskRay added inline comments.
clang/test/Driver/x86-march.c
128

And use --target= for new tests. -target is legacy.

compiler-rt/lib/builtins/cpu_model.c
478 ↗(On Diff #468387)

fallthrough?

llvm/docs/ReleaseNotes.rst
143

-mcpu=xxx and -mcpu=yyy are now supported.

llvm/lib/Support/Host.cpp
843

fallthrough?

skan added inline comments.Oct 21 2022, 2:04 AM
compiler-rt/lib/builtins/cpu_model.c
111 ↗(On Diff #468387)

I see. But if possible, could we split "ZHAOXIN_FAM7H_LUJIAZUI" to another patch?

FreddyYe updated this revision to Diff 471938.Oct 31 2022, 2:29 AM
FreddyYe marked an inline comment as done.

Rebase.

FreddyYe updated this revision to Diff 472203.Oct 31 2022, 7:23 PM
FreddyYe marked 7 inline comments as done.

Address comments and update to align with gcc. See my latest comments.

FreddyYe retitled this revision from [WIP][X86] Support -march=raptorlake, meteorlake to [X86] Support -march=raptorlake, meteorlake.Oct 31 2022, 7:23 PM
FreddyYe added a comment.EditedOct 31 2022, 7:26 PM

For saving capacity of ProcessorSubtypes, gcc decided to not support part of compiler features of these two cpus:

__builtin_cpu_is("meteorlake")
__attribute__((target("arch=raptorlake")))
... some others I don't know.

Gcc's related patch:
raptorlake and meteorlake
Updated to align with gcc first. Welcome opinions and review!

clang/test/Preprocessor/predefined-arch-macros.c
2245

I merged them with the check-prefix.

compiler-rt/lib/builtins/cpu_model.c
111 ↗(On Diff #468387)

Related change is removed. See my latest comment.

478 ↗(On Diff #468387)

Good catch! While related changes are removed, see my latest comment.

llvm/lib/Support/Host.cpp
843

Good catch!

FreddyYe added a comment.EditedNov 1 2022, 1:23 AM

For saving capacity of ProcessorSubtypes, gcc decided to not support part of compiler features of these two cpus:

__builtin_cpu_is("meteorlake")
__attribute__((target("arch=raptorlake")))
... some others I don't know.

Gcc's related patch:
raptorlake and meteorlake
Updated to align with gcc first. Welcome opinions and review!

GCC may supported these two features. I'm confirming... Stay tuned. Sorry for noise.

MaskRay accepted this revision.Nov 1 2022, 2:41 PM
MaskRay added inline comments.
clang/test/Driver/x86-march.c
91

You can just write --target=x86_64. Then the line is short enough that you do not need wrapping. Not necessary to follow other lines.

This revision is now accepted and ready to land.Nov 1 2022, 2:41 PM
FreddyYe updated this revision to Diff 472496.Nov 1 2022, 8:35 PM
FreddyYe marked an inline comment as done.

Add X86_CPU_SUBTYPE_ALIAS and address comments.

For saving capacity of ProcessorSubtypes, gcc decided to not support part of compiler features of these two cpus:

__builtin_cpu_is("meteorlake")
__attribute__((target("arch=raptorlake")))
... some others I don't know.

Gcc's related patch:
raptorlake and meteorlake
Updated to align with gcc first. Welcome opinions and review!

GCC may supported these two features. I'm confirming... Stay tuned. Sorry for noise.

Confirmed they still supported these two features. So I added X86_CPU_SUBTYPE_ALIAS to support so. Pls review.

FreddyYe requested review of this revision.Nov 1 2022, 10:28 PM
pengfei added inline comments.Nov 1 2022, 10:46 PM
clang/lib/CodeGen/CGBuiltin.cpp
12942–12943

Mo difference with X86_CPU_SUBTYPE?

FreddyYe updated this revision to Diff 472822.Nov 2 2022, 6:17 PM
FreddyYe marked an inline comment as done.

Address comments.

clang/lib/CodeGen/CGBuiltin.cpp
12942–12943

Good catch!

pengfei accepted this revision.Nov 2 2022, 11:30 PM

LGTM.

This revision is now accepted and ready to land.Nov 2 2022, 11:30 PM
skan accepted this revision.Nov 2 2022, 11:49 PM
This revision was landed with ongoing or failed builds.Nov 3 2022, 7:06 PM
This revision was automatically updated to reflect the committed changes.