Support Virtual Tuples and Memtuples in SlotGetAttrCodegen

We can avoid generating multiple versions of the slot_getattr. Once we
deform any of the attributes in the tuples, we make it a virtual tuple.
At code generation time, we know exactly how many need to be deformed
and can in fact go ahead deform all the way. This way we don't need to
worry about the case when slot_getattr is called on a virtual tuple with
attnum > nvalid - that is deformation is partially complete.

To enable this, we need to collect information from all the code
generators that depend on SlotGetAttrCodegen before it is generated. We
maintain a static map (keyed on the manager and the slot) to instances of
SlotGetAttrCodegen. We also introduce a InitDependencies phase that
happens before the GenerateCode phase, when dependants of
SlotGetAttrCodegen can retrieve instances from the static map.

src/backend/codegen/codegen_manager.cc

@@ -40,6 +40,13 @@ bool CodegenManager::EnrollCodeGenerator(
 }
 
 unsigned int CodegenManager::GenerateCode() {
+  // First, allow all code generators to initialize their dependencies
+  for (size_t i = 0; i < enrolled_code_generators_.size(); ++i) {
+    // NB: This list is still volatile at this time, as more generators may be
+    // enrolled as we iterate to initialize dependencies.
+    enrolled_code_generators_[i]->InitDependencies();
+  }
+  // Then ask them to generate code
   unsigned int success_count = 0;
   for (std::unique_ptr<CodegenInterface>& generator :
       enrolled_code_generators_) {

src/backend/codegen/exec_eval_expr_codegen.cc

@@ -59,30 +59,32 @@ ExecEvalExprCodegen::ExecEvalExprCodegen(
                   kExecEvalExprPrefix,
                   regular_func_ptr, ptr_to_regular_func_ptr),
       exprstate_(exprstate),
-      econtext_(econtext),
-      plan_state_(plan_state) {
+      plan_state_(plan_state),
+      gen_info_(econtext, nullptr, nullptr, nullptr, 0),
+      slot_getattr_codegen_(nullptr),
+      expr_tree_generator_(nullptr) {
 }
 
-void ExecEvalExprCodegen::PrepareSlotGetAttr(
-    gpcodegen::GpCodegenUtils* codegen_utils, ExprTreeGeneratorInfo* gen_info) {
+bool ExecEvalExprCodegen::InitDependencies() {
+  OpExprTreeGenerator::InitializeSupportedFunction();
+  ExprTreeGenerator::VerifyAndCreateExprTree(
+        exprstate_, &gen_info_, &expr_tree_generator_);
+  // Prepare dependent slot_getattr() generation
+  PrepareSlotGetAttr();
+  return true;
+}
+
+void ExecEvalExprCodegen::PrepareSlotGetAttr() {
+  TupleTableSlot* slot = nullptr;
   assert(nullptr != plan_state_);
   switch (nodeTag(plan_state_)) {
     case T_SeqScanState:
     case T_TableScanState:
       // Generate dependent slot_getattr() implementation for the given slot
-      if (gen_info->max_attr > 0) {
-        TupleTableSlot* slot = reinterpret_cast<ScanState*>(plan_state_)
+      if (gen_info_.max_attr > 0) {
+        slot = reinterpret_cast<ScanState*>(plan_state_)
             ->ss_ScanTupleSlot;
         assert(nullptr != slot);
-        std::string slot_getattr_func_name = "slot_getattr_"
-            + std::to_string(reinterpret_cast<uint64_t>(slot)) + "_"
-            + std::to_string(gen_info->max_attr);
-        bool ok = SlotGetAttrCodegen::GenerateSlotGetAttr(
-            codegen_utils, slot_getattr_func_name, slot, gen_info->max_attr,
-            &gen_info->llvm_slot_getattr_func);
-        if (!ok) {
-          elog(DEBUG1, "slot_getattr generation for ExecEvalExpr failed!");
-        }
       }
       break;
     case T_AggState:
@@ -90,12 +92,16 @@ void ExecEvalExprCodegen::PrepareSlotGetAttr(
       // deformed in which case, we can avoid generating and calling the
       // generated slot_getattr(). This may not be true always, but calling the
       // regular slot_getattr() will still preserve correctness.
-      gen_info->llvm_slot_getattr_func = nullptr;
       break;
     default:
       elog(DEBUG1,
           "Attempting to generate ExecEvalExpr for an unsupported operator!");
   }
+
+  if (nullptr != slot) {
+    slot_getattr_codegen_ = SlotGetAttrCodegen::GetCodegenInstance(
+        manager(), slot, gen_info_.max_attr);
+  }
 }
 
 bool ExecEvalExprCodegen::GenerateExecEvalExpr(
@@ -104,11 +110,24 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr(
   assert(NULL != codegen_utils);
   if (nullptr == exprstate_ ||
       nullptr == exprstate_->expr ||
-      nullptr == econtext_) {
+      nullptr == gen_info_.econtext) {
     return false;
   }
-  // TODO(krajaraman): move to better place
-  OpExprTreeGenerator::InitializeSupportedFunction();
+
+  if (nullptr == expr_tree_generator_.get()) {
+    return false;
+  }
+
+  // In case the generation above either failed or was not needed,
+  // we revert to use the external slot_getattr()
+  if (nullptr == slot_getattr_codegen_) {
+    gen_info_.llvm_slot_getattr_func =
+        codegen_utils->GetOrRegisterExternalFunction(slot_getattr,
+                                                     "slot_getattr");
+  } else {
+    slot_getattr_codegen_->GenerateCode(codegen_utils);
+    gen_info_.llvm_slot_getattr_func = slot_getattr_codegen_->GetGeneratedFunction();
+  }
 
   llvm::Function* exec_eval_expr_func = CreateFunction<ExecEvalExprFn>(
       codegen_utils, GetUniqueFuncName());
@@ -122,33 +141,10 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr(
   llvm::BasicBlock* llvm_error_block = codegen_utils->CreateBasicBlock(
         "error_block", exec_eval_expr_func);
 
-  auto irb = codegen_utils->ir_builder();
-
-  // Check if we can codegen. If so create ExprTreeGenerator
-  ExprTreeGeneratorInfo gen_info(econtext_,
-                                 exec_eval_expr_func,
-                                 llvm_error_block,
-                                 nullptr,
-                                 0);
-
-  std::unique_ptr<ExprTreeGenerator> expr_tree_generator(nullptr);
-  bool can_generate = ExprTreeGenerator::VerifyAndCreateExprTree(
-      exprstate_, &gen_info, &expr_tree_generator);
-  if (!can_generate ||
-      expr_tree_generator.get() == nullptr) {
-    return false;
-  }
-
-  // Prepare dependent slot_getattr() generation
-  PrepareSlotGetAttr(codegen_utils, &gen_info);
+  gen_info_.llvm_main_func = exec_eval_expr_func;
+  gen_info_.llvm_error_block = llvm_error_block;
 
-  // In case the generation above failed either or it was not needed,
-  // we revert to use the external slot_getattr()
-  if (nullptr == gen_info.llvm_slot_getattr_func) {
-    gen_info.llvm_slot_getattr_func =
-        codegen_utils->GetOrRegisterExternalFunction(slot_getattr,
-                                                     "slot_getattr");
-  }
+  auto irb = codegen_utils->ir_builder();
 
   irb->SetInsertPoint(llvm_entry_block);
 
@@ -158,11 +154,10 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr(
       "Codegen'ed expression evaluation called!");
 #endif
 
-
   // Generate code from expression tree generator
   llvm::Value* value = nullptr;
-  bool is_generated = expr_tree_generator->GenerateCode(codegen_utils,
-                                                        gen_info,
+  bool is_generated = expr_tree_generator_->GenerateCode(codegen_utils,
+                                                        gen_info_,
                                                         llvm_isnull_arg,
                                                         &value);
   if (!is_generated ||

src/backend/codegen/exec_variable_list_codegen.cc

@@ -62,9 +62,22 @@ ExecVariableListCodegen::ExecVariableListCodegen(
                   regular_func_ptr,
                   ptr_to_regular_func_ptr),
       proj_info_(proj_info),
-      slot_(slot) {
+      slot_(slot),
+      max_attr_(0),
+      slot_getattr_codegen_(nullptr) {
 }
 
+bool ExecVariableListCodegen::InitDependencies() {
+  assert(proj_info_ != nullptr);
+
+  // Find the largest attribute index in projInfo->pi_targetlist
+  max_attr_ = *std::max_element(
+      proj_info_->pi_varNumbers,
+      proj_info_->pi_varNumbers + list_length(proj_info_->pi_targetlist));
+  slot_getattr_codegen_ = SlotGetAttrCodegen::GetCodegenInstance(
+      manager(), slot_, max_attr_);
+  return true;
+}
 
 bool ExecVariableListCodegen::GenerateExecVariableList(
     gpcodegen::GpCodegenUtils* codegen_utils) {
@@ -90,17 +103,12 @@ bool ExecVariableListCodegen::GenerateExecVariableList(
     }
   }
 
-  // Find the largest attribute index in projInfo->pi_targetlist
-  int max_attr = *std::max_element(
-      proj_info_->pi_varNumbers,
-      proj_info_->pi_varNumbers + list_length(proj_info_->pi_targetlist));
-
   // System attribute
-  if (max_attr <= 0) {
+  if (max_attr_ <= 0) {
     elog(DEBUG1, "Cannot generate code for ExecVariableList"
                  "because max_attr is negative (i.e., system attribute).");
     return false;
-  } else if (max_attr > slot_->tts_tupleDescriptor->natts) {
+  } else if (max_attr_ > slot_->tts_tupleDescriptor->natts) {
     elog(DEBUG1, "Cannot generate code for ExecVariableList"
                  "because max_attr is greater than natts.");
     return false;
@@ -127,7 +135,7 @@ bool ExecVariableListCodegen::GenerateExecVariableList(
       "fallback", exec_variable_list_func);
 
   // Generation-time constants
-  llvm::Value* llvm_max_attr = codegen_utils->GetConstant(max_attr);
+  llvm::Value* llvm_max_attr = codegen_utils->GetConstant(max_attr_);
   llvm::Value* llvm_slot = codegen_utils->GetConstant(slot_);
 
   // Function arguments to ExecVariableList
@@ -138,21 +146,17 @@ bool ExecVariableListCodegen::GenerateExecVariableList(
 
 
   // Generate slot_getattr for attributes all the way to max_attr
-  std::string slot_getattr_func_name =
-      "slot_getattr_" + std::to_string(max_attr);
-  llvm::Function* slot_getattr_func = nullptr;
-  bool ok = SlotGetAttrCodegen::GenerateSlotGetAttr(
-      codegen_utils,
-      slot_getattr_func_name,
-      slot_,
-      max_attr,
-      &slot_getattr_func);
-  if (!ok) {
+  assert(nullptr != slot_getattr_codegen_);
+  slot_getattr_codegen_->GenerateCode(codegen_utils);
+  llvm::Function* slot_getattr_func = slot_getattr_codegen_->GetGeneratedFunction();
+
+  // In case the above generation failed, no point in continuing since that was
+  // the most crucial part of ExecVariableList code generation.
+  if (nullptr == slot_getattr_func) {
     elog(DEBUG1, "Cannot generate code for ExecVariableList "
                  "because slot_getattr generation failed!");
     return false;
   }
-  assert(nullptr != slot_getattr_func);
 
   // Entry block
   // -----------

src/backend/codegen/include/codegen/base_codegen.h

@@ -51,6 +51,10 @@ class BaseCodegen: public CodegenInterface {
     SetToRegular(regular_func_ptr_, ptr_to_chosen_func_ptr_);
   }
 
+  bool InitDependencies() override {
+    return true;
+  }
+
   bool GenerateCode(gpcodegen::GpCodegenUtils* codegen_utils) final {
     bool valid_generated_functions = true;
     valid_generated_functions &= GenerateCodeInternal(codegen_utils);
@@ -128,10 +132,6 @@ class BaseCodegen: public CodegenInterface {
     return regular_func_ptr_;
   }
 
-  gpcodegen::CodegenManager* manager() const {
-    return manager;
-  }
-
   /**
    * @brief Sets up the caller to use the corresponding regular version of the
    *        target function.
@@ -166,16 +166,20 @@ class BaseCodegen: public CodegenInterface {
                        const std::string& orig_func_name,
                        FuncPtrType regular_func_ptr,
                        FuncPtrType* ptr_to_chosen_func_ptr)
-  : orig_func_name_(orig_func_name),
+  : manager_(manager),
+    orig_func_name_(orig_func_name),
     unique_func_name_(CodegenInterface::GenerateUniqueName(orig_func_name)),
     regular_func_ptr_(regular_func_ptr),
     ptr_to_chosen_func_ptr_(ptr_to_chosen_func_ptr),
-    is_generated_(false),
-    manager_(manager) {
+    is_generated_(false) {
     // Initialize the caller to use regular version of target function.
     SetToRegular(regular_func_ptr, ptr_to_chosen_func_ptr);
   }
 
+  gpcodegen::CodegenManager* manager() const {
+    return manager_;
+  }
+
   /**
    * @brief Generates specialized code at run time.
    *
@@ -217,12 +221,12 @@ class BaseCodegen: public CodegenInterface {
   }
 
  private:
+  gpcodegen::CodegenManager* manager_;
   std::string orig_func_name_;
   std::string unique_func_name_;
   FuncPtrType regular_func_ptr_;
   FuncPtrType* ptr_to_chosen_func_ptr_;
   bool is_generated_;
-  gpcodegen::CodegenManager* manager_;
   // To track uncompiled llvm functions it creates and erase from
   // llvm module on failed generations.
   std::vector<llvm::Function*> uncompiled_generated_functions_;

src/backend/codegen/include/codegen/codegen_interface.h

@@ -15,7 +15,6 @@
 #include <string>
 #include <vector>
 
-
 namespace gpcodegen {
 
 /** \addtogroup gpcodegen
@@ -32,6 +31,13 @@ class CodegenInterface {
  public:
   virtual ~CodegenInterface() = default;
 
+  /**
+   * @brief Hook to request for and initialize any dependencies
+   *
+   * @return true on success
+   */
+  virtual bool InitDependencies() = 0;
+
   /**
    * @brief Generates specialized code at run time.
    *

src/backend/codegen/include/codegen/exec_eval_expr_codegen.h

@@ -13,9 +13,10 @@
 #ifndef GPCODEGEN_EXECEVALEXPR_CODEGEN_H_  // NOLINT(build/header_guard)
 #define GPCODEGEN_EXECEVALEXPR_CODEGEN_H_
 
-#include "codegen/codegen_wrapper.h"
 #include "codegen/base_codegen.h"
+#include "codegen/codegen_wrapper.h"
 #include "codegen/expr_tree_generator.h"
+#include "codegen/slot_getattr_codegen.h"
 
 namespace gpcodegen {
 
@@ -48,6 +49,8 @@ class ExecEvalExprCodegen: public BaseCodegen<ExecEvalExprFn> {
 
   virtual ~ExecEvalExprCodegen() = default;
 
+  bool InitDependencies() override;
+
  protected:
   /**
    * @brief Generate code for expression evaluation.
@@ -71,9 +74,12 @@ class ExecEvalExprCodegen: public BaseCodegen<ExecEvalExprFn> {
 
  private:
   ExprState *exprstate_;
-  ExprContext *econtext_;
   PlanState* plan_state_;
 
+  ExprTreeGeneratorInfo gen_info_;
+  SlotGetAttrCodegen* slot_getattr_codegen_;
+  std::unique_ptr<ExprTreeGenerator> expr_tree_generator_;
+
   static constexpr char kExecEvalExprPrefix[] = "ExecEvalExpr";
 
   /**
@@ -88,8 +94,7 @@ class ExecEvalExprCodegen: public BaseCodegen<ExecEvalExprFn> {
    * @brief Prepare generation of dependent slot_getattr() if necessary
    * @return true on successful generation.
    **/
-  void PrepareSlotGetAttr(gpcodegen::GpCodegenUtils* codegen_utils,
-                          ExprTreeGeneratorInfo* gen_info);
+  void PrepareSlotGetAttr();
 };
 
 /** @} */

src/backend/codegen/include/codegen/exec_variable_list_codegen.h

@@ -14,6 +14,7 @@
 #define GPCODEGEN_EXECVARIABLELIST_CODEGEN_H_
 
 #include "codegen/codegen_wrapper.h"
+#include "codegen/slot_getattr_codegen.h"
 #include "codegen/base_codegen.h"
 
 namespace gpcodegen {
@@ -43,6 +44,8 @@ class ExecVariableListCodegen: public BaseCodegen<ExecVariableListFn> {
 
   virtual ~ExecVariableListCodegen() = default;
 
+  bool InitDependencies() override;
+
  protected:
   /**
    * @brief Generate code for the code path ExecVariableList > slot_getattr >
@@ -83,6 +86,9 @@ class ExecVariableListCodegen: public BaseCodegen<ExecVariableListFn> {
   ProjectionInfo* proj_info_;
   TupleTableSlot* slot_;
 
+  int max_attr_;
+  SlotGetAttrCodegen* slot_getattr_codegen_;
+
 
   static constexpr char kExecVariableListPrefix[] = "ExecVariableList";
 

src/backend/codegen/include/codegen/slot_getattr_codegen.h

@@ -14,73 +14,160 @@
 #define GPCODEGEN_SLOT_GETATTR_CODEGEN_H_
 
 #include <string>
+#include <utility>
 
 #include "codegen/codegen_wrapper.h"
+#include "codegen/base_codegen.h"
 #include "codegen/utils/gp_codegen_utils.h"
 
+extern "C" {
+#include "postgres.h"  // NOLINT(build/include)
+#include "utils/elog.h"
+#include "access/htup.h"
+#include "nodes/execnodes.h"
+#include "executor/tuptable.h"
+
+}
+
 namespace gpcodegen {
 
 /** \addtogroup gpcodegen
  *  @{
  */
 
-class SlotGetAttrCodegen {
+class SlotGetAttrCodegen : public BaseCodegen<SlotGetAttrFn> {
+ public:
+  /**
+   * @brief Request code generation for the codepath slot_getattr >
+   * _slot_getsomeattr > slot_deform_tuple for the given slot and max_attr
+   *
+   * @param codegen_utils Utilities for easy code generation
+   * @param slot          Use the TupleDesc from this slot to generate
+   * @param max_attr      Generate slot deformation up to this many attributes
+   * @param out_func      Return the llvm::Function that will be generated
+   *
+   * @note This method does not actually do any code generation, but simply
+   * caches the information necessary for code generation when
+   * GenerateSlotGetAttr() is called. This way we can de-duplicate multiple
+   * requests for multiple slot_getattr() implementation producing only one per
+   * slot.
+   *
+   **/
+  static SlotGetAttrCodegen* GetCodegenInstance(
+      gpcodegen::CodegenManager* manager,
+      TupleTableSlot* slot,
+      int max_attr);
+
+  virtual ~SlotGetAttrCodegen();
+
   /**
    * @brief Generate code for the codepath slot_getattr > _slot_getsomeattr >
-   * slot_deform_tuple
+   * slot_deform_tuple for the given slot and max_attr
+   *
+   * @param codegen_utils Utilities for easy code generation
    *
-   * @param codegen_utils
-   * @param function_name Name of the generated function
-   * @param slot Use the TupleDesc for this slot to generate
-   * @param max_attr Generate slot deformation upto this many attributes
-   * @param out_func Return the generated llvm::Function
+   * Based on parameters given to SlotGetAttr::GetCodegenInstance(), the maximum
+   * max_attr is tracked for each slot. Then for each slot, we generate code for
+   * slot_getattr() that deforms tuples in that slot up to the maximum max_attr.
    *
    * @note This is a wrapper around GenerateSlotGetAttrInternal that handles the
    * case when generation fails, and cleans up a possible broken function by
    * removing it from the module.
-   **/
- public:
-  static bool GenerateSlotGetAttr(
-      gpcodegen::GpCodegenUtils* codegen_utils,
-      const std::string& function_name,
-      TupleTableSlot* slot,
-      int max_attr,
-      llvm::Function** out_func);
+   *
+   * TODO(shardikar, krajaraman) Remove this wrapper after a shared code
+   * generation framework implementation is complete.
+   */
+  bool GenerateCodeInternal(gpcodegen::GpCodegenUtils* codegen_utils) override;
+
+  /*
+   * @return A pointer to the yet un-compiled llvm::Function that will be
+   * generated and populate by this module
+   *
+   * @note This is initialized as NULL when on creation. Only if after code
+   * generation is performed by calling SlotGetAttrCodegen::GenerateCode and is
+   * successful, can this be assumed return a valid llvm::Function*
+   */
+  llvm::Function* GetGeneratedFunction() {
+    return llvm_function_;
+  }
 
  private:
+  /**
+   * @brief Constructor for SlotGetAttrCodegen
+   *
+   * @note The call to the constructor of BaseCodegen passes in a pointer to a
+   * dummy SlotGetAttrFn stored in this class. Thus SlotGetAttrCodegen can
+   * inherit all the functionality of BaseCodegen, including the pointer
+   * swapping logic, with low risk. However the pointer swapping will be of no
+   * consequence.
+   */
+  SlotGetAttrCodegen(gpcodegen::CodegenManager* manager,
+                     TupleTableSlot* slot,
+                     int max_attr)
+  : BaseCodegen(manager, kSlotGetAttrPrefix, slot_getattr, &dummy_func_),
+    slot_(slot),
+    max_attr_(max_attr),
+    llvm_function_(nullptr) {
+  }
+
   /**
    * @brief Generate code for the codepath slot_getattr > _slot_getsomeattr >
    * slot_deform_tuple
    *
-   * @param codegen_utils
-   * @param function_name Name of the generated function
-   * @param slot Use the TupleDesc for this slot to generate
-   * @param max_attr Generate slot deformation upto this many attributes
-   * @param out_func Return the generated llvm::Function
+   * @param codegen_utils Utilities for easy code generation
+   * @param slot          Use the TupleDesc from this slot to generate
+   * @param max_attr      Generate slot deformation up to this many attributes
+   * @param out_func      Return the llvm::Function that will be generated
    *
    * @return true on success generation; false otherwise
    *
    * @note Generate code for code path slot_getattr > * _slot_getsomeattrs >
    * slot_deform_tuple. slot_getattr() will eventually call slot_deform_tuple
    * (through _slot_getsomeattrs), which fetches all yet unread attributes of
-   * the slot until the given attribute. Moreover, instead of looping over the
-   * target list one at a time, this approach uses slot_getattr only once, with
-   * the largest attribute index from the target list.
-   *
+   * the slot until the given attribute.
    *
    * This implementation does not support:
-   *  (1) Variable length attributes
-   *  (2) Attributes passed by reference
+   *  (1) Attributes passed by reference
    *
    * If at execution time, we see any of the above types of attributes,
    * we fall backs to the regular function.
    **/
-  static bool GenerateSlotGetAttrInternal(
+  bool GenerateSlotGetAttr(
       gpcodegen::GpCodegenUtils* codegen_utils,
-      const std::string& function_name,
       TupleTableSlot* slot,
       int max_attr,
-      llvm::Function** out_func);
+      llvm::Function* out_func);
+
+  /**
+   * @brief Removes the entry of this SlotGetAttrCodegen from the static cache.
+   */
+  void RemoveSelfFromCache();
+
+  TupleTableSlot* slot_;
+  // Max attribute to deform to
+  int max_attr_;
+  // Primary function to be generated and populated
+  llvm::Function* llvm_function_;
+  // A dummy function pointer that can be swapped by the BaseCodegen implementation
+  SlotGetAttrFn dummy_func_;
+
+  static constexpr char kSlotGetAttrPrefix[] = "slot_getattr";
+  /**
+   * Utility map indexed by CodegenManager pointer to track all instances of
+   * SlotGetAttrCodegen created by SlotGetAttrCodegen::GetCodegenInstance.
+   * Each entry of the utility map contains another map keyed on the de-duplication key
+   * used by SlotGetAttrCodegen::GetCodegenInstance i.e slot.
+   *
+   * Entries are inserted into this map on creation of SlotGetAttrCodegen objects, and
+   * removed on destruction. When an inner-map for some manager becomes empty
+   * (when the manager itself is destroyed), the entry for that manager is
+   * erased from the outer-map as well
+   *
+   * Map of the form: { manager -> { slot -> slot_getattr_codegen } }
+   */
+  typedef std::unordered_map<TupleTableSlot*, SlotGetAttrCodegen*> SlotGetAttrCodegenCache;
+  static std::unordered_map<gpcodegen::CodegenManager*, SlotGetAttrCodegenCache> codegen_cache_by_manager;
+
 };
 
 /** @} */

src/backend/codegen/slot_getattr_codegen.cc

@@ -48,49 +48,100 @@ class Value;
 
 using gpcodegen::SlotGetAttrCodegen;
 
+constexpr char SlotGetAttrCodegen::kSlotGetAttrPrefix[];
+
 // TODO(shardikar): Retire this GUC after performing experiments to find the
 // tradeoff of codegen-ing slot_getattr() (potentially by measuring the
 // difference in the number of instructions) when one of the first few
 // attributes is varlen.
 extern const int codegen_varlen_tolerance;
 
-// TODO(shardikar, krajaraman) Remove this wrapper after implementing an
-// interface to share code generation logic.
-bool SlotGetAttrCodegen::GenerateSlotGetAttr(
-    gpcodegen::GpCodegenUtils* codegen_utils,
-    const std::string& function_name,
+std::unordered_map<gpcodegen::CodegenManager*,
+    SlotGetAttrCodegen::SlotGetAttrCodegenCache> SlotGetAttrCodegen::codegen_cache_by_manager;
+
+SlotGetAttrCodegen* SlotGetAttrCodegen::GetCodegenInstance(
+    gpcodegen::CodegenManager* manager,
     TupleTableSlot *slot,
-    int max_attr,
-    llvm::Function** out_func) {
+    int max_attr) {
+
+  // Create an cache entry for this manager if it doesn't already exist
+  auto it = codegen_cache_by_manager[manager].find(slot);
+
+  SlotGetAttrCodegen* generator = nullptr;
+  if (it != codegen_cache_by_manager[manager].end()) {
+    // For a slot already seen before, update max_attr value only
+    generator = it->second;
+    generator->max_attr_ = std::max(generator->max_attr_, max_attr);
+  } else {
+    // For a slot we haven't see before, create and add a new object
+    generator = new SlotGetAttrCodegen(manager, slot, max_attr);
+    codegen_cache_by_manager[manager].insert(std::make_pair(slot, generator));
+    // Enroll this in the manager so that it can take ownership
+    manager->EnrollCodeGenerator(CodegenFuncLifespan_Parameter_Invariant,
+                                 generator);
+  }
 
-  *out_func = nullptr;
+  assert(nullptr != generator);
+  return generator;
+}
 
-  bool ret = GenerateSlotGetAttrInternal(
-      codegen_utils, function_name, slot, max_attr, out_func);
+void SlotGetAttrCodegen::RemoveSelfFromCache() {
+  CodegenManager* manager = this->manager();
 
-  assert(nullptr != *out_func);
+  assert(manager != nullptr && codegen_cache_by_manager.find(manager) != codegen_cache_by_manager.end());
+  std::unordered_map<TupleTableSlot*, SlotGetAttrCodegen*>& cache_map =
+      codegen_cache_by_manager[manager];
+  auto it = codegen_cache_by_manager[manager].find(slot_);
 
-  if (!ret ||
-      (codegen_validate_functions && llvm::verifyFunction(**out_func))) {
-    (*out_func)->eraseFromParent();
-    *out_func = nullptr;
-    return false;
+  assert(it != cache_map.end() && it->second == this);
+  // Delete this SlotGetAttr from it's manager
+  cache_map.erase(it);
+
+  // Clean up manager data out of the map if this was the last instance
+  if (cache_map.empty()) {
+    codegen_cache_by_manager.erase(manager);
+  }
+}
+
+SlotGetAttrCodegen::~SlotGetAttrCodegen() {
+  RemoveSelfFromCache();
+}
+
+bool SlotGetAttrCodegen::GenerateCodeInternal(
+    gpcodegen::GpCodegenUtils* codegen_utils) {
+
+  // This function may be called multiple times, but it should generate code only once
+  if (IsGenerated()) {
+    return true;
   }
 
-  return ret;
+  // Give the function a human readable name
+  std::string function_name = GetUniqueFuncName() + "_" +
+      std::to_string(reinterpret_cast<uint64_t>(slot_)) + "_" +
+      std::to_string(max_attr_);
+  llvm::Function* function = CreateFunction<SlotGetAttrFn>(codegen_utils, function_name);
+
+  bool isGenerated = GenerateSlotGetAttr(codegen_utils, slot_, max_attr_, function);
+
+  if (isGenerated) {
+    elog(DEBUG1, "slot_getattr was generated successfully!");
+    assert(nullptr != function);
+    llvm_function_ = function;
+    return true;
+  } else {
+    elog(DEBUG1, "slot_getattr generation failed!");
+    llvm_function_ = nullptr;
+    return false;
+  }
 }
 
-bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
+bool SlotGetAttrCodegen::GenerateSlotGetAttr(
     gpcodegen::GpCodegenUtils* codegen_utils,
-    const std::string& function_name,
     TupleTableSlot *slot,
     int max_attr,
-    llvm::Function** out_func) {
+    llvm::Function* slot_getattr_func) {
 
   // So looks like we're going to generate code
-  *out_func = codegen_utils->CreateFunction<SlotGetAttrFn>(function_name);
-  llvm::Function* slot_getattr_func = *out_func;
-
   auto irb = codegen_utils->ir_builder();
 
   // BasicBlock of function entry.
@@ -99,9 +150,15 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
   // BasicBlock for checking correct slot
   llvm::BasicBlock* slot_check_block = codegen_utils->CreateBasicBlock(
       "slot_check", slot_getattr_func);
-  // BasicBlock for checking tuple type.
-  llvm::BasicBlock* tuple_type_check_block = codegen_utils->CreateBasicBlock(
-      "tuple_type_check", slot_getattr_func);
+  // BasicBlock for checking virtual tuple type.
+  llvm::BasicBlock* virtual_tuple_check_block = codegen_utils->CreateBasicBlock(
+      "virtual_tuple_check", slot_getattr_func);
+  // BasicBlock for checking memtuple type.
+  llvm::BasicBlock* memtuple_check_block = codegen_utils->CreateBasicBlock(
+      "memtuple_check", slot_getattr_func);
+  // BasicBlock for handling memtuple.
+  llvm::BasicBlock* memtuple_block = codegen_utils->CreateBasicBlock(
+      "memtuple", slot_getattr_func);
   // BasicBlock for heap tuple check.
   llvm::BasicBlock* heap_tuple_check_block = codegen_utils->CreateBasicBlock(
       "heap_tuple_check", slot_getattr_func);
@@ -111,6 +168,9 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
   // BasicBlock for final computations.
   llvm::BasicBlock* final_block = codegen_utils->CreateBasicBlock(
       "final", slot_getattr_func);
+  // BasicBlock for return
+  llvm::BasicBlock* return_block = codegen_utils->CreateBasicBlock(
+      "return", slot_getattr_func);
   // BasicBlock for fall back.
   llvm::BasicBlock* fallback_block = codegen_utils->CreateBasicBlock(
       "fallback", slot_getattr_func);
@@ -118,6 +178,9 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
   // External functions
   llvm::Function* llvm_memset =
       codegen_utils->GetOrRegisterExternalFunction(memset, "memset");
+  llvm::Function* llvm_memtuple_getattr =
+      codegen_utils->GetOrRegisterExternalFunction(memtuple_getattr,
+                                                   "memtuple_getattr");
   llvm::Function* llvm_slot_deform_tuple =
       codegen_utils->GetOrRegisterExternalFunction(slot_deform_tuple,
                                                    "slot_deform_tuple");
@@ -140,6 +203,20 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
       DEBUG1,
       "Codegen'ed slot_getattr called!");
 #endif
+  // Retrieve slot's PRIVATE variables
+  llvm::Value* llvm_slot_PRIVATE_tts_isnull /* bool* */ =
+      irb->CreateLoad(codegen_utils->GetPointerToMember(
+          llvm_slot, &TupleTableSlot::PRIVATE_tts_isnull));
+  llvm::Value* llvm_slot_PRIVATE_tts_values /* Datum* */ =
+      irb->CreateLoad(codegen_utils->GetPointerToMember(
+          llvm_slot, &TupleTableSlot::PRIVATE_tts_values));
+  llvm::Value* llvm_slot_PRIVATE_tts_nvalid_ptr /* int* */ =
+      codegen_utils->GetPointerToMember(
+          llvm_slot, &TupleTableSlot::PRIVATE_tts_nvalid);
+  llvm::Value* llvm_slot_tts_mt_bind /* MemTupleBinding* */ =
+      irb->CreateLoad(codegen_utils->GetPointerToMember(
+          llvm_slot, &TupleTableSlot::tts_mt_bind));
+
   // We start a sequence of checks to ensure that everything is fine and
   // we do not need to fall back.
   irb->CreateBr(slot_check_block);
@@ -152,39 +229,61 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
   // in as an argument to slot_getattr
   irb->CreateCondBr(
       irb->CreateICmpEQ(llvm_slot, llvm_slot_arg),
-      tuple_type_check_block /* true */,
+      virtual_tuple_check_block /* true */,
       fallback_block /* false */);
 
+  // Virtual tuple check block
+  // -------------------------
 
-  // Tuple type check block
-  // ----------------------
-  // We fall back if we see a virtual tuple or mem tuple,
-  // but it's possible to partially handle those cases also
-
-  irb->SetInsertPoint(tuple_type_check_block);
+  irb->SetInsertPoint(virtual_tuple_check_block);
+  // (slot->PRIVATE_tts_flags & TTS_VIRTUAL) != 0
   llvm::Value* llvm_slot_PRIVATE_tts_flags_ptr =
       codegen_utils->GetPointerToMember(
           llvm_slot, &TupleTableSlot::PRIVATE_tts_flags);
-  llvm::Value* llvm_slot_PRIVATE_tts_memtuple =
-      irb->CreateLoad(codegen_utils->GetPointerToMember(
-          llvm_slot, &TupleTableSlot::PRIVATE_tts_memtuple));
-
-  // (slot->PRIVATE_tts_flags & TTS_VIRTUAL) != 0
   llvm::Value* llvm_tuple_is_virtual = irb->CreateICmpNE(
       irb->CreateAnd(
           irb->CreateLoad(llvm_slot_PRIVATE_tts_flags_ptr),
           codegen_utils->GetConstant(TTS_VIRTUAL)),
           codegen_utils->GetConstant(0));
 
+  // If it is indeed a virtual tuple, we must have already deformed this tuple,
+  // so go straight to returning the contained values
+  irb->CreateCondBr(
+      llvm_tuple_is_virtual,
+      return_block /* true */,
+      memtuple_check_block /* false */);
+
+
+  // Memtuple type check block
+  // ----------------------
+
+  irb->SetInsertPoint(memtuple_check_block);
+
   // slot->PRIVATE_tts_memtuple != NULL
+  llvm::Value* llvm_slot_PRIVATE_tts_memtuple =
+      irb->CreateLoad(codegen_utils->GetPointerToMember(
+          llvm_slot, &TupleTableSlot::PRIVATE_tts_memtuple));
   llvm::Value* llvm_tuple_has_memtuple = irb->CreateICmpNE(
       llvm_slot_PRIVATE_tts_memtuple,
       codegen_utils->GetConstant((MemTuple) NULL));
 
-  // Fall back if tuple is virtual or memtuple is null
   irb->CreateCondBr(
-      irb->CreateOr(llvm_tuple_is_virtual, llvm_tuple_has_memtuple),
-      fallback_block /*true*/, heap_tuple_check_block /*false*/);
+      llvm_tuple_has_memtuple,
+      memtuple_block /*true*/, heap_tuple_check_block /*false*/);
+
+  // Memtuple Block
+  // --------------
+
+  irb->SetInsertPoint(memtuple_block);
+  // return memtuple_getattr(slot->PRIVATE_tts_memtuple,
+  //    slot->tts_mt_bind, attnum, isnull);
+  llvm::Value* llvm_memtuple_ret = irb->CreateCall(llvm_memtuple_getattr, {
+      llvm_slot_PRIVATE_tts_memtuple,
+      llvm_slot_tts_mt_bind,
+      llvm_attnum_arg,
+      llvm_isnull_ptr_arg});
+  irb->CreateRet(llvm_memtuple_ret);
+
 
   // HeapTuple check block
   // ---------------------
@@ -238,17 +337,6 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
           llvm_max_attr);
   // }}}
 
-  // Retrieve slot's PRIVATE variables
-  llvm::Value* llvm_slot_PRIVATE_tts_isnull /* bool* */ =
-      irb->CreateLoad(codegen_utils->GetPointerToMember(
-          llvm_slot, &TupleTableSlot::PRIVATE_tts_isnull));
-  llvm::Value* llvm_slot_PRIVATE_tts_values /* Datum* */ =
-      irb->CreateLoad(codegen_utils->GetPointerToMember(
-          llvm_slot, &TupleTableSlot::PRIVATE_tts_values));
-  llvm::Value* llvm_slot_PRIVATE_tts_nvalid_ptr /* int* */ =
-      codegen_utils->GetPointerToMember(
-          llvm_slot, &TupleTableSlot::PRIVATE_tts_nvalid);
-
   llvm::Value* llvm_heaptuple_t_data_t_hoff = irb->CreateLoad(
       codegen_utils->GetPointerToMember(llvm_heaptuple_t_data,
                                         &HeapTupleHeaderData::t_hoff));
@@ -541,11 +629,13 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
           llvm_slot_PRIVATE_tts_flags_ptr);
 
   // }}}
+  irb->CreateBr(return_block);
 
+  // Return block
+  // ------------
   // slot_getattr() after calling _slot_getsomeattrs() {{{
-  //
-  // TODO(hardikar): We can optimize this further by getting rid of
-  // the follow computation and changing the signature of this function
+
+  irb->SetInsertPoint(return_block);
 
   // *isnull = slot->PRIVATE_tts_isnull[attnum-1];
   llvm::Value* llvm_isnull_from_slot_val =
@@ -570,8 +660,6 @@ bool SlotGetAttrCodegen::GenerateSlotGetAttrInternal(
   llvm_error->addIncoming(codegen_utils->GetConstant(0),
       slot_check_block);
   llvm_error->addIncoming(codegen_utils->GetConstant(1),
-      tuple_type_check_block);
-  llvm_error->addIncoming(codegen_utils->GetConstant(2),
       heap_tuple_check_block);
 
   codegen_utils->CreateElog(

src/backend/codegen/tests/codegen_framework_unittest.cc

@@ -92,9 +92,11 @@ MulFunc mul_func_ptr = nullptr;
 
 class SumCodeGenerator : public BaseCodegen<SumFunc> {
  public:
-  explicit SumCodeGenerator(SumFunc regular_func_ptr,
+  explicit SumCodeGenerator(gpcodegen::CodegenManager* manager,
+                            SumFunc regular_func_ptr,
                              SumFunc* ptr_to_regular_func_ptr) :
-                             BaseCodegen(kAddFuncNamePrefix,
+                             BaseCodegen(manager,
+                                         kAddFuncNamePrefix,
                                          regular_func_ptr,
                                          ptr_to_regular_func_ptr) {
   }
@@ -122,11 +124,13 @@ class SumCodeGenerator : public BaseCodegen<SumFunc> {
 
 class MulOverflowCodeGenerator : public BaseCodegen<MulFunc> {
  public:
-  explicit MulOverflowCodeGenerator(MulFunc regular_func_ptr,
+  explicit MulOverflowCodeGenerator(gpcodegen::CodegenManager* manager,
+                                    MulFunc regular_func_ptr,
                                     MulFunc* ptr_to_regular_func_ptr) :
-                                    BaseCodegen(kMulFuncNamePrefix,
-                                         regular_func_ptr,
-                                         ptr_to_regular_func_ptr) {
+                                    BaseCodegen(manager,
+                                                kMulFuncNamePrefix,
+                                                regular_func_ptr,
+                                                ptr_to_regular_func_ptr) {
   }
 
   virtual ~MulOverflowCodeGenerator() = default;
@@ -178,9 +182,11 @@ class MulOverflowCodeGenerator : public BaseCodegen<MulFunc> {
 
 class FailingCodeGenerator : public BaseCodegen<SumFunc> {
  public:
-  explicit FailingCodeGenerator(SumFunc regular_func_ptr,
+  explicit FailingCodeGenerator(gpcodegen::CodegenManager* manager,
+                                SumFunc regular_func_ptr,
                                 SumFunc* ptr_to_regular_func_ptr):
-                                BaseCodegen(kFailingFuncNamePrefix,
+                                BaseCodegen(manager,
+                                            kFailingFuncNamePrefix,
                                             regular_func_ptr,
                                             ptr_to_regular_func_ptr) {
   }
@@ -200,9 +206,11 @@ template <bool GEN_SUCCESS>
 class UncompilableCodeGenerator : public BaseCodegen<UncompilableFunc> {
  public:
   explicit UncompilableCodeGenerator(
+      gpcodegen::CodegenManager* manager,
       UncompilableFunc regular_func_ptr,
       UncompilableFunc* ptr_to_regular_func_ptr)
-  : BaseCodegen(kUncompilableFuncNamePrefix,
+  : BaseCodegen(manager,
+                kUncompilableFuncNamePrefix,
                 regular_func_ptr,
                 ptr_to_regular_func_ptr) {
   }
@@ -233,9 +241,11 @@ class DatumToCppCastGenerator :
 
  public:
   explicit DatumToCppCastGenerator(
+      gpcodegen::CodegenManager* manager,
       DatumCastTemplateFn regular_func_ptr,
       DatumCastTemplateFn* ptr_to_regular_func_ptr):
-      BaseCodegen<DatumCastTemplateFn>(kDatumToCppCastFuncNamePrefix,
+      BaseCodegen<DatumCastTemplateFn>(manager,
+                  kDatumToCppCastFuncNamePrefix,
                   regular_func_ptr,
                   ptr_to_regular_func_ptr) {
   }
@@ -268,9 +278,11 @@ class CppToDatumCastGenerator :
   using DatumCastTemplateFn = DatumCastFn<Datum, src_type>;
  public:
   explicit CppToDatumCastGenerator(
+      gpcodegen::CodegenManager* manager,
       DatumCastTemplateFn regular_func_ptr,
       DatumCastTemplateFn* ptr_to_regular_func_ptr):
-      BaseCodegen<DatumCastTemplateFn>(kCppToDatumCastFuncNamePrefix,
+      BaseCodegen<DatumCastTemplateFn>(manager,
+                  kCppToDatumCastFuncNamePrefix,
                   regular_func_ptr,
                   ptr_to_regular_func_ptr) {
   }
@@ -330,7 +342,7 @@ class CodegenManagerTest : public ::testing::Test {
 
   template <typename ClassType, typename FuncType>
   void EnrollCodegen(FuncType reg_func, FuncType* ptr_to_chosen_func) {
-    ClassType* code_gen = new ClassType(reg_func, ptr_to_chosen_func);
+    ClassType* code_gen = new ClassType(manager_.get(), reg_func, ptr_to_chosen_func);
     ASSERT_TRUE(reg_func == *ptr_to_chosen_func);
     ASSERT_TRUE(manager_->EnrollCodeGenerator(
         CodegenFuncLifespan_Parameter_Invariant,
@@ -343,12 +355,12 @@ class CodegenManagerTest : public ::testing::Test {
                       const std::vector<CppType>& values) {
     DatumCastFn<Datum, CppType> CppToDatumCgFn = CppToDatumReg;
     CppToDatumCastGenerator<CppType>* cpp_datum_gen =
-        new CppToDatumCastGenerator<CppType>(CppToDatumReg, &CppToDatumCgFn);
+        new CppToDatumCastGenerator<CppType>(manager_.get(), CppToDatumReg, &CppToDatumCgFn);
 
 
     DatumCastFn<CppType, Datum> DatumToCppCgFn = DatumToCppReg;
     DatumToCppCastGenerator<CppType>* datum_cpp_gen =
-        new DatumToCppCastGenerator<CppType>(DatumToCppReg, &DatumToCppCgFn);
+        new DatumToCppCastGenerator<CppType>(manager_.get(), DatumToCppReg, &DatumToCppCgFn);
 
     ASSERT_TRUE(manager_->EnrollCodeGenerator(
         CodegenFuncLifespan_Parameter_Invariant, cpp_datum_gen));
@@ -378,7 +390,8 @@ class CodegenManagerTest : public ::testing::Test {
 
 TEST_F(CodegenManagerTest, TestGetters) {
   sum_func_ptr = nullptr;
-  SumCodeGenerator* code_gen = new SumCodeGenerator(SumFuncRegular,
+  SumCodeGenerator* code_gen = new SumCodeGenerator(manager_.get(),
+                                                    SumFuncRegular,
                                                     &sum_func_ptr);
 
   EXPECT_EQ(SumCodeGenerator::kAddFuncNamePrefix,
@@ -580,7 +593,8 @@ TEST_F(CodegenManagerTest, MulOverFlowTest) {
 
 TEST_F(CodegenManagerTest, ResetTest) {
   sum_func_ptr = nullptr;
-  SumCodeGenerator* code_gen = new SumCodeGenerator(SumFuncRegular,
+  SumCodeGenerator* code_gen = new SumCodeGenerator(manager_.get(),
+                                                    SumFuncRegular,
                                                     &sum_func_ptr);
 
   ASSERT_TRUE(manager_->EnrollCodeGenerator(