[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng added inline comments.


================
Comment at: llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:469
+    if (STI.getFeatureBits()[RISCV::FeatureVendorXVentanaCondOps]) {
+      LLVM_DEBUG(dbgs() << "Trying Vemtama custom opcode table:\n");
+      Result = decodeInstruction(DecoderTableVentana32, MI, Insn, Address, this,
----------------
Typo, should be "Trying Ventana..."


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1785
+//===----------------------------------------------------------------------===//
+// Vendor extensions
+//===----------------------------------------------------------------------===//
----------------
reames wrote:
> How do we want to manage td files for vendor extensions?
> 
> I put them inline - which is probably not the right answer.  I'm leaning towards a vendor specific td file with extensions split out if complexity justifies it.  So, this patch would add a RISCVInstInfoXVentana.td.  
> 
> Is that the precedent we want here?
I would suggest put into `RISCVInstInfoXVentana.td` instead of inline here, I could imagine once this get merged, T-head extensions might try to upstream too, so put into separated now could prevent they reference this commit and try to inline their extensions IMO :)


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

https://reviews.llvm.org/D137350