Fix silent bad codegen in signed comparison

While working on #5956, I uncovered a silent bad codegen bug in signed
comparison. The way the code was written, we'd do an unsigned set after a
signed compare/jump from the high comparison. However, in this case, we
need to make sure that we are honoring the sign of the comparison done on
the high part of the operation, and the lo comparison would still want to
be an unsigned compare followed by an unsigned set. This change splits
comparisons into two cases: 1) A signed comparison where we have two jumps
for the high part (GT_GT, GT_LT, GT_GE, and GT_LE), which require a signed
set if the high comparisons are true, and an unsigned set if not, and 2)
An unsigned comparison, or a comparison where the sign does not matter for
the compare (GT_NE, GT_EQ), which just require an unsigned set for both
the high and lo operations (the code we already had in genCompareLong).

When we compare longs, we don't need to have both a jg/ja and a jne
instruction for the hi compare for the case where we enregister the result
of the compare, since the set will do the right thing. We only need to
check the lo halves if the hi halves were equal.

For long compares where the result isn't stored in a register, we need to use
signed jumps for the high comparison, and unsigned jumps for the low compare.
Currently, we generate a signed jump in the code for GT_JTRUE that is used by
both compares. This change modifies the logic for these compares/JTRUEs.
We separate the logic into genJTrueLong. In genJTrueLong, we use similar
logic as the legacy backend for the jumps, using a signed jump after the
high compare when we have signed operands, and unsigned jump when we have
a unsigned operands, and an unsigned jump for the low compare.

This change also modifies genJumpKindsForTreeLongHi, since it is now only
used for the uncontained case, where we need to jump to the bbJumpDest if
the compare is true, so we need to use the actual jump type, rather than
just checking for inequality (as inequality only tells us if we need to
check the low halves, not if the condition is true or not).

This change also adds test cases for the two silent bad codegen cases this
commit fixes.

1 files changed, 27 insertions, 0 deletions

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_6239/app.config b/tests/src/JIT/Regression/JitBlue/GitHub_6239/app.config
new file mode 100644
index 0000000000..62803f5972
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_6239/app.config
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+  <runtime>
+    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
+      <dependentAssembly>
+        <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+    </assemblyBinding>
+  </runtime>
+</configuration>
\ No newline at end of file