From 924622f5e522bed59b86ac043985ecf0242b40db Mon Sep 17 00:00:00 2001 From: Chung Shien Chai Date: Sat, 15 Jul 2023 17:46:43 -0700 Subject: [PATCH 1/2] Issue 1248 - fix bug bintoi_charvec() --- libs/libopenfpgautil/src/openfpga_decode.cpp | 32 +++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/libs/libopenfpgautil/src/openfpga_decode.cpp b/libs/libopenfpgautil/src/openfpga_decode.cpp index 988bec456..6e69b230b 100644 --- a/libs/libopenfpgautil/src/openfpga_decode.cpp +++ b/libs/libopenfpgautil/src/openfpga_decode.cpp @@ -2,7 +2,6 @@ * This file includes functions that are used to decode integer to binary *vectors or the reverse operation ***************************************************************************************/ -#include /* Headers from vtrutil library */ #include "openfpga_decode.h" @@ -109,12 +108,27 @@ std::string combine_two_1hot_str(const std::string& code1, * Output: * index | 0 | 1 | 2 * ret | 0 | 0 | 1 +* +* ToDo: Need to revisit and change all the feature that call to this function +* Had studied the code, should be safe to make the change +* Apparently we only want to store 0 or 1 (binary vector) +* Yet we store it in a vector on size_t (8 Bytes) +* We are using 8x memory that we supposed to +* uint8_t is good enough +* Not a bug, but is a serious unoptimized issue ********************************************************************/ std::vector itobin_vec(const size_t& in_int, const size_t& bin_len) { + /* bin_len must be in valid range*/ + VTR_ASSERT(bin_len > 0 && bin_len <= 64); std::vector ret(bin_len, 0); /* Make sure we do not have any overflow! */ - VTR_ASSERT((in_int < pow(2., bin_len))); + /* If the length is 64 bits, in_int can be any number since this is the max bit length, and BTW pow(2, 64) + itself should be zero (from 64bits size_t perspective). Once we fix the bintoi_charvec() bug, without + this change, it will cause assertion */ + if (bin_len < 64) { + VTR_ASSERT(in_int < (size_t(1) << bin_len)); + } size_t temp = in_int; for (size_t i = 0; i < bin_len; i++) { @@ -140,10 +154,17 @@ std::vector itobin_vec(const size_t& in_int, const size_t& bin_len) { * which has a smaller memory footprint than size_t ********************************************************************/ std::vector itobin_charvec(const size_t& in_int, const size_t& bin_len) { + /* bin_len must be in valid range*/ + VTR_ASSERT(bin_len > 0 && bin_len <= 64); std::vector ret(bin_len, '0'); /* Make sure we do not have any overflow! */ - VTR_ASSERT((in_int < pow(2., bin_len))); + /* If the length is 64 bits, in_int can be any number since this is the max bit length, and BTW pow(2, 64) + itself should be zero (from 64bits size_t perspective). Once we fix the bintoi_charvec() bug, without + this change, it will cause assertion */ + if (bin_len < 64) { + VTR_ASSERT(in_int < (size_t(1) << bin_len)); + } size_t temp = in_int; for (size_t i = 0; i < bin_len; i++) { @@ -170,11 +191,14 @@ std::vector itobin_charvec(const size_t& in_int, const size_t& bin_len) { * which has a smaller memory footprint than size_t ********************************************************************/ size_t bintoi_charvec(const std::vector& bin) { + /* bin.size() must be in valid range*/ + VTR_ASSERT(bin.size() > 0 && bin.size() <= 64); + size_t ret = 0; for (size_t i = 0; i < bin.size(); ++i) { if ('1' == bin[i]) { - ret += pow(2., i); + ret |= ((size_t)(1) << i); } } From b2f5b493c252e1202c6337b8497bd92c9624b3cf Mon Sep 17 00:00:00 2001 From: Chung Shien Chai Date: Sun, 16 Jul 2023 13:08:40 -0700 Subject: [PATCH 2/2] Fix the cpp-format --- libs/libopenfpgautil/src/openfpga_decode.cpp | 33 +++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/libs/libopenfpgautil/src/openfpga_decode.cpp b/libs/libopenfpgautil/src/openfpga_decode.cpp index 6e69b230b..30aa2d128 100644 --- a/libs/libopenfpgautil/src/openfpga_decode.cpp +++ b/libs/libopenfpgautil/src/openfpga_decode.cpp @@ -5,6 +5,7 @@ /* Headers from vtrutil library */ #include "openfpga_decode.h" + #include "vtr_assert.h" /* begin namespace openfpga */ @@ -108,14 +109,14 @@ std::string combine_two_1hot_str(const std::string& code1, * Output: * index | 0 | 1 | 2 * ret | 0 | 0 | 1 -* -* ToDo: Need to revisit and change all the feature that call to this function -* Had studied the code, should be safe to make the change -* Apparently we only want to store 0 or 1 (binary vector) -* Yet we store it in a vector on size_t (8 Bytes) -* We are using 8x memory that we supposed to -* uint8_t is good enough -* Not a bug, but is a serious unoptimized issue + * + * ToDo: Need to revisit and change all the feature that call to this function + * Had studied the code, should be safe to make the change + * Apparently we only want to store 0 or 1 (binary vector) + * Yet we store it in a vector on size_t (8 Bytes) + * We are using 8x memory that we supposed to + * uint8_t is good enough + * Not a bug, but is a serious unoptimized issue ********************************************************************/ std::vector itobin_vec(const size_t& in_int, const size_t& bin_len) { /* bin_len must be in valid range*/ @@ -123,9 +124,10 @@ std::vector itobin_vec(const size_t& in_int, const size_t& bin_len) { std::vector ret(bin_len, 0); /* Make sure we do not have any overflow! */ - /* If the length is 64 bits, in_int can be any number since this is the max bit length, and BTW pow(2, 64) - itself should be zero (from 64bits size_t perspective). Once we fix the bintoi_charvec() bug, without - this change, it will cause assertion */ + /* If the length is 64 bits, in_int can be any number since this is the max + bit length, and BTW pow(2, 64) itself should be zero (from 64bits size_t + perspective). Once we fix the bintoi_charvec() bug, without this change, it + will cause assertion */ if (bin_len < 64) { VTR_ASSERT(in_int < (size_t(1) << bin_len)); } @@ -159,9 +161,10 @@ std::vector itobin_charvec(const size_t& in_int, const size_t& bin_len) { std::vector ret(bin_len, '0'); /* Make sure we do not have any overflow! */ - /* If the length is 64 bits, in_int can be any number since this is the max bit length, and BTW pow(2, 64) - itself should be zero (from 64bits size_t perspective). Once we fix the bintoi_charvec() bug, without - this change, it will cause assertion */ + /* If the length is 64 bits, in_int can be any number since this is the max + bit length, and BTW pow(2, 64) itself should be zero (from 64bits size_t + perspective). Once we fix the bintoi_charvec() bug, without this change, it + will cause assertion */ if (bin_len < 64) { VTR_ASSERT(in_int < (size_t(1) << bin_len)); } @@ -193,7 +196,7 @@ std::vector itobin_charvec(const size_t& in_int, const size_t& bin_len) { size_t bintoi_charvec(const std::vector& bin) { /* bin.size() must be in valid range*/ VTR_ASSERT(bin.size() > 0 && bin.size() <= 64); - + size_t ret = 0; for (size_t i = 0; i < bin.size(); ++i) {