Fixing functional issue in DB query 11 (oneapi-src#841)

* Fixing bug in DB query 11

* Updated cache depth

* Fixed typo :/

* Changes to MapJoin to try and fix issue :(

* Reset old loop structure for compute loop in Q11

* Printing kernel time as well as total time (including memory transfer)

* Printing more performance results

* Changes to Q11 to improve performance
std::endl for some messages to ensure buffer is flushed
Updated README with new output example

* Creating new, more useful, CachedMemory
More explicit messages between kernels to try and address hang

* Removed debug prints

* Removed ArrayMap from MapJoin in favour of separate 'data' and 'valid'
map arrays passed in by caller. This allows for the compiler to create
a more optimal memory system.
Updated q9 and q11 for to use this new MapJoin

* Removing ivdep from map join loops in query9 and query11 since they are
not needed anymore with change to loop induction variable indexing.

* Samples JSON, adding SF_SMALL=1 to Windows CMake generation

Author: tyoungsc

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/README.md

@@ -16,16 +16,7 @@ The [oneAPI Programming Guide](https://software.intel.com/en-us/oneapi-programmi
 _Notice: This example design is only officially supported for the Intel® FPGA PAC D5005 (with Intel Stratix® 10 SX)_
 
 **Performance**
-In this design, we accelerate four database queries as *offload accelerators*. In an offload accelerator scheme, the queries are performed by transferring the relevant data from the CPU host to the FPGA, starting the query kernel on the FPGA, and copying the results back. This means that the relevant performance number is the latency (i.e., the wall clock time) from when the query is requested to the time the output data is accessible by the host. This includes the time to transfer data between the CPU and FPGA over PCIe (with an approximate read and write bandwidth of 6877 and 6582 MB/s, respectively). As shown in the table below, most of the total query time is spent transferring the data between the CPU and FPGA, and the query kernels themselves are a small portion of the total latency.
-
-The performance data below was gathered using the Intel® FPGA PAC D5005 (with Intel Stratix® 10 SX) with a database scale factor (SF) of 1. Please see the [Database files](#database-files) section for more information on generating data for a scale factor of 1.
-
-| Query | Approximate Data Transfer Time (ms) | Measured Total Query Processing Time (ms)
-|:---   |:---                                 |:---
-| 1     | 35                                  | 39
-| 9     | 37                                  | 43
-| 11    | 5                                   | 11
-| 12    | 16                                  | 26
+In this design, we accelerate four database queries as *offload accelerators*. In an offload accelerator scheme, the queries are performed by transferring the relevant data from the CPU host to the FPGA, starting the query kernel on the FPGA, and copying the results back. This means that the relevant performance number is the processing time (i.e., the wall clock time) from when the query is requested to the time the output data is accessible by the host. This includes the time to transfer data between the CPU and FPGA over PCIe (with an approximate read and write bandwidth of 6877 and 6582 MB/s, respectively). As shown in the table below, most of the total query time is spent transferring the data between the CPU and FPGA, and the query kernels themselves are a small portion of the total latency.
 
 ## Purpose
 The database in this tutorial has 8-tables and a set of 21 business-oriented queries with broad industry-wide relevance. This reference design shows how four queries can be accelerated using the Intel® FPGA PAC D5005 (with Intel Stratix® 10 SX) and oneAPI. To do so, we create a set of common database operators (found in the `src/db_utils/` directory) that are are combined in different ways to build the four queries.
@@ -232,7 +223,9 @@ You should see the following output in the console:
     Validating query 1 test results
     Running Q1 within 90 days of 1998-12-1
     Validating query 1 test results
-    Processing time: 40.2986 ms
+    Total processing time: 34.389 ms
+    Kernel processing time: 3.16621 ms
+    Throughput: 315.835 queries/s
     PASSED
     ```
     NOTE: the scale factor 1 (SF=1) database files (`../data/sf1`) are **not** shipped with this reference design. Please refer to the [Database files](#database-files) section for information on how to generate these files yourself.

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/sample.json

@@ -145,7 +145,7 @@
           "cd ../..",
           "mkdir build-q1",
           "cd build-q1",
-          "cmake -G \"NMake Makefiles\" ../ReferenceDesigns/db -DQUERY=1",
+          "cmake -G \"NMake Makefiles\" ../ReferenceDesigns/db -DQUERY=1 -DSF_SMALL=1",
           "nmake report"
         ]
       },
@@ -156,7 +156,7 @@
           "cd ../..",
           "mkdir build-q11",
           "cd build-q11",
-          "cmake -G \"NMake Makefiles\" ../ReferenceDesigns/db -DQUERY=11",
+          "cmake -G \"NMake Makefiles\" ../ReferenceDesigns/db -DQUERY=11 -DSF_SMALL=1",
           "nmake report"
         ]
       },
@@ -167,7 +167,7 @@
           "cd ../..",
           "mkdir build-q12",
           "cd build-q12",
-          "cmake -G \"NMake Makefiles\" ../ReferenceDesigns/db -DQUERY=12",
+          "cmake -G \"NMake Makefiles\" ../ReferenceDesigns/db -DQUERY=12 -DSF_SMALL=1",
           "nmake report"
         ]
       }

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/src/db.cpp

@@ -268,8 +268,15 @@ int main(int argc, char* argv[]) {
           std::accumulate(total_latency.begin() + 1, total_latency.end(), 0.0) /
           (double)(runs - 1);
 
+      double kernel_latency_avg =
+        std::accumulate(kernel_latency.begin() + 1, kernel_latency.end(), 0.0) /
+        (double)(runs - 1);
+
       // print the performance results
       std::cout << "Processing time: " << total_latency_avg << " ms\n";
+      std::cout << "Kernel time: " << kernel_latency_avg << " ms\n";
+      std::cout << "Throughput: " << ((1 / kernel_latency_avg) * 1e3)
+                << " queries/s\n";
 #endif
 
       std::cout << "PASSED\n";
@@ -325,7 +332,7 @@ bool DoQuery1(queue& q, Database& dbinfo, std::string& db_root_dir,
   unsigned int low_date_compact = low_date.ToCompact();
 
   std::cout << "Running Q1 within " << DELTA << " days of " << date.year << "-"
-            << date.month << "-" << date.day << "\n";
+            << date.month << "-" << date.day << std::endl;
 
   // the query output data
   std::array<DBDecimal, kQuery1OutSize> sum_qty = {0}, sum_base_price = {0},
@@ -378,7 +385,7 @@ bool DoQuery9(queue& q, Database& dbinfo, std::string& db_root_dir,
   // convert the colour regex to uppercase characters (convention)
   transform(colour.begin(), colour.end(), colour.begin(), ::toupper);
 
-  std::cout << "Running Q9 with colour regex: " << colour << "\n";
+  std::cout << "Running Q9 with colour regex: " << colour << std::endl;
 
   // the output of the query
   std::array<DBDecimal, 25 * 2020> sum_profit;
@@ -424,7 +431,8 @@ bool DoQuery11(queue& q, Database& dbinfo, std::string& db_root_dir,
   transform(nation.begin(), nation.end(), nation.begin(), ::toupper);
 
   std::cout << "Running Q11 for nation " << nation.c_str()
-            << " (key=" << (int)(dbinfo.n.name_key_map[nation]) << ")\n";
+            << " (key=" << (int)(dbinfo.n.name_key_map[nation]) << ")"
+            << std::endl;
 
   // the query output
   std::vector<DBIdentifier> partkeys(kPartTableSize);
@@ -492,7 +500,7 @@ bool DoQuery12(queue& q, Database& dbinfo, std::string& db_root_dir,
 
   std::cout << "Running Q12 between years " << low_date.year << " and "
             << high_date.year << " for SHIPMODES " << shipmode1 << " and "
-            << shipmode2 << "\n";
+            << shipmode2 << std::endl;;
 
   // the output of the query
   std::array<DBDecimal, 2> high_line_count, low_line_count;

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/src/db_utils/Accumulator.hpp

@@ -89,13 +89,13 @@ class BRAMAccumulator {
   // initialize the memory entries
   void Init() {
     // initialize the memory entries
-    for (IndexType i = 0; i < size; i++) {
+    for (int i = 0; i < size; i++) {
       mem[i] = 0;
     }
 
 // initialize the cache
 #pragma unroll
-    for (IndexType i = 0; i < cache_size + 1; i++) {
+    for (int i = 0; i < cache_size + 1; i++) {
       cache_value[i] = 0;
       cache_tag[i] = 0;
     }
@@ -104,34 +104,33 @@ class BRAMAccumulator {
   // accumulate 'value' into register 'index' (i.e. registers[index] += value)
   void Accumulate(IndexType index, StorageType value) {
     // get value from memory
-    StorageType currVal = mem[index];
+    StorageType curr_val = mem[index];
 
 // check if value is in cache
 #pragma unroll
-    for (IndexType i = 0; i < cache_size + 1; i++) {
+    for (int i = 0; i < cache_size + 1; i++) {
       if (cache_tag[i] == index) {
-        currVal = cache_value[i];
+        curr_val = cache_value[i];
       }
     }
 
     // write the new value to both the shift register cache and the local mem
-    const StorageType newVal = currVal + value;
-    mem[index] = cache_value[cache_size] = newVal;
+    StorageType new_val = curr_val + value;
+    mem[index] = new_val;
+    cache_value[cache_size] = new_val;
     cache_tag[cache_size] = index;
 
 // Cache is just a shift register, so shift it
 // pushing into back of the shift register done above
 #pragma unroll
-    for (IndexType i = 0; i < cache_size; i++) {
+    for (int i = 0; i < cache_size; i++) {
       cache_value[i] = cache_value[i + 1];
       cache_tag[i] = cache_tag[i + 1];
     }
   }
 
   // get the value of memory at 'index'
-  StorageType Get(IndexType index) {
-    return mem[index];
-  }
+  StorageType Get(IndexType index) { return mem[index]; }
 
   // internal storage
   StorageType mem[size];

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/src/db_utils/CachedMemory.hpp

@@ -0,0 +1,73 @@
+#ifndef __CACHED_MEMORY_HPP__
+#define __CACHED_MEMORY_HPP__
+
+template <typename StorageType, int n, int cache_n,
+          typename IndexType = int>
+class CachedMemory {
+  // static asserts
+  static_assert(n > 0);
+  static_assert(cache_n >= 0);
+  static_assert(std::is_arithmetic<StorageType>::value,
+                "StorageType must be arithmetic to support accumulation");
+  static_assert(std::is_integral<IndexType>::value,
+                "IndexType must be an integral type");
+  static_assert(std::numeric_limits<IndexType>::max() >= (n - 1),
+                "IndexType must be large enough to index the entire array");
+
+public:
+  CachedMemory() {}
+  
+  void Init(StorageType init_val = 0) {
+    for (int i = 0; i < n; i++) {
+      mem[i] = init_val;
+    }
+    #pragma unroll
+    for (int i = 0; i < cache_n + 1; i++) {
+      cache_value[i] = init_val;
+      cache_tag[i] = 0;
+    }
+  }
+
+  auto Get(IndexType idx) {
+    // grab the value from memory
+    StorageType ret = mem[idx];
+
+    // check for this value in the cache as well
+    #pragma unroll
+    for (int i = 0; i < cache_n + 1; i++) {
+      if (cache_tag[i] == idx) {
+        ret = cache_value[i];
+      }
+    }
+
+    return ret;
+  }
+
+  void Set(IndexType idx, StorageType val) {
+    // store the new value in the actual memory, and the start of the shift
+    // register cache
+    mem[idx] = val;
+    cache_value[cache_n] = val;
+    cache_tag[cache_n] = idx;
+
+    // shift the shift register cache
+    #pragma unroll
+    for (int i = 0; i < cache_n; i++) {
+      cache_value[i] = cache_value[i + 1];
+      cache_tag[i] = cache_tag[i + 1];
+    }
+  }
+
+private:
+  // internal storage
+  StorageType mem[n];
+
+  // internal cache for hiding write latency
+  [[intel::fpga_register]]
+  StorageType cache_value[cache_n + 1];
+
+  [[intel::fpga_register]]
+  int cache_tag[cache_n + 1];
+};
+
+#endif /* __CACHED_MEMORY_HPP__ */

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/src/db_utils/MapJoin.hpp

@@ -11,45 +11,13 @@
 #include "Unroller.hpp"
 #include "Tuple.hpp"
 #include "StreamingData.hpp"
-#include "ShannonIterator.hpp"
-
-//
-// ArrayMap class
-//
-template <typename Type, int size>
-class ArrayMap {
-  // static asserts
-  static_assert(size > 0,
-    "size must be positive and non-zero");
-  static_assert(std::is_same<bool, decltype(Type().valid)>::value,
-    "Type must have a 'valid' boolean member");
-
- public:
-  void Init() {
-    for (unsigned int i = 0; i < size; i++) {
-      valid[i] = false;
-    }
-  }
-
-  std::pair<bool, Type> Get(unsigned int key) {
-    return {valid[key], map[key]};
-  }
-
-  void Set(unsigned int key, Type data) {
-    map[key] = data;
-    valid[key] = true;
-  }
-
-  Type map[size];
-  bool valid[size];
-};
 
 //
 // MapJoin implementation
 //
 template<typename MapType, typename T2Pipe, typename T2Data, int t2_win_size,
          typename JoinPipe, typename JoinType>
-void MapJoin(MapType& map) {
+void MapJoin(MapType map_data[], bool map_valid[]) {
   //////////////////////////////////////////////////////////////////////////////
   // static asserts
   static_assert(t2_win_size > 0,
@@ -89,12 +57,10 @@ void MapJoin(MapType& map) {
         const unsigned int t2_key =
             in_data.data.template get<j>().PrimaryKey();
 
-        auto [data_valid, map_data] = map.Get(t2_key);
-
-        if (t2_win_valid && data_valid) {
+        if (t2_win_valid && map_valid[t2_key]) {
           // NOTE: order below important if Join() overrides valid
           join_data.data.template get<j>().valid = true;
-          join_data.data.template get<j>().Join(map_data,
+          join_data.data.template get<j>().Join(map_data[t2_key],
                                                 in_data.data.template get<j>());
         }
       });

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/src/dbdata.cpp

@@ -507,7 +507,7 @@ bool Database::ValidateQ1(std::string db_root_dir,
                           std::array<DBDecimal, 3 * 2>& avg_price,
                           std::array<DBDecimal, 3 * 2>& avg_discount,
                           std::array<DBDecimal, 3 * 2>& count) {
-  std::cout << "Validating query 1 test results\n";
+  std::cout << "Validating query 1 test results" << std::endl;
 
   // populate date row by row (as presented in the file)
   std::string path(db_root_dir + kSeparator + "answers" + kSeparator + "q1.out");
@@ -630,7 +630,7 @@ bool Database::ValidateQ1(std::string db_root_dir,
 //
 bool Database::ValidateQ9(std::string db_root_dir,
                           std::array<DBDecimal, 25 * 2020>& sum_profit) {
-  std::cout << "Validating query 9 test results\n";
+  std::cout << "Validating query 9 test results" << std::endl;
 
   // populate date row by row (as presented in the file)
   std::string path(db_root_dir + kSeparator + "answers" + kSeparator + "q9.out");
@@ -684,7 +684,7 @@ bool Database::ValidateQ9(std::string db_root_dir,
 bool Database::ValidateQ11(std::string db_root_dir,
                            std::vector<DBIdentifier>& partkeys,
                            std::vector<DBDecimal>& partkey_values) {
-  std::cout << "Validating query 11 test results\n";
+  std::cout << "Validating query 11 test results" << std::endl;
 
   // populate date row by row (as presented in the file)
   std::string path(db_root_dir + kSeparator + "answers" + kSeparator + "q11.out");

DirectProgramming/DPC++FPGA/ReferenceDesigns/db/src/query11/pipe_types.hpp

@@ -10,24 +10,6 @@
 
 using namespace sycl;
 
-//
-// A single row of the SUPPLIER table
-// with a subset of the columns (needed for this query)
-//
-class SupplierRow {
- public:
-  // SupplierRow() : valid(false), suppkey(0), nationkey(0) {}
-  SupplierRow() {}
-  SupplierRow(bool v_valid, DBIdentifier v_suppkey, unsigned char v_nationkey)
-      : valid(v_valid), suppkey(v_suppkey), nationkey(v_nationkey) {}
-
-  DBIdentifier PrimaryKey() const { return suppkey; }
-
-  bool valid;
-  DBIdentifier suppkey;
-  unsigned char nationkey;
-};
-
 //
 // A single row of the PARTSUPPLIER table
 // with a subset of the columns (needed for this query)
@@ -71,11 +53,11 @@ class SupplierPartSupplierJoined {
 
   DBIdentifier PrimaryKey() const { return partkey; }
 
-  void Join(const SupplierRow& s_row, const PartSupplierRow& ps_row) {
+  void Join(const unsigned char nation_key, const PartSupplierRow& ps_row) {
     partkey = ps_row.partkey;
     availqty = ps_row.availqty;
     supplycost = ps_row.supplycost;
-    nationkey = s_row.nationkey;
+    nationkey = nation_key;
   }
 
   bool valid;