Skip to content

Conversation

@mshockwave
Copy link
Member

And make all unsigned and signed versions of min/max matchers commutative, since we already made a precedent of m_GAdd that is commutative by default.

And make all unsigned and signed versions of min/max matchers
commutative, since we already made a precedent of m_GAdd that is
commutative by default.
@llvmbot
Copy link
Member

llvmbot commented Dec 24, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: Min-Yih Hsu (mshockwave)

Changes

And make all unsigned and signed versions of min/max matchers commutative, since we already made a precedent of m_GAdd that is commutative by default.


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

2 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h (+16-4)
  • (modified) llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp (+26)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h b/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
index ea6ed322e9b192..95d28834900e00 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
@@ -538,15 +538,27 @@ m_GAShr(const LHS &L, const RHS &R) {
 }
 
 template <typename LHS, typename RHS>
-inline BinaryOp_match<LHS, RHS, TargetOpcode::G_SMAX, false>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_SMAX, true>
 m_GSMax(const LHS &L, const RHS &R) {
-  return BinaryOp_match<LHS, RHS, TargetOpcode::G_SMAX, false>(L, R);
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_SMAX, true>(L, R);
 }
 
 template <typename LHS, typename RHS>
-inline BinaryOp_match<LHS, RHS, TargetOpcode::G_SMIN, false>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_SMIN, true>
 m_GSMin(const LHS &L, const RHS &R) {
-  return BinaryOp_match<LHS, RHS, TargetOpcode::G_SMIN, false>(L, R);
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_SMIN, true>(L, R);
+}
+
+template <typename LHS, typename RHS>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_UMAX, true>
+m_GUMax(const LHS &L, const RHS &R) {
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_UMAX, true>(L, R);
+}
+
+template <typename LHS, typename RHS>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_UMIN, true>
+m_GUMin(const LHS &L, const RHS &R) {
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_UMIN, true>(L, R);
 }
 
 // Helper for unary instructions (G_[ZSA]EXT/G_TRUNC) etc
diff --git a/llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp b/llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp
index 59a86fa5646f36..2088a3f81ed57a 100644
--- a/llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp
@@ -224,6 +224,32 @@ TEST_F(AArch64GISelMITest, MatchBinaryOp) {
   auto MIBAddCst = B.buildAdd(s64, MIBCst, Copies[0]);
   auto MIBUnmerge = B.buildUnmerge({s32, s32}, B.buildConstant(s64, 42));
 
+  // Match min/max, and make sure they're commutative.
+  auto SMin = B.buildSMin(s64, Copies[2], MIBAdd);
+  EXPECT_TRUE(mi_match(SMin.getReg(0), *MRI,
+                       m_GSMin(m_GAdd(m_Reg(Src1), m_Reg(Src2)), m_Reg(Src0))));
+  EXPECT_EQ(Src0, Copies[2]);
+  EXPECT_EQ(Src1, Copies[0]);
+  EXPECT_EQ(Src2, Copies[1]);
+  auto SMax = B.buildSMax(s64, Copies[2], MIBAdd);
+  EXPECT_TRUE(mi_match(SMax.getReg(0), *MRI,
+                       m_GSMax(m_GAdd(m_Reg(Src1), m_Reg(Src2)), m_Reg(Src0))));
+  EXPECT_EQ(Src0, Copies[2]);
+  EXPECT_EQ(Src1, Copies[0]);
+  EXPECT_EQ(Src2, Copies[1]);
+  auto UMin = B.buildUMin(s64, Copies[2], MIBAdd);
+  EXPECT_TRUE(mi_match(UMin.getReg(0), *MRI,
+                       m_GUMin(m_GAdd(m_Reg(Src1), m_Reg(Src2)), m_Reg(Src0))));
+  EXPECT_EQ(Src0, Copies[2]);
+  EXPECT_EQ(Src1, Copies[0]);
+  EXPECT_EQ(Src2, Copies[1]);
+  auto UMax = B.buildUMax(s64, Copies[2], MIBAdd);
+  EXPECT_TRUE(mi_match(UMax.getReg(0), *MRI,
+                       m_GUMax(m_GAdd(m_Reg(Src1), m_Reg(Src2)), m_Reg(Src0))));
+  EXPECT_EQ(Src0, Copies[2]);
+  EXPECT_EQ(Src1, Copies[0]);
+  EXPECT_EQ(Src2, Copies[1]);
+
   // m_BinOp with opcode.
   // Match binary instruction, opcode and its non-commutative operands.
   match = mi_match(MIBAddCst, *MRI,

@mshockwave mshockwave merged commit 831e1ac into llvm:main Dec 26, 2024
10 checks passed
@mshockwave mshockwave deleted the patch/gisel-combine-umin-umax branch December 26, 2024 17:28
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 26, 2024

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-win running on as-builder-10 while building llvm at step 17 "test-check-lldb-api".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/52

Here is the relevant piece of the build log for the reference
Step 17 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure)
...
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteKill.py (1178 of 1218)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteSingleStep.py (1179 of 1218)
PASS: lldb-api :: tools/lldb-server/TestGdbRemotePlatformFile.py (1180 of 1218)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteRegisterState.py (1181 of 1218)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteAuxvSupport.py (1182 of 1218)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteLaunch.py (1183 of 1218)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteExpeditedRegisters.py (1184 of 1218)
UNSUPPORTED: lldb-api :: tools/lldb-server/TestPtyServer.py (1185 of 1218)
UNSUPPORTED: lldb-api :: tools/lldb-server/commandline/TestGdbRemoteConnection.py (1186 of 1218)
UNRESOLVED: lldb-api :: tools/lldb-server/TestGdbRemote_qThreadStopInfo.py (1187 of 1218)
******************** TEST 'lldb-api :: tools/lldb-server/TestGdbRemote_qThreadStopInfo.py' FAILED ********************
Script:
--
C:/Python312/python.exe C:/buildbot/as-builder-10/lldb-x-aarch64/llvm-project/lldb\test\API\dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/./lib --env LLVM_INCLUDE_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/include --env LLVM_TOOLS_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin --arch aarch64 --build-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex --lldb-module-cache-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex/module-cache-lldb\lldb-api --clang-module-cache-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex/module-cache-clang\lldb-api --executable C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/lldb.exe --compiler C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/clang.exe --dsymutil C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/dsymutil.exe --make C:/ninja/make.exe --llvm-tools-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin --lldb-obj-root C:/buildbot/as-builder-10/lldb-x-aarch64/build/tools/lldb --lldb-libs-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/./lib --platform-url connect://jetson-agx-0086.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot c:/buildbot/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\API\tools\lldb-server -p TestGdbRemote_qThreadStopInfo.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 831e1ac12e766ae8c94d8d735d8f32c8d319e576)
  clang revision 831e1ac12e766ae8c94d8d735d8f32c8d319e576
  llvm revision 831e1ac12e766ae8c94d8d735d8f32c8d319e576
Setting up remote platform 'remote-linux'

Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-0086.lab.llvm.org:1234'...

Connected.

Setting remote platform working directory to '/home/ubuntu/lldb-tests'...

Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap']


--
Command Output (stderr):
--
UNSUPPORTED: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt_debugserver (TestGdbRemote_qThreadStopInfo.TestGdbRemote_qThreadStopInfo.test_qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt_debugserver) (test case does not fall in any category of interest for this run) 

FAIL: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt_llgs (TestGdbRemote_qThreadStopInfo.TestGdbRemote_qThreadStopInfo.test_qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt_llgs)

2024-12-26 09:55:17,196 WARNING  failed to send kill packet to debug monitor: <class 'ConnectionResetError'>; ignoring

UNSUPPORTED: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_qThreadStopInfo_works_for_multiple_threads_debugserver (TestGdbRemote_qThreadStopInfo.TestGdbRemote_qThreadStopInfo.test_qThreadStopInfo_works_for_multiple_threads_debugserver) (test case does not fall in any category of interest for this run) 

PASS: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_qThreadStopInfo_works_for_multiple_threads_llgs (TestGdbRemote_qThreadStopInfo.TestGdbRemote_qThreadStopInfo.test_qThreadStopInfo_works_for_multiple_threads_llgs)

======================================================================

ERROR: test_qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt_llgs (TestGdbRemote_qThreadStopInfo.TestGdbRemote_qThreadStopInfo.test_qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt_llgs)

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.

4 participants