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

[SYCL][E2E] Initialize arch_flag to an empty string by default #16200

Open
wants to merge 2 commits into
base: sycl
Choose a base branch
from

Conversation

ayylol
Copy link
Contributor

@ayylol ayylol commented Nov 27, 2024

In an environment with no devices this variable is never set, causing a confusing error when we try to define the clangxx expansion. Here we set it to an empty string by default so that instead of exiting with a python error we report all tests as unsupported (due to having no devices).

This same issue is why arch_flag was set in build-only mode, so I removed the line that did that too.

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, btw what was the confusing error?

@ayylol
Copy link
Contributor Author

ayylol commented Nov 27, 2024

lgtm, btw what was the confusing error?

an error due to arch_flag not being defined

llvm-lit: /home/llvm/llvm/utils/lit/lit/TestingConfig.py:156: fatal: unable to parse config file '/home/llvm/sycl/test-e2e/lit.cfg.py', traceback: Traceback (most recent call last):
  File "/home/llvm/llvm/utils/lit/lit/TestingConfig.py", line 144, in load_from_path
    exec(compile(data, path, "exec"), cfg_globals, None)
  File "/home/llvm/sycl/test-e2e/lit.cfg.py", line 819, in <module>
    " " + config.dpcpp_compiler + " " + config.cxx_flags + " " + arch_flag,
NameError: name 'arch_flag' is not defined

Very niche case, since it can only happen if we have no devices, which is why it needed to be set explicitly for build-only

@sarnex
Copy link
Contributor

sarnex commented Nov 27, 2024

cool thx

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