[PATCH] D93013: [RISCV] Define vadd intrinsics and lower to V instructions.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2255
+    const TargetRegisterClass *RC = TLI->getRegClassFor(ValVT);
+    if (RC->hasSuperClassEq(&RISCV::VRRegClass)) {
+      Reg = State.AllocateReg(ArgVRs);
----------------
Will RC ever be a subclass? Can we just check RC == &RISCV::VRRegClass?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2394
+  if (LocVT.getSimpleVT().isScalableVector()) {
+    RC = TLI->getRegClassFor(LocVT.getSimpleVT());
+  } else {
----------------
Would getRegClassFor work for the scalar case too?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:24
+[{
+    SDLoc DL(N);
+
----------------
Put this inside the if since its only needed if we create nodes.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:26
+
+    if (auto *C = dyn_cast<ConstantSDNode>(N)) {
+      if (C->isNullValue()) {
----------------
This can be simplified slightly to

```
auto *C = dyn_cast<ConstantSDNode(N);
if (C && C->isNullValue()
```

Then you don't need two levels of indentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93013/new/

https://reviews.llvm.org/D93013