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

[yeputons-gh]

Summary

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. Do the same for note messages about inferred id-dependencies. They previously pointed to the declaration of the original id-dependent variable, not the assignment from it.

There are six cases of assignment inferrence in the code, all are tested: a variable initialized from a variable/field, a variable assigned from a variable/field, a field assigned from a variable/field.

Depends on D145305

[yeputons-gh]

Test inferred assignments and fix notes for them as well.

[yeputons-gh]

clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp

126

This line was already correct prior to this patch.

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

42

It previously pointed at int ThreadIDAssigned = 0, not ThreadIDAssigned = get_local_id(0) * 2.

61–63

This became ThreadIDVarFromVar, see above.

180–210

I mirror all id-dependent tests with non-id-dependent to make sure https://github.com/llvm/llvm-project/issues/52790 does not creep back.

clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp

if (const auto *ChildExpression = dyn_cast<Expr>(Child))
         return Result;
   return nullptr;
 }
 
 void IdDependentBackwardBranchCheck::saveIdDepVar(const Stmt *Statement,
                                                   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());
 }
 
 void IdDependentBackwardBranchCheck::saveIdDepField(const Stmt *Statement,
                                                     const FieldDecl *Field) {
   // Record that this field is thread-dependent.
   IdDepFieldsMap[Field] = IdDependencyRecord(
       Field, Statement->getBeginLoc(),
       Twine("assignment of ID-dependent field ") + Field->getNameAsString());
 }
 
 void IdDependentBackwardBranchCheck::saveIdDepVarFromPotentialReference(
     const DeclRefExpr *RefExpr, const MemberExpr *MemExpr,
     const VarDecl *PotentialVar) {
   // If the variable is already in IdDepVarsMap, ignore it.
   if (IdDepVarsMap.find(PotentialVar) != IdDepVarsMap.end())
     return;
   std::string Message;
   llvm::raw_string_ostream StringStream(Message);
   StringStream << "inferred assignment of ID-dependent value from "
                   "ID-dependent ";
   if (RefExpr) {
     const auto *RefVar = dyn_cast<VarDecl>(RefExpr->getDecl());
     // If variable isn't ID-dependent, but RefVar is.
     if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) {
       StringStream << "variable " << RefVar->getNameAsString();
-      IdDepVarsMap[PotentialVar] = IdDependencyRecord(
-          PotentialVar, PotentialVar->getBeginLoc(), Message);
+      IdDepVarsMap[PotentialVar] =
+          IdDependencyRecord(PotentialVar, RefExpr->getBeginLoc(), Message);
       return; // Optional, as we only match only one of `RefExpr` or `MemExpr`
     }
   }
   if (MemExpr) {
     const auto *RefField = dyn_cast<FieldDecl>(MemExpr->getMemberDecl());
     // If variable isn't ID-dependent, but RefField is.
     if (IdDepFieldsMap.find(RefField) != IdDepFieldsMap.end()) {
       StringStream << "member " << RefField->getNameAsString();
-      IdDepVarsMap[PotentialVar] = IdDependencyRecord(
-          PotentialVar, PotentialVar->getBeginLoc(), Message);
+      IdDepVarsMap[PotentialVar] =
+          IdDependencyRecord(PotentialVar, MemExpr->getBeginLoc(), Message);
       return;
     }
   }
 }
 
 void IdDependentBackwardBranchCheck::saveIdDepFieldFromPotentialReference(
     const DeclRefExpr *RefExpr, const MemberExpr *MemExpr,
     const FieldDecl *PotentialField) {
   // If the field is already in IdDepFieldsMap, ignore it.
   if (IdDepFieldsMap.find(PotentialField) != IdDepFieldsMap.end())
     return;
   std::string Message;
   llvm::raw_string_ostream StringStream(Message);
   StringStream << "inferred assignment of ID-dependent member from "
                   "ID-dependent ";
   if (RefExpr) {
     const auto *RefVar = dyn_cast<VarDecl>(RefExpr->getDecl());
     // If field isn't ID-dependent, but RefVar is.
     if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) {
       StringStream << "variable " << RefVar->getNameAsString();
-      IdDepFieldsMap[PotentialField] = IdDependencyRecord(
-          PotentialField, PotentialField->getBeginLoc(), Message);
+      IdDepFieldsMap[PotentialField] =
+          IdDependencyRecord(PotentialField, RefExpr->getBeginLoc(), Message);
       return; // Optional, as we only match only one of `RefExpr` or `MemExpr`
     }
   }
   if (MemExpr) {
     const auto *RefField = dyn_cast<FieldDecl>(MemExpr->getMemberDecl());
     if (IdDepFieldsMap.find(RefField) != IdDepFieldsMap.end()) {
       StringStream << "member " << RefField->getNameAsString();
-      IdDepFieldsMap[PotentialField] = IdDependencyRecord(
-          PotentialField, PotentialField->getBeginLoc(), Message);
+      IdDepFieldsMap[PotentialField] =
+          IdDependencyRecord(PotentialField, MemExpr->getBeginLoc(), Message);
       return;
     }
   }
 }
 
 IdDependentBackwardBranchCheck::LoopType
 IdDependentBackwardBranchCheck::getLoopType(const Stmt *Loop) {
   switch (Loop->getStmtClass()) {

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

void error() {
   }
 
   do {
     accumulator++;
   } while (j < ThreadID);
   // 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);
 
   for (int i = 0; i < Example.IDDepField; i++) {
     // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
     // CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent field IDDepField
     accumulator++;
   }
 
   while (j < Example.IDDepField) {
     // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
     // CHECK-NOTES: :[[@LINE-10]]:3: note: assignment of ID-dependent field IDDepField
     accumulator++;
   }
 
   do {
     accumulator++;
   } while (j < Example.IDDepField);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
   // CHECK-NOTES: :[[@LINE-18]]:3: note: assignment of ID-dependent field IDDepField
 
   // ==== Inferred Assignments ====
-  int ThreadID2 = ThreadID * 2;
+  int ThreadIDVarFromVar = ThreadID * 2;
+  for (int i = 0; i < ThreadIDVarFromVar; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-3]]:28: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+    accumulator++;
+  }
+
+  int ThreadIDVarAssignFromVar;
+  ThreadIDVarAssignFromVar = ThreadID * 2;
+  for (int i = 0; i < ThreadIDVarAssignFromVar; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarAssignFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-3]]:30: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+    accumulator++;
+  }
+
+  int ThreadIDVarFromMember = Example.IDDepField * 2;
+  for (int i = 0; i < ThreadIDVarFromMember; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarFromMember' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-3]]:31: note: inferred assignment of ID-dependent value from ID-dependent member IDDepField
+    accumulator++;
+  }
+
+  int ThreadIDVarAssignFromMember;
+  ThreadIDVarAssignFromMember = Example.IDDepField * 2;
+  for (int i = 0; i < ThreadIDVarAssignFromMember; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarAssignFromMember' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-3]]:33: note: inferred assignment of ID-dependent value from ID-dependent member IDDepField
+    accumulator++;
+  }
+
+  struct { int IDDepFieldFromVar; } ExampleFromVar;
+  ExampleFromVar.IDDepFieldFromVar = ThreadID * 2;
+  for (int i = 0; i < ExampleFromVar.IDDepFieldFromVar; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 'IDDepFieldFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-3]]:38: note: inferred assignment of ID-dependent member from ID-dependent variable ThreadID
+    accumulator++;
+  }
 
-  for (int i = 0; i < ThreadID2; i++) {
-    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadID2' and may cause performance degradation [altera-id-dependent-backward-branch]
-    // CHECK-NOTES: :[[@LINE-4]]:3: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+  struct { int IDDepFieldFromMember; } ExampleFromMember;
+  ExampleFromMember.IDDepFieldFromMember = Example.IDDepField * 2;
+  for (int i = 0; i < ExampleFromMember.IDDepFieldFromMember; i++) {
+    // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 'IDDepFieldFromMember' and may cause performance degradation [altera-id-dependent-backward-branch]
+    // CHECK-NOTES: :[[@LINE-3]]:44: note: inferred assignment of ID-dependent member from ID-dependent member IDDepField
     accumulator++;
   }
 
   // ==== Unused Inferred Assignments ====
   int UnusedThreadID = Example.IDDepField; // OK: not used in any loops
 
   struct { int IDDepField; } UnusedStruct;
   UnusedStruct.IDDepField = ThreadID * 2; // OK: not used in any loops
void success() {
   while (j < NotThreadID) {
     accumulator++;
   }
 
   do {
     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);
 
   for (int i = 0; i < Example.NotIDDepField; i++) {
     accumulator++;
   }
 
   while (j < Example.NotIDDepField) {
     accumulator++;
   }
 
   do {
     accumulator++;
   } while (j < Example.NotIDDepField);
 
   // ==== Inferred Assignments ====
-  int NotThreadID2 = NotThreadID * 2;
+  int NotThreadIDVarFromVar = NotThreadID * 2;
+  for (int i = 0; i < NotThreadIDVarFromVar; i++) {
+    accumulator++;
+  }
+
+  int NotThreadIDVarAssignFromVar;
+  NotThreadIDVarAssignFromVar = NotThreadID * 2;
+  for (int i = 0; i < NotThreadIDVarAssignFromVar; i++) {
+    accumulator++;
+  }
+
+  int NotThreadIDVarFromMember = Example.NotIDDepField * 2;
+  for (int i = 0; i < NotThreadIDVarFromMember; i++) {
+    accumulator++;
+  }
+
+  int NotThreadIDVarAssignFromMember;
+  NotThreadIDVarAssignFromMember = Example.NotIDDepField * 2;
+  for (int i = 0; i < NotThreadIDVarAssignFromMember; i++) {
+    accumulator++;
+  }
+
+  struct { int NotIDDepFieldFromVar; } ExampleFromVar;
+  ExampleFromVar.NotIDDepFieldFromVar = NotThreadID * 2;
+  for (int i = 0; i < ExampleFromVar.NotIDDepFieldFromVar; i++) {
+    accumulator++;
+  }
 
-  for (int i = 0; i < NotThreadID2; i++) {
+  struct { int NotIDDepFieldFromMember; } ExampleFromMember;
+  ExampleFromMember.NotIDDepFieldFromMember = Example.NotIDDepField * 2;
+  for (int i = 0; i < ExampleFromMember.NotIDDepFieldFromMember; i++) {
     accumulator++;
   }
 
   // ==== Unused Inferred Assignments ====
   int UnusedNotThreadID = Example.NotIDDepField;
 
   struct { int NotIDDepField; } UnusedStruct;
   UnusedStruct.NotIDDepField = NotThreadID * 2;
 }