Skip to content

Conversation

@michaelmaitland
Copy link
Contributor

@michaelmaitland michaelmaitland commented Dec 18, 2024

These instructions are covered by the existing tests. We don't add them to
isSupported because of VXSAT. This decision was made in #120358.

These instructions are covered by the existing tests. We do not add them to
isSupportedInstr because they have a tied def which means they will never
get to that point in isCandidate.
@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2024

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

Author: Michael Maitland (michaelmaitland)

Changes

These instructions are covered by the existing tests. We do not add them to isSupportedInstr because they have a tied def which means they will never get to that point in isCandidate.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp (+5)
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index e8719d02cfa0aa..9ee6ec2498d309 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -378,6 +378,11 @@ static OperandInfo getOperandInfo(const MachineOperand &MO,
   case RISCV::VASUBU_VX:
   case RISCV::VASUB_VV:
   case RISCV::VASUB_VX:
+  // Vector Single-Width Fractional Multiply with Rounding and Saturation
+  // EEW=SEW. EMUL=LMUL. The instruction produces 2*SEW product internally but
+  // saturates to fit into SEW bits.
+  case RISCV::VSMUL_VV:
+  case RISCV::VSMUL_VX:
   // Vector Single-Width Scaling Shift Instructions
   // EEW=SEW. EMUL=LMUL.
   case RISCV::VSSRL_VI:

@topperc
Copy link
Collaborator

topperc commented Dec 18, 2024

We do not add them to isSupportedInstr because they have a tied def which means they will never get to that point in isCandidate.

They don't have a tied def.

@michaelmaitland
Copy link
Contributor Author

We do not add them to isSupportedInstr because they have a tied def which means they will never get to that point in isCandidate.

They don't have a tied def.

Updated to say multiple def instead of tied.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelmaitland
Copy link
Contributor Author

I've reverted this patch back to the LGTM, since we decided in #120358 that we will not be adding saturated instructions to isSupportedInstr because of VXSAT.

@michaelmaitland michaelmaitland merged commit 50a457d into llvm:main Dec 30, 2024
5 of 8 checks passed
@michaelmaitland michaelmaitland deleted the vsmul-vlopt branch December 30, 2024 14:01
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.

3 participants