[PATCH] D80464: [CUDA] Missing __syncthreads intrinsic in __clang_cuda_device_functions.h

2020-05-22 Thread Boris Staletic via Phabricator via cfe-commits
bstaletic created this revision.
bstaletic added a reviewer: aprantl.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.

Seems like the `__syncthreads` is missing from the 
`clang/lib/Headers/__clang_cuda_device_functions.h` file. To be honest, I don't 
know much about CUDA. This issue was noticed by a YouCompleteMe user who then 
made a pull request:

https://github.com/ycm-core/ycmd/pull/1438

I did not create any tests, because a similar patch did not include tests:

https://reviews.llvm.org/D43602


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80464

Files:
  clang/lib/Headers/__clang_cuda_device_functions.h


Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -519,6 +519,7 @@
   return __nv_fast_sincosf(__a, __s, __c);
 }
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }
 __DEVICE__ int __syncthreads_count(int __a) { return __nvvm_bar0_popc(__a); }
 __DEVICE__ int __syncthreads_or(int __a) { return __nvvm_bar0_or(__a); }


Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -519,6 +519,7 @@
   return __nv_fast_sincosf(__a, __s, __c);
 }
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }
 __DEVICE__ int __syncthreads_count(int __a) { return __nvvm_bar0_popc(__a); }
 __DEVICE__ int __syncthreads_or(int __a) { return __nvvm_bar0_or(__a); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D80464: [CUDA] Missing __syncthreads intrinsic in __clang_cuda_device_functions.h

2020-05-23 Thread Boris Staletic via Phabricator via cfe-commits
bstaletic marked an inline comment as done.
bstaletic added a comment.

This doesn't seem to actually compile:

  In file included from :1:
  In file included from 
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/clang/test/Headers/../../lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:29:
  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/lib/clang/11.0.0/include/__clang_cuda_device_functions.h:522:16:
 error: static declaration of '__syncthreads' follows non-static declaration
  __DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
 ^
  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/lib/clang/11.0.0/include/__clang_cuda_device_functions.h:522:16:
 note: previous implicit declaration is here
  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/lib/clang/11.0.0/include/__clang_cuda_device_functions.h:522:45:
 error: use of undeclared identifier '__nvvm_bar0'
  __DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
  ^

Looks like `__nvvm_bar0` is not declared/defined anywhere. When grepping, 
compared to `__nvvm_bar0_and`, these two are missing:

  llvm/include/llvm/IR/IntrinsicsNVVM.td
  1034:  def int_nvvm_barrier0_and : GCCBuiltin<"__nvvm_bar0_and">,
  
  clang/include/clang/Basic/BuiltinsNVPTX.def
  408:BUILTIN(__nvvm_bar0_and, "ii", "")

Should I add `BUILTIN(__nvvm_bar0, "v", "")` to `BuiltinsNVPTX.def` and 
whatever needs to be added to the `IntrinsicsNVVM.td`?




Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:522
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }

This doesn't seem to actually compile:

```
In file included from :1:
In file included from 
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/clang/test/Headers/../../lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:29:
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/lib/clang/11.0.0/include/__clang_cuda_device_functions.h:522:16:
 error: static declaration of '__syncthreads' follows non-static declaration
__DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
   ^
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/lib/clang/11.0.0/include/__clang_cuda_device_functions.h:522:16:
 note: previous implicit declaration is here
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/lib/clang/11.0.0/include/__clang_cuda_device_functions.h:522:45:
 error: use of undeclared identifier '__nvvm_bar0'
__DEVICE__ int __syncthreads(void) { return __nvvm_bar0(); }
^
```

Looks like `__nvvm_bar0` is not declared/defined anywhere. When grepping, 
compared to `__nvvm_bar0_and(int)`, these two are missing:

```
llvm/include/llvm/IR/IntrinsicsNVVM.td
1034:  def int_nvvm_barrier0_and : GCCBuiltin<"__nvvm_bar0_and">,
clang/include/clang/Basic/BuiltinsNVPTX.def
408:BUILTIN(__nvvm_bar0_and, "ii", "")
```

Should I add `BUILTIN(__nvvm_bar0, "v", "")` to `BuiltinsNVPTX.def` and 
whatever needs to be added to the `IntrinsicsNVVM.td`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80464/new/

https://reviews.llvm.org/D80464



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90599: Fix a leak in `ASTUnit::LoadFromCommandLine()` wehn compiler invocation fails

2020-11-07 Thread Boris Staletic via Phabricator via cfe-commits
bstaletic updated this revision to Diff 303624.
bstaletic added a comment.

Fixed the code formatting error.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90599/new/

https://reviews.llvm.org/D90599

Files:
  clang/lib/Frontend/ASTUnit.cpp


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1757,8 +1757,11 @@
 
 CI = createInvocationFromCommandLine(
 llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
-if (!CI)
+if (!CI) {
+  for (const auto &RF : RemappedFiles)
+delete RF.second;
   return nullptr;
+}
   }
 
   // Override any files that need remapping


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1757,8 +1757,11 @@
 
 CI = createInvocationFromCommandLine(
 llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
-if (!CI)
+if (!CI) {
+  for (const auto &RF : RemappedFiles)
+delete RF.second;
   return nullptr;
+}
   }
 
   // Override any files that need remapping
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90599: Fix a leak in `ASTUnit::LoadFromCommandLine()` wehn compiler invocation fails

2020-11-02 Thread Boris Staletic via Phabricator via cfe-commits
bstaletic created this revision.
bstaletic added a reviewer: aprantl.
bstaletic added projects: clang, clang-c.
Herald added a subscriber: cfe-commits.
bstaletic requested review of this revision.

1. Here 

 we allocate buffers to hold the contents of unsaved files.
2. The `RemappedFiles` vector is then passed to 
`ASTUnit::LoadFromCommandLine()` as an `ArrayRef`, here 

3. In ASTUnit::LoadFromCommandLine() 

 we try to make a compiler invocation and exit early if that fails 
.
4. If we did exit early from that function, both Unit and UnitErr are nullptr 
,
 making the function exit without ever cleaning up the memory allocations from 
step 1.

Notes:

1. The function allocating all those `RemappedFile` objects is not actually the 
same as the one deallocating, in the happy case.

1.1. I did not figure out where exactly does it get deleted, because the above 
seems to be enough data for now.

2. My attempted solution is to iterate over remapped files and deallocate in 
this block 
,
 but it seems odd to deallocate through `ArrayRef`.

Fixes https://bugs.llvm.org/show_bug.cgi?id=47832


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90599

Files:
  clang/lib/Frontend/ASTUnit.cpp


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1758,7 +1758,11 @@
 CI = createInvocationFromCommandLine(
 llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
 if (!CI)
+{
+  for (const auto &RF : RemappedFiles)
+delete RF.second;
   return nullptr;
+}
   }
 
   // Override any files that need remapping


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1758,7 +1758,11 @@
 CI = createInvocationFromCommandLine(
 llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
 if (!CI)
+{
+  for (const auto &RF : RemappedFiles)
+delete RF.second;
   return nullptr;
+}
   }
 
   // Override any files that need remapping
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits