Skip to content

Conversation

jithunnair-amd
Copy link
Collaborator

Fix static_cast logic in hipify script for better coverage
Also remove obsolete code for output directory not existing

…solete code for output directory not existing
@jithunnair-amd jithunnair-amd requested a review from ezyang as a code owner July 6, 2018 21:30
@iotamudelta
Copy link

Thanks! Did you check if unit tests are running?

@iotamudelta
Copy link

As per the review for pytorch#8812 w/ upstream: could you add to the documentation of the function an example for before vs after? Thanks

Copy link

@Jorghi12 Jorghi12 left a comment

Choose a reason for hiding this comment

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

The changes look good! Once it passes CI we should merge. It appears there wer flaky tests the first time it ran through.

the_type = argument_types[arg_idx]
the_arg = arg.replace("\n", "").strip()
the_arg = arg.replace("\n", "").replace("\\", "").strip()
Copy link

Choose a reason for hiding this comment

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

Why is replace("\\", "") added here?

Copy link
Collaborator Author

@jithunnair-amd jithunnair-amd Jul 9, 2018

Choose a reason for hiding this comment

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

There are instances where the kernel invocation is part of a preprocessor multiline macro. That's where the "\\" comes from (for the literal backslash used in a multiline macro).

@iotamudelta
Copy link

LGTM after the recent documentation additions. Thanks!

@iotamudelta iotamudelta merged commit e852434 into ROCm:master Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants