[AMDGPU] Fix HasFP32Denormals check in FDIV32 lowering#66212
Merged
Conversation
arsenm
reviewed
Sep 13, 2023
arsenm
left a comment
Contributor
There was a problem hiding this comment.
Missing test changes.
This is also a stop gap, we really need to address the fixme later and restore the correct mode
Member
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-backend-amdgpu ChangesFixes SWDEV-403219 -- Full diff: https://github.com//pull/66212.diff1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 777fe76df1151cd..998904bf08820c0 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -9589,7 +9589,7 @@ SDValue SITargetLowering::LowerFDIV32(SDValue Op, SelectionDAG &DAG) const {
const SIMachineFunctionInfo *Info = MF.getInfo<SIMachineFunctionInfo>();
const DenormalMode DenormMode = Info->getMode().FP32Denormals;
- const bool HasFP32Denormals = DenormMode == DenormalMode::getIEEE();
+ const bool HasFP32Denormals = DenormMode != DenormalMode::getPreserveSign();
if (!HasFP32Denormals) {
// Note we can't use the STRICT_FMA/STRICT_FMUL for the non-strict FDIV
|
0f23a32 to
a394495
Compare
arsenm
requested changes
Sep 13, 2023
arsenm
left a comment
Contributor
There was a problem hiding this comment.
Also need the equivalent change for globalisel
a394495 to
74ec83d
Compare
Fixes SWDEV-403219
74ec83d to
97af8bb
Compare
arsenm
approved these changes
Sep 13, 2023
arsenm
left a comment
Contributor
There was a problem hiding this comment.
Please fix the dynamic mode restore in a follow up
| @@ -4737,7 +4737,7 @@ bool AMDGPULegalizerInfo::legalizeFDIV32(MachineInstr &MI, | |||
|
|
|||
| // FIXME: This mishandles dynamic denormal mode. We need to query the | |||
Contributor
There was a problem hiding this comment.
This is still broken, we're just changing the mode incorrectly in the other direction
kstoimenov
pushed a commit
to kstoimenov/llvm-project
that referenced
this pull request
Sep 14, 2023
Fixes SWDEV-403219
This was referenced Sep 14, 2023
ZijunZhaoCCK
pushed a commit
to ZijunZhaoCCK/llvm-project
that referenced
this pull request
Sep 19, 2023
Fixes SWDEV-403219
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes SWDEV-403219