[PATCH] D70010: [OpenMP][Offloading] Replaced default stream with an actual per-device unblocking stream in NVPTX implementation

George Rokos via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 8 13:11:37 PST 2019


grokos added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:260
+    // Destroy streams
+    for (auto &stream : Streams)
+      if (stream) {
----------------
jdoerfert wrote:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > 1. Use real type instead of `auto`
> > > > 2. Variables must start with the upper case letter.
> > > Sure but just to keep align with existing code. Do I need to update existing code as well?
> > In a separate patch, please
> You can go either way but I would just keep it as the surrounding code for now.
> Making the code blend in is arguably a good think and we have not decided how we handle the plugins in the short term (wrt. coding standard).
> 
> @JonChesterfield @grokos What do you think? Should we run once over the plugin and adjust the coding style now or keep it consistent for the time being?
I would say let's keep it consistent. Later on, we can adjust the code style for the whole library but until then I prefer consistency over mixed styles.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D70010





More information about the llvm-commits mailing list