Non-standard assembler usage about riscv-arch-test HOT 16 CLOSED

https://reviews.llvm.org/D57319 (LLVM 9) has fixed sw a0, (0)(t3)

https://reviews.llvm.org/D92293 will fix la sp, (_end + 4096) - ((64 / 8) * 36)

from riscv-arch-test.

Historically, the behavior of the GNU assembler has been the de facto spec, though I realize that's a bit hegemonic.

Philosophically, a constant value is a valid address. My preference in this case is to maximize compatibility: update the spec to clarify that constant-valued addresses are legal addresses for the purposes of la, and encourage the LLVM team to follow suit.

from riscv-arch-test.

Same issue in compiling riscv-tests with

clang -static -mcmodel=medany -nostdlib -nostartfiles -mno-relax --target=riscv64 -mcpu=rocket-rv64 -fuse-ld=lld -mabi=lp64 -march=rv64g -fvisibility=hidden -Ienv/v -Iisa/macros/scalar -I/usr/riscv64-elf/include -L/usr/riscv64-elf/lib env/v/entry.S -DENTROPY=0 env/v/*.c isa/rv64si/sbreak.S

clang is not happy with symbol defined in ld script.

env/v/entry.S:65:10: error: operand must be a bare symbol name
  la sp, (_end + 4096) - ((64 / 8) * 36)

I have a pending LLVM MC patch to fix this. You can use la sp, _end + 4096 - ((64 / 8) * 36) for now.

from riscv-arch-test.

Hi Jeremy,

The erro is in the file: compliance_io.h that is managed and provided by the target. Since the compilation, linking and execution of the test is responsibility of the target, the framework cannot impose any restrictions there. If ris5cy wants the compile the tests with clang and gcc then it should change the respective compliance_io.h.

I did a grep and quick glance indicates that atleast this particular case isn't occurring anywhere else in the tests.

from riscv-arch-test.

@aswaterman Agreed the spec needs changing, since it says "symbol". Clang is clear what is a symbol and what is a constant. Initial implementations inherently define specifications - that is common in many areas. We just need to clean up the gaps, so the spec becomes the golden reference.

@neelgala It is reasonable for the compliance test suite to require compliance with the spec, but where bugs are found, responsibility for resolution needs to be placed on the target owner. So this issue should be assigned to that person. Do you know who owns the RI5CY target?

from riscv-arch-test.

I think @bluewww was the author of the PR for adding ri5scy.

I just wanted to say that it is a bug only if RI5CY wants to be Clang compatible. The compliace framework declares a combination of SDK and Device Target as RISC-V compliant.

Also, I think #107 and #106 (post resolution) should probably end up as guidelines within the Test-format spec.

Maybe the RI5CY guys could fix this.

from riscv-arch-test.

I track it for now, but I don't really use clang. The quick fix would probably be to define a symbol with the correct constant and load that.

from riscv-arch-test.

from riscv-arch-test.

The other fix (which I've mentioned elsewhere) is to change the LA in riscv-target/ri5cy/compliance_io.h to be LI.

from riscv-arch-test.

Same issue in compiling riscv-tests with

clang -static -mcmodel=medany -nostdlib -nostartfiles -mno-relax --target=riscv64 -mcpu=rocket-rv64 -fuse-ld=lld -mabi=lp64 -march=rv64g -fvisibility=hidden -Ienv/v -Iisa/macros/scalar -I/usr/riscv64-elf/include -L/usr/riscv64-elf/lib env/v/entry.S -DENTROPY=0 env/v/*.c isa/rv64si/sbreak.S

clang is not happy with symbol defined in ld script.

env/v/entry.S:65:10: error: operand must be a bare symbol name
  la sp, (_end + 4096) - ((64 / 8) * 36)

from riscv-arch-test.

Is this something that can be closed now?

from riscv-arch-test.

No comments, so I'm closing this. Reopen if someone can show it is still failing.

from riscv-arch-test.

Sorry to reopen this issue.
Now I am trying to compile la ra,_f2802x_usdelay-5, which generates a wrong result.
While when I compile la ra,_f2802x_usdelay+0-5, the result is OK.
LLVM version:13.0.1.
Source code:

_f2802x_usdelay:
	addi  sp,sp,-20
	sw    a1,16(sp) 
	csrr  a1,0x7c0
	sw    a1,12(sp)
	csrr  a1,mstatus
	la ra,_f2802x_usdelay+0-5

Disassembly for la ra,_f2802x_usdelay+0-5:

00010000 <_f2802x_usdelay>:
   10000:	1131                	addi	sp,sp,-20
   10002:	c82e                	sw	a1,16(sp)
   10004:	7c0025f3          	csrr	a1,loop
   10008:	c62e                	sw	a1,12(sp)
   1000a:	300025f3          	csrr	a1,mstatus
   1000e:	00000097          	auipc	ra,0x0
   10012:	fed08093          	addi	ra,ra,-19 # fffb <SysPwrCtrlRegs+0x239b>

Disassembly for la ra,_f2802x_usdelay-5:

00010000 <_f2802x_usdelay>:
   10000:	1131                	addi	sp,sp,-20
   10002:	c82e                	sw	a1,16(sp)
   10004:	7c0025f3          	csrr	a1,loop
   10008:	c62e                	sw	a1,12(sp)
   1000a:	300025f3          	csrr	a1,mstatus
   1000e:	00000097          	auipc	ra,0x0
   10012:	ff708093          	addi	ra,ra,-9 # 10005 <_f2802x_usdelay+0x5>

@allenjbaum
@MaskRay

from riscv-arch-test.

Not sure whether this is an asm spec bug or an LLVM bug, but it doesn’t seem like a bug in this repo. I suggest raising this issue elsewhere and using your workaround locally (or use GNU tools as a workaround).

from riscv-arch-test.

from riscv-arch-test.

OK, I have posted this issue to llvm maillist.

from riscv-arch-test.