improve code readability

proper variable naming and whitelining between functions

Author: prigoyal

tc/core/tc2halide.cc

@@ -497,18 +497,18 @@ Expr reductionUpdate(Expr e) {
 // contain kReductionUpdate intrinsics.  These may have to be removed
 // in order to be able to apply internal Halide analysis passes on them.
 void translateComprehension(
-    const lang::Comprehension& c,
+    const lang::Comprehension& comprehension,
     const map<string, Parameter>& params,
     bool throwWarnings,
     map<string, Function>* funcs,
     FunctionBounds* bounds) {
   Function f;
-  auto it = funcs->find(c.ident().name());
+  auto it = funcs->find(comprehension.ident().name());
   if (it != funcs->end()) {
     f = it->second;
   } else {
-    f = Function(c.ident().name());
-    (*funcs)[c.ident().name()] = f;
+    f = Function(comprehension.ident().name());
+    (*funcs)[comprehension.ident().name()] = f;
   }
   // Function is the internal Halide IR type for a pipeline
   // stage. Func is the front-end class that wraps it. Here it's
@@ -517,7 +517,7 @@ void translateComprehension(
 
   vector<Var> lhs;
   vector<Expr> lhs_as_exprs;
-  for (lang::Ident id : c.indices()) {
+  for (lang::Ident id : comprehension.indices()) {
     lhs.push_back(Var(id.name()));
     lhs_as_exprs.push_back(lhs.back());
   }
@@ -526,17 +526,17 @@ void translateComprehension(
   // in the future we may consider using Halide Let bindings when they
   // are supported later
   map<string, Expr> lets;
-  for (auto wc : c.whereClauses()) {
+  for (auto wc : comprehension.whereClauses()) {
     if (wc->kind() == lang::TK_LET) {
       auto let = lang::Let(wc);
       lets[let.name().name()] = translateExpr(let.rhs(), params, *funcs, lets);
     }
   }
 
-  Expr rhs = translateExpr(c.rhs(), params, *funcs, lets);
+  Expr rhs = translateExpr(comprehension.rhs(), params, *funcs, lets);
 
   std::vector<Expr> all_exprs;
-  for (auto wc : c.whereClauses()) {
+  for (auto wc : comprehension.whereClauses()) {
     if (wc->kind() == lang::TK_EXISTS) {
       all_exprs.push_back(
           translateExpr(lang::Exists(wc).exp(), params, *funcs, lets));
@@ -560,7 +560,7 @@ void translateComprehension(
   // values (2) +=!, TK_PLUS_EQ_B which first sets the tensor to the identity
   // for the reduction and then applies the reduction.
   bool should_zero = false;
-  switch (c.assignment()->kind()) {
+  switch (comprehension.assignment()->kind()) {
     case lang::TK_PLUS_EQ_B:
       should_zero = true; // fallthrough
     case lang::TK_PLUS_EQ:
@@ -592,12 +592,13 @@ void translateComprehension(
     case '=':
       break;
     default:
-      throw lang::ErrorReport(c) << "Unimplemented reduction "
-                                 << c.assignment()->range().text() << "\n";
+      throw lang::ErrorReport(comprehension)
+          << "Unimplemented reduction "
+          << comprehension.assignment()->range().text() << "\n";
   }
 
   // Tag reductions as such
-  if (c.assignment()->kind() != '=') {
+  if (comprehension.assignment()->kind() != '=') {
     rhs = reductionUpdate(rhs);
   }
 
@@ -637,7 +638,7 @@ void translateComprehension(
   Scope<Interval> solution;
 
   // Put anything explicitly specified with a 'where' class in the solution
-  for (auto constraint_ : c.whereClauses()) {
+  for (auto constraint_ : comprehension.whereClauses()) {
     if (constraint_->kind() != lang::TK_RANGE_CONSTRAINT)
       continue;
     auto constraint = lang::RangeConstraint(constraint_);
@@ -658,7 +659,8 @@ void translateComprehension(
 
   // Infer the rest
   all_exprs.push_back(rhs);
-  forwardBoundsInference(all_exprs, *bounds, c, throwWarnings, &solution);
+  forwardBoundsInference(
+      all_exprs, *bounds, comprehension, throwWarnings, &solution);
 
   // TODO: What if subsequent updates have incompatible bounds
   // (e.g. an in-place stencil)?. The .bound directive will use the
@@ -669,7 +671,7 @@ void translateComprehension(
 
   for (Var v : lhs) {
     if (!solution.contains(v.name())) {
-      throw lang::ErrorReport(c)
+      throw lang::ErrorReport(comprehension)
           << "Free variable " << v
           << " was not solved in range inference. May not be used right-hand side";
     }
@@ -693,7 +695,7 @@ void translateComprehension(
     for (size_t i = 0; i < unbound.size(); i++) {
       auto v = unbound[unbound.size() - 1 - i];
       if (!solution.contains(v->name)) {
-        throw lang::ErrorReport(c)
+        throw lang::ErrorReport(comprehension)
             << "Free variable " << v << " is unconstrained. "
             << "Use a 'where' clause to set its range.";
       }

tc/lang/sema.h

@@ -166,6 +166,7 @@ struct Sema {
     }
     return expr_to_type.at(ref);
   }
+
   // associate a type with this expression
   TreeRef withType(TreeRef expr, TreeRef type) {
     auto inserted = expr_to_type.emplace(expr, type).second;
@@ -179,6 +180,7 @@ struct Sema {
     }
     return TensorType(typ);
   }
+
   TreeRef matchAllTypes(TreeRef list, TreeRef matched_type = nullptr) {
     for (auto e : list->trees()) {
       if (!matched_type)
@@ -188,23 +190,27 @@ struct Sema {
     }
     return matched_type;
   }
+
   TreeRef expectIntegral(TreeRef e) {
     if (TypeInfo(typeOfExpr(e)).code() == TypeInfo::Float) {
       throw ErrorReport(e) << " expected integral type but found "
                            << kindToString(typeOfExpr(e)->kind());
     }
     return e;
   }
+
   void expectBool(TreeRef anchor, int token) {
     if (token != TK_BOOL) {
       throw ErrorReport(anchor)
           << "expected boolean but found " << kindToString(token);
     }
   }
+
   TreeRef expectBool(TreeRef exp) {
     expectBool(exp, typeOfExpr(exp)->kind());
     return exp;
   }
+
   TreeRef lookupVarOrCreateIndex(Ident ident) {
     TreeRef type = lookup(ident, false);
     if (!type) {
@@ -216,6 +222,7 @@ struct Sema {
     }
     return type;
   }
+
   TreeRef checkExp(TreeRef exp, bool allow_access) {
     switch (exp->kind()) {
       case TK_APPLY: {
@@ -339,6 +346,7 @@ struct Sema {
         throw ErrorReport(exp) << "NYI - semantic checking for " << exp;
     }
   }
+
   // This is the entry function for semantic analysis. It is called by
   // tc2halide to associate type with each node of the tree and to also make
   // sure that the tree is sematically correct. For example: a variable
@@ -352,7 +360,7 @@ struct Sema {
   //
   // Type checking is also done by small amount of code
   //
-  // The method 'withType' can be used to associate the type with a given node
+  // The method 'withType' is used to associate the type with a given node
   //
   TreeRef checkFunction(TreeRef func_) {
     auto func = Def(func_);
@@ -385,21 +393,27 @@ struct Sema {
         Def::create(func.range(), func.name(), params_, returns_, statements_);
     return r;
   }
+
   TreeRef indexType(TreeRef anchor) {
-    return c(TK_INT32, anchor->range(), {});
+    return createCompound(TK_INT32, anchor->range(), {});
   }
+
   TreeRef dimType(TreeRef anchor) {
     return indexType(anchor);
   }
+
   TreeRef floatType(TreeRef anchor) {
-    return c(TK_FLOAT, anchor->range(), {});
+    return createCompound(TK_FLOAT, anchor->range(), {});
   }
+
   TreeRef boolType(TreeRef anchor) {
-    return c(TK_BOOL, anchor->range(), {});
+    return createCompound(TK_BOOL, anchor->range(), {});
   }
+
   void checkDim(Ident dim) {
     insert(env, dim, dimType(dim), false);
   }
+
   TreeRef checkTensorType(TreeRef type) {
     auto tt = TensorType(type);
     for (const auto& d : tt.dims()) {
@@ -409,18 +423,21 @@ struct Sema {
     }
     return type;
   }
+
   TreeRef checkParam(TreeRef param) {
     auto p = Param(param);
     TreeRef type_ = checkTensorType(p.type());
     insert(env, p.ident(), type_, true);
     live_input_names.insert(p.ident().name());
     return param;
   }
+
   TreeRef checkReturn(TreeRef ret) {
     auto r = Param(ret);
     TreeRef real_type = lookup(env, r.ident(), true);
     return ret;
   }
+
   TreeRef checkList(TreeRef list, std::function<TreeRef(TreeRef)> fn) {
     TC_ASSERT(list, list->kind() == TK_LIST);
     TreeList r;
@@ -429,6 +446,7 @@ struct Sema {
     }
     return List::create(list->range(), std::move(r));
   }
+
   TreeRef checkRangeConstraint(RangeConstraint rc) {
     // RCs are checked _before_ the rhs of the TC, so
     // it is possible the index is not in the environment yet
@@ -441,11 +459,13 @@ struct Sema {
     auto e = expectIntegral(checkExp(rc.end(), false));
     return RangeConstraint::create(rc.range(), rc.ident(), s, e);
   }
+
   TreeRef checkLet(Let l) {
     auto rhs = checkExp(l.rhs(), true);
     insert(let_env, l.name(), typeOfExpr(rhs), true);
     return Let::create(l.range(), l.name(), rhs);
   }
+
   TreeRef checkWhereClause(TreeRef ref) {
     if (ref->kind() == TK_LET) {
       return checkLet(Let(ref));
@@ -456,6 +476,7 @@ struct Sema {
       return checkRangeConstraint(RangeConstraint(ref));
     }
   }
+
   // Semantic checking for the statements/comprehensions in a TC Def.
   TreeRef checkStmt(TreeRef stmt_) {
     auto stmt = Comprehension(stmt_);
@@ -467,11 +488,13 @@ struct Sema {
       insert(index_env, index, typ, true);
     }
 
-    // make dimension variables for each dimension of the output tensor
+    // check that the input is not used for output - inputs are immutable
     std::string name = stmt.ident().name();
     if (inputParameters.count(name) > 0) {
       throw ErrorReport(stmt_) << "TC inputs are immutable";
     }
+
+    // make dimension variables for each dimension of the output tensor
     TreeList output_indices;
     int n = stmt.indices().size();
     for (int i = 0; i < n; ++i) {
@@ -578,6 +601,7 @@ struct Sema {
 
     return result;
   }
+
   static bool isUninitializedReductionOperation(TreeRef assignment) {
     switch (assignment->kind()) {
       case TK_PLUS_EQ:
@@ -589,6 +613,7 @@ struct Sema {
         return false;
     }
   }
+
   bool isNotInplace(TreeRef assignment) {
     switch (assignment->kind()) {
       case TK_PLUS_EQ_B:
@@ -600,6 +625,7 @@ struct Sema {
         return false;
     }
   }
+
   std::string dumpEnv() {
     std::stringstream ss;
     std::vector<std::pair<std::string, TreeRef>> elems(env.begin(), env.end());
@@ -618,6 +644,7 @@ struct Sema {
 
  private:
   using Env = std::unordered_map<std::string, TreeRef>;
+
   void
   insert(Env& the_env, Ident ident, TreeRef value, bool must_be_undefined) {
     std::string name = ident.name();
@@ -630,6 +657,7 @@ struct Sema {
       throw ErrorReport(ident) << name << " already defined";
     }
   }
+
   TreeRef lookup(Ident ident, bool required) {
     TreeRef v = lookup(index_env, ident, false);
     if (!v)
@@ -638,6 +666,7 @@ struct Sema {
       v = lookup(env, ident, required);
     return v;
   }
+
   TreeRef lookup(Env& the_env, Ident ident, bool required) {
     std::string name = ident.name();
     auto it = the_env.find(name);
@@ -647,10 +676,12 @@ struct Sema {
     }
     return it == the_env.end() ? nullptr : it->second;
   }
-  TreeRef c(int kind, const SourceRange& range, TreeList&& trees) {
+
+  TreeRef createCompound(int kind, const SourceRange& range, TreeList&& trees) {
     return Compound::create(kind, range, std::move(trees));
   }
-  TreeRef s(const std::string& s) {
+
+  TreeRef createString(const std::string& s) {
     return String::create(s);
   }