rules_nixpkgs icon indicating copy to clipboard operation
rules_nixpkgs copied to clipboard

CXX_FLAGS should allow for overload

Open dmadisetti opened this issue 3 years ago • 4 comments

Describe the bug Explicit definition of -x c++ means I cannot use cc_* to compile cuda with clang... https://github.com/tweag/rules_nixpkgs/blob/master/toolchains/cc/cc.nix#L164 https://github.com/tweag/rules_nixpkgs/blob/0d40e42ef8f2369638ee240cb81f1e6cab62a48d/toolchains/cc/cc.nix#L164

I don't see a way overloading this, and I don't think the explicit definition is needed - since compilers generally can figure that out by themselves. Currently just have a patch file that removes this line

To Reproduce

nixpkgs_cc_configure(                                                                                                                                                                                                             
    name = "nixpkgs_config_cc",                                                                                                                                                                                                   
    nix_file_content = """                                                                                                                                                                                                        
      with import <nixpkgs> { config = {allowUnfree = true; }; overlays = []; }; buildEnv {                                                                                                                                       
        name = "bazel-cc-toolchain";                                                                                                                                                                                              
        paths = [ clang_13 cudatoolkit_11 ];                                                                                                                                                                                      
        buildInputs = [  ];                                                                                                                                                                                                       
      }                                                                                                                                                                                                                           
    """,                                                                                                                                                                                                                          
    repository = "@nixpkgs",                                                                                                                                                                                                      
)

and try compiling some cuda (example: https://gist.github.com/dpiponi/1502434) basically the same as clang++ <cuda specific flags> -x c++ example.cu, which will break since syntax unrecognized and c++ takes precedent.

Environment NixOS unstable on master

dmadisetti avatar Apr 12 '22 21:04 dmadisetti

I don't have an answer, maybe it's a clang change? https://github.com/tweag/rules_nixpkgs/commit/6d6fc1b608a79ac2036688d640c3d13c68e158f2

I can investigate the earliest clang version that's compatible.

dmadisetti avatar Apr 12 '22 22:04 dmadisetti

See https://github.com/tweag/rules_nixpkgs/pull/128#issuecomment-729835453 and https://github.com/tweag/rules_nixpkgs/pull/128#issuecomment-732941362 for why the -x c++ flag is necessary.

I don't think the explicit definition is needed - since compilers generally can figure that out by themselves.

In short, the compilers, at least as provided by nixpkgs, figure this out based on how they're called. E.g. if called as gcc it's C and if called as g++ it's C++. But, Bazel doesn't make this distinction, it always calls the cc binary provided by the toolchain, meaning this detection doesn't work. To work around this we pass -x c++. But, as it's only included in CXX_FLAGS Bazel shouldn't pass that flag if it's compiling code that is not considered C++. E.g. it shouldn't pass that flag on sources ending on .c. Perhaps there's a way to instruct cc_library that the given code is not C++, but C in your use-case?

aherrmann avatar Apr 13 '22 07:04 aherrmann

Unfortunately, Bazel is regarding .cu files as C++: https://github.com/bazelbuild/bazel/blob/394ddb82b311ea7edbe2522736b0b0202903ddb6/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java#L30-L34

  // .cu and .cl are CUDA and OpenCL source extensions, respectively. They are expected to only be
  // supported with clang. Bazel is not officially supporting these targets, and the extensions are
  // listed only as long as they work with the existing C++ actions.
  public static final FileType CPP_SOURCE =
      FileType.of(".cc", ".cpp", ".cxx", ".c++", ".C", ".cu", ".cl");

So, it will add the cxx_flags to the compiler call when the source file has a .cu extension.

I don't know much about CUDA. Is CUDA code C++ or C? Maybe this needs to be fixed in Bazel then?

avdv avatar May 02 '22 14:05 avdv

cuda is cuda (there is some specific syntax that makes gpu specific code not valid c/c++), but it builds upon c++. It does requires an extra layer to compile correctly. clang++ can detect it automatically, but you can explicitly pass in -x cuda. Passing in -x c++ overrides the auto-detect and any -x cuda that I can pass in.

I do have this working locally without the flag- but I also understand if this is a hard stop, since cuda isn't properly supported by bazel to begin with

dmadisetti avatar May 03 '22 06:05 dmadisetti