Skip to content

Conversation

michaelmaitland
Copy link
Contributor

@michaelmaitland michaelmaitland commented Dec 11, 2024

No description provided.

…tructions

We don't add tests for incompatible LMUL since it would break the type system.
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2024

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

Author: Michael Maitland (michaelmaitland)

Changes

We don't add tests for incompatible LMUL since it would break the MIR type system.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp (+19)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir (+25)
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index dabf36480f1dcf..de00c06e5caf46 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -247,6 +247,25 @@ static OperandInfo getOperandInfo(const MachineInstr &MI,
     llvm_unreachable("Configuration setting instructions do not read or write "
                      "vector registers");
 
+  // Vector Store Whole Register Instructions
+  // EMUL=nr. EEW=eew. Since in-register byte layouts are idential to in-memory
+  // byte layouts, the same data is writen to destination register regardless
+  // of EEW. eew is just a hint to the hardware and has not functional impact.
+  // Therefore, it is be okay if we ignore eew and always use the same EEW to
+  // create more optimization opportunities.
+  // FIXME: Instead of using any SEW, we really should return the SEW in the
+  // instruction and add a field to OperandInfo that says the SEW is just a hint
+  // so that this optimization can use any sew to construct a ratio.
+  case RISCV::VS1R_V:
+    return OperandInfo(RISCVII::VLMUL::LMUL_1, 0);
+  case RISCV::VS2R_V:
+    return OperandInfo(RISCVII::VLMUL::LMUL_2, 0);
+  case RISCV::VS4R_V:
+    return OperandInfo(RISCVII::VLMUL::LMUL_4, 0);
+  case RISCV::VS8R_V:
+    return OperandInfo(RISCVII::VLMUL::LMUL_8, 0);
+
+
   // Vector Integer Arithmetic Instructions
   // Vector Single-Width Integer Add and Subtract
   case RISCV::VADD_VI:
diff --git a/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir b/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
index 1071ee53610854..ee953132d2cde2 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
@@ -483,3 +483,28 @@ body: |
     %x:vr = PseudoVADD_VV_MF4 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0
     %y:vr = PseudoVNSRL_WV_MF2 $noreg, $noreg, %x, 1, 3 /* e8 */, 0
 ...
+---
+name: vsNr_v
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vsNr_v
+    ; CHECK: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:gpr = ADDI $x0, 1
+    ; CHECK-NEXT: VS1R_V %x, %y
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0
+    %y:gpr = ADDI $x0, 1
+    VS1R_V %x, %y,
+...
+# FIXME: We can optimize this
+---
+name: vsNr_v_eew
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vsNr_v_eew
+    ; CHECK: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:gpr = ADDI $x0, 1
+    ; CHECK-NEXT: VS1R_V %x, %y
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0
+    %y:gpr = ADDI $x0, 1
+    VS1R_V %x, %y
+...

Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

Can we not get a different LMUL to typecheck with MIR if we use MF2/MF4/MF8? They should all use the same register class

@michaelmaitland
Copy link
Contributor Author

Can we not get a different LMUL to typecheck with MIR if we use MF2/MF4/MF8? They should all use the same register class

Good point. fixed

%x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0
VS1R_V %x, $noreg
...
# FIXME: We can optimize this
Copy link
Collaborator

Choose a reason for hiding this comment

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

What would the optimization be? We didn't optimize the vsNr_v case.

// FIXME: Instead of using any SEW, we really should return the SEW in the
// instruction and add a field to OperandInfo that says the SEW is just a hint
// so that this optimization can use any sew to construct a ratio.
case RISCV::VS1R_V:
Copy link
Collaborator

Choose a reason for hiding this comment

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

These instructions don't have an SEW or VL operand so I think we would fail this check before we get here

    const MCInstrDesc &Desc = UserMI.getDesc();                                  
    if (!RISCVII::hasVLOp(Desc.TSFlags) || !RISCVII::hasSEWOp(Desc.TSFlags)) {   
      LLVM_DEBUG(dbgs() << "    Abort due to lack of VL or SEW, assume that"     
                           " use VLMAX\n");                                      
      CanReduceVL = false;                                                       
      break;                                                                     
    } 

@michaelmaitland
Copy link
Contributor Author

No optimization opportunity here that I can think of. And we're not hitting this case in getOperandInfo.

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