Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for passing defines list to ispc rules, also added linu… #2

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

parvit
Copy link

@parvit parvit commented Mar 15, 2023

Part of the fix for Vertexwahn/rules_oidn#1

In this change the ispc_cc_library method is changed to receive an additional collection of strings that are joined in a series of define command line switches for the ispc binary.

eg.

  1. passing ["OIDN_DNNL"] produces "-DOIDN_DNNL"
  2. passing ["OIDN_DNNL","OIDN_FILTER_RT"] produces "-DOIDN_DNNL -DOIDN_FILTER_RT"

Included also "--target-os=linux" for simmetry with the other platform definitions.

@Vertexwahn Vertexwahn merged commit bdedbc6 into Vertexwahn:main Mar 15, 2023
@Vertexwahn
Copy link
Owner

@parvit Just seeing that the merge broke my unit test :( - would be nice if the defines would be optional and there would be a test, that makes sure defines work....

@parvit
Copy link
Author

parvit commented Mar 15, 2023 via email

@Vertexwahn
Copy link
Owner

@Vertexwahn
Copy link
Owner

@parvit

git clone https://github.com/Vertexwahn/rules_ispc.git
cd rules_ispc
cd tests
bazel build //square:main
bazel run //square:main

-> should be "green" == should compile without errors

@parvit
Copy link
Author

parvit commented Mar 15, 2023 via email

@Vertexwahn
Copy link
Owner

Vertexwahn commented Mar 15, 2023

@parvit It is not a test in the sense of a unit test - its a test in the sense of "does it compile without errors" - bazel run //square:main will run square demo - this does not have any defines - currently it will fail with errors since the defines argument is not provided
There should be also a second "demo" in the test folder that expect a define - just to make sure that handing over defines works currently and in the future

@parvit
Copy link
Author

parvit commented Mar 15, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants