llvm

llvm copied to clipboard

Describe the bug A truncated version of the sycl.hpp path works when it should not. It should only be able to find the header with sycl/sycl.hpp. Bug found when adding test to #6769.

To Reproduce Compile the below cpp file with the -fsycl option. This should not compile as the include path should be sycl/sycl.hpp, however it does successfully find the file path.

Build llvm as normal. Set environment paths

export PATH=$PATH_TO_LLVM/build/bin:$PATH
export LD_LIBRARY_PATH=$PATH_TO_LLVM/build/lib:$LD_LIBRARY_PATH

Create source file test_file.cpp

#include <sycl.hpp>

int main () {
  sycl::queue Q;

  return 0;
}

Compile file, this will compile when it should not be able to find the sycl.hpp path.

clang++ -fsycl test_file.cpp

Environment (please complete the following information):

• OS: Linux, Ubuntu
• Target device and vendor: Intel CPU/GPU
• DPC++ version: commit c7289d0b2d9ba40a5797d169dc15c575d92a1535

@intel/dpcpp-clang-driver-reviewers, I suppose clang driver adds additional include paths or system paths, where SYCL headers is found w/o additional directory prefix. Could you take a look, please?

By default, the driver will add bin/../include/sycl as a search directory. This has basically always been in place. Should this no longer be the case?

I see following: "-internal-isystem" "bin/../include/sycl" "-internal-isystem" "bin/../include"

I think we added "bin/../include/sycl" to enable #include <CL/sycl.hpp>, but it has a side effect, which described in this issue. To fix we probably should deploy CL directory to include rather than include/sycl and after that we can remove "-internal-isystem" "bin/../include/sycl".

It would be interesting to have a test checking that if we have a user-provided sycl.hpp #include <sycl.hpp> or `#include "sycl.hpp" can include this file.

It would be interesting to have a test checking that if we have a user-provided sycl.hpp #include <sycl.hpp> or `#include "sycl.hpp" can include this file.

This sounds like a basic functionality and I'm 100% sure it's already covered by existing tests. I think it user-provided sycl.hpp is located in system paths or user also provided -I <path-to-user-provided-sycl.hpp>, then there will be no issues.

My point is that only include directory should be added by default and adding include/sycl might cause unexpected side effect like including a wrong header.

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days.

AFAIK this issue has not yet been address, so I have removed the Stale label.