[PATCH] D143029: [RISCV] Add vendor-defined XTHeadBa (address-generation) extension

Thu Feb 2 12:28:02 PST 2023

philipp.tomsich added inline comments.

================
Comment at: llvm/test/MC/RISCV/rv64xtheadba-valid.s:10
+# CHECK-ASM: encoding: [0x8b,0x12,0x73,0x02]
+th.addsl t0, t1, t2, 1
+# CHECK-ASM-AND-OBJ: th.addsl t0, t1, t2, 2
----------------
reames wrote:
> Is 0 a valid encoding?  If so, can you add a test covering that?
> 
> p.s. The choice to use uimm2 seems slightly odd here since it wastes one value for the shift.  Was the 0 encoding possibly used for something else?  If it was, maybe we need a different immediate type or an assert somewhere?
Yes, 0 is a valid encoding.
Yes, it wastes encoding space and is non-sensical and should normally not be generated.

We'll add the test case.  We already have the equivalent in the binutils tests (see https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/riscv/x-thead-ba.s;h=5081c06e6aa85ec4ec0882780be30b25a80e11b1;hb=8254c3d2c94ae5458095ea6c25446ba89134b9da)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143029