[PATCH] D108598: [ARC] Add codegen for the readcyclecounter intrinsic along with disassembly for associated instructions

[Mark Schimmel via Phabricator via llvm-commits]

marksl requested changes to this revision.
marksl added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/ARC/ARCISelLowering.cpp:172
+  //       when the LegalizeI64 predicate is available.
+  setOperationAction(ISD::READCYCLECOUNTER, MVT::i64, Custom);
 }
----------------
Please use 
    setOperationAction(ISD::READCYCLECOUNTER,MVT::i64,
                       isTypeLegal(MVT::i64)?Legal:Custom);



================
Comment at: llvm/lib/Target/ARC/ARCInstrFormats.td:417
+
+  let DecoderMethod = "DecodeSOPwithRU6orRS12";
+}
----------------
Can this one be DecodeSOPwithRS12?


================
Comment at: llvm/lib/Target/ARC/ARCInstrFormats.td:439
+
+  let DecoderMethod = "DecodeSOPwithRU6orRS12";
+}
----------------
Can this one be DecodeSOPwithRU6?


================
Comment at: llvm/lib/Target/ARC/Disassembler/ARCDisassembler.cpp:322
+  DecodeGPR32RegisterClass(Inst, DstB, Address, Decoder);
+  using Field = decltype(Insn);
+  Field Lower = fieldFromInstruction(Insn, 6, 6);
----------------
thomasjohns wrote:
> This aims to decode either form:
> `...ssssssSSSSSS`
> or
> `...uuuuuu000000`
> (S12 or U6). I assumed the leading bit of the S12 marks the sign. Is this correct @marksl ?
I'd rather see you use two separate functions:  DecodeSOPwithRU6 and DecodeSOPwithRS12


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108598