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

Remove OP_REQUIRES call from helper function. #49396

Merged
merged 1 commit into from May 21, 2021

Conversation

geetachavan1
Copy link
Contributor

Since OP_REQUIRES macro expands to a return; (among other), calling it in a helper function only ends the helper function's execution earlier, but the kernel will still run from start to end. Thus, all the expected validations are actually broken/useless as the code ploughs through the next crash anyway.

PiperOrigin-RevId: 369524386
Change-Id: I54f6cf9328445675ccc392e661b04336b229c9da

Since `OP_REQUIRES` macro expands to a `return;` (among other), calling it in a helper function only ends the helper function's execution earlier, but the kernel will still run from start to end. Thus, all the expected validations are actually broken/useless as the code ploughs through the next crash anyway.

PiperOrigin-RevId: 369524386
Change-Id: I54f6cf9328445675ccc392e661b04336b229c9da
@google-cla google-cla bot added the cla: yes label May 20, 2021
@gbaned gbaned self-assigned this May 21, 2021
@gbaned gbaned added comp:core issues related to core part of tensorflow size:M CL Change Size: Medium labels May 21, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation May 21, 2021
@gbaned gbaned requested a review from mihaimaruseac May 21, 2021 10:24
@gbaned gbaned assigned mihaimaruseac and unassigned gbaned May 21, 2021
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer May 21, 2021
@mihaimaruseac mihaimaruseac merged commit 69661c2 into tensorflow:r2.4 May 21, 2021
PR Queue automation moved this from Approved by Reviewer to Merged May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:core issues related to core part of tensorflow size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

3 participants