Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Oct 24, 2025

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2025

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

Author: Craig Topper (topperc)

Changes

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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+11-1)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoP.td (+5)
  • (modified) llvm/test/CodeGen/RISCV/rv64p.ll (+2-4)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 1c930acd9c4a0..be9d71c23f471 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -433,6 +433,8 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
   if (Subtarget.hasStdExtP() ||
       (Subtarget.hasVendorXCValu() && !Subtarget.is64Bit())) {
     setOperationAction(ISD::ABS, XLenVT, Legal);
+    if (Subtarget.is64Bit())
+      setOperationAction(ISD::ABS, MVT::i32, Custom);
   } else if (Subtarget.hasShortForwardBranchOpt()) {
     // We can use PseudoCCSUB to implement ABS.
     setOperationAction(ISD::ABS, XLenVT, Legal);
@@ -14816,8 +14818,16 @@ void RISCVTargetLowering::ReplaceNodeResults(SDNode *N,
     assert(N->getValueType(0) == MVT::i32 && Subtarget.is64Bit() &&
            "Unexpected custom legalisation");
 
+    if (Subtarget.hasStdExtP()) {
+      SDValue Src = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64,
+                                N->getOperand(0));
+      SDValue Abs = DAG.getNode(RISCVISD::ABSW, DL, MVT::i64, Src);
+      Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, Abs));
+      return;
+    }
+
     if (Subtarget.hasStdExtZbb()) {
-      // Emit a special ABSW node that will be expanded to NEGW+MAX at isel.
+      // Emit a special node that will be expanded to NEGW+MAX at isel.
       // This allows us to remember that the result is sign extended. Expanding
       // to NEGW+MAX here requires a Freeze which breaks ComputeNumSignBits.
       SDValue Src = DAG.getNode(ISD::SIGN_EXTEND, DL, MVT::i64,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
index cc085bb6c9fd7..4cbbba3aa68cb 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
@@ -1461,5 +1461,10 @@ let Predicates = [HasStdExtP, IsRV32] in {
 // Codegen patterns
 //===----------------------------------------------------------------------===//
 
+def riscv_absw : RVSDNode<"ABSW", SDTIntUnaryOp>;
+
 let Predicates = [HasStdExtP] in
 def : PatGpr<abs, ABS>;
+
+let Predicates = [HasStdExtP, IsRV64] in
+def : PatGpr<riscv_absw, ABSW>;
diff --git a/llvm/test/CodeGen/RISCV/rv64p.ll b/llvm/test/CodeGen/RISCV/rv64p.ll
index cb07f945a582a..f937f44f13320 100644
--- a/llvm/test/CodeGen/RISCV/rv64p.ll
+++ b/llvm/test/CodeGen/RISCV/rv64p.ll
@@ -297,8 +297,7 @@ declare i32 @llvm.abs.i32(i32, i1 immarg)
 define i32 @abs_i32(i32 %x) {
 ; CHECK-LABEL: abs_i32:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    sext.w a0, a0
-; CHECK-NEXT:    abs a0, a0
+; CHECK-NEXT:    absw a0, a0
 ; CHECK-NEXT:    ret
   %abs = tail call i32 @llvm.abs.i32(i32 %x, i1 true)
   ret i32 %abs
@@ -307,8 +306,7 @@ define i32 @abs_i32(i32 %x) {
 define signext i32 @abs_i32_sext(i32 signext %x) {
 ; CHECK-LABEL: abs_i32_sext:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    abs a0, a0
-; CHECK-NEXT:    sext.w a0, a0
+; CHECK-NEXT:    absw a0, a0
 ; CHECK-NEXT:    ret
   %abs = tail call i32 @llvm.abs.i32(i32 %x, i1 true)
   ret i32 %abs

@github-actions
Copy link

github-actions bot commented Oct 24, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@pfusik
Copy link
Contributor

pfusik commented Oct 27, 2025

Where's the spec of the P extension?
I don't see it at https://github.com/riscv/riscv-isa-manual and at riscv.org I found https://riscv.atlassian.net/browse/RVS-1135 which links to https://github.com/riscv/riscv-p-spec which contains KABSW not ABSW.

@4vtomat
Copy link
Member

4vtomat commented Oct 27, 2025

Where's the spec of the P extension? I don't see it at https://github.com/riscv/riscv-isa-manual and at riscv.org I found https://riscv.atlassian.net/browse/RVS-1135 which links to https://github.com/riscv/riscv-p-spec which contains KABSW not ABSW.

It's defined here: https://www.jhauser.us/RISCV/ext-P/RVP-baseInstrs-Sail-017.txt

@4vtomat
Copy link
Member

4vtomat commented Oct 27, 2025

Is there any reason we can't pattern match ISD::ABS -> absw, e.g. def : PatGprGpr<binop_allwusers<abs>, ABSW>;?

@topperc
Copy link
Collaborator Author

topperc commented Oct 27, 2025

Is there any reason we can't pattern match ISD::ABS -> absw, e.g. def : PatGprGpr<binop_allwusers<abs>, ABSW>;?

An i64 abs would use bit 63 as the sign bit. We need to use bit 31. Having all W users isn't enough.

Copy link
Member

@4vtomat 4vtomat left a comment

Choose a reason for hiding this comment

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

LGTM~

Copy link
Contributor

@pfusik pfusik 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 521fb93 into llvm:main Oct 30, 2025
9 checks passed
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
@topperc topperc deleted the pr/absw branch October 30, 2025 16:59
luciechoi pushed a commit to luciechoi/llvm-project that referenced this pull request Nov 1, 2025
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