public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>
To: Keith Packard <keithp@keithp.com>, newlib@sourceware.org
Subject: Re: [PATCH] arm/strlen-thumb2-Os.S: Correct assembly syntax for ldrb instruction
Date: Fri, 15 May 2020 16:44:49 +0100	[thread overview]
Message-ID: <a1ed7682-b8e8-4ef3-347f-1594ee84a67f@foss.arm.com> (raw)
In-Reply-To: <87mu69usxj.fsf@keithp.com>

On 15/05/2020 16:19, Keith Packard wrote:
> Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes:
> 
>> IIRC the .w was deliberate to keep the alignment right for some
>> subsequent instructions.
> 
> I'm not sure what you mean by 'alignment' here -- would the assembler
> actually insert no-ops to ensure that the instruction were aligned
> somehow? I can't find any mention of this additional meaning of the '.w'
> qualifier and find that unlikely as it would increase code size?
> 
> It sounds like gas and llvm have a different interpretation of the '.w'
> qualifier for this instruction. The ARMv7-M Architecture Reference
> Manual (ARM DDI 0403E.d (ID070218), says the '.W' means:
> 
> .W      Meaning wide, specifies that the assembler must select a 32-bit
>         encoding for the instruction. If this is not possible, an
>         assembler error is produced.
> 
> LDRB offers three encodings, called T1, T2 and T3 in the specs. T1 and
> T2 have equivalent flexibility: loading indirect from a register with an
> optional offset (with T1 being 16 bit and T2 being 32 bit). Selecting
> the 32-bit T2 form extends the offset from 5 to 12 bits, so I imagine
> that an environment that couldn't replace instructions at link time
> might use the T2 form to make room for larger relocation values in case
> the offset weren't known at assembly time?
> 
> However, only the 32-bit T3 encoding offers the post-indexed mode
> required by the code here, and for that, there is no need to clarify
> which to select using the .N or .W qualifiers. And, when reading the
> description of the three encodings, only T2 includes the '.W' qualifier
> in its syntax, although T1 doesn't include '.N', which kinda indicates
> that the qualifiers should always be accepted, even if they aren't
> necessary.
> 
> Hrm. For this instruction, perhaps the intent in the specification is to
> use the .W to force a T2 encoding *over a T3 encoding*. The T3 encoding
> also supports register-indexed mode, but with only an 8-bit offset, so
> using .W might indicate that even if the offset *could* fit in the 8-bit
> T3 form, that the assembler should instead select the T2 encoding. I
> dunno.
> 
>> LLVM's assembler needs fixing if it doesn't accept '.w'.
> 
> Yes, it seems like that would be a good idea; having the '.w' in this
> case is harmless as only the T3 encoding can possibly work. There is
> already a discussion about this in the llvm world; I don't know if or
> when that will result in a fix being applied.
> 
> I dug through the gas source and couldn't find any place where the '.w'
> qualifier would affect the output in this case though, so removing it
> should be harmless for gas.
> 
> I'm just responding to a bug report from a user trying to use
> clang to build the library, and for that, fixing the source code to work
> with both gas and clang in ways that don't appear to affect the output
> of gas at all seemed like a reasonable option to me.
> 

The assembler syntax for ldrb in the the copy of the Arm ARM that I have
to hand has:

LDRB<c><q>  <Rt>, [<Rn> {, #+/-<imm>}]  Offset: index==TRUE, wback==FALSE
LDRB<c><q>  <Rt>, [<Rn>, #+/-<imm>]!  Pre-indexed: index==TRUE, wback==TRUE
LDRB<c><q>  <Rt>, [<Rn>], #+/-<imm>  Post-indexed: index==FALSE, wback==TRUE

And <q> is the .N .W qualifier.

So clearly .W should be acceptable, even if it is the only permitted
from for this case.

R.

      reply	other threads:[~2020-05-15 15:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-12 17:58 Keith Packard
2020-05-15 13:25 ` Richard Earnshaw
2020-05-15 14:38   ` Emmanuel Blot
2020-05-15 15:19   ` Keith Packard
2020-05-15 15:44     ` Richard Earnshaw [this message]

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=a1ed7682-b8e8-4ef3-347f-1594ee84a67f@foss.arm.com \
    --to=richard.earnshaw@foss.arm.com \
    --cc=keithp@keithp.com \
    --cc=newlib@sourceware.org \
    /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).