[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