Skip to content

Add workarounds in pyHIPIFY for __forceinline__ and std:: math functions in HIP #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 13, 2018
Merged

Add workarounds in pyHIPIFY for __forceinline__ and std:: math functions in HIP #27

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
25f1b2d
Add workarounds in pyHIPIFY for __forceinline__ and std:: math functi…
jithunnair-amd Jul 10, 2018
759417e
Adding doc string to function
jithunnair-amd Jul 10, 2018
9f85af0
Revert accidental python version change
jithunnair-amd Jul 12, 2018
31240b6
Add comments to mention correctness caveat for replace_math_functions…
jithunnair-amd Jul 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions tools/amd_build/pyHIPIFY/hipify-python.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,18 @@ def disable_asserts(input_string):
output_string = output_string.replace(input_string[start:p_end + 1], "")
return output_string

def replace_forceinline(input_string):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function needed?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jithunnair-amd could you add a doc string here with the link to HIP not implying static for forceinline?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iotamudelta please don't do this, the correct fix is to change how HIP handles __forceinline__ to match what CUDA does (it actually implies language inline there). Please ask the HIP maintainers to correct that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexVlx yes, we will. for the time being we need things to compile reliably in all our environments. it's a hack, unfortunately. @jithunnair-amd could you create HIP issue w/ that and link it here for documentation purposes? thanks! :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ROCm/hip#572

output_string = input_string
output_string = re.sub("__forceinline__", "inline", output_string)
return output_string

def replace_math_functions(input_string):
""" Replace std:: invocations of match functions with non-std:: versions"""
output_string = input_string
output_string = re.sub("std::exp\(", "::exp(", output_string)
output_string = re.sub("std::log\(", "::log(", output_string)
output_string = re.sub("std::pow\(", "::pow(", output_string)
return output_string

def disable_function(input_string, function, replace_style):
""" Finds and disables a function in a particular file.
Expand Down Expand Up @@ -497,6 +509,12 @@ def preprocessor(filepath, stats):
if not filepath.endswith("THCGeneral.h.in"):
output_source = disable_asserts(output_source)

# Replace std:: with non-std:: versions
output_source = replace_math_functions(output_source)

# Replace __forceinline__ with inline
output_source = replace_forceinline(output_source)

# Overwrite file contents
fileobj.seek(0)
fileobj.write(output_source)
Expand Down