Add performance warning for rsSetElementAt

This warning proposes the use of typed rsSetElementAt calls in
case typed versions of this method are available.

Change-Id: I8d3b9bbd50b085d4e04db0008d7bf39733e0c663

5 files changed, 186 insertions, 0 deletions

diff --git a/slang_rs_check_ast.cpp b/slang_rs_check_ast.cpp
index 845db4c..14ee822 100644
--- a/slang_rs_check_ast.cpp
+++ b/slang_rs_check_ast.cpp
@@ -36,6 +36,117 @@ void RSCheckAST::VisitStmt(clang::Stmt *S) {
   }
 }
 
+void RSCheckAST::WarnOnSetElementAt(clang::CallExpr *E) {
+  clang::FunctionDecl *Decl;
+  clang::DiagnosticsEngine &DiagEngine = C.getDiagnostics();
+  Decl = clang::dyn_cast_or_null<clang::FunctionDecl>(E->getCalleeDecl());
+
+  if (!Decl || Decl->getNameAsString() != std::string("rsSetElementAt")) {
+    return;
+  }
+
+  clang::Expr *Expr;
+  clang::ImplicitCastExpr *ImplCast;
+  Expr = E->getArg(1);
+  ImplCast = clang::dyn_cast_or_null<clang::ImplicitCastExpr>(Expr);
+
+  if (!ImplCast) {
+    return;
+  }
+
+  const clang::Type *Ty;
+  const clang::VectorType *VectorTy;
+  const clang::BuiltinType *ElementTy;
+  Ty = ImplCast->getSubExpr()->getType()->getPointeeType()
+    ->getUnqualifiedDesugaredType();
+  VectorTy = clang::dyn_cast_or_null<clang::VectorType>(Ty);
+
+  if (VectorTy) {
+    ElementTy = clang::dyn_cast_or_null<clang::BuiltinType>(
+      VectorTy->getElementType()->getUnqualifiedDesugaredType());
+  } else {
+    ElementTy = clang::dyn_cast_or_null<clang::BuiltinType>(
+      Ty->getUnqualifiedDesugaredType());
+  }
+
+  if (!ElementTy) {
+    return;
+  }
+
+  // We only support vectors with 2, 3 or 4 elements.
+  if (VectorTy) {
+    switch (VectorTy->getNumElements()) {
+    default:
+      return;
+    case 2:
+    case 3:
+    case 4:
+      break;
+    }
+  }
+
+  const char *Name;
+
+  switch (ElementTy->getKind()) {
+    case clang::BuiltinType::Float:
+      Name = "float";
+      break;
+    case clang::BuiltinType::Double:
+      Name = "double";
+      break;
+    case clang::BuiltinType::Char_S:
+      Name = "char";
+      break;
+    case clang::BuiltinType::Short:
+      Name = "short";
+      break;
+    case clang::BuiltinType::Int:
+      Name = "int";
+      break;
+    case clang::BuiltinType::Long:
+      Name = "long";
+      break;
+    case clang::BuiltinType::UChar:
+      Name = "uchar";
+      break;
+    case clang::BuiltinType::UShort:
+      Name = "ushort";
+      break;
+    case clang::BuiltinType::UInt:
+      Name = "uint";
+      break;
+    case clang::BuiltinType::ULong:
+      Name = "ulong";
+      break;
+    default:
+      return;
+  }
+
+  clang::DiagnosticBuilder DiagBuilder =  DiagEngine.Report(
+    clang::FullSourceLoc(E->getLocStart(), mSM),
+    mDiagEngine.getCustomDiagID( clang::DiagnosticsEngine::Warning,
+    "untyped rsSetElementAt() can reduce performance. "
+    "Use rsSetElementAt_%0%1() instead."));
+  DiagBuilder << Name;
+
+  if (VectorTy) {
+    DiagBuilder << VectorTy->getNumElements();
+  } else {
+    DiagBuilder << "";
+  }
+
+  return;
+}
+
+void RSCheckAST::VisitCallExpr(clang::CallExpr *E) {
+  WarnOnSetElementAt(E);
+
+  for (clang::CallExpr::arg_iterator AI = E->arg_begin(), AE = E->arg_end();
+       AI != AE; ++AI) {
+    Visit(*AI);
+  }
+}
+
 void RSCheckAST::ValidateFunctionDecl(clang::FunctionDecl *FD) {
   if (!FD) {
     return;
diff --git a/slang_rs_check_ast.h b/slang_rs_check_ast.h
index 1700cb7..53be3cf 100644
--- a/slang_rs_check_ast.h
+++ b/slang_rs_check_ast.h
@@ -36,6 +36,14 @@ class RSCheckAST : public clang::StmtVisitor<RSCheckAST> {
   bool mIsFilterscript;
   bool mInKernel;
 
+  /// @brief Emit warnings for inapproriate uses of rsSetElementAt
+  ///
+  /// We warn in case generic rsSetElementAt() is used even though the user
+  /// could have used a typed rsSetElementAt_<type>() call. Typed calls
+  /// allow more aggressive optimization (e.g. due to better alias analysis
+  /// results). Hence, we want to steer the users to use them.
+  void WarnOnSetElementAt(clang::CallExpr*);
+
  public:
   explicit RSCheckAST(clang::ASTContext &Con, unsigned int TargetAPI,
                       bool IsFilterscript)
@@ -47,6 +55,8 @@ class RSCheckAST : public clang::StmtVisitor<RSCheckAST> {
 
   void VisitStmt(clang::Stmt *S);
 
+  void VisitCallExpr(clang::CallExpr *CE);
+
   void VisitCastExpr(clang::CastExpr *CE);
 
   void VisitExpr(clang::Expr *E);
diff --git a/tests/P_warnings_rsSetElementAt/setelementat.rs b/tests/P_warnings_rsSetElementAt/setelementat.rs
new file mode 100644
index 0000000..032a456
--- /dev/null
+++ b/tests/P_warnings_rsSetElementAt/setelementat.rs
@@ -0,0 +1,52 @@
+#pragma version(1)
+#pragma rs java_package_name(foo)
+
+rs_allocation A;
+static void foo() {
+    // Basic scalar and floating point types.
+    float a = 4.0f;
+    double d = 4.0f;
+    float2 a2 = {4.0f, 4.0f};
+    float3 a3 = {4.0f, 4.0f, 4.0f};
+    float4 a4 = {4.0f, 4.0f, 4.0f, 4.0f};
+    char c = 4;
+    uchar uc = 4;
+    short s = 4;
+    ushort us = 4;
+    int i = 4;
+    uint ui = 4;
+    long l = 4;
+    ulong ul = 4;
+
+    rsSetElementAt(A, &a, 0, 0);
+    rsSetElementAt(A, &d, 0, 0);
+    rsSetElementAt(A, &a2, 0, 0);
+    rsSetElementAt(A, &a3, 0, 0);
+    rsSetElementAt(A, &a4, 0, 0);
+    rsSetElementAt(A, &c, 0, 0);
+    rsSetElementAt(A, &uc, 0, 0);
+    rsSetElementAt(A, &s, 0, 0);
+    rsSetElementAt(A, &us, 0, 0);
+    rsSetElementAt(A, &i, 0, 0);
+    rsSetElementAt(A, &ui, 0, 0);
+    rsSetElementAt(A, &l, 0, 0);
+    rsSetElementAt(A, &ul, 0, 0);
+
+    // No warnings for complex data types
+    struct {
+        int A;
+        int B;
+    } P;
+    rsSetElementAt(A, &P, 0, 0);
+
+    // No warning for 'long long'
+    long long LL = 4.0f;
+    rsSetElementAt(A, &LL, 0, 0);
+
+    // Unsupported vector width
+    typedef int int5 __attribute__((ext_vector_type(5)));
+    int5 i5 = {5, 5, 5, 5, 5};
+
+    rsSetElementAt(A, &i5, 0, 0);
+}
+
diff --git a/tests/P_warnings_rsSetElementAt/stderr.txt.expect b/tests/P_warnings_rsSetElementAt/stderr.txt.expect
new file mode 100644
index 0000000..71b7aab
--- /dev/null
+++ b/tests/P_warnings_rsSetElementAt/stderr.txt.expect
@@ -0,0 +1,12 @@
+setelementat.rs:21:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_float() instead.
+setelementat.rs:22:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_double() instead.
+setelementat.rs:23:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_float2() instead.
+setelementat.rs:24:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_float3() instead.
+setelementat.rs:25:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_float4() instead.
+setelementat.rs:26:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_char() instead.
+setelementat.rs:27:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_uchar() instead.
+setelementat.rs:28:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_short() instead.
+setelementat.rs:29:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_ushort() instead.
+setelementat.rs:30:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_int() instead.
+setelementat.rs:31:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_uint() instead.
+setelementat.rs:32:5: warning: untyped rsSetElementAt() can reduce performance. Use rsSetElementAt_long() instead.
diff --git a/tests/P_warnings_rsSetElementAt/stdout.txt.expect b/tests/P_warnings_rsSetElementAt/stdout.txt.expect
new file mode 100644
index 0000000..b10911b
--- /dev/null
+++ b/tests/P_warnings_rsSetElementAt/stdout.txt.expect
@@ -0,0 +1 @@
+Generating ScriptC_setelementat.java ...