openrisc.lists.librecores.org archive mirror
 help / color / mirror / Atom feed
From: Samuel Holland <samuel@sholland.org>
To: Stafford Horne <shorne@gmail.com>
Cc: openrisc@lists.librecores.org, binutils@sourceware.org
Subject: Re: [PATCH] or1k: Add support for a little-endian target variant
Date: Thu, 9 Jun 2022 20:39:38 -0500	[thread overview]
Message-ID: <780e40a3-17eb-c1c2-6572-1fd41757e963@sholland.org> (raw)
In-Reply-To: <YqHXrxh5BRTk48o7@antec>

Hi Stafford,

Thanks for the review.

On 6/9/22 6:21 AM, Stafford Horne wrote:
> On Thu, Jun 09, 2022 at 01:01:33AM -0500, Samuel Holland wrote:
>> While not officially sanctioned by the architecture spec, little-endian
>> or1k processors do exist in the wild, for example the Allwinner AR100.
>> Let's add native support for this, instead of hacks like using objcopy
>> to byteswap ELF file contents.
> 
> Hello,
> 
> In general I have no objections to this.  If there are processors that
> are little endian it makes sense to support it.  Do you have any details
> of how people built for these before? I am curious.

We post-process the binary with "objcopy --reverse-bytes 4".

https://github.com/crust-firmware/crust/blob/master/Makefile#L193
https://github.com/intelligent-agent/klipper/blob/master/src/ar100/Makefile#L39
https://github.com/nfeske/genode-allwinner/blob/scp/src/scp/Makefile#L22

The processor has both a word-invariant mode (the default), and a byte-invariant
mode:

https://linux-sunxi.org/AR100#Byte-invariant_Ranges

With a big-endian toolchain, in the word-invariant mode, all GCC optimizations
work fine, but sharing data with other software is painful. Sub-32-bit memory
accesses are reversed within each group of 4 bytes, which affects strings and
struct layouts.

In the byte-invariant mode, sharing strings and structs causes no problems, but
some GCC optimizations like -fstore-merging are broken.

>> diff --git a/gas/config/tc-or1k.c b/gas/config/tc-or1k.c
>> index ae4e3452f48..9dc5a46f2e2 100644
>> --- a/gas/config/tc-or1k.c
>> +++ b/gas/config/tc-or1k.c
>> @@ -58,8 +58,16 @@ const char FLT_CHARS[]            = "dD";
>>  #define OR1K_SHORTOPTS "m:"
>>  const char * md_shortopts = OR1K_SHORTOPTS;
>>  
>> +enum
>> +{
>> +  OPTION_LITTLE_ENDIAN = OPTION_MD_BASE,
>> +  OPTION_BIG_ENDIAN
>> +};
>> +
>>  struct option md_longopts[] =
>>  {
>> +  {"EB", no_argument, NULL, OPTION_BIG_ENDIAN},
>> +  {"EL", no_argument, NULL, OPTION_LITTLE_ENDIAN},
>>    {NULL, no_argument, NULL, 0}
>>  };
>>  size_t md_longopts_size = sizeof (md_longopts);
>> @@ -67,14 +75,30 @@ size_t md_longopts_size = sizeof (md_longopts);
>>  unsigned long or1k_machine = 0; /* default */
>>  
>>  int
>> -md_parse_option (int c ATTRIBUTE_UNUSED, const char * arg ATTRIBUTE_UNUSED)
>> +md_parse_option (int c, const char * arg ATTRIBUTE_UNUSED)
>>  {
>> -  return 0;
>> +  switch (c)
>> +    {
>> +    case OPTION_BIG_ENDIAN:
>> +      target_big_endian = 1;
>> +      break;
>> +    case OPTION_LITTLE_ENDIAN:
>> +      target_big_endian = 0;
>> +      break;
>> +    default:
>> +      return 0;
>> +    }
>> +
>> +  return 1;
>>  }
>>  
>>  void
>> -md_show_usage (FILE * stream ATTRIBUTE_UNUSED)
>> +md_show_usage (FILE * stream)
>>  {
>> +  fprintf (stream, _(" OR1K-specific assembler options:\n"));
>> +  fprintf (stream, _("\
>> +  --EB			generate code for a big endian machine\n\
>> +  --EL			generate code for a little endian machine\n"));
>>  }
> 
> Aboce you mention -EB, -EL, here is is --EB, --EL.

They are long options. I will fix the changelog.

> Does this setup big endian as the default?  We should specify that in the
> options.  i.e. "generate code for a big endian machine, this is the default."
> 
> But I am not sure how that defaulting works now.  I will try to build this
> and understand better.

The default depends on the target. It comes from gas/configure.tgt.

Regards,
Samuel

  parent reply	other threads:[~2022-06-10  1:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-09  6:01 [PATCH] or1k: Add support for a little-endian target variant Samuel Holland
2022-06-09 11:21 ` Stafford Horne
2022-06-10  1:00   ` Alan Modra
2022-06-10  1:39   ` Samuel Holland [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-06-09  6:03 Samuel Holland
2022-06-09 11:29 ` Stafford Horne
2022-06-10  2:07   ` Samuel Holland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=780e40a3-17eb-c1c2-6572-1fd41757e963@sholland.org \
    --to=samuel@sholland.org \
    --cc=binutils@sourceware.org \
    --cc=openrisc@lists.librecores.org \
    --cc=shorne@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).