[Openmp-commits] [PATCH] D76843: [Openmp] Libomptarget plugin for NEC SX-Aurora

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added inline comments.


================
Comment at: openmp/libomptarget/plugins/ve/src/rtl.cpp:298
+      }
+    } else {
+      proc_handle = veo_proc_create_static(ID, tmp_name);
----------------
simoll wrote:
> manorom wrote:
> > simoll wrote:
> > > else after return
> > I don't think so: If the process handle for a dynamic process is created successfully, we do not return but also do not want to visit the else branch.
> This is simply a matter of coding style. You can rewrite the `else` to
> 
> ```
> if (!is_dyn) {
> }
> ```
> 
> and it is clear under which condition it will be executed.
This would repeat the condition on `is_dyn` which is less desirable IMHO. I agree with @manorom here: The else is not on the branch with the `return`, which is handling a possible error (ie `!proc_handle`). The code is properly indented, so at least to me it's obvious what's happening here:

```lang=c
if (...) {
  // veo_proc_create
  if (error) {
    // log
    return NULL;
  }
  // success
} else {
  // veo_proc_create_static
  if (error) {
    // log
    return NULL;
  }
}


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

https://reviews.llvm.org/D76843