Skip to content
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.

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

Rename reshape to avoid acc tracer collision #838

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tissue3
Copy link
Contributor

@tissue3 tissue3 commented Jul 21, 2023

Summary:
There is an AIT bug
See this acc_graph: P795462043, and model-generated.h: P795461857
When the node name is the same as function name - in this case, reshape_14
we will have

expression preceding parentheses of apparent call must have (pointer-to-) function type

This is because reshape_14 is both tensor and function (see model-generated.h: P795461857)
The fix can be either add func name to MEMO set (D47657410), or name our reshape function so that it would cause any name collision to the name produced by the acc tracer (this diff).

We probably chose the latter because the former may might end up with a very large name set and long compilation time.
The later approach isn't general but I guess we don’t have many cases where we name our kernel launchers the same way as what the acc tracer names tensors.

Differential Revision: D47675840

Summary:
There is an AIT bug
See this acc_graph: P795462043, and model-generated.h: P795461857
When the node name is the same as function name - in this case, reshape_14
we will have
```
expression preceding parentheses of apparent call must have (pointer-to-) function type
```
This is because reshape_14 is both tensor and function (see model-generated.h: P795461857)
The fix can be either add func name to MEMO set (D47657410), or name our reshape function so that it would cause any name collision to the name produced by the acc tracer (this diff).

We probably chose the latter because the former may might end up with a very large name set and long compilation time.
The later approach isn't general but I guess we don’t have many cases where we name our kernel launchers the same way as what the acc tracer names tensors.

Differential Revision: D47675840

fbshipit-source-id: 21395181fb7610cc5feaccf953dbee0dfc900f51
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Jul 21, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47675840

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants