[RISCV][VLOPT] Add support for more instructions in vl-opt-op-info.mir #119416

michaelmaitland · 2024-12-10T17:13:20Z

Specifically, add ones where EMUL=LMUL and EEW=SEW.

llvmbot · 2024-12-10T17:13:47Z

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

Author: Michael Maitland (michaelmaitland)

Changes

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

1 Files Affected:

(modified) llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir (+30)

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..32c5bf2f12e22f 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
@@ -1,6 +1,36 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
 # RUN: llc %s -o - -mtriple=riscv64 -mattr=+v -run-pass=riscv-vl-optimizer -verify-machineinstrs | FileCheck %s
 
+---
+name: vop_vi
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vop_vi
+    ; CHECK: %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, 1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:vr = PseudoVADD_VI_M1 $noreg, %x, 9, 1, 3 /* e8 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, -1, 3 /* e8 */, 0
+    %y:vr = PseudoVADD_VI_M1 $noreg, %x, 9, 1, 3 /* e8 */, 0
+...
+---
+name: vop_vi_incompatible_eew
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vop_vi_incompatible_eew
+    ; CHECK: %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, -1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:vr = PseudoVADD_VI_M1 $noreg, %x, 9, 1, 4 /* e16 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, -1, 3 /* e8 */, 0
+    %y:vr = PseudoVADD_VI_M1 $noreg, %x, 9, 1, 4 /* e16 */, 0
+...
+---
+name: vop_vi_incompatible_emul
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vop_vi_incompatible_emul
+    ; CHECK: %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, -1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:vr = PseudoVADD_VI_MF2 $noreg, %x, 9, 1, 3 /* e8 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, -1, 3 /* e8 */, 0
+    %y:vr = PseudoVADD_VI_MF2 $noreg, %x, 9, 1, 3 /* e8 */, 0
+...
 ---
 name: vop_vv
 body: |

topperc · 2024-12-10T17:57:46Z

EMUL=EEW in the title doesn't make sense.

michaelmaitland · 2024-12-10T18:03:35Z

EMUL=EEW in the title doesn't make sense.

Updated

llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir

topperc · 2024-12-10T20:51:04Z

llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir

+    ; CHECK: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:vr = PseudoVMV_V_I_M1 $noreg, %x, 1, 3 /* e8 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0
+    %y:vr = PseudoVMV_V_I_M1 $noreg, %x, 1, 3 /* e8 */, 0


This should be VMV_V_V_M1?

topperc · 2024-12-10T20:51:36Z

llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir

+    ; CHECK: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 4 /* e16 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:vr = PseudoVMV_V_V_M1 %x, $noreg, 1, 4 /* e16 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 4 /* e16 */, 0
+    %y:vr = PseudoVMV_V_V_M1 %x, $noreg, 1, 4 /* e16 */, 0


This is testing the pasthru operand which can never fold regardless of the EEW being incompatible

Now that we have testing of all instructions in the isSupportedInstr switch, and better coverage of getOperandInfo, I think it is a good time to enable this by default. I'd like for llvm#112231 and llvm#119416 to land before this patch, so it'd be great for anyone reviewing this to check those out first.

lukel97

The existing vop_vv_* test cases should cover the EEW=SEW EMUL=EMUL format instructions.

I think the original structure of this test was to test the various formats/combinations of EEW and EMUL, but with that said I'm not opposed if we want to test specific opcodes either

michaelmaitland · 2024-12-11T02:42:12Z

The existing vop_vv_* test cases should cover the EEW=SEW EMUL=EMUL format instructions.

I think the original structure of this test was to test the various formats/combinations of EEW and EMUL, but with that said I'm not opposed if we want to test specific opcodes either

While its true that we cover the return statements in getOperandInfo already, this patch adds some tests for instructions that definitely have different properties than whats currently being tested. Just because an instruction is grouped up with a common return statement, it doesn't mean that it actually belongs there. I'd like to build this test up over time to get increased confidence that instructions grouped under a common return statement actually belong there.

lukel97 · 2024-12-11T15:29:55Z

llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir

+    ; CHECK-NEXT: %z:vr = PseudoVMV_V_X_M1 %x, %y, 1, 4 /* e16 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 4 /* e16 */, 0
+    %y:gpr = ADDI $x0, 1
+    %z:vr = PseudoVMV_V_X_M1 %x, %y, 1, 4 /* e16 */, 0


Are the EEWs here not compatible?

Sorry for typo. Fixed.

lukel97 · 2024-12-11T15:30:42Z

llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir

+    ; CHECK-LABEL: name: vop_vi
+    ; CHECK: %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, 1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %y:vr = PseudoVADD_VI_M1 $noreg, %x, 9, 1, 3 /* e8 */, 0 /* tu, mu */
+    %x:vr = PseudoVADD_VI_M1 $noreg, $noreg, 9, -1, 3 /* e8 */, 0


Nit, can we keep the instruction not under test VADD_VV to make it easier to tell which operand info we should be looking out for?

lukel97 · 2024-12-11T15:32:06Z

The existing vop_vv_* test cases should cover the EEW=SEW EMUL=EMUL format instructions.

I think the original structure of this test was to test the various formats/combinations of EEW and EMUL, but with that said I'm not opposed if we want to test specific opcodes either

While its true that we cover the return statements in getOperandInfo already, this patch adds some tests for instructions that definitely have different properties than whats currently being tested. Just because an instruction is grouped up with a common return statement, it doesn't mean that it actually belongs there. I'd like to build this test up over time to get increased confidence that instructions grouped under a common return statement actually belong there.

That sounds reasonable to me

michaelmaitland · 2024-12-17T14:23:37Z

ping

Now that we have testing of all instructions in the isSupportedInstr switch, and better coverage of getOperandInfo, I think it is a good time to enable this by default. I'd like for llvm#112231 and llvm#119416 to land before this patch, so it'd be great for anyone reviewing this to check those out first.

…-info.mir

topperc

LGTM

llvm-ci · 2024-12-17T18:01:11Z

LLVM Buildbot has detected a new failure on builder cross-project-tests-sie-ubuntu running on doug-worker-1a while building llvm at step 2 "checkout".

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

Here is the relevant piece of the build log for the reference

Step 2 (checkout) failure: update (failure)
git version 2.25.1
fatal: unable to access 'https://github.com/llvm/llvm-project.git/': HTTP/2 stream 0 was not closed cleanly: CANCEL (err 8)
fatal: unable to access 'https://github.com/llvm/llvm-project.git/': HTTP/2 stream 0 was not closed cleanly: CANCEL (err 8)

michaelmaitland added the backend:RISC-V label Dec 10, 2024

michaelmaitland requested review from lukel97 and topperc December 10, 2024 17:13

michaelmaitland changed the title ~~[RISCV][VLOPT] Add support for vop_vi style instructions in vl-opt-op-info.mir~~ [RISCV][VLOPT] Add support for vop_vi and vmerge in vl-opt-op-info.mir Dec 10, 2024

michaelmaitland changed the title ~~[RISCV][VLOPT] Add support for vop_vi and vmerge in vl-opt-op-info.mir~~ [RISCV][VLOPT] Add support for more SEW=EEW EMUL=EEW instructions in vl-opt-op-info.mir Dec 10, 2024

michaelmaitland force-pushed the vop-vi-vlopt branch 3 times, most recently from c7fbec8 to 811fd1b Compare December 10, 2024 17:53

michaelmaitland changed the title ~~[RISCV][VLOPT] Add support for more SEW=EEW EMUL=EEW instructions in vl-opt-op-info.mir~~ [RISCV][VLOPT] Add support for more instructions in vl-opt-op-info.mir Dec 10, 2024

topperc reviewed Dec 10, 2024

View reviewed changes

llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir Show resolved Hide resolved

topperc reviewed Dec 10, 2024

View reviewed changes

michaelmaitland requested a review from topperc December 10, 2024 21:04

michaelmaitland mentioned this pull request Dec 10, 2024

[RISCV][VLOPT] Enable the RISCVVLOptimizer by default #119461

Merged

lukel97 reviewed Dec 11, 2024

View reviewed changes

michaelmaitland requested a review from lukel97 December 11, 2024 15:24

lukel97 reviewed Dec 11, 2024

View reviewed changes

michaelmaitland requested a review from lukel97 December 11, 2024 15:46

michaelmaitland added 3 commits December 17, 2024 08:05

[RISCV][VLOPT] Add support for vop_vi style instructions in vl-opt-op…

d788a48

…-info.mir

fixup! test vmerge instructions

ffb96b8

fixup! test vmv.v.* instructions

113c667

michaelmaitland added 3 commits December 17, 2024 08:09

remove redundant tests

7a3cb3b

fixup! respond to review

1222a59

fixup! respond to review

a4827db

michaelmaitland force-pushed the vop-vi-vlopt branch from e423d02 to a4827db Compare December 17, 2024 16:10

topperc approved these changes Dec 17, 2024

View reviewed changes

michaelmaitland merged commit 904849f into llvm:main Dec 17, 2024
8 checks passed

michaelmaitland deleted the vop-vi-vlopt branch December 17, 2024 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RISCV][VLOPT] Add support for more instructions in vl-opt-op-info.mir #119416

[RISCV][VLOPT] Add support for more instructions in vl-opt-op-info.mir #119416

michaelmaitland commented Dec 10, 2024 •

edited

Loading

llvmbot commented Dec 10, 2024

topperc commented Dec 10, 2024

michaelmaitland commented Dec 10, 2024

topperc Dec 10, 2024

topperc Dec 10, 2024

lukel97 left a comment

michaelmaitland commented Dec 11, 2024

lukel97 Dec 11, 2024

michaelmaitland Dec 11, 2024

lukel97 Dec 11, 2024

michaelmaitland Dec 11, 2024

lukel97 commented Dec 11, 2024

michaelmaitland commented Dec 17, 2024

topperc left a comment

llvm-ci commented Dec 17, 2024

[RISCV][VLOPT] Add support for more instructions in vl-opt-op-info.mir #119416

[RISCV][VLOPT] Add support for more instructions in vl-opt-op-info.mir #119416

Conversation

michaelmaitland commented Dec 10, 2024 • edited Loading

llvmbot commented Dec 10, 2024

topperc commented Dec 10, 2024

michaelmaitland commented Dec 10, 2024

topperc Dec 10, 2024

Choose a reason for hiding this comment

topperc Dec 10, 2024

Choose a reason for hiding this comment

lukel97 left a comment

Choose a reason for hiding this comment

michaelmaitland commented Dec 11, 2024

lukel97 Dec 11, 2024

Choose a reason for hiding this comment

michaelmaitland Dec 11, 2024

Choose a reason for hiding this comment

lukel97 Dec 11, 2024

Choose a reason for hiding this comment

michaelmaitland Dec 11, 2024

Choose a reason for hiding this comment

lukel97 commented Dec 11, 2024

michaelmaitland commented Dec 17, 2024

topperc left a comment

Choose a reason for hiding this comment

llvm-ci commented Dec 17, 2024

michaelmaitland commented Dec 10, 2024 •

edited

Loading