| Message ID | 20221113204119.4061447-1-philipp.tomsich@vrull.eu |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Use .p2align for code-alignment | expand |
On 11/13/22 13:41, Philipp Tomsich wrote: > RISC-V's .p2align (currently) ignores the max-skip argument. As we > have experimental patches underway to address this in a > backwards-compatible manner, let's prepare GCC for the day when > binutils gets updated. > > gcc/ChangeLog: > > * config/riscv/riscv.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Implement. What are the implications if we start using p2align immediately when the current (broken?) state of the assembler? I'm pretty sure configure is already turning on HAVE_GAS_SKIP_P2ALIGN. From a native risc-v build: auto-host.h:#define HAVE_GAS_MAX_SKIP_P2ALIGN 1 jeff
Jeff, On Tue, 15 Nov 2022 at 17:37, Jeff Law <jeffreyalaw@gmail.com> wrote: > > > On 11/13/22 13:41, Philipp Tomsich wrote: > > RISC-V's .p2align (currently) ignores the max-skip argument. As we > have experimental patches underway to address this in a > backwards-compatible manner, let's prepare GCC for the day when > binutils gets updated. > > gcc/ChangeLog: > > * config/riscv/riscv.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Implement. > > > What are the implications if we start using p2align immediately when the current (broken?) state of the assembler? I'm pretty sure configure is already turning on HAVE_GAS_SKIP_P2ALIGN. From a native risc-v build: > > > auto-host.h:#define HAVE_GAS_MAX_SKIP_P2ALIGN 1 This is your tree, which has the (partial fix — i.e., the best we can do without breaking backward compatibility) for .p2align. When building against upstream binutils, this should not be defined. Philipp.
On Tue, 15 Nov 2022 at 22:14, Philipp Tomsich <philipp.tomsich@vrull.eu> wrote: > > Jeff, > > On Tue, 15 Nov 2022 at 17:37, Jeff Law <jeffreyalaw@gmail.com> wrote: > > > > > > On 11/13/22 13:41, Philipp Tomsich wrote: > > > > RISC-V's .p2align (currently) ignores the max-skip argument. As we > > have experimental patches underway to address this in a > > backwards-compatible manner, let's prepare GCC for the day when > > binutils gets updated. > > > > gcc/ChangeLog: > > > > * config/riscv/riscv.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Implement. > > > > > > What are the implications if we start using p2align immediately when the current (broken?) state of the assembler? I'm pretty sure configure is already turning on HAVE_GAS_SKIP_P2ALIGN. From a native risc-v build: > > > > > > auto-host.h:#define HAVE_GAS_MAX_SKIP_P2ALIGN 1 > > This is your tree, which has the (partial fix — i.e., the best we can > do without breaking backward compatibility) for .p2align. > When building against upstream binutils, this should not be defined. After rereading the generated configure-script, I've changed my mind The check the does nothing to ensure that the max-skip is actually honored and only tests whether it is accepted. Having the max-skip silently ignored may still be the lesser evil. Philipp.
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index 2d0d170645c..c216173cf6b 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -938,6 +938,24 @@ typedef struct { fprintf (STREAM, "\t.word\t%sL%d-%sL%d\n", \ LOCAL_LABEL_PREFIX, VALUE, LOCAL_LABEL_PREFIX, REL) +#ifdef HAVE_GAS_MAX_SKIP_P2ALIGN +/* Support for -falign-* switches. Use .p2align to ensure that code + sections are padded with NOP instructions, rather than zeros. */ +#define ASM_OUTPUT_MAX_SKIP_ALIGN(FILE, LOG, MAX_SKIP) \ + do \ + { \ + if ((LOG) != 0) \ + { \ + if ((MAX_SKIP) == 0) \ + fprintf ((FILE), "\t.p2align %d\n", (int) (LOG)); \ + else \ + fprintf ((FILE), "\t.p2align %d,,%d\n", \ + (int) (LOG), (int) (MAX_SKIP)); \ + } \ + } while (0) + +#endif /* HAVE_GAS_MAX_SKIP_P2ALIGN */ + /* This is how to output an assembler line that says to advance the location counter to a multiple of 2**LOG bytes. */
RISC-V's .p2align (currently) ignores the max-skip argument. As we have experimental patches underway to address this in a backwards-compatible manner, let's prepare GCC for the day when binutils gets updated. gcc/ChangeLog: * config/riscv/riscv.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Implement. Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> --- gcc/config/riscv/riscv.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)