[PATCH] D143439: [RISCV] Add vendor-defined XTheadBb (basic bit-manipulation) extension

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:318
+    if (Subtarget.is64Bit())
+      setOperationAction({ISD::CTLZ, ISD::CTLZ_ZERO_UNDEF}, MVT::i32, Custom);
+  }
----------------
without these two lines to promote i32, I suppose we would get zext i32 to i64, ff1, addi? Is the sequence used for ctlzw better than that?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td:232
+def : Pat<(and GPR:$rs1, 1), (TH_EXTU GPR:$rs1, 0, 0)>;
+def : Pat<(and GPR:$rs1, 0xff), (TH_EXTU GPR:$rs1, 7, 0)>;
+def : Pat<(and GPR:$rs1, 0xffff), (TH_EXTU GPR:$rs1, 15, 0)>;
----------------
Why is TH_EXTU better than ANDI for uimm11 immediates?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td:254
+               (SRLW i64:$rs1, (SUB X0, i64:$rs2)))>;
+def : Pat<(sra (bswap i64:$rs1), (i64 32)), (TH_REVW i64:$rs1)>;
+def : Pat<(riscv_clzw i64:$rs1),
----------------
The sra will very likely be turned into an srl if the next instruction doesn't use the upper bits. You probably need this pattern too

```
def : Pat<(binop_allwusers<srl> (bswap i64:$rs1), i64 32)), (TH_REVW i64:$rs1)>;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143439