This is an archive of the discontinued LLVM Phabricator instance.

[HLSL] Enable vector types for hlsl.
ClosedPublic

Authored by python3kgae on May 5 2022, 3:34 PM.

Details

Summary

Vector types in hlsl is using clang ext_vector_type.
Declaration of vector types is in builtin header hlsl.h.
hlsl.h will be included by default for hlsl shader.

Diff Detail

Event Timeline

python3kgae created this revision.May 5 2022, 3:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2022, 3:34 PM
python3kgae requested review of this revision.May 5 2022, 3:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2022, 3:34 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Anastasia added inline comments.May 24 2022, 7:22 AM
clang/lib/Basic/LangOptions.cpp
121

Is this header expected to be large? You might want to flag up in the description of the review and the comments in the header itself what content is expected to be there.

If the file is expected to be large it might make sense to add a flag that would disable this include. You can then for example use bare clang without this header for all the tests that don't require functionality from the header to reduce the testing time.

clang/test/CodeGenHLSL/basic_types.hlsl
2

Technically mapping into IR types might be target specific, so passing the triple is necessary for this test to work correctly. Alternatively you can switch to AST dump checking, however even that might be target specific in some cases.

python3kgae added inline comments.May 24 2022, 5:44 PM
clang/lib/Basic/LangOptions.cpp
121

It might be large when more things are added.
I'll add an option to disable the include.

clang/test/CodeGenHLSL/basic_types.hlsl
2

The option -T lib_6_7 decides the triple.

"-Tlib_6_7 -Fo -" will be translated into
"-cc1" "-triple" "dxil--shadermodel6.7-library" "-o" "-" "-x" "hlsl" for cc1.

Add hlsl-no-stdinc to not include the hlsl header.

Anastasia added inline comments.May 26 2022, 10:45 AM
clang/lib/Driver/ToolChains/Clang.cpp
3490

Sharing command line options is great, let's just update the help message straight away that this flag now also applies to HLSL: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L6053

Update comment for finclude_default_header.

Add newline at end of file

Anastasia accepted this revision.May 30 2022, 7:50 AM

LGTM! Thanks

Please make sure to reflect in your commit message that you are adding the clang internal headers too.

This revision is now accepted and ready to land.May 30 2022, 7:50 AM
This revision was landed with ongoing or failed builds.May 30 2022, 9:05 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.May 30 2022, 10:02 AM

Looks like this breaks tests on Mac/arm: http://45.33.8.238/macm1/36297/step_7.txt

Please take a look and revert for now if it takes a while to fix. (Maybe the test just needs an explicit triple?)

Hi thakis,
The issue could be fixed by https://reviews.llvm.org/D125585
Could you take a look?
Thanks

python3kgae reopened this revision.May 31 2022, 1:53 PM

Reopen for the Mac/arm test fail.

This revision is now accepted and ready to land.May 31 2022, 1:53 PM

Recover after fix Mac/arm test issue.

This revision was landed with ongoing or failed builds.May 31 2022, 1:54 PM
This revision was automatically updated to reflect the committed changes.

Hi, this patch causes an issue with the CMake Xcode build configuration, if I try to use xcode as the generator with CMake, using the build command:

xcrun cmake -G Xcode -DCMAKE_BUILD_TYPE:STRING=Debug -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE -DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DLLVM_ENABLE_PROJECTS='clang;' -DCMAKE_IGNORE_PATH="/usr/lib;/usr/local/lib;/lib" -DLLVM_TARGETS_TO_BUILD="X86;ARM;AArch64" ../llvm

I get:

CMake Error in /Users/shubham/Development/llvm-project/clang/lib/Headers/CMakeLists.txt:
  The custom command generating

    /Users/shubham/Development/llvm-project/build/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)/lib/clang/15.0.0/include/hlsl.h

  is attached to multiple targets:

    hlsl-resource-headers
    clang-resource-headers

  but none of these is a common dependency of the other(s).  This is not
  allowed by the Xcode "new build system".


CMake Error in /Users/shubham/Development/llvm-project/third-party/benchmark/CMakeLists.txt:
  The custom command generating

    /Users/shubham/Development/llvm-project/build/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)/lib/clang/15.0.0/include/hlsl.h

  is attached to multiple targets:

    hlsl-resource-headers
    clang-resource-headers

  but none of these is a common dependency of the other(s).  This is not
  allowed by the Xcode "new build system".


CMake Generate step failed.  Build files cannot be regenerated correctly.

This is similar to the issue that was found in this patch: https://reviews.llvm.org/D123498

Hi Xiang,

I would be more than happy to test the fix for you :). Though it is 11:45 pm for me now and I am going to bed. But I can take a look tomorrow.

Thanks,

Shubham