[llvm] 695fdef - [RISCV] Bugfix for 90f91683 noticed in follow up work

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 15 08:32:26 PST 2022


Author: Philip Reames
Date: 2022-12-15T08:32:05-08:00
New Revision: 695fdef0ef03eba1169e1c91bdefb24c60cb8ab8

URL: https://github.com/llvm/llvm-project/commit/695fdef0ef03eba1169e1c91bdefb24c60cb8ab8
DIFF: https://github.com/llvm/llvm-project/commit/695fdef0ef03eba1169e1c91bdefb24c60cb8ab8.diff

LOG: [RISCV] Bugfix for 90f91683 noticed in follow up work

I went to extend this locally, and then promptly tripped across a bug which is possible with the landed patch.  The problematic case is:
vsetvli zero, 4, <some vtype>
vmv.x.s x1, v0
vsetvli a0, zero, <same type>

In this case, the naive rewrite - what I had implemented - would form:
vsetvli zero, zero, <same vtype>
vmv.x.s x1, v0

This is, amusingly, correct for the vmv.x.s, but is incorrect for the instructions which follow the sequence and probably rely on VL=VLMAX.  (The VL before the sequence is unknown, and thus doesn't have to be VLMAX.)

I plan to rework the rewrite code to be more robust here, but I wanted to directly fix the bug first.  Sorry for the lack of test; I didn't manage to reproduce this without an additional optimization change after a few minutes of trying.

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index c991b99ab726..62a92ab26bd7 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -1216,6 +1216,13 @@ static bool canMutatePriorConfig(const MachineInstr &PrevMI,
     if (MI.getOperand(1).isReg() &&
         RISCV::X0 != MI.getOperand(1).getReg())
       return false;
+
+    // TODO: We need to change the result register to allow this rewrite
+    // without the result forming a vl preserving vsetvli which is not
+    // a correct state merge.
+    if (PrevMI.getOperand(0).getReg() == RISCV::X0 &&
+        MI.getOperand(1).isReg())
+      return false;
   }
 
   if (!PrevMI.getOperand(2).isImm() || !MI.getOperand(2).isImm())


        


More information about the llvm-commits mailing list