Re: [PATCH 2/2] RISC-V: Add riscv{32, 64}be with big endian as default

[Marcus Comstedt]

Hi again.

Sorry, but I just realized that the "-melf" part isn't the same in all
of {elf,linux,freebsd}.h; linux.h has a LD_EMUL_SUFFIX which the other
two are lacking.  Which means that the only truly common part is

  %{mbig-endian:-EB} \
  %{mlittle-endian:-EL} \

So is it worth a define to change two lines duplicated in three files
into one line duplicated in three files?  The define would not even
cover all of the endianness stuff.


  // Marcus

Re: [PATCH 2/2] RISC-V: Add riscv{32, 64}be with big endian as default

[Marcus Comstedt]

Kito Cheng  writes:

> Yeah, but I'd like to include following 2 lines too:
>
> %{mbig-endian:-EB} \
> %{mlittle-endian:-EL} \
>
> I saw it's just the same among 3 files.

Ah, I see.  Then it becomes a little more of a mixed grab bag.

I see that SuperH has a spec "subtarget_link_spec" which includes
-EL/-EB as well as other stuff, and then includes that into a
SH_LINK_SPEC, which also adds the link emulation as well as
e.g. mrelax.

Maybe we could take a page from their playbook but skip the extra
complexity, and just use "RISCV_LINK_SPEC" for the common stuff?

What about "%{mno-relax:--no-relax}"?  It's the same in linux.h and
elf.h, but missing from freebsd.h.  Intentional, or is this another
candidate for putting in the common define?


  // Marcus

Re: [PATCH 2/2] RISC-V: Add riscv{32, 64}be with big endian as default

[Kito Cheng via Gcc-patches]

> > Could you extract the endian related LINK_SPEC change to
> > ENDIAN_LINK_SPEC to riscv.h, so that we can prevent
> > duplicate this several times.
>
> You mean a define which expands to
>
>   "-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv"

Yeah, but I'd like to include following 2 lines too:

%{mbig-endian:-EB} \
%{mlittle-endian:-EL} \

I saw it's just the same among 3 files.

>
> ?  Sure, but I don't think ENDIAN_LINK_SPEC would be a good name for
> it since it defines the word size as well as the endianness, and also
> ELF in general.
>
> Maybe ELF_LINK_SPEC?  The word size and endianness are also ELF
> properties (as encoded in EI_CLASS and EI_DATA).

Either ENDIAN_LINK_SPEC or ELF_LINK_SPEC is ok to me,
I don't have strong preference on nanming.

>
>
>   // Marcus
>
>

Re: [PATCH 2/2] RISC-V: Add riscv{32, 64}be with big endian as default

[Marcus Comstedt]

Kito Cheng  writes:

>> diff --git a/gcc/config/riscv/elf.h b/gcc/config/riscv/elf.h
>> index 973efdaed7b..7e65e499031 100644
>> --- a/gcc/config/riscv/elf.h
>> +++ b/gcc/config/riscv/elf.h
>> @@ -18,7 +18,7 @@ along with GCC; see the file COPYING3.  If not see
>>  .  */
>>
>>  #define LINK_SPEC "\
>> --melf" XLEN_SPEC "lriscv \
>> +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \
>
> Could you extract the endian related LINK_SPEC change to
> ENDIAN_LINK_SPEC to riscv.h, so that we can prevent
> duplicate this several times.

You mean a define which expands to

  "-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv"

?  Sure, but I don't think ENDIAN_LINK_SPEC would be a good name for
it since it defines the word size as well as the endianness, and also
ELF in general.

Maybe ELF_LINK_SPEC?  The word size and endianness are also ELF
properties (as encoded in EI_CLASS and EI_DATA).


  // Marcus

Re: [PATCH 2/2] RISC-V: Add riscv{32, 64}be with big endian as default

[Kito Cheng via Gcc-patches]

> diff --git a/gcc/config/riscv/elf.h b/gcc/config/riscv/elf.h
> index 973efdaed7b..7e65e499031 100644
> --- a/gcc/config/riscv/elf.h
> +++ b/gcc/config/riscv/elf.h
> @@ -18,7 +18,7 @@ along with GCC; see the file COPYING3.  If not see
>  .  */
>
>  #define LINK_SPEC "\
> --melf" XLEN_SPEC "lriscv \
> +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \

Could you extract the endian related LINK_SPEC change to
ENDIAN_LINK_SPEC to riscv.h, so that we can prevent
duplicate this several times.



>  %{mno-relax:--no-relax} \
>  %{mbig-endian:-EB} \
>  %{mlittle-endian:-EL} \
> diff --git a/gcc/config/riscv/freebsd.h b/gcc/config/riscv/freebsd.h
> index f3aca9f7673..6018e7bb764 100644
> --- a/gcc/config/riscv/freebsd.h
> +++ b/gcc/config/riscv/freebsd.h
> @@ -40,7 +40,7 @@ along with GCC; see the file COPYING3.  If not see
>
>  #undef LINK_SPEC
>  #define LINK_SPEC "\
> -  -melf" XLEN_SPEC "lriscv \
> +  -melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv  \
>%{p:%nconsider using `-pg' instead of `-p' with gprof (1)}   \
>%{v:-V}  \
>%{assert*} %{R*} %{rpath*} %{defsym*}\
> diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
> index e74f5d3f914..fce5b896e6e 100644
> --- a/gcc/config/riscv/linux.h
> +++ b/gcc/config/riscv/linux.h
> @@ -58,7 +58,7 @@ along with GCC; see the file COPYING3.  If not see
>"%{mabi=ilp32:_ilp32}"
>
>  #define LINK_SPEC "\
> --melf" XLEN_SPEC "lriscv" LD_EMUL_SUFFIX " \
> +-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv" LD_EMUL_SUFFIX " \
>  %{mno-relax:--no-relax} \
>  %{mbig-endian:-EB} \
>  %{mlittle-endian:-EL} \
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index ff41795a031..06f4739db79 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -5499,6 +5499,11 @@ riscv_asan_shadow_offset (void)
>  #undef TARGET_ASAN_SHADOW_OFFSET
>  #define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
>
> +#ifdef TARGET_BIG_ENDIAN_DEFAULT
> +#undef  TARGET_DEFAULT_TARGET_FLAGS
> +#define TARGET_DEFAULT_TARGET_FLAGS (MASK_BIG_ENDIAN)
> +#endif
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>
>  #include "gt-riscv.h"
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index 0b667d2e8b9..3cc3e864a3e 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -30,6 +30,12 @@ along with GCC; see the file COPYING3.  If not see
>  /* Target CPU versions for D.  */
>  #define TARGET_D_CPU_VERSIONS riscv_d_target_versions
>
> +#ifdef TARGET_BIG_ENDIAN_DEFAULT
> +#define DEFAULT_ENDIAN_SPEC"b"
> +#else
> +#define DEFAULT_ENDIAN_SPEC"l"
> +#endif
> +
>  /* Default target_flags if no switches are specified  */
>
>  #ifndef TARGET_DEFAULT
> --
> 2.26.2
>