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

[Thomas Johnson via Phabricator via llvm-commits]

thomasjohns added inline comments.


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


================
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);
----------------
marksl wrote:
> 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
Thanks, agreed that makes sense to me to separate them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108598