[Openmp-commits] [PATCH] D103090: [libomptarget][nfc][amdgpu] Factor out setting upper bounds

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:716
+    return true;
+  } else {
+    return false;
----------------
JonChesterfield wrote:
> dhruvachak wrote:
> > How about making the change that clang-tidy is suggesting here?
> > 
> > Another question: Any specific reason why you are not using std::min? Is it because emitting the DP becomes a bit cumbersome?
> Pretty much. min doesn't say whether it made a change or not, and we've tied debug printing to whether a change was made.
> 
> I can't think of a reason why else after a return would be considered bad. Will try google on that term.
> ...advises to reduce indentation where possible and where it makes understanding code easier. Early exit is one of the suggested enforcements of that. Please do not use else or else if after something that interrupts control flow

That seems uncompelling in this case, but it's trivial to shuffle the code to avoid the diagnostic. E.g. latest version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103090