[PATCH] D145321: [clang-tidy] altera-id-dependent-backward-branch: fix direct assignment notes

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
yeputons-gh requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Previously a note was always issued on a variable declaration itself. Now it
points to the actual assignment of `get_local_id` to the variable, like with
fields.

Depends on D145305 <https://reviews.llvm.org/D145305>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145321

Files:
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===================================================================
--- 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -34,6 +34,15 @@
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
   // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable 
ThreadID
 
+  int ThreadIDAssigned = 0;
+  ThreadIDAssigned = get_local_id(0) * 2;
+
+  for (int i = 0; i < ThreadIDAssigned; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadIDAssigned' and may cause 
performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable 
ThreadIDAssigned
+    accumulator++;
+  }
+
   struct { int IDDepField; } Example;
   Example.IDDepField = get_local_id(0);
 
@@ -107,6 +116,13 @@
     accumulator++;
   } while (j < NotThreadID);
 
+  int NotThreadIDAssigned = 0;
+  NotThreadIDAssigned = foo(0) * 2;
+
+  for (int i = 0; i < NotThreadIDAssigned; i++) {
+    accumulator++;
+  }
+
   struct { int NotIDDepField; } Example;
   Example.NotIDDepField = foo(0);
 
Index: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -114,7 +114,7 @@
                                                   const VarDecl *Variable) {
   // Record that this variable is thread-dependent.
   IdDepVarsMap[Variable] =
-      IdDependencyRecord(Variable, Variable->getBeginLoc(),
+      IdDependencyRecord(Variable, Statement->getBeginLoc(),
                          Twine("assignment of ID-dependent variable ") +
                              Variable->getNameAsString());
 }



Index: clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -34,6 +34,15 @@
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
   // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable ThreadID
 
+  int ThreadIDAssigned = 0;
+  ThreadIDAssigned = get_local_id(0) * 2;
+
+  for (int i = 0; i < ThreadIDAssigned; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDAssigned' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable ThreadIDAssigned
+    accumulator++;
+  }
+
   struct { int IDDepField; } Example;
   Example.IDDepField = get_local_id(0);
 
@@ -107,6 +116,13 @@
     accumulator++;
   } while (j < NotThreadID);
 
+  int NotThreadIDAssigned = 0;
+  NotThreadIDAssigned = foo(0) * 2;
+
+  for (int i = 0; i < NotThreadIDAssigned; i++) {
+    accumulator++;
+  }
+
   struct { int NotIDDepField; } Example;
   Example.NotIDDepField = foo(0);
 
Index: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -114,7 +114,7 @@
                                                   const VarDecl *Variable) {
   // Record that this variable is thread-dependent.
   IdDepVarsMap[Variable] =
-      IdDependencyRecord(Variable, Variable->getBeginLoc(),
+      IdDependencyRecord(Variable, Statement->getBeginLoc(),
                          Twine("assignment of ID-dependent variable ") +
                              Variable->getNameAsString());
 }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits