This is an archive of the discontinued LLVM Phabricator instance.

[HLSL][clang][Driver] Support validator version command line option.
ClosedPublic

Authored by python3kgae on Apr 15 2022, 4:19 PM.

Details

Summary

The DXIL validator version option(/validator-version) decide the validator version when compile hlsl.
The format is major.minor like 1.0.

In normal case, the value of validator version should be got from DXIL validator. Before we got DXIL validator ready for llvm/main, DXIL validator version option is added first to set validator version.

It will affect code generation for DXIL, so it is treated as a code gen option.

A new member std::string DxilValidatorVersion is added to clang::CodeGenOptions.

Then CGHLSLRuntime is added to clang::CodeGenModule.
It is used to translate clang::CodeGenOptions::DxilValidatorVersion into a ModuleFlag under key "dx.valver" at end of clang code generation.

Diff Detail

Event Timeline

python3kgae created this revision.Apr 15 2022, 4:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2022, 4:19 PM
python3kgae requested review of this revision.Apr 15 2022, 4:19 PM
python3kgae edited subscribers, added: beanz; removed: mgorny, arphaman, kuter and 6 others.
MaskRay added inline comments.Apr 15 2022, 4:28 PM
clang/include/clang/Driver/Options.td
6739

delete

6749

Remove period from the last sentence of HelpText.
Use just one space for sentence separator. See other options for the prevailing style stating the format.

I see many dxil related options have violated this and I am going to fix them soon.

clang/lib/CodeGen/CodeGenModule.cpp
843
llvm/test/Transforms/OpenMP/icv_tracking.ll
2 ↗(On Diff #423183)

opt -S -foo tests the new PM as well due to EnableNewPassManager.

The old style (inherited from legacy pass manager) run lines are being phased out. Don't add new one.

Fix format in Options.td.

nikic resigned from this revision.Apr 19 2022, 12:51 AM
python3kgae marked an inline comment as done.

Use c_str instead of data to avoid test fail.

beanz added inline comments.Apr 26 2022, 12:48 PM
clang/include/clang/Basic/CodeGenOptions.h
194 ↗(On Diff #424810)

Rather than adding this to CodeGenOptions, it may make sense to put this in TargetOptions instead as it relates specifically to the DXIL target.

clang/lib/CodeGen/CGHLSLRuntime.h
39

Make sure to setup your code editor to add newlines to the end of files, this does turn into a compiler warning because ISO C requires it :)

clang/unittests/Driver/ToolChainTest.cpp
615

Can you share this struct definition with the one in the other test case?

703

Probably worth having some test cases for completely bogus things like -validator-version blah, or -validator-version -some-other-flag.

Fix issues mentioned in comments.

beanz added a comment.Apr 26 2022, 3:09 PM

I pointed out a few (not all) the places where you have unneeded brackets. Also all your new files don't have newlines at the end of them (the C standard specifies that as a requirement although pretty much all compilers just issue a warning).

clang/lib/CodeGen/CodeGenModule.cpp
843

As @MaskRay said you don't need the bracket here.

clang/lib/Driver/ToolChains/Clang.cpp
3478

Omitting the bracket around the for makes this even more confusing... please omit the brackets around the if too.

6242

no brackets here either

clang/lib/Driver/ToolChains/HLSL.cpp
171

no bracket here

clang/test/CodeGenHLSL/validator_version.hlsl
11

no bracket here

clang/unittests/Driver/ToolChainTest.cpp
536

here too.

Remove useless brackets.

beanz retitled this revision from [HLSL][clang][Driver] Support target profile command line option. to [HLSL][clang][Driver] Support validator version command line option..Apr 28 2022, 7:58 AM
beanz accepted this revision.Apr 29 2022, 3:27 PM

LGTM.

This revision is now accepted and ready to land.Apr 29 2022, 3:27 PM

NO change, fix UNKNOWN REVISION when arc land.

This revision was landed with ongoing or failed builds.Apr 29 2022, 4:49 PM
This revision was automatically updated to reflect the committed changes.