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.

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

Jump to bottom

[compiler] fix inf/nan convert to i32 on x86_64 arch #378

Merged
merged 1 commit into from
Jun 28, 2024
Merged

[compiler] fix inf/nan convert to i32 on x86_64 arch #378

merged 1 commit into from
Jun 28, 2024

Conversation

jianwenyyy
Copy link
Collaborator

@jianwenyyy jianwenyyy commented Jun 27, 2024 •
edited by qingyunqu
Loading

  1. on x86_64, [inf, -inf, nan] will be converted to [INT32_MIN, INT32_MIN, INT32_MIN] due to UB. However, on arm_aarch64/nvgpu, [inf, -inf, nan] will be converted to [INT32_MAX, INT32_MIN, 0].
  2. so we add compare and select during HloToLinalg and add target & arch option to control it.

@jianwenyyy jianwenyyy force-pushed the yjw/fix_inf_nan_cvt branch 2 times, most recently from 3b30e7e to 329efd7 Compare June 28, 2024 04:30
@qingyunqu qingyunqu changed the title [compiler] fix inf/nan convert on x86_64 arch [compiler] fix inf/nan convert to i32 on x86_64 arch Jun 28, 2024
qingyunqu
qingyunqu approved these changes Jun 28, 2024
View reviewed changes
Copy link
Collaborator

@qingyunqu qingyunqu left a comment

Choose a reason for hiding this comment

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

LGTM

@qingyunqu qingyunqu merged commit 473bb38 into main Jun 28, 2024
4 checks passed
@qingyunqu qingyunqu deleted the yjw/fix_inf_nan_cvt branch June 28, 2024 08:08
Vremold added a commit that referenced this pull request Jul 4, 2024
@Vremold
COPYBARA SYNC:
281a13e 
  - 9a41649 [onnx-frontend] fix some error (#372)
  - 473bb38 [compiler] fix inf/nan convert to i32 on x86_64 arch (#378)

GitOrigin-RevId: 9a41649
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qingyunqu qingyunqu qingyunqu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jianwenyyy @qingyunqu