Page MenuHomePhabricator

[X86] NFC: Convert LLVM command-line flag to target attribute
AbandonedPublic

Authored by davezarzycki on Oct 28 2019, 1:52 AM.

Details

Summary

Swift and perhaps other users of LLVM do not leave cl::opt style command-line option parsing enabled due to C++ global constructor overhead, therefore let's convert the VZEROUPPER option to a target attribute.

Furthermore, this change stops the conflation of VZEROUPPER emission with enabling/disabling fast partial YMM/ZMM updates. The compiler doesn't intentionally emit partial updates at the moment, but it could.

This patch also verifies that clang can pass arbitrary target features.

Diff Detail

Event Timeline

davezarzycki created this revision.Oct 28 2019, 1:52 AM

Add a comment to the ReleaseNotes X86 section?

lib/Target/X86/X86.td
316

Just say "some X86 processors" like above.

davezarzycki edited the summary of this revision. (Show Details)

Addressed recent feedback.

davezarzycki marked an inline comment as done.Oct 31 2019, 2:08 AM

Ping. If it helps, I can move the remaining few uses of cl::opt by the x86 backend to code gen attributes for consistency.

Doesn’t this leave the FastPartialWriteFlag unused?

With this change there is no way to disable the pass from the clang command line.

A brief google search suggests Swift has a -Xllvm option to reach the llvm command line parser or did at one time.

A brief google search suggests Swift has a -Xllvm option to reach the llvm command line parser or did at one time.

Here you go: https://forums.swift.org/t/micro-architectural-tuning/28269/3

Quote: "-Xllvm flags are compiled out of Release builds, since they impact startup time."

Doesn’t this leave the FastPartialWriteFlag unused?

With this change there is no way to disable the pass from the clang command line.

Yes, this leaves FeatureFastPartialYMMorZMMWrite unused. I looked into wiring up some code gen to use the flag, but I got distracted.

Your right about the clang behavior. I'll need to update the patch.

davezarzycki edited the summary of this revision. (Show Details)

This update also verifies that clang can pass arbitrary target features.

I still don’t understand why we’re not just renaming the existing flag? What are you planning to connect the old flag to?

I think I’d like to see the existing feature renamed and leave the command line alone. I’m not sure -target-feature can be reached from the clang driver rather than cc1. I’ll work on adding -mno-vzeroupper to clang to match gcc and then we can remove the command line.

I still don’t understand why we’re not just renaming the existing flag? What are you planning to connect the old flag to?

I didn't remove it because it seemed like information would be "lost". Said differently, I didn't know that some AMD CPUs have fast partial updates too.

I made a quick attempt at using the "fast partial" flag so that it wouldn't be left unused, but I got distracted. (For example using SSE move instructions instead of VINSERT*)

And yes, the -target-feature flag can be reached from the driver via -Xclang.

arsenm added a subscriber: arsenm.Nov 1 2019, 9:57 AM

Terminology needs fixing. This is adding a subtarget feature, not a target attribute which would be a different thing

davezarzycki abandoned this revision.Nov 3 2019, 11:56 PM

Closed in favor of D69786