From 8548d751020663786928e0b497e2ccfb56b45acc Mon Sep 17 00:00:00 2001 From: jithunnair-amd Date: Fri, 6 Jul 2018 16:28:05 -0500 Subject: [PATCH 1/3] Fix static_cast logic in hipify script for better coverage. Remove obsolete code for output directory not existing --- tools/amd_build/pyHIPIFY/hipify-python.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/tools/amd_build/pyHIPIFY/hipify-python.py b/tools/amd_build/pyHIPIFY/hipify-python.py index 607adb2c1e116c..e4af9b449fa8a5 100755 --- a/tools/amd_build/pyHIPIFY/hipify-python.py +++ b/tools/amd_build/pyHIPIFY/hipify-python.py @@ -726,7 +726,8 @@ def add_static_casts(directory, extensions, KernelTemplateParams): # Check if we have templating + static_cast information argument_strings = [input_source[arg["start"]:arg["end"]] for arg in arguments] - kernel_name = argument_strings[0].strip() + original_kernel_name_with_template = argument_strings[0].strip() + kernel_name = original_kernel_name_with_template.split("<")[0].strip() ignore = ["upscale"] if kernel_name in KernelTemplateParams and kernel_name not in ignore: # Add template to the kernel @@ -740,21 +741,20 @@ def add_static_casts(directory, extensions, KernelTemplateParams): kernel_params = argument_strings[5:] for arg_idx, arg in enumerate(kernel_params): if arg_idx in argument_types: - arg = kernel_params[arg_idx].strip() the_type = argument_types[arg_idx] - the_arg = arg.replace("\n", "").strip() + the_arg = arg.replace("\n", "").replace("\\", "").strip() if the_type in ["int", "const int", "int64_t", "THCIndex_t *", "const int *", "ptrdiff_t", "long", "const int64_t*", "int64_t *", "double"]: static_argument = "static_cast<%s>(%s)" % (the_type, the_arg) - static_argument = arg.replace(the_arg, static_argument) - - # Update to static_cast - new_kernel_launch = re.sub(r'\b(%s)\b' % - arg, lambda x: static_argument, new_kernel_launch) + def replace_arg(match): + return match.group(1) + static_argument + match.group(3) + # Update to static_cast, account for cases where argument is at start/end of string + new_kernel_launch = re.sub(r'(^|\W)(%s)(\W|$)' % re.escape(the_arg), replace_arg, new_kernel_launch) + # Add template type if "THCUNN" in filepath.split("/") and "generic" not in filepath.split("/"): kernel_name_with_template = kernel_name_with_template.replace("", "") - new_kernel_launch = re.sub(r'\b(%s)\b' % kernel_name, + new_kernel_launch = re.sub(r'\b%s\b' % original_kernel_name_with_template, lambda x: kernel_name_with_template, new_kernel_launch) # Replace Launch @@ -835,11 +835,6 @@ def main(): print("The project folder specified does not exist.") return - # Make sure output directory exists. - if not os.path.exists(args.output_directory): - print("The output folder already exists.") - return - # If no output directory, provide a default one. if args.output_directory is "": args.project_directory = args.project_directory[0:- From 2da81c71d9d63ddf8dce0f312bc427cfb3bf382e Mon Sep 17 00:00:00 2001 From: jithunnair-amd Date: Mon, 9 Jul 2018 12:09:17 -0500 Subject: [PATCH 2/3] Replace old string replacement technique with new technique using .format() --- tools/amd_build/pyHIPIFY/hipify-python.py | 61 +++++++++++------------ 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/tools/amd_build/pyHIPIFY/hipify-python.py b/tools/amd_build/pyHIPIFY/hipify-python.py index e4af9b449fa8a5..47850ad2d7d663 100755 --- a/tools/amd_build/pyHIPIFY/hipify-python.py +++ b/tools/amd_build/pyHIPIFY/hipify-python.py @@ -107,7 +107,7 @@ def filename_ends_with_extension(filename, extensions): def inside_included_directories(dirpath, rootpath, include_dirs): """Helper method to see if filename within included directories""" - return reduce(lambda result, included_directory: re.match(r'(%s)\b' % os.path.join(rootpath, included_directory), dirpath) or result, include_dirs, None) + return reduce(lambda result, included_directory: re.match(r'{0}\b'.format(os.path.join(rootpath, included_directory)), dirpath) or result, include_dirs, None) def walk_over_directory(rootpath, extensions, show_detailed=False, include_dirs=None): @@ -169,19 +169,19 @@ def compute_stats(stats): unsupported_calls = set(cuda_call for (cuda_call, _filepath) in stats["unsupported_calls"]) # Print the number of unsupported calls - print("Total number of unsupported CUDA function calls: %d" % (len(unsupported_calls))) + print("Total number of unsupported CUDA function calls: {0:d}".format(len(unsupported_calls))) # Print the list of unsupported calls print(", ".join(unsupported_calls)) # Print the number of kernel launches - print("\nTotal number of replaced kernel launches: %d" % (len(stats["kernel_launches"]))) + print("\nTotal number of replaced kernel launches: {0:d}".format(len(stats["kernel_launches"]))) def processKernelLaunches(string, stats): """ Replace the CUDA style Kernel launches with the HIP style kernel launches.""" # Concat the namespace with the kernel names. (Find cleaner way of doing this later). - string = re.sub(r'([ ]+)(detail?)::[ ]+\\\n[ ]+', lambda inp: "%s%s::" % (inp.group(1), inp.group(2)), string) + string = re.sub(r'([ ]+)(detail?)::[ ]+\\\n[ ]+', lambda inp: "{0}{1}::".format(inp.group(1), inp.group(2)), string) def grab_method_and_template(in_kernel): # The positions for relevant kernel components. @@ -368,7 +368,7 @@ def disable_function(input_string, function, replace_style): } # Create function string to search for - function_string = "%s%s%s" % ( + function_string = "{0}{1}{2}".format( func_info["return_type"], func_info["function_name"], func_info["function_args"] @@ -378,8 +378,7 @@ def disable_function(input_string, function, replace_style): info["function_start"] = input_string.find(function_string) else: # Automatically detect signature. - the_match = re.search(r"(((.*) (\*)?)(%s)(\([^{)]*\)))\s*{" % - (function.replace("(", "\(").replace(")", "\)")), input_string) + the_match = re.search(r"(((.*) (\*)?)({0})(\([^{{)]*\)))\s*{{".format(function.replace("(", "\(").replace(")", "\)")), input_string) if the_match is None: return input_string @@ -430,12 +429,12 @@ def disable_function(input_string, function, replace_style): elif replace_style == disablefuncmode.STUB: # void return type if func_info["return_type"] == "void" or func_info["return_type"] == "static void": - stub = "%s{\n}" % (function_string) + stub = "{0}{{\n}}".format(function_string) # pointer return type elif "*" in func_info["return_type"]: - stub = "%s{\nreturn %s;\n}" % (function_string, "NULL") # nullptr + stub = "{0}{{\nreturn {1};\n}}".format(function_string, "NULL") # nullptr else: - stub = "%s{\n%s stub_var;\nreturn stub_var;\n}" % (function_string, func_info["return_type"]) + stub = "{0}{{\n{1} stub_var;\nreturn stub_var;\n}}".format(function_string, func_info["return_type"]) output_string = input_string.replace(function_body, stub) @@ -443,30 +442,30 @@ def disable_function(input_string, function, replace_style): elif replace_style == disablefuncmode.HCC_MACRO: output_string = input_string.replace( function_body, - "#if !defined(__HIP_PLATFORM_HCC__)\n%s\n#endif" % function_body) + "#if !defined(__HIP_PLATFORM_HCC__)\n{0}\n#endif".format(function_body)) # Add HIP Preprocessors. elif replace_style == disablefuncmode.DEVICE_MACRO: output_string = input_string.replace( function_body, - "#if !defined(__HIP_DEVICE_COMPILE__)\n%s\n#endif" % function_body) + "#if !defined(__HIP_DEVICE_COMPILE__)\n{0}\n#endif".format(function_body)) # Throw an exception at runtime. elif replace_style == disablefuncmode.EXCEPTION: - stub = "%s{\n%s;\n}" % ( + stub = "{0}{{\n{1};\n}}".format( function_string, - 'throw std::runtime_error("The function %s is not implemented.")' % - function_string.replace("\n", " ")) + 'throw std::runtime_error("The function {0} is not implemented.")'.format( + function_string.replace("\n", " "))) output_string = input_string.replace(function_body, stub) elif replace_style == disablefuncmode.ASSERT: - stub = "%s{\n%s;\n}" % ( + stub = "{0}{{\n{1};\n}}".format( function_string, 'assert(0)') output_string = input_string.replace(function_body, stub) elif replace_style == disablefuncmode.EMPTY: - stub = "%s{\n;\n}" % (function_string) + stub = "{0}{{\n;\n}}".format(function_string) output_string = input_string.replace(function_body, stub) return output_string @@ -489,7 +488,7 @@ def preprocessor(filepath, stats): stats["unsupported_calls"].append((cuda_type, filepath)) if cuda_type in output_source: - output_source = re.sub(r'\b(%s)\b' % cuda_type, lambda x: hip_type, output_source) + output_source = re.sub(r'\b({0})\b'.format(cuda_type), lambda x: hip_type, output_source) # Perform Kernel Launch Replacements output_source = processKernelLaunches(output_source, stats) @@ -512,7 +511,7 @@ def file_specific_replacement(filepath, search_string, replace_string, strict=Fa with openf(filepath, "r+") as f: contents = f.read() if strict: - contents = re.sub(r'\b(%s)\b' % search_string, lambda x: replace_string, contents) + contents = re.sub(r'\b({0})\b'.format(search_string), lambda x: replace_string, contents) else: contents = contents.replace(search_string, replace_string) f.seek(0) @@ -524,8 +523,8 @@ def file_add_header(filepath, header): with openf(filepath, "r+") as f: contents = f.read() if header[0] != "<" and header[-1] != ">": - header = '"%s"' % header - contents = ('#include %s \n' % header) + contents + header = '"{0}"'.format(header) + contents = ('#include {0} \n'.format(header)) + contents f.seek(0) f.write(contents) f.truncate() @@ -595,7 +594,7 @@ def get_kernel_template_params(the_file, KernelDictionary): break if len(template_arguments) == 1 and template_arguments[0].strip() in ["Dtype", "T"]: # Updates kernel - kernel_with_template = "%s" % (kernel_name) + kernel_with_template = "{0}".format(kernel_name) else: kernel_with_template = kernel_name formatted_args = {} @@ -613,10 +612,10 @@ def get_kernel_template_params(the_file, KernelDictionary): kernel_params = kernel.group(2).split(",")[1:] if kernel_gen_type == 1: - kernel_args = {1: "int", 2: "%s *" % kernel_params[0], 3: kernel_params[1]} + kernel_args = {1: "int", 2: "{0} *".format(kernel_params[0]), 3: kernel_params[1]} if kernel_gen_type == 2: - kernel_args = {1: "int", 2: "%s *" % kernel_params[0], 3: kernel_params[1], 4: kernel_params[2]} + kernel_args = {1: "int", 2: "{0} *".format(kernel_params[0]), 3: kernel_params[1], 4: kernel_params[2]} # Argument at position 1 should be int KernelDictionary[kernel_name] = {"kernel_with_template": kernel_name, "arg_types": kernel_args} @@ -628,7 +627,7 @@ def disable_unsupported_function_call(function, input_string, replacement): output_string = input_string # Find all calls to the function - calls = re.finditer(r"\b%s\b" % function, input_string) + calls = re.finditer(r"\b{0}\b".format(function), input_string) # Do replacements for call in calls: @@ -666,7 +665,7 @@ def disable_module(input_file): last = list(re.finditer(r"#include .*\n", txt))[-1] end = last.end() - disabled = "%s#if !defined(__HIP_PLATFORM_HCC__)\n%s\n#endif" % (txt[0:end], txt[end:]) + disabled = "{0}#if !defined(__HIP_PLATFORM_HCC__)\n{1}\n#endif".format(txt[0:end], txt[end:]) f.seek(0) f.write(disabled) @@ -744,17 +743,17 @@ def add_static_casts(directory, extensions, KernelTemplateParams): the_type = argument_types[arg_idx] the_arg = arg.replace("\n", "").replace("\\", "").strip() if the_type in ["int", "const int", "int64_t", "THCIndex_t *", "const int *", "ptrdiff_t", "long", "const int64_t*", "int64_t *", "double"]: - static_argument = "static_cast<%s>(%s)" % (the_type, the_arg) + static_argument = "static_cast<{0}>({1})".format(the_type, the_arg) def replace_arg(match): return match.group(1) + static_argument + match.group(3) # Update to static_cast, account for cases where argument is at start/end of string - new_kernel_launch = re.sub(r'(^|\W)(%s)(\W|$)' % re.escape(the_arg), replace_arg, new_kernel_launch) + new_kernel_launch = re.sub(r'(^|\W)({0})(\W|$)'.format(re.escape(the_arg)), replace_arg, new_kernel_launch) # Add template type if "THCUNN" in filepath.split("/") and "generic" not in filepath.split("/"): kernel_name_with_template = kernel_name_with_template.replace("", "") - new_kernel_launch = re.sub(r'\b%s\b' % original_kernel_name_with_template, + new_kernel_launch = re.sub(r'\b{0}\b'.format(original_kernel_name_with_template), lambda x: kernel_name_with_template, new_kernel_launch) # Replace Launch @@ -922,7 +921,7 @@ def main(): s_constants = [] if not os.path.exists(filepath): - print("\n" + bcolors.WARNING + "YAML Warning: File %s does not exist." % filepath + bcolors.ENDC) + print("\n" + bcolors.WARNING + "YAML Warning: File {0} does not exist.".format(filepath) + bcolors.ENDC) continue with openf(filepath, "r+") as f: @@ -934,7 +933,7 @@ def main(): # Disable Constants w\ Boundary. for const in constants: - txt = re.sub(r"\b%s\b" % const, constants[const], txt) + txt = re.sub(r"\b{0}\b".format(const), constants[const], txt) # Disable Constants for s_const in s_constants: From acfacbc451c7b35b503080633d42a4273bc61aa5 Mon Sep 17 00:00:00 2001 From: jithunnair-amd Date: Mon, 9 Jul 2018 14:10:00 -0500 Subject: [PATCH 3/3] Add comments for add_static_cast and extract_arguments functions --- tools/amd_build/pyHIPIFY/hipify-python.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tools/amd_build/pyHIPIFY/hipify-python.py b/tools/amd_build/pyHIPIFY/hipify-python.py index afc615ee2832e4..974a7793bbb167 100755 --- a/tools/amd_build/pyHIPIFY/hipify-python.py +++ b/tools/amd_build/pyHIPIFY/hipify-python.py @@ -671,9 +671,14 @@ def disable_module(input_file): f.write(disabled) f.truncate() - def extract_arguments(start, string): - """ Return the list of arguments in the upcoming function parameter closure""" + """ Return the list of arguments in the upcoming function parameter closure + This function needs a string that contains function arguments fully encapsulated within opening and closing parantheses. + Eg: + string (input): '(blocks, threads, 0, THCState_getCurrentStream(state))' + arguments (output): '[{'start': 1, 'end': 7}, {'start': 8, 'end': 16}, {'start': 17, 'end': 19}, {'start': 20, 'end': 53}]' + """ + arguments = [] closures = { "<": 0, @@ -708,9 +713,18 @@ def extract_arguments(start, string): return arguments +# Add static_cast to ensure that the type of kernel arguments matches that in the corresponding kernel definition def add_static_casts(directory, extensions, KernelTemplateParams): - """Added necessary static casts to kernel launches due to issue in HIP""" + """Added necessary static casts to kernel launches to match kernel argument type to corresponding kernel definition + Eg. + old_kernel_launch: ' createBatchGemmBuffer, grid, block, 0, THCState_getCurrentStream(state), + (const real**)d_result, THCTensor_(data)(state, ra__), + ra__->stride[0], num_batches' + new_kernel_launch: ' createBatchGemmBuffer, grid, block, 0, THCState_getCurrentStream(state), + (const real**)d_result, THCTensor_(data)(state, ra__), + static_cast(ra__->stride[0]), static_cast(num_batches)' + """ # Add static_casts<> to all kernel launches. for (dirpath, _dirnames, filenames) in os.walk(directory): for filename in filenames: