Skip to content

Conversation

kaushik-quicinc
Copy link
Contributor

The provided regression, Hexagon/inst_setcc_uno_uo.ll, fails prior to this patch.

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added backend:Hexagon llvm:mc Machine (object) code labels May 20, 2025
@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-hexagon

Author: None (kaushik-quicinc)

Changes

The provided regression, Hexagon/inst_setcc_uno_uo.ll, fails prior to this patch.


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

2 Files Affected:

  • (modified) llvm/lib/Target/Hexagon/HexagonISelLoweringHVX.cpp (+4)
  • (added) llvm/test/MC/Hexagon/inst_setcc_uno_uo.ll (+20)
diff --git a/llvm/lib/Target/Hexagon/HexagonISelLoweringHVX.cpp b/llvm/lib/Target/Hexagon/HexagonISelLoweringHVX.cpp
index fbbcacf0d713e..af3682ab38fa1 100644
--- a/llvm/lib/Target/Hexagon/HexagonISelLoweringHVX.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonISelLoweringHVX.cpp
@@ -349,6 +349,8 @@ HexagonTargetLowering::initializeHVXLowering() {
   setCondCodeAction(ISD::SETULE, MVT::v64f16, Expand);
   setCondCodeAction(ISD::SETUGE, MVT::v64f16, Expand);
   setCondCodeAction(ISD::SETULT, MVT::v64f16, Expand);
+  setCondCodeAction(ISD::SETUO, MVT::v64f16, Expand);
+  setCondCodeAction(ISD::SETO, MVT::v64f16, Expand);
 
   setCondCodeAction(ISD::SETNE,  MVT::v32f32, Expand);
   setCondCodeAction(ISD::SETLE,  MVT::v32f32, Expand);
@@ -362,6 +364,8 @@ HexagonTargetLowering::initializeHVXLowering() {
   setCondCodeAction(ISD::SETULE, MVT::v32f32, Expand);
   setCondCodeAction(ISD::SETUGE, MVT::v32f32, Expand);
   setCondCodeAction(ISD::SETULT, MVT::v32f32, Expand);
+  setCondCodeAction(ISD::SETUO, MVT::v32f32, Expand);
+  setCondCodeAction(ISD::SETO, MVT::v32f32, Expand);
 
   // Boolean vectors.
 
diff --git a/llvm/test/MC/Hexagon/inst_setcc_uno_uo.ll b/llvm/test/MC/Hexagon/inst_setcc_uno_uo.ll
new file mode 100644
index 0000000000000..fa94f60b088c8
--- /dev/null
+++ b/llvm/test/MC/Hexagon/inst_setcc_uno_uo.ll
@@ -0,0 +1,20 @@
+;; RUN: llc -mv79 -mhvx %s -o - | FileCheck %s
+source_filename = "isnan.c"
+target datalayout = "e-m:e-p:32:32:32-a:0-n16:32-i64:64:64-i32:32:32-i16:16:16-i1:8:8-f32:32:32-f64:64:64-v32:32:32-v64:64:64-v512:512:512-v1024:1024:1024-v2048:2048:2048"
+target triple = "hexagon"
+
+; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite, inaccessiblemem: readwrite)
+define dso_local void @foo(ptr noundef readonly captures(none) %a, ptr noundef writeonly captures(none) %isnan_a) local_unnamed_addr #0 {
+entry:
+  %arrayidx = getelementptr inbounds nuw float, ptr %a, i32 0
+  %0 = load <32 x float>, ptr %arrayidx, align 4
+  %.ripple.vectorized = fcmp uno <32 x float> %0, zeroinitializer
+  %arrayidx1 = getelementptr inbounds nuw i8, ptr %isnan_a, i32 0
+  %storedv.ripple.LS.instance = zext <32 x i1> %.ripple.vectorized to <32 x i8>
+  store <32 x i8> %storedv.ripple.LS.instance, ptr %arrayidx1, align 1
+  ret void
+}
+
+;; CHECK: vcmp.eq
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite, inaccessiblemem: readwrite) "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="hexagonv79" "target-features"="+hmx,+hvx-length128b,+hvxv79,+v79,-long-calls" }

ret void
}

;; CHECK: vcmp.eq
Copy link
Member

Choose a reason for hiding this comment

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

Are there any other stable outputs from this test that could be verified beyond vcmp.eq? For example, is there a polarity/direction of the comparison?

Also: MVT::v64f16 and MVT::v32f32 types seem to be supported, but does that mean other ones cannot be supported? If so, should we have new tests to verify more types than the ones that are used in foo above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SETO and SETUO should be only for floats. I don't think other types would come into play?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Added a check for f16.)

Copy link
Member

Choose a reason for hiding this comment

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

Ok -- I'm not really experienced w/the compiler so forgive questions that appear out of left field.

@androm3da
Copy link
Member

androm3da commented May 22, 2025

This test seems to expose an issue with v68-and-later (with HVX 64 byte mode). Not sure if it's on the baseline or introduced by your change, though.

./bin/llc --mtriple=hexagon --mcpu=hexagonv68 --mattr=+hvxv68,+hvx-length64b < ../llvm-project/llvm/test/MC/Hexagon/inst_setcc_uno_uo.ll > hvx64.s
llc: /local/mnt/workspace/upstream/llvm-project/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp:649: void llvm::DAGTypeLegalizer::ReplaceValueWith(SDValue, SDValue): Assertion `From.getNode() != To.getNode() && "Potential legalization loop!"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.

@kaushik-quicinc
Copy link
Contributor Author

This test seems to expose an issue with v68-and-later (with HVX 64 byte mode). Not sure if it's on the baseline or introduced by your change, though.

Yes, the issue with the type legalizer is on the baseline too.

@kaushik-quicinc
Copy link
Contributor Author

Duplicate of #158740.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:Hexagon llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants