Skip to content

[X86][AVX512] Check input-types to COMX #118606

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

Merged
merged 9 commits into from
Dec 9, 2024
Merged

Conversation

abhishek-kaushik22
Copy link
Contributor

Supported types for COMX are f16, f32 and f64.
Without this check there's a crash on f80 types.
Fixes: #118605

@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-backend-x86

Author: None (abhishek-kaushik22)

Changes

Supported types for COMX are f16, f32 and f64.
Without this check there's a crash on f80 types.
Fixes: #118605


Full diff: https://github.com/llvm/llvm-project/pull/118606.diff

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+7-2)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 96b03feaa45803..c8d6bd7c622127 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -24228,8 +24228,13 @@ SDValue X86TargetLowering::LowerSETCC(SDValue Op, SelectionDAG &DAG) const {
   if (Subtarget.hasAVX10_2()) {
     if (CC == ISD::SETOEQ || CC == ISD::SETUNE) {
       auto NewCC = (CC == ISD::SETOEQ) ? X86::COND_E : (X86::COND_NE);
-      return getSETCC(NewCC, DAG.getNode(X86ISD::UCOMX, dl, MVT::i32, Op0, Op1),
-                      dl, DAG);
+      auto isValidType = [&](MVT Type) {
+        return Type == MVT::f16 || Type == MVT::f32 || Type == MVT::f64;
+      };
+      if (isValidType(Op0.getSimpleValueType()) &&
+          isValidType(Op1.getSimpleValueType()))
+        return getSETCC(
+            NewCC, DAG.getNode(X86ISD::UCOMX, dl, MVT::i32, Op0, Op1), dl, DAG);
     }
   }
   // Handle floating point.

@abhishek-kaushik22
Copy link
Contributor Author

@mahesh-attarde @phoebewang @e-kud can you please review?

Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

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

LGTM.

@abhishek-kaushik22
Copy link
Contributor Author

@phoebewang can you please merge?

@phoebewang phoebewang merged commit d6501dc into llvm:main Dec 9, 2024
8 checks passed
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.

LLVM ERROR: Cannot select: ... = X86ISD::UCOMX ..., ...
3 participants