On Tue, 25 Jun 2019 15:58:53 PDT (-0700), pins...@gmail.com wrote:
On Tue, Jun 25, 2019 at 3:46 PM Ilia Diachkov <ilia.diach...@optimitech.com> wrote:Hello, This patch adds new machine specific option -malign-data={word,abi} to RISC-V port. The option switches alignment of global variables and constants of array/record/union types. The default value (-malign-data=word) keeps existing way of alignment computation. Another option value (-malign-data=abi) makes data natural alignment. It avoids extra spaces between data to reduce code size. The measured code size reduction is about 0.4% at -Os on EEMBC automotive 1.1 tests and SPEC2006 C/C++ benchmarks. The patch was tested in riscv-gnu-toolchain by dejagnu. Please check the patch into the trunk.Hmm, may I suggest use "natural" rather than "abi" and 32bit or 64bit rather than "word"; it is not obvious what abi means and it is not obvious what word means here; it could be either 32bit or 64bit depending on the option.
It's actually worse: in RISC-V "word" always means 32-bit (BITS_PER_WORD is a GCC name). "natural" seems like a good term for the "align to the size of the type". The RISC-V term for "the width of an integer register" is "xlen", so I think that's a good bet for the other option.
Also my other suggestion is create a new macro where you pass riscv_align_data_type == riscv_align_data_type_word for the "(ALIGN) < BITS_PER_WORD) " check to reduce the code duplication.
Additionally, has this been tested with "-mstrict-align"? The generated code can be awful, but if it's not correct then we should throw an error on that combination.
Thanks, Andrew PinskiBest regards, Ilia. gcc/ * config/riscv/riscv-opts.h (struct riscv_align_data): Added. * config/riscv/riscv.c (riscv_constant_alignment): Use riscv_align_data_type. * config/riscv/riscv.h (DATA_ALIGNMENT): Use riscv_align_data_type. (LOCAL_ALIGNMENT): Set to old DATA_ALIGNMENT value. * config/riscv/riscv.opt (malign-data): New. * doc/invoke.texi (RISC-V Options): Document -malign-data=.
