diff --git a/include/hc_printf.hpp b/include/hc_printf.hpp index cb7ccb4ea3a..3e176272566 100644 --- a/include/hc_printf.hpp +++ b/include/hc_printf.hpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include @@ -15,16 +14,11 @@ // The printf on the accelerator is only enabled when // The HCC_ENABLE_ACCELERATOR_PRINTF is defined - -// Disabling hc::printf it's broken on certain platform -// and this experimental feature is not longer being maintained. -#ifdef HCC_ENABLE_ACCELERATOR_PRINTF - #undef HCC_ENABLE_ACCELERATOR_PRINTF - #warning "The experimental hc::printf is no longer supported and is disabled" -#endif +// +//#define HCC_ENABLE_ACCELERATOR_PRINTF (1) // Indicate whether hc::printf is supported -//#define HC_FEATURE_PRINTF (1) +#define HC_FEATURE_PRINTF (1) // Enable extra debug messages #define HC_PRINTF_DEBUG (0) @@ -49,12 +43,8 @@ namespace hc { */ union PrintfPacketData { - uint32_t ui; - int32_t i; uint64_t uli; int64_t li; - hc::half h; - float f; double d; void* ptr; const void* cptr; @@ -98,14 +88,14 @@ enum PrintfPacketDataType { class PrintfPacket { public: void clear() [[hc,cpu]] { type = PRINTF_UNUSED; } - void set(uint32_t d) [[hc,cpu]] { type = PRINTF_UINT32_T; data.ui = d; } - void set(int32_t d) [[hc,cpu]] { type = PRINTF_INT32_T; data.i = d; } + void set(uint32_t d) [[hc,cpu]] { type = PRINTF_UINT32_T; data.uli = d; } + void set(int32_t d) [[hc,cpu]] { type = PRINTF_INT32_T; data.li = d; } void set(uint64_t d) [[hc,cpu]] { type = PRINTF_UINT64_T; data.uli = d; } void set(int64_t d) [[hc,cpu]] { type = PRINTF_INT64_T; data.li = d; } void set(unsigned long long d) [[hc,cpu]] { type = PRINTF_UINT64_T; data.uli = d; } void set(long long d) [[hc,cpu]] { type = PRINTF_INT64_T; data.li = d; } - void set(hc::half d) [[hc,cpu]] { type = PRINTF_HALF; data.h = d; } - void set(float d) [[hc,cpu]] { type = PRINTF_FLOAT; data.f = d; } + void set(hc::half d) [[hc,cpu]] { type = PRINTF_HALF; data.d = d; } + void set(float d) [[hc,cpu]] { type = PRINTF_FLOAT; data.d = d; } void set(double d) [[hc,cpu]] { type = PRINTF_DOUBLE; data.d = d; } void set(void* d) [[hc,cpu]] { type = PRINTF_VOID_PTR; data.ptr = d; } void set(const void* d) [[hc,cpu]] { type = PRINTF_CONST_VOID_PTR; data.cptr = d; } @@ -134,14 +124,14 @@ static inline PrintfPacket* createPrintfBuffer(const unsigned int numElements) { // Initialize the Header elements of the Printf Buffer printfBuffer[PRINTF_BUFFER_SIZE].type = PRINTF_BUFFER_SIZE; - printfBuffer[PRINTF_BUFFER_SIZE].data.ui = numElements; + printfBuffer[PRINTF_BUFFER_SIZE].data.uli = numElements; // Header includes a helper string buffer which holds all char* args // PrintfPacket is 12 bytes, equivalent string buffer size used printfBuffer[PRINTF_STRING_BUFFER].type = PRINTF_STRING_BUFFER; printfBuffer[PRINTF_STRING_BUFFER].data.ptr = hc::internal::am_alloc_host_coherent(sizeof(char) * numElements * 12); printfBuffer[PRINTF_STRING_BUFFER_SIZE].type = PRINTF_STRING_BUFFER_SIZE; - printfBuffer[PRINTF_STRING_BUFFER_SIZE].data.ui = numElements * 12; + printfBuffer[PRINTF_STRING_BUFFER_SIZE].data.uli = numElements * 12; // Using one atomic offset to maintain order and atomicity printfBuffer[PRINTF_OFFSETS].type = PRINTF_OFFSETS; @@ -210,7 +200,7 @@ PrintfError process_str_batch(PrintfPacket* queue, int poffset, unsigned int& so unsigned int str_len = string_length(string); unsigned int sb_offset = soffset; char* string_buffer = (char*) queue[PRINTF_STRING_BUFFER].data.ptr; - if (!string_buffer || soffset + str_len + 1 > queue[PRINTF_STRING_BUFFER_SIZE].data.ui){ + if (!string_buffer || soffset + str_len + 1 > queue[PRINTF_STRING_BUFFER_SIZE].data.uli){ return PRINTF_STRING_BUFFER_OVERFLOW; } copy_n(&string_buffer[sb_offset], string, str_len + 1); @@ -260,10 +250,10 @@ static inline PrintfError printf(PrintfPacket* queue, All... all) [[hc,cpu]] { if (!queue) { error = PRINTF_BUFFER_NULLPTR; } - else if (count_arg + 1 + queue[PRINTF_OFFSETS].data.uia[0] > queue[PRINTF_BUFFER_SIZE].data.ui) { + else if (count_arg + 1 + queue[PRINTF_OFFSETS].data.uia[0] > queue[PRINTF_BUFFER_SIZE].data.uli) { error = PRINTF_BUFFER_OVERFLOW; } - else if (!queue[PRINTF_STRING_BUFFER].data.ptr || count_char + queue[PRINTF_OFFSETS].data.uia[1] > queue[PRINTF_STRING_BUFFER_SIZE].data.ui){ + else if (!queue[PRINTF_STRING_BUFFER].data.ptr || count_char + queue[PRINTF_OFFSETS].data.uia[1] > queue[PRINTF_STRING_BUFFER_SIZE].data.uli){ error = PRINTF_STRING_BUFFER_OVERFLOW; } else { @@ -280,10 +270,10 @@ static inline PrintfError printf(PrintfPacket* queue, All... all) [[hc,cpu]] { unsigned int poffset = (unsigned int)old_off.uia[0]; unsigned int soffset = (unsigned int)old_off.uia[1]; - if (poffset + count_arg + 1 > queue[PRINTF_BUFFER_SIZE].data.ui) { + if (poffset + count_arg + 1 > queue[PRINTF_BUFFER_SIZE].data.uli) { error = PRINTF_BUFFER_OVERFLOW; } - else if (soffset + count_char > queue[PRINTF_STRING_BUFFER_SIZE].data.ui){ + else if (soffset + count_char > queue[PRINTF_STRING_BUFFER_SIZE].data.uli){ error = PRINTF_STRING_BUFFER_OVERFLOW; } else { diff --git a/lib/hsa/mcwamp_hsa.cpp b/lib/hsa/mcwamp_hsa.cpp index 0148736fe2d..1a90fefc091 100644 --- a/lib/hsa/mcwamp_hsa.cpp +++ b/lib/hsa/mcwamp_hsa.cpp @@ -3333,89 +3333,106 @@ class HSADevice final : public KalmarDevice namespace hc { -#ifdef HC_PRINTF_SUPPORT_ENABLE +static void printArgument(const std::string &spec, const PrintfPacket *ptr) { + switch (spec.back()) { + case 'c': + case 'd': + case 'i': + std::printf(spec.c_str(), ptr->data.li); + return; + case 'o': + case 'u': + case 'x': + case 'X': + std::printf(spec.c_str(), ptr->data.uli); + return; + case 'f': + case 'F': + case 'e': + case 'E': + case 'g': + case 'G': + case 'a': + case 'A': + std::printf(spec.c_str(), ptr->data.d); + return; + case 's': + case 'p': + std::printf(spec.c_str(), ptr->data.cptr); + return; + } +} -// regex for finding format string specifiers -static const std::regex specifierPattern("(%){1}[-+#0]*[0-9]*((.)[0-9]+){0,1}([hl]*)([diuoxXfFeEgGaAcsp]){1}"); -static const std::regex signedIntegerPattern("(%){1}[-+#0]*[0-9]*((.)[0-9]+){0,1}([hl]*)([cdi]){1}"); -static const std::regex unsignedIntegerPattern("(%){1}[-+#0]*[0-9]*((.)[0-9]+){0,1}([hl]*)([uoxX]){1}"); -static const std::regex floatPattern("(%){1}[-+#0]*[0-9]*((.)[0-9]+){0,1}([fFeEgGaA]){1}"); -static const std::regex pointerPattern("(%){1}[ps]"); -static const std::regex doubleAmpersandPattern("(%){2}"); -static const std::string ampersand("%"); +void format(const std::string &fmt, const PrintfPacket *ptr, const PrintfPacket *end) { + const char conv_specifiers[] = "diuoxXfFeEgGaAcsp"; -static inline void processPrintfPackets(PrintfPacket* packets, const unsigned int numPackets) { + size_t point = 0; - for (unsigned int i = 0; i < numPackets; ) { + while (true) { + // Each segment of the format string delineated by [mark, point) + // is handled seprately. + auto mark = point; + point = fmt.find('%', point); - unsigned int numPrintfArgs = packets[i++].data.ui; - if (numPrintfArgs == 0) - continue; + // Two different cases where a literal segment is printed out. + // 1. When the point reaches the end of the format string. + // 2. When the point is at the start of a format specifier. + if (point == std::string::npos) { + std::printf("%s", &fmt[mark]); + return; + } - // get the format - unsigned int formatStringIndex = i++; - assert(packets[formatStringIndex].type == PRINTF_CHAR_PTR - || packets[formatStringIndex].type == PRINTF_CONST_CHAR_PTR); - std::string formatString((const char*)packets[formatStringIndex].data.cptr); - std::smatch specifierMatches; - -#if HC_PRINTF_DEBUG - std::printf("%s:%d \t number of matches = %d\n", __FUNCTION__, __LINE__, (int)specifierMatches.size()); -#endif + std::printf("%.*s", (int)(point - mark), &fmt[mark]); - for (unsigned int j = 1; j < numPrintfArgs; ++j, ++i) { + mark = point; + ++point; - if (!std::regex_search(formatString, specifierMatches, specifierPattern)) { - // More printf argument than format specifier?? - // Just skip to the next printf request - i+=(numPrintfArgs - j); - break; - } + // Handle the simplest specifier, '%%'. + if (fmt[point] == '%') { + std::printf("%%"); + ++point; + continue; + } - std::string specifier = specifierMatches.str(); -#if HC_PRINTF_DEBUG - std::cout << " (specifier found: " << specifier << ") "; -#endif + // If we have run out of arguments, bail out. + if (ptr == end) { + return; + } - // print the substring before the specifier - // clean up all the double ampersands - std::string prefix = specifierMatches.prefix(); - prefix = std::regex_replace(prefix,doubleAmpersandPattern,ampersand); - std::printf("%s",prefix.c_str()); - - std::smatch specifierTypeMatch; - if (std::regex_search(specifier, specifierTypeMatch, unsignedIntegerPattern)) { - std::printf(specifier.c_str(), packets[i].data.ui); - } else if (std::regex_search(specifier, specifierTypeMatch, signedIntegerPattern)) { - std::printf(specifier.c_str(), packets[i].data.i); - } else if (std::regex_search(specifier, specifierTypeMatch, floatPattern)) { - if (packets[i].type == PRINTF_HALF) - std::cout << static_cast(packets[i].data.h); - else if (packets[i].type == PRINTF_FLOAT) - std::printf(specifier.c_str(), packets[i].data.f); - else - std::printf(specifier.c_str(), packets[i].data.d); - } else if (std::regex_search(specifier, specifierTypeMatch, pointerPattern)) { - std::printf(specifier.c_str(), packets[i].data.cptr); - } - else { - assert(false); - } - formatString = specifierMatches.suffix(); + // Undefined behaviour if we don't see a conversion specifier. + point = fmt.find_first_of(conv_specifiers, point); + if (point == std::string::npos) { + return; } - // print the substring after the last specifier - // clean up all the double ampersands before printing - formatString = std::regex_replace(formatString,doubleAmpersandPattern,ampersand); - std::printf("%s",formatString.c_str()); + ++point; + + // [mark,point) now contains a complete specifier. + const std::string spec(fmt, mark, point - mark); + printArgument(spec, ptr); + ++ptr; } - std::flush(std::cout); } -#else +static inline void processPrintfPackets(PrintfPacket* packets, const unsigned int numPackets) { + auto pkt = packets; + auto end = pkt + numPackets; -static inline void processPrintfPackets(PrintfPacket* packets, const unsigned int numPackets) { } + while (pkt != end) { + unsigned int numPrintfArgs = pkt->data.uli; + ++pkt; + if (numPrintfArgs == 0) + continue; + + // get the format + assert(pkt->type == PRINTF_CHAR_PKT || pkt->type == PRINTF_CONST_CHAR_PKT); + const std::string formatString((const char*)pkt->data.cptr); + ++pkt; -#endif // #ifdef HC_PRINTF_SUPPORT_ENABLE + format(formatString, pkt, end); + pkt += numPrintfArgs - 1; + } + std::flush(std::cout); +} static inline void processPrintfBuffer(PrintfPacket* gpuBuffer) { @@ -3425,7 +3442,7 @@ static inline void processPrintfBuffer(PrintfPacket* gpuBuffer) { // check whether the printf buffer is non-empty if (cursor != PRINTF_HEADER_SIZE) { - unsigned int bufferSize = gpuBuffer[PRINTF_BUFFER_SIZE].data.ui; + unsigned int bufferSize = gpuBuffer[PRINTF_BUFFER_SIZE].data.uli; unsigned int numPackets = ((bufferSize diff --git a/tests/Unit/HSA/printf_error_check.cpp b/tests/Unit/Printf/printf_error_check.cpp similarity index 99% rename from tests/Unit/HSA/printf_error_check.cpp rename to tests/Unit/Printf/printf_error_check.cpp index 9d9a43139ce..d689972e599 100644 --- a/tests/Unit/HSA/printf_error_check.cpp +++ b/tests/Unit/Printf/printf_error_check.cpp @@ -1,4 +1,3 @@ -// XFAIL: * // RUN: %hc %s -DHCC_ENABLE_ACCELERATOR_PRINTF -DCHECK_PRINTF_ERROR -lhc_am -o %t.out && %t.out | %FileCheck %s #include diff --git a/tests/Unit/HSA/printf_excess_args.cpp b/tests/Unit/Printf/printf_excess_args.cpp similarity index 98% rename from tests/Unit/HSA/printf_excess_args.cpp rename to tests/Unit/Printf/printf_excess_args.cpp index 527e1336ba4..f70cdc392d1 100644 --- a/tests/Unit/HSA/printf_excess_args.cpp +++ b/tests/Unit/Printf/printf_excess_args.cpp @@ -1,4 +1,3 @@ -// XFAIL: * // RUN: %hc %s -DHCC_ENABLE_ACCELERATOR_PRINTF -lhc_am -o %t.out && %t.out | %FileCheck %s #include diff --git a/tests/Unit/HSA/printf_minimal.cpp b/tests/Unit/Printf/printf_minimal.cpp similarity index 96% rename from tests/Unit/HSA/printf_minimal.cpp rename to tests/Unit/Printf/printf_minimal.cpp index 179e7f0d4a0..a06d50a61a0 100644 --- a/tests/Unit/HSA/printf_minimal.cpp +++ b/tests/Unit/Printf/printf_minimal.cpp @@ -1,4 +1,3 @@ -// XFAIL: * // RUN: %hc %s -DHCC_ENABLE_ACCELERATOR_PRINTF -lhc_am -o %t.out && %t.out | %FileCheck %s #include diff --git a/tests/Unit/HSA/printf_ptr_addr.cpp b/tests/Unit/Printf/printf_ptr_addr.cpp similarity index 98% rename from tests/Unit/HSA/printf_ptr_addr.cpp rename to tests/Unit/Printf/printf_ptr_addr.cpp index 9fec625d8bc..7dfc8e1aa17 100644 --- a/tests/Unit/HSA/printf_ptr_addr.cpp +++ b/tests/Unit/Printf/printf_ptr_addr.cpp @@ -1,4 +1,3 @@ -// XFAIL: * // RUN: %hc %s -DHCC_ENABLE_ACCELERATOR_PRINTF -lhc_am -o %t.out && %t.out | %FileCheck %s #include diff --git a/tests/Unit/HSA/printf_supported_types.cpp b/tests/Unit/Printf/printf_supported_types.cpp similarity index 100% rename from tests/Unit/HSA/printf_supported_types.cpp rename to tests/Unit/Printf/printf_supported_types.cpp diff --git a/tests/lit.cfg b/tests/lit.cfg index ae20cf40c04..596e700c4d6 100644 --- a/tests/lit.cfg +++ b/tests/lit.cfg @@ -53,7 +53,7 @@ if os.environ.get('HCC_DEFAULT_GPU'): if os.environ.get('HCC_ENABLE_PRINTF'): config.environment['HCC_ENABLE_PRINTF'] = os.environ['HCC_ENABLE_PRINTF'] else: - config.environment['HCC_ENABLE_PRINTF'] = "1" + config.environment['HCC_ENABLE_PRINTF'] = "0" if os.environ.get('AMDGPU_OBJ_CODEGEN'): config.environment['AMDGPU_OBJ_CODEGEN'] = os.environ['AMDGPU_OBJ_CODEGEN']