Skip to content

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 3, 2025

This pattern appears to be a copy of the pattern in Sh3AddPat but using sh3add.uw instead of sh3add. I think this is a mistake and the pattern should be the equivalent of the first pattern from Sh1Add_UWPat and Sh2Add_UWPat.

These classes were created to share with Andes in a788a1a, but there was so many test changes in there that we must have overlooked the changes to Zba codegen.

This pattern appears to be a copy of the pattern in Sh3AddPat
but using sh3add.uw instead of sh3add. I think this is mistake and
the pattern should be the equivalent of the first pattern from
Sh1Add_UWPat and Sh2Add_UWPat.

These classes were created to share with Andes in a788a1a, but
there was so many test changes in there that we must have overlooked the
changes to Zba codegen.
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

This pattern appears to be a copy of the pattern in Sh3AddPat but using sh3add.uw instead of sh3add. I think this is mistake and the pattern should be the equivalent of the first pattern from Sh1Add_UWPat and Sh2Add_UWPat.

These classes were created to share with Andes in a788a1a, but there was so many test changes in there that we must have overlooked the changes to Zba codegen.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZb.td (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rv64zba.ll (+1-9)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZb.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
index ce21d831250b2..8d9b7776ea430 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
@@ -808,9 +808,9 @@ multiclass Sh2Add_UWPat<Instruction sh2add_uw> {
 }
 
 multiclass Sh3Add_UWPat<Instruction sh3add_uw> {
-  def : Pat<(i64 (add_like_non_imm12 (and GPR:$rs1, 0xFFFFFFF8),
+  def : Pat<(i64 (add_like_non_imm12 (and (shl GPR:$rs1, (i64 3)), 0x7FFFFFFFF),
                                      (XLenVT GPR:$rs2))),
-            (sh3add_uw (XLenVT (SRLIW GPR:$rs1, 3)), GPR:$rs2)>;
+            (sh3add_uw GPR:$rs1, GPR:$rs2)>;
   // Use SRLI to clear the LSBs and SHXADD_UW to mask and shift.
   def : Pat<(i64 (add_like_non_imm12 (and GPR:$rs1, 0x7FFFFFFF8),
                                      (XLenVT GPR:$rs2))),
diff --git a/llvm/test/CodeGen/RISCV/rv64zba.ll b/llvm/test/CodeGen/RISCV/rv64zba.ll
index c028d25169749..7fd76262d547a 100644
--- a/llvm/test/CodeGen/RISCV/rv64zba.ll
+++ b/llvm/test/CodeGen/RISCV/rv64zba.ll
@@ -409,15 +409,11 @@ define i64 @sh3adduw_2(i64 %0, i64 %1) {
 ;
 ; RV64ZBA-LABEL: sh3adduw_2:
 ; RV64ZBA:       # %bb.0:
-; RV64ZBA-NEXT:    slli a0, a0, 3
-; RV64ZBA-NEXT:    srli a0, a0, 3
 ; RV64ZBA-NEXT:    sh3add.uw a0, a0, a1
 ; RV64ZBA-NEXT:    ret
 ;
 ; RV64XANDESPERF-LABEL: sh3adduw_2:
 ; RV64XANDESPERF:       # %bb.0:
-; RV64XANDESPERF-NEXT:    slli a0, a0, 3
-; RV64XANDESPERF-NEXT:    srli a0, a0, 3
 ; RV64XANDESPERF-NEXT:    nds.lea.d.ze a0, a1, a0
 ; RV64XANDESPERF-NEXT:    ret
   %3 = shl i64 %0, 3
@@ -436,15 +432,11 @@ define i64 @sh3adduw_3(i64 %0, i64 %1) {
 ;
 ; RV64ZBA-LABEL: sh3adduw_3:
 ; RV64ZBA:       # %bb.0:
-; RV64ZBA-NEXT:    slli a0, a0, 3
-; RV64ZBA-NEXT:    srli a0, a0, 3
 ; RV64ZBA-NEXT:    sh3add.uw a0, a0, a1
 ; RV64ZBA-NEXT:    ret
 ;
 ; RV64XANDESPERF-LABEL: sh3adduw_3:
 ; RV64XANDESPERF:       # %bb.0:
-; RV64XANDESPERF-NEXT:    slli a0, a0, 3
-; RV64XANDESPERF-NEXT:    srli a0, a0, 3
 ; RV64XANDESPERF-NEXT:    nds.lea.d.ze a0, a1, a0
 ; RV64XANDESPERF-NEXT:    ret
   %3 = shl i64 %0, 3
@@ -2681,7 +2673,7 @@ define i64 @srliw_3_sh3add(ptr %0, i32 signext %1) {
 ; RV64ZBA-LABEL: srliw_3_sh3add:
 ; RV64ZBA:       # %bb.0:
 ; RV64ZBA-NEXT:    srliw a1, a1, 3
-; RV64ZBA-NEXT:    sh3add.uw a0, a1, a0
+; RV64ZBA-NEXT:    sh3add a0, a1, a0
 ; RV64ZBA-NEXT:    ld a0, 0(a0)
 ; RV64ZBA-NEXT:    ret
 ;

; RV64ZBA-LABEL: srliw_3_sh3add:
; RV64ZBA: # %bb.0:
; RV64ZBA-NEXT: srliw a1, a1, 3
; RV64ZBA-NEXT: sh3add.uw a0, a1, a0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

srliw always puts 0s in the upper 32 bits so the .uw here isn't necessary. So I don't think the mistake can cause a miscompile.

@topperc topperc requested review from lukel97 and preames October 6, 2025 21:30
Copy link
Contributor

@tclin914 tclin914 left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit 08078fb into llvm:main Oct 7, 2025
11 checks passed
@topperc topperc deleted the pr/sh3add_uw branch October 7, 2025 02:34
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 7, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-windows running on linaro-armv8-windows-msvc-05 while building llvm at step 6 "test".

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

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-api :: functionalities/ptrauth_diagnostics/LDRAA_error/TestPtrauthLDRAADiagnostic.py (596 of 2310)
UNSUPPORTED: lldb-api :: functionalities/ptrauth_diagnostics/brkC47x_code/TestPtrauthBRKc47xDiagnostic.py (597 of 2310)
UNSUPPORTED: lldb-api :: functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/TestPtrauthBRKc47xX16Invalid.py (598 of 2310)
PASS: lldb-api :: functionalities/rerun/TestRerun.py (599 of 2310)
UNSUPPORTED: lldb-api :: functionalities/rerun_and_expr/TestRerunAndExpr.py (600 of 2310)
UNSUPPORTED: lldb-api :: functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py (601 of 2310)
PASS: lldb-api :: functionalities/return-value/TestReturnValue.py (602 of 2310)
PASS: lldb-api :: functionalities/recursion/TestValueObjectRecursion.py (603 of 2310)
PASS: lldb-api :: functionalities/reverse-execution/TestReverseContinueBreakpoints.py (604 of 2310)
PASS: lldb-api :: functionalities/reverse-execution/TestReverseExecutionNotSupported.py (605 of 2310)
FAIL: lldb-api :: functionalities/reverse-execution/TestReverseContinueWatchpoints.py (606 of 2310)
******************** TEST 'lldb-api :: functionalities/reverse-execution/TestReverseContinueWatchpoints.py' FAILED ********************
Script:
--
C:/Users/tcwg/scoop/apps/python/current/python.exe C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/llvm-project/lldb\test\API\dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./lib --env LLVM_INCLUDE_DIR=C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/include --env LLVM_TOOLS_DIR=C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./bin --arch aarch64 --build-dir C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/lldb-test-build.noindex --lldb-module-cache-dir C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/lldb-test-build.noindex/module-cache-lldb\lldb-api --clang-module-cache-dir C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/lldb-test-build.noindex/module-cache-clang\lldb-api --executable C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./bin/lldb.exe --compiler C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./bin/clang.exe --dsymutil C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./bin/dsymutil.exe --make C:/Users/tcwg/scoop/shims/make.exe --llvm-tools-dir C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./bin --lldb-obj-root C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/tools/lldb --lldb-libs-dir C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/./lib --cmake-build-type Release --skip-category=watchpoint C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\API\functionalities\reverse-execution -p TestReverseContinueWatchpoints.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision 08078fb359b68d88ee3edbb01a910af7b8cde548)
  clang revision 08078fb359b68d88ee3edbb01a910af7b8cde548
  llvm revision 08078fb359b68d88ee3edbb01a910af7b8cde548

Watchpoint 1 hit:
old value: 2
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 2
Skipping the following test categories: ['watchpoint', 'libc++', 'libstdcxx', 'dwo', 'dsym', 'gmodules', 'debugserver', 'objc', 'fork', 'pexpect']

An exception happened when receiving the response from the gdb server. Closing the client...


--
Command Output (stderr):
--
lldb-server exiting...

PASS: LLDB (C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe-aarch64) :: test_reverse_continue_skip_watchpoint (TestReverseContinueWatchpoints.TestReverseContinueWatchpoints.test_reverse_continue_skip_watchpoint)

lldb-server exiting...

PASS: LLDB (C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe-aarch64) :: test_reverse_continue_skip_watchpoint_async (TestReverseContinueWatchpoints.TestReverseContinueWatchpoints.test_reverse_continue_skip_watchpoint_async)

lldb-server exiting...


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