Warnings

.gitignore

@@ -23,3 +23,5 @@ CMakeFiles/
 
 **/capstone-config-version.cmake
 **/capstone-config.cmake
+
+**/simeng-fileio-test.txt

.jenkins/build_arm22.sh

@@ -15,3 +15,7 @@ module load tools/cmake
 build armclang armclang++
 test
 run
+
+buildRelease armclang armclang++
+test
+run

.jenkins/build_gcc10.sh

@@ -15,3 +15,7 @@ module load tools/cmake
 build gcc g++
 test
 run
+
+buildRelease gcc g++
+test
+run

.jenkins/build_gcc7.sh

@@ -15,3 +15,7 @@ module load tools/cmake
 build gcc g++
 test
 run
+
+buildRelease gcc g++
+test
+run

.jenkins/build_gcc8.sh

@@ -15,3 +15,7 @@ module load tools/cmake
 build gcc g++
 test
 run
+
+buildRelease gcc g++
+test
+run

.jenkins/build_gcc9.sh

@@ -15,3 +15,7 @@ module load tools/cmake
 build gcc g++
 test
 run
+
+buildRelease gcc g++
+test
+run

.jenkins/build_test_run.sh

@@ -36,6 +36,16 @@ build () {
     cmake --build build --target install
 }
 
+# Build common function
+buildRelease () {
+    cd "$SIMENG_TOP" || exit
+    rm -rf build/* install/*
+
+    cmake -B build -S . -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX="$SIMENG_INSTALL" -DSIMENG_ENABLE_TESTS=ON -DSIMENG_USE_EXTERNAL_LLVM=ON -DLLVM_DIR=/home/br-simeng/llvm14.0.5/install-gcc7/lib/cmake/llvm/ -DCMAKE_C_COMPILER=$1 -DCMAKE_CXX_COMPILER=$2
+    cmake --build build -j
+    cmake --build build --target install
+}
+
 # Run tests
 test () {
     cd "$SIMENG_BUILD" || exit
@@ -52,16 +62,16 @@ run () {
     echo "Simulation without configuration file argument:"
     cat run
     echo ""
-    compare_outputs "$(grep "retired:" run | rev | cut -d ' ' -f1 | rev)" "3145731" "retired instructions"
-    compare_outputs "$(grep "cycles:" run | rev | cut -d ' ' -f1 | rev)" "3145732" "simulated cycles"
+    compare_outputs "$(grep "retired:" run | rev | cut -d ' ' -f1 | rev)" "6708" "retired instructions"
+    compare_outputs "$(grep "cycles:" run | rev | cut -d ' ' -f1 | rev)" "7955" "simulated cycles"
     echo ""
 
     ./bin/simeng "$SIMENG_TOP"/configs/tx2.yaml > run
     echo "Simulation with configuration file argument:"
     cat run
     echo ""
-    compare_outputs "$(grep "retired:" run | rev | cut -d ' ' -f1 | rev)" "3145732" "retired instructions"
-    compare_outputs "$(grep "cycles:" run | rev | cut -d ' ' -f1 | rev)" "1048588" "simulated cycles"
+    compare_outputs "$(grep "retired:" run | rev | cut -d ' ' -f1 | rev)" "6724" "retired instructions"
+    compare_outputs "$(grep "cycles:" run | rev | cut -d ' ' -f1 | rev)" "8677" "simulated cycles"
     echo ""
 }
 

CMakeLists.txt

@@ -71,8 +71,8 @@ set(CMAKE_MACOSX_RPATH 1)
 # Enable PIC for libraries
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
-# Enable additional compiler warnings for all targets
-add_compile_options(-Wall)
+# Create variable to enable additional compiler warnings for SimEng targets only
+set(SIMENG_COMPILE_OPTIONS -Wall -pedantic -Werror) #-Wextra
 
 # Disable RTTI for all targets
 add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fno-rtti>)
@@ -183,6 +183,8 @@ if(SIMENG_ENABLE_TESTS)
     set(LLVM_ENABLE_BINDINGS OFF)
     set(LLVM_INSTALL_UTILS OFF)
 
+    set(LLVM_ENABLE_WARNINGS OFF)
+
     # XXX all LLVM specific cmake variables must be set BEFORE FetchContent_MakeAvailable otherwise they have no effect
     FetchContent_MakeAvailable_SubDir_Args(llvm llvm-14.0.5.src EXCLUDE_FROM_ALL)
     # make sure we get the headers too

docs/sphinx/developer/components/coreinstance.rst

@@ -9,7 +9,7 @@ Process the config file
     Either the passed configuration file path, or default configuration string, is used to generate the model configuration class. All subsequent parameterised instantiations of simulation objects utilise this configuration class.
 
 Create the image process
-    From the passed workload path, or default set of instructions, a process image is created. A region of host memory is populated with workload data (e.g. instructions), a region for the HEAP, and an initial stack frame. References to it are then passed between various simulation objects to serve as the underlying process memory space.
+    From the passed workload path, or default binary, a process image is created. A region of host memory is populated with workload data (e.g. instructions), a region for the HEAP, and an initial stack frame. References to it are then passed between various simulation objects to serve as the underlying process memory space.
 
 Construct on-chip cache interfaces
     Based on the supplied configuration options, the on-chip cache interfaces are constructed. These interfaces sit on top of a reference to the process memory space constructed prior. Currently, only L1 instruction and data caches are supported and the interfaces are defined under the :ref:`L1-Data-Memory <l1dcnf>` and  :ref:`L1-Instruction-Memory <l1icnf>` config options.

docs/sphinx/user/running_simeng.rst

@@ -7,7 +7,7 @@ SimEng uses a configuration file and a program binary to produce a cycle-accurat
 
         <simeng_install_directory>/bin/simeng <config file> <binary>
 
-If no arguments are passed to SimEng, default options are used. The default configuration file is tuned to a ThunderX2 processor. The default program binary is defined in ``SimEng/src/include/simeng/CoreInstance.hh`` under the ``hex[]`` array which contains a set of raw instructions in a hexadecimal format.
+If no arguments are passed to SimEng, default options are used. The default configuration file is tuned to a ThunderX2 processor. The default program is a binary compiled to AArch64 found at ``SimEng/SimEngDefaultProgram``. This prints a welcome message to the console.
 
 Whilst a configuration file can be specified without a program (will use default program), a specified program must be accompanied by a configuration file.
 

src/include/simeng/CoreInstance.hh

@@ -21,19 +21,6 @@
 #include "simeng/pipeline/A64FXPortAllocator.hh"
 #include "simeng/pipeline/BalancedPortAllocator.hh"
 
-// Program used when no executable is provided; counts down from
-// 1024*1024, with an independent `orr` at the start of each branch.
-static uint32_t hex_[] = {
-    0x320C03E0,  // orr w0, wzr, #1048576
-    0x320003E1,  // orr w0, wzr, #1
-    0x71000400,  // subs w0, w0, #1
-    0x54FFFFC1,  // b.ne -8
-                 // .exit:
-    0xD2800000,  // mov x0, #0
-    0xD2800BC8,  // mov x8, #94
-    0xD4000001,  // svc #0
-};
-
 namespace simeng {
 
 /** A class to create a SimEng core instance from a supplied config. */
@@ -46,7 +33,7 @@ class CoreInstance {
 
   /** CoreInstance with source code assembled by LLVM and a model configuration.
    */
-  CoreInstance(char* assembledSource, size_t sourceSize,
+  CoreInstance(uint8_t* assembledSource, size_t sourceSize,
                ryml::ConstNodeRef config = config::SimInfo::getConfig());
 
   ~CoreInstance();
@@ -75,10 +62,10 @@ class CoreInstance {
   std::shared_ptr<char> getProcessImage() const;
 
   /** Getter for the size of the created process image. */
-  const uint64_t getProcessImageSize() const;
+  uint64_t getProcessImageSize() const;
 
   /* Getter for heap start. */
-  const uint64_t getHeapStart() const;
+  uint64_t getHeapStart() const;
 
  private:
   /** Generate the appropriate simulation objects as parameterised by the
@@ -111,7 +98,7 @@ class CoreInstance {
   simeng::kernel::Linux kernel_;
 
   /** Reference to source assembled by LLVM. */
-  char* source_ = nullptr;
+  uint8_t* source_ = nullptr;
 
   /** Size of the source code assembled by LLVM. */
   size_t sourceSize_ = 0;

src/include/simeng/RegisterValue.hh

@@ -54,7 +54,7 @@ class RegisterValue {
    */
   RegisterValue(const char* ptr, uint16_t bytes, uint16_t capacity)
       : bytes(capacity) {
-    assert(capacity >= bytes && "Capacity is less then requested bytes");
+    assert(capacity >= bytes && "Capacity is less than requested bytes");
     char* dest;
     if (isLocal()) {
       dest = this->value;

src/include/simeng/arch/Architecture.hh

@@ -52,7 +52,7 @@ class Architecture {
    * Writes into the supplied macro-op vector, and returns the number of bytes
    * consumed to produce it; a value of 0 indicates too few bytes were present
    * for a valid decoding. */
-  virtual uint8_t predecode(const void* ptr, uint16_t bytesAvailable,
+  virtual uint8_t predecode(const uint8_t* ptr, uint16_t bytesAvailable,
                             uint64_t instructionAddress,
                             MacroOp& output) const = 0;
 

src/include/simeng/arch/aarch64/ArchInfo.hh

@@ -37,9 +37,9 @@ class ArchInfo : public simeng::arch::ArchInfo {
     uint16_t predCount = regConfig["Predicate-Count"].as<uint16_t>();
     uint16_t condCount = regConfig["Conditional-Count"].as<uint16_t>();
     uint16_t matCount = regConfig["Matrix-Count"].as<uint16_t>();
-    // Matrix-Count multiplied by (SVL/8) as internal representation of
-    // ZA is a block of row-vector-registers. Therefore we need to
-    // convert physical counts from whole-ZA to rows-in-ZA.
+    // Matrix-Count multiplied by (SVL/8) as internal representation of ZA is a
+    // block of row-vector-registers. Therefore, we need to convert physical
+    // counts from whole-ZA to rows-in-ZA.
     matCount *= zaSize_;
     physRegStruct_ = {{8, gpCount},
                       {256, fpCount},

src/include/simeng/arch/aarch64/Architecture.hh

@@ -25,7 +25,7 @@ class Architecture : public arch::Architecture {
   /** Pre-decode instruction memory into a macro-op of `Instruction`
    * instances. Returns the number of bytes consumed to produce it (always 4),
    * and writes into the supplied macro-op vector. */
-  uint8_t predecode(const void* ptr, uint16_t bytesAvailable,
+  uint8_t predecode(const uint8_t* ptr, uint16_t bytesAvailable,
                     uint64_t instructionAddress,
                     MacroOp& output) const override;
 

src/include/simeng/arch/aarch64/helpers/float.hh

@@ -157,6 +157,12 @@ RegisterValue scvtf_FixedPoint(
  * Returns single value of type D. */
 template <typename D, typename N>
 D fcvtzu_integer(srcValContainer& sourceValues) {
+  // Ensure types so that we know behaviour of inaccurate type conversions
+  static_assert((std::is_same<float, N>() || std::is_same<double, N>()) &&
+                "N is not a valid type which should be float or double");
+  static_assert((std::is_same<uint32_t, D>() || std::is_same<uint64_t, D>()) &&
+                "D is not a valid type which should be int32_t or int64_t");
+
   N input = sourceValues[0].get<N>();
   D result = static_cast<D>(0);
 
@@ -165,7 +171,18 @@ D fcvtzu_integer(srcValContainer& sourceValues) {
     if (std::isinf(input)) {
       // Account for Infinity
       result = std::numeric_limits<D>::max();
-    } else if (input > std::numeric_limits<D>::max()) {
+    } else if (static_cast<double>(input) >=
+               static_cast<double>(std::numeric_limits<D>::max())) {
+      // Cast to double to ensure no precision errors. Float can't store uint32
+      // or uint64 max values accurately as not enough bits available. This
+      // causes unwanted comparison behaviour
+      //
+      // max() will be either 4294967295 or 18446744073709551615
+      // Casting to float results in the following (incorrect) values 4294967296
+      // (+1) or 18446744073709551616 (+1)
+      //
+      // Casting to double results in no erroneous conversion.
+
       // Account for the source value being larger than the
       // destination register can support
       result = std::numeric_limits<D>::max();

src/include/simeng/arch/aarch64/helpers/neon.hh

@@ -146,7 +146,7 @@ RegisterValue vecCountPerByte(srcValContainer& sourceValues) {
   const uint8_t* n = sourceValues[0].getAsVector<uint8_t>();
   T out[16 / sizeof(T)] = {0};
   for (int i = 0; i < I; i++) {
-    for (int j = 0; j < (sizeof(T) * 8); j++) {
+    for (size_t j = 0; j < (sizeof(T) * 8); j++) {
       // Move queried bit to LSB and extract via an AND operator
       out[i] += ((n[i] >> j) & 1);
     }
@@ -187,10 +187,10 @@ RegisterValue vecExtVecs_index(
   const uint64_t index = static_cast<uint64_t>(metadata.operands[3].imm);
   T out[16 / sizeof(T)] = {0};
 
-  for (int i = index; i < I; i++) {
+  for (uint64_t i = index; i < I; i++) {
     out[i - index] = n[i];
   }
-  for (int i = 0; i < index; i++) {
+  for (uint64_t i = 0; i < index; i++) {
     out[I - index + i] = m[i];
   }
   return {out, 256};
@@ -816,27 +816,27 @@ RegisterValue vecTbl(
   assert(I == 8 || I == 16);
 
   // Vm contains the indices to fetch from table
-  const int8_t* Vm =
+  const uint8_t* Vm =
       sourceValues[metadata.operandCount - 2]
-          .getAsVector<int8_t>();  // final operand is vecMovi_imm
+          .getAsVector<uint8_t>();  // final operand is vecMovi_imm
 
   // All sourceValues except the first and last are the vector registers to
   // construct the table from
   const uint8_t n_table_regs = metadata.operandCount - 2;
 
   // Create table from vectors. All table sourceValues must be of 16b format.
-  int tableSize = 16 * n_table_regs;
-  uint8_t table[tableSize];
-  for (int i = 0; i < n_table_regs; i++) {
-    const int8_t* currentVector = sourceValues[i].getAsVector<int8_t>();
-    for (int j = 0; j < 16; j++) {
+  const uint16_t tableSize = 16 * n_table_regs;
+  std::vector<uint8_t> table(tableSize, 0);
+  for (uint8_t i = 0; i < n_table_regs; i++) {
+    const uint8_t* currentVector = sourceValues[i].getAsVector<uint8_t>();
+    for (uint8_t j = 0; j < 16; j++) {
       table[16 * i + j] = currentVector[j];
     }
   }
 
-  int8_t out[16 / sizeof(int8_t)] = {0};
+  uint8_t out[16 / sizeof(uint8_t)] = {0};
   for (int i = 0; i < I; i++) {
-    unsigned int index = Vm[i];
+    uint8_t index = Vm[i];
 
     // If an index is out of range for the table, the result for that lookup
     // is 0

src/include/simeng/arch/aarch64/helpers/sve.hh

@@ -508,8 +508,8 @@ RegisterValue sveFcvtPredicated(srcValContainer& sourceValues,
 
   // Stores size of largest type out of D and N
   int lts = std::max(sizeof(D), sizeof(N));
-  bool sourceLarger = (sizeof(D) < sizeof(N)) ? true : false;
-  bool sameDandN = (sizeof(D) == sizeof(N)) ? true : false;
+  bool sourceLarger = (sizeof(D) < sizeof(N));
+  bool sameDandN = (sizeof(D) == sizeof(N));
 
   const uint16_t partition_num = VL_bits / (lts * 8);
   D out[256 / sizeof(D)] = {0};
@@ -543,14 +543,19 @@ RegisterValue sveFcvtPredicated(srcValContainer& sourceValues,
 template <typename D, typename N>
 RegisterValue sveFcvtzsPredicated(srcValContainer& sourceValues,
                                   const uint16_t VL_bits) {
+  static_assert((std::is_same<float, N>() || std::is_same<double, N>()) &&
+                "N is not a valid type which should be float or double");
+  static_assert((std::is_same<int32_t, D>() || std::is_same<int64_t, D>()) &&
+                "D is not a valid type which should be int32_t or int64_t");
+
   const D* d = sourceValues[0].getAsVector<D>();
   const uint64_t* p = sourceValues[1].getAsVector<uint64_t>();
   const N* n = sourceValues[2].getAsVector<N>();
 
   // Stores size of largest type out of D and N
   int lts = std::max(sizeof(D), sizeof(N));
-  bool sameType = (sizeof(D) == sizeof(N)) ? true : false;
-  bool sourceLarger = (sizeof(D) < sizeof(N)) ? true : false;
+  bool sameType = (sizeof(D) == sizeof(N));
+  bool sourceLarger = (sizeof(D) < sizeof(N));
 
   const uint16_t partition_num = VL_bits / (lts * 8);
   D out[256 / sizeof(D)] = {0};
@@ -561,7 +566,21 @@ RegisterValue sveFcvtzsPredicated(srcValContainer& sourceValues,
     int indexN = ((!sourceLarger) & (!sameType)) ? (2 * i) : i;
 
     if (p[i / (64 / lts)] & shifted_active) {
-      if (n[indexN] > std::numeric_limits<D>::max())
+      if (static_cast<double>(n[indexN]) >=
+          static_cast<double>(std::numeric_limits<D>::max()))
+        // Cast to double to reduce precision errors. Float can't store int32
+        // or int64 max values accurately as not enough bits available. This
+        // causes unwanted comparison behaviour. Double also can't accurately
+        // represent int64.MaxValue. Non-strict comparison used to capture this
+        // case
+        //
+        // max() will be either 2147483647 or 9223372036854775807
+        // Casting to float results in the following (incorrect) values
+        // 2147483648 (+1) or 9223372036854775808 (+1)
+        //
+        // Casting to double results in 2147483647 (+0) or incorrect
+        // 9223372036854775808(+1)
+
         out[indexOut] = std::numeric_limits<D>::max();
       else if (n[indexN] < std::numeric_limits<D>::lowest())
         out[indexOut] = std::numeric_limits<D>::lowest();

src/include/simeng/arch/riscv/ArchInfo.hh

@@ -25,7 +25,7 @@ const uint8_t addressAlignMask = 0x3;
 const uint8_t addressAlignMaskCompressed = 0x1;
 const uint8_t minInstWidthBytes = 4;
 const uint8_t minInstWidthBytesCompressed = 2;
-};  // namespace constantsPool
+}  // namespace constantsPool
 
 /** A class to hold and generate riscv specific architecture configuration
  * options. */

src/include/simeng/arch/riscv/Architecture.hh

@@ -24,7 +24,7 @@ class Architecture : public arch::Architecture {
   /** Pre-decode instruction memory into a macro-op of `Instruction`
    * instances. Returns the number of bytes consumed to produce it (0 if
    * failure), and writes into the supplied macro-op vector. */
-  uint8_t predecode(const void* ptr, uint16_t bytesAvailable,
+  uint8_t predecode(const uint8_t* ptr, uint16_t bytesAvailable,
                     uint64_t instructionAddress,
                     MacroOp& output) const override;