Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

balign asm statement broken #298

Closed
cliffordwolf opened this issue Nov 18, 2017 · 12 comments
Closed

balign asm statement broken #298

cliffordwolf opened this issue Nov 18, 2017 · 12 comments

Comments

@cliffordwolf
Copy link

Consider the following assembler code:

j foobar
.ascii "xxxxx"
.byte 0x00
.balign 4
foobar:
nop

An older version of riscv32-unknown-elf-as produces the expected result (output of riscv32-unknown-elf-objdump -d):

00000000 <foobar-0xc>:
   0:	00c0006f          	j	c <foobar>
   4:	7878                	flw	fa4,116(s0)
   6:	7878                	flw	fa4,116(s0)
   8:	0078                	addi	a4,sp,12
	...

0000000c <foobar>:
   c:	00000013          	nop

But with the current version of riscv-gnu-toolchain (bf5697a) I get the following result instead:

00000000 <foobar-0xa>:
   0:	00a0006f          	j	a <foobar>
   4:	7878                	flw	fa4,116(s0)
   6:	7878                	flw	fa4,116(s0)
   8:	0078                	addi	a4,sp,12

0000000a <foobar>:
   a:	00000013          	nop
	...

The symbol foobar is not aligned to a multiple of 4. It looks like the .balign statement was simply ignored.

In both cases I just ran riscv32-unknown-elf-as test.s and then riscv32-unknown-elf-objdump -d a.out.

@palmer-dabbelt
Copy link
Contributor

as produces object files, not executables. RISC-V handles alignment via linker relaxation, so object files won't have the expected alignment. If you 'objdump -dr a.out' you should see a R_RISCV_ALGIN relocation.

$ riscv64-unknown-elf-objdump -dr test.o                                                                                                                                                                                                                                                                                                                                                                                                                                                                              test.o:     file format elf64-littleriscv                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                  Disassembly of section .text:

0000000000000000 <foobar-0xa>:
   0:   00a0006f                j       a <foobar>
                        0: R_RISCV_JAL  foobar
   4:   7878                    ld      a4,240(s0)
   6:   7878                    ld      a4,240(s0)
   8:   0078                    addi    a4,sp,12

000000000000000a <foobar>:
   a:   00000013                nop
                        a: R_RISCV_ALIGN        *ABS*
        ...

You should see the correct alignment in an executable (ie, produced by the linker). I haven't gotten to the specific case of the R_RISCV_ALIGN, but there's a bit of blog content on some other relocations and on linker relaxation

https://www.sifive.com/blog/2017/08/21/all-aboard-part-2-relocations/
https://www.sifive.com/blog/2017/08/28/all-aboard-part-3-linker-relaxation-in-riscv-toolchain/

@wallento
Copy link
Contributor

Hi @palmer-dabbelt, the bug report is based on this issue: YosysHQ/picorv32#50

The linker ultimately gives the error 6 bytes required for alignment to 8 bytes boundary.

@cliffordwolf
Copy link
Author

Here is the complete error I get in the original test case:

/opt/riscv32i/bin/riscv32-unknown-elf-gcc -Os -ffreestanding -nostdlib -o firmware/firmware.elf \
	-Wl,-Bstatic,-T,firmware/sections.lds,-Map,firmware/firmware.map,--strip-debug \
	firmware/start.o firmware/irq.o firmware/print.o firmware/sieve.o firmware/multest.o firmware/stats.o tests/ori.o tests/xor.o tests/or.o tests/andi.o tests/lui.o tests/srai.o tests/sra.o tests/blt.o tests/bge.o tests/sub.o tests/mul.o tests/lw.o tests/lb.o tests/sw.o tests/bgeu.o tests/div.o tests/beq.o tests/auipc.o tests/rem.o tests/jalr.o tests/sll.o tests/mulhu.o tests/mulh.o tests/divu.o tests/add.o tests/slt.o tests/lbu.o tests/lhu.o tests/sh.o tests/simple.o tests/sb.o tests/bltu.o tests/slli.o tests/remu.o tests/xori.o tests/lh.o tests/srli.o tests/jal.o tests/addi.o tests/and.o tests/srl.o tests/j.o tests/bne.o tests/mulhsu.o tests/slti.o -lgcc
/opt/riscv32i/lib/gcc/riscv32-unknown-elf/7.2.0/../../../../riscv32-unknown-elf/bin/ld: tests/auipc.o(.text+0x32): 6 bytes required for alignmentto 8-byte boundary, but only 4 present
/opt/riscv32i/lib/gcc/riscv32-unknown-elf/7.2.0/../../../../riscv32-unknown-elf/bin/ld: can't relax section: Bad value
collect2: error: ld returned 1 exit status

@palmer-dabbelt you are right, I can link the object file I get in the case of the above simple assembler input. I'll try to create a stand-alone test case that reproduces the linker error.

@cliffordwolf
Copy link
Author

Also: Maybe we can fix the "alignmentto" typo? :)
https://twitter.com/oe1cxw/status/929506482332119041

@cliffordwolf
Copy link
Author

Try this test case:

.text
.global _start
_start:
nop

.prname_next: 
nop
j .prname_done
j .prname_next

.test_name: 
.ascii "auipc"
.byte 0x00
.balign 4
.prname_done: 
nop

test_2:
.align 3

With current riscv-gnu-toolchain:

$ riscv32-unknown-elf-gcc -march=rv32i -Os -ffreestanding -nostdlib -o test.elf test.s
/opt/riscv32i/lib/gcc/riscv32-unknown-elf/7.2.0/../../../../riscv32-unknown-elf/bin/ld: /tmp/ccdQLHYQ.o(.text+0x1a): 6 bytes required for alignmentto 8-byte boundary, but only 4 present
/opt/riscv32i/lib/gcc/riscv32-unknown-elf/7.2.0/../../../../riscv32-unknown-elf/bin/ld: can't relax section: Bad value
collect2: error: ld returned 1 exit status

With an older version:

$ riscv32-unknown-elf-gcc -march=rv32i -Os -ffreestanding -nostdlib -o test.elf test.s
$ riscv32-unknown-elf-objdump -d test.elf

test.elf:     file format elf32-littleriscv


Disassembly of section .text:

00010058 <_start>:
   10058:	00000013          	nop

0001005c <.prname_next>:
   1005c:	00000013          	nop
   10060:	0100006f          	j	10070 <.prname_done>
   10064:	ff9ff06f          	j	1005c <.prname_next>

00010068 <.test_name>:
   10068:	7561                	lui	a0,0xffff8
   1006a:	7069                	0x7069
   1006c:	00000063          	beqz	zero,1006c <.test_name+0x4>

00010070 <.prname_done>:
   10070:	00000013          	nop

00010074 <test_2>:
   10074:	00000013          	nop

@jim-wilson
Copy link
Collaborator

jim-wilson commented Nov 19, 2017 via email

@jim-wilson
Copy link
Collaborator

jim-wilson commented Nov 19, 2017 via email

@jim-wilson
Copy link
Collaborator

jim-wilson commented Nov 19, 2017 via email

@cliffordwolf
Copy link
Author

I thought the default filling byte for .balign is zero bytes, not nops. But apparently this is implementation defined in .text sections. From as.info:

The second expression (also absolute) gives the fill value to be
stored in the padding bytes. It (and the comma) may be omitted. If it
is omitted, the padding bytes are normally zero. However, on some
systems, if the section is marked as containing code and the fill value
is omitted, the space is filled with no-op instructions.

Is stuff like this (risc-v specific implementation defined behavior) documented anywhere?

Explicitly setting the filling byte to zero .balign 4, 0 solves the issue with my code.

@jim-wilson
Copy link
Collaborator

jim-wilson commented Nov 19, 2017 via email

@cliffordwolf
Copy link
Author

Emitting alignment directives to align branch target labels for better
performance is a common compiler optimization.

I thought that's what .align is for. But now I see that as.info has the same paragraph for .align.

Clearly the problem was that I had incorrect assumptions about the differences between the two directives. I always used something like .align 2 to pad with nops to a multiple of 4 and .balign 4 to pad with whatever to a multiple of 4. But now I see that it's not even standardized if the parameter to .align specifies the alignment in zero bits in the next address or in bytes..

So in the future I will now use .balign 4 and .balign 4, 0 for the two use cases. Thanks for helping me understand what went wrong here.

The risc-v one only
mentions options. It needs to be expanded to mention the .option pseudo-op
and could probably also mention a few other things. That will have to be a
long term goal to improve the docs.

Yes please. :) I did not know about .option [no]relax and .option [no]rvc until you mentioned it.

@jim-wilson
Copy link
Collaborator

jim-wilson commented Nov 19, 2017 via email

davidlt pushed a commit to davidlt/opensbi that referenced this issue Oct 23, 2019
Resolves the follow compilation error (payload variant for SiFive FU540 with
U-Boot and embedded FDT):

    /usr/bin/ld: /build/platform/sifive/fu540/firmware/fw_payload.o(.text+0x1961): 15 bytes required for alignment to 16-byte boundary, but only 14 present
    /usr/bin/ld: can't relax section: bad value
    collect2: error: ld returned 1 exit status

Noticed while compiling 5.4-rc3+ kernel in Fedora/RISCV.

For more details see:
riscv-collab/riscv-gnu-toolchain#298

Signed-off-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
avpatel added a commit to avpatel/opensbi that referenced this issue Oct 23, 2019
We get following compile error for FW_PAYLOAD with latest GCC
binutils:
fw_payload.o(.text+0x1961): 15 bytes required for alignment to 16-byte
boundary, but only 14 present

Further investigating, it turn-out to be a known issue with RISC-V
GCC binutils.
(Refer, riscv-collab/riscv-gnu-toolchain#298)

As a work-around, we disable relaxation when including DTB and
PAYLOAD binary in fw_payload.S.

Reported-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
avpatel added a commit to avpatel/opensbi that referenced this issue Oct 23, 2019
We get following compile error for FW_PAYLOAD with latest GCC
binutils:
fw_payload.o(.text+0x1961): 15 bytes required for alignment to 16-byte
boundary, but only 14 present

Further investigating, it turn-out to be a known issue with RISC-V
GCC binutils.
(Refer, riscv-collab/riscv-gnu-toolchain#298)

As a work-around, we disable relaxation when including DTB and
PAYLOAD binary in fw_payload.S.

Reported-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
avpatel added a commit to avpatel/opensbi that referenced this issue Oct 28, 2019
We get following compile error for FW_PAYLOAD with latest GCC
binutils:
fw_payload.o(.text+0x1961): 15 bytes required for alignment to 16-byte
boundary, but only 14 present

Further investigating, it turn-out to be a known issue with RISC-V
GCC binutils.
(Refer, riscv-collab/riscv-gnu-toolchain#298)

As a work-around, we disable relaxation when including DTB and
PAYLOAD binary in fw_payload.S.

Reported-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Tested-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants