[PATCH] D143036: [RISCV] Add vendor-defined XTHeadBs (single-bit) extension

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:838
+    // If C2 is (1 << ShAmt) use bexti or th.tst if possible.
+    if ((Subtarget->hasStdExtZbs() || Subtarget->hasVendorXTHeadBs()) &&
+        ShAmt + 1 == TrailingOnes) {
----------------
Maybe put into a variable like `bool HasBitTest = Subtarget->hasStdExtZbs() || Subtarget->hasVendorXTHeadBs()` then use it here and on line 969 for the Skip?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1252
+  if (Subtarget.hasVendorXTHeadBs())
+    return C != NULL;
+  // We can use ANDI+SEQZ/SNEZ as a bit test. Y contains the bit position.
----------------
We don't use NULL in llvm. Use nullptr.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8709
+  if (Subtarget.is64Bit() &&
+      (Subtarget.hasStdExtZbs() || Subtarget.hasVendorXTHeadBs()) &&
+      VT == MVT::i1 && N0.getValueType() == MVT::i32 &&
----------------
I think this code was for variable shifts is it relevant for this extension?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8957
+  if (Subtarget.is64Bit() &&
+      (Subtarget.hasStdExtZbs() || Subtarget.hasVendorXTHeadBs()) &&
       N->getValueType(0) == MVT::i32 && isOneConstant(N->getOperand(1)) &&
----------------
I think this code was for variable shifts is it relevant for this extension?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143036