[RISCV] Introduction of Zcb instructions in lr/sc sequences may not meet forward progress guarantee

[asb]

I've written up a fuller description here riscv/riscv-isa-manual#1938 in a request to confirm/clarify the current wording. Zcb introduced some new compressed instructions that map from 'I' instructions (c.zext.b and c.not). RISCVExpandAtomicPass takes great care in only producing instructions within the lr/sc sequence that confirm with the forward progess guarantee, but for at least the atomicrmw nand expensions, we produce a xori rd, rs, -1 (for which not rd, rs is a pseudoinstruction), and the final stage compression later converts this to c.not. A plausible reading of the current wording on the forward progress guarantee suggests that the presence of c.not breaks it. Or alternatively, it's just that the text hasn't been updated since Zcb was merged and that was never the intent.

Filing an issue to keep track of this in case we need to make adjustments, depending on the outcome of riscv/riscv-isa-manual#1938

$ cat t.c
char c;

void foo(void) {
  __sync_fetch_and_nand (&c, 1);
}
$ clang --target=riscv64-linux-gnu -march=rv64gc_zcb -O3 t.c -c -o t.clang.o
$ llvm-objdump -d -Mno-aliases t.clang.o

t.clang.o:	file format elf64-littleriscv

Disassembly of section .text:

0000000000000000 <foo>:
       0: 00000517     	auipc	a0, 0x0
       4: 00050513     	addi	a0, a0, 0x0
       8: ffc57593     	andi	a1, a0, -0x4
       c: 890d         	c.andi	a0, 0x3
       e: 050e         	c.slli	a0, 0x3
      10: 0ff00613     	addi	a2, zero, 0xff
      14: 00a6163b     	sllw	a2, a2, a0
      18: 4685         	c.li	a3, 0x1
      1a: 00a6953b     	sllw	a0, a3, a0
      1e: 1605a6af     	lr.w.aqrl	a3, (a1)
      22: 00a6f733     	and	a4, a3, a0
      26: 9f75         	c.not	a4
      28: 8f35         	c.xor	a4, a3
      2a: 8f71         	c.and	a4, a2
      2c: 8f35         	c.xor	a4, a3
      2e: 1ae5a72f     	sc.w.rl	a4, a4, (a1)
      32: e301         	c.bnez	a4, 0x32 <foo+0x32>
      34: 8082         	c.jr	ra

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Alex Bradbury (asb)

Details

I've written up a fuller description here https://github.com/riscv/riscv-isa-manual/issues/1938 in a request to confirm/clarify the current wording. Zcb introduced some new compressed instructions that map from 'I' instructions (c.zext.b and c.not). RISCVExpandAtomicPass takes great care in only producing instructions within the lr/sc sequence that confirm with the forward progess guarantee, but for at least the atomicrmw nand expensions, we produce a `xori rd, rs, -1` (for which `not rd, rs` is a pseudoinstruction), and the final stage compression later converts this to c.not. A plausible reading of the current wording on the forward progress guarantee suggests that the presence of `c.not` breaks it. Or alternatively, it's just that the text hasn't been updated since Zcb was merged and that was never the intent.

Filing an issue to keep track of this in case we need to make adjustments, depending on the outcome of riscv/riscv-isa-manual#1938

$ cat t.c
char c;

void foo(void) {
  __sync_fetch_and_nand (&c, 1);
}
$ clang --target=riscv64-linux-gnu -march=rv64gc_zcb -O3 t.c -c -o t.clang.o
$ llvm-objdump -d -Mno-aliases t.clang.o

t.clang.o:	file format elf64-littleriscv

Disassembly of section .text:

0000000000000000 <foo>:
       0: 00000517     	auipc	a0, 0x0
       4: 00050513     	addi	a0, a0, 0x0
       8: ffc57593     	andi	a1, a0, -0x4
       c: 890d         	c.andi	a0, 0x3
       e: 050e         	c.slli	a0, 0x3
      10: 0ff00613     	addi	a2, zero, 0xff
      14: 00a6163b     	sllw	a2, a2, a0
      18: 4685         	c.li	a3, 0x1
      1a: 00a6953b     	sllw	a0, a3, a0
      1e: 1605a6af     	lr.w.aqrl	a3, (a1)
      22: 00a6f733     	and	a4, a3, a0
      26: 9f75         	c.not	a4
      28: 8f35         	c.xor	a4, a3
      2a: 8f71         	c.and	a4, a2
      2c: 8f35         	c.xor	a4, a3
      2e: 1ae5a72f     	sc.w.rl	a4, a4, (a1)
      32: e301         	c.bnez	a4, 0x32 <foo+0x32>
      34: 8082         	c.jr	ra

[asb]

After some discussion and review from the RISC-V ARC, the spec has been updated to explicitly state that Zcb instructions are allowed (in preference to an earlier proposal to give a more open definition that would cover any future compressed instructions that map to a single I instruction). riscv/riscv-isa-manual@372edf9 As such, there's nothing to fix here for now.

The following two scenarios are important to keep in mind for the future:

• If another standard compressed extension introduces new instructions that map to I instructions that aren't otherwise disallowed in lr/sc loops then hopefully the ISA manual is updated to allow them. If not we should query if it was an oversight.
• For any vendor extensions doing the same, we should clarify that they extend the forward progress guarantee to cover those instructions. (Realistically, the answer is surely yes).