Skip to content

Move var and std overloads to Functions.cpp and remove native:: reference#48683

Closed
ezyang wants to merge 2 commits intogh/ezyang/876/basefrom
gh/ezyang/876/head
Closed

Move var and std overloads to Functions.cpp and remove native:: reference#48683
ezyang wants to merge 2 commits intogh/ezyang/876/basefrom
gh/ezyang/876/head

Conversation

@ezyang
Copy link
Copy Markdown
Contributor

@ezyang ezyang commented Dec 2, 2020

Stack from ghstack:

I want to delete NativeFunctions.h from Functions.h header. To do this I must remove all references to native:: However; I also must avoid trampling over iseeyuan's work of making ATen compilable without reference to ATen_cpu. In this particular case, I fix the Functions.h problem by moving it to a cpp file, and removing the native:: short-circuit (ostensibly there for performances). This also fixes a hypothetical correctness bug where these would not dispatch properly if the underlying functions no longer uniformly used a single native:: implementation.

Signed-off-by: Edward Z. Yang ezyang@fb.com

Differential Revision: D25261843

Overall goal: delete NativeFunctions.h from Functions.h header.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
Overall goal: delete NativeFunctions.h from Functions.h header.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
ezyang added a commit that referenced this pull request Dec 2, 2020
Overall goal: delete NativeFunctions.h from Functions.h header.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ghstack-source-id: fbca6f1
Pull Request resolved: #48683
@ezyang ezyang requested review from bhosmer and iseeyuan December 2, 2020 01:38
@ezyang ezyang changed the title Move var and std overloads to Functions.cpp Move var and std overloads to Functions.cpp and remove native:: reference Dec 2, 2020
Copy link
Copy Markdown
Contributor

@iseeyuan iseeyuan left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 2, 2020

Codecov Report

Merging #48683 (bb39d31) into gh/ezyang/876/base (6969196) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@                  Coverage Diff                   @@
##           gh/ezyang/876/base   #48683      +/-   ##
======================================================
- Coverage               80.39%   80.39%   -0.01%     
======================================================
  Files                    1830     1830              
  Lines                  192491   192491              
======================================================
- Hits                   154755   154750       -5     
- Misses                  37736    37741       +5     

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ezyang merged this pull request in e097f88.

shaibagon pushed a commit to shaibagon/pytorch that referenced this pull request Dec 3, 2020
…ence (pytorch#48683)

Summary:
Pull Request resolved: pytorch#48683

I want to delete NativeFunctions.h from Functions.h header. To do this I must remove all references to native:: However; I also must avoid trampling over iseeyuan's work of making ATen compilable without reference to ATen_cpu. In this particular case, I fix the Functions.h problem by moving it to a cpp file, and removing the native:: short-circuit (ostensibly there for performances). This also fixes a hypothetical correctness bug where these would not dispatch properly if the underlying functions no longer uniformly used a single native:: implementation.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: Imported from OSS

Reviewed By: bhosmer

Differential Revision: D25261843

Pulled By: ezyang

fbshipit-source-id: 05ca6555fbf1062f9b22d868c8cb88fdf8e4c24b
@ezyang
Copy link
Copy Markdown
Contributor Author

ezyang commented Dec 3, 2020

cc @peterbell10

@facebook-github-bot facebook-github-bot deleted the gh/ezyang/876/head branch December 6, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants