[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Frank Derry Wanye via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 29 11:47:11 PDT 2021


ffrankies marked an inline comment as done.
ffrankies added a comment.

@Eugene.Zelenko @aaron.ballman Are there any more changes that need to be made to this check or comments that need to be addressed?



================
Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:200
+  if (isa<DoStmt>(Loop))
+    return DO_LOOP; // loop_type = 0;
+  else if (isa<WhileStmt>(Loop))
----------------
Eugene.Zelenko wrote:
> Is loop ID is not enough? Why does comment with numerical code used? Same below.
Removed the comment with numerical code (I simply added it to avoid having to check the header file to see their numerical value). The LoopType is used in the diagnostics to select and emit the correct loop type as part of the diagnostic message, e.g.: 
```
diag(CondExpr->getBeginLoc(),
        "backward branch (%select{do|while|for}0 loop) is ID-dependent due "
        "to ID function call and may cause performance degradation")
           << Type;
```
I'm assuming the loop ID is a unique object identifier, so I don't think it'll serve the same purpose.


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

https://reviews.llvm.org/D70094



More information about the cfe-commits mailing list