[RISCV] Verify SEW/VecPolicy immediate values

[reames]

Summary

Copy the asserts from the printing code, and turn them into actual verifier rules. Doing this revealed an existing bug - see D133868.

[asb]

@reames what's your thinking about the current asserts in the printing code - to keep them, or not?

I think it definitely makes sense to add these verifier rules and the code seems like an accurate translation from the printing code to my eyes. I'd appreciate if someone else a little more familiar with the vector instructions would give a quick additional review, but otherwise LGTM.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1224

MI.getDesc() here should be the same as Desc 2 lines up. But it looks like we took the long way to create Desc at the top of this function.

[reames]

In D133869#3790380, @asb wrote:

@reames what's your thinking about the current asserts in the printing code - to keep them, or not?

Had not planned to remove them. A data point is that the verifier does not use that printing path, so having them doesn't influence verification failures.

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1224

Will reuse, update pending.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1225

The immediate operand stores 64 bits, should we check all 64 bits and make sure its small enough to be a shift amount before using it to shift?

1234

Should we check all 64 bits of the immediate?

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

break;
         if (!Ok) {
           ErrInfo = "Invalid immediate";
           return false;
         }
       }
     }
   }
 
+  const uint64_t TSFlags = Desc.TSFlags;
+  if (RISCVII::hasSEWOp(TSFlags)) {
+    unsigned OpIdx = RISCVII::getSEWOpNum(Desc);
+    uint64_t Log2SEW = MI.getOperand(OpIdx).getImm();
+    if (Log2SEW > 31) {
+      ErrInfo = "Unexpected SEW value";
+      return false;
+    }
+    unsigned SEW = Log2SEW ? 1 << Log2SEW : 8;
+    if (!RISCVVType::isValidSEW(SEW)) {
+      ErrInfo = "Unexpected SEW value";
+      return false;
+    }
+  }
+  if (RISCVII::hasVecPolicyOp(TSFlags)) {
+    unsigned OpIdx = RISCVII::getVecPolicyOpNum(Desc);
+    uint64_t Policy = MI.getOperand(OpIdx).getImm();
+    if (Policy > (RISCVII::TAIL_AGNOSTIC | RISCVII::MASK_AGNOSTIC)) {
+      ErrInfo = "Invalid Policy Value";
+      return false;
+    }
+  }
+
   return true;
 }
 
 // Return true if get the base operand, byte offset of an instruction and the
 // memory width. Width is the size of memory that is being loaded/stored.
 bool RISCVInstrInfo::getMemOperandWithOffsetWidth(
     const MachineInstr &LdSt, const MachineOperand *&BaseReg, int64_t &Offset,
     unsigned &Width, const TargetRegisterInfo *TRI) const {