[PATCH] D116580: [M68k] Add addressing modes ARIPI and ARIPD support for BTST

Thu Jan 27 02:55:12 PST 2022

myhsu accepted this revision.
myhsu added a comment.
This revision is now accepted and ready to land.

In D116580#3257039 <https://reviews.llvm.org/D116580#3257039>, @Jim wrote:

> In D116580#3247458 <https://reviews.llvm.org/D116580#3247458>, @myhsu wrote:
>
>> This revision seems to be in conflict with D116993 <https://reviews.llvm.org/D116993> . How are you going to resolve it? (e.g. rebasing this one on top of D116993 <https://reviews.llvm.org/D116993> and setup a dependency)
>
> This revision is parent of D116993 <https://reviews.llvm.org/D116993>. It doesn't depend on D116993 <https://reviews.llvm.org/D116993>. I am going to resolve issue in D116993 <https://reviews.llvm.org/D116993> you mentioned.

Sorry I missed this comment. LGTM now

================
Comment at: llvm/test/MC/M68k/Bits/Classes/MxBTST_MI.s:15
+; CHECK-SAME: encoding: [0x08,0x18,0x00,0xff]
+btst	#-1, (%a0)+
+
----------------
Jim wrote:
> ricky26 wrote:
> > I'm not sure whether this is defined behaviour. The spec I have says that BTST with a register for the bit number wraps, but doesn't specify that behaviour for the immediate mode, so it probably works. Additionally, I don't think it would be signed, but again, I suspect it'll still work (-1 will be interpreted as 31).
> The spec for BTST mentioned `When a data register is the destination, any of the 32 bits can be specified by a modulo 32- bit number. When a memory location is the destination, the operation is a byte operation, and the bit number is modulo 8.`  
> 
> Maybe have another revision to forbid signed number as immediate operand in the future.
> 
> The reference spec: https://www.nxp.com/files-static/archives/doc/ref_manual/M68000PRM.pdf
> Maybe have another revision to forbid signed number as immediate operand in the future.

GNU AS currently supports negative immediate so I'm incline not to remove this feature.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116580