From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Matthias Kaehlcke" <mka@chromium.org>,
"Catalin Marinas" <catalin.marinas@arm.com>,
"Will Deacon" <will.deacon@arm.com>,
"Christoffer Dall" <christoffer.dall@linaro.org>,
"Marc Zyngier" <marc.zyngier@arm.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>, "Tejun Heo" <tj@kernel.org>,
"Christoph Lameter" <cl@linux.com>,
"Vladimir Murzin" <vladimir.murzin@arm.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
"KVM devel mailing list" <kvm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Grant Grundler" <grundler@chromium.org>,
"Greg Hackmann" <ghackmann@google.com>,
"Michael Davidson" <md@google.com>
Subject: Re: [PATCH v2] arm64: Add ASM modifier for xN register operands
Date: Fri, 28 Apr 2017 15:33:33 +0100 [thread overview]
Message-ID: <20170428143333.GA5292@leverpostej> (raw)
In-Reply-To: <CAKv+Gu9s2t+Nt7JCcTxkr5BYSN05f_zxYkKzfrD-4owWQLvF_Q@mail.gmail.com>
On Fri, Apr 28, 2017 at 08:18:52AM +0100, Ard Biesheuvel wrote:
> On 27 April 2017 at 23:52, Matthias Kaehlcke <mka@chromium.org> wrote:
> > El Thu, Apr 27, 2017 at 12:02:56PM +0100 Mark Rutland ha dit:
> >> On Wed, Apr 26, 2017 at 02:46:16PM -0700, Matthias Kaehlcke wrote:
> >> > Many inline assembly statements don't include the 'x' modifier when
> >> > using xN registers as operands. This is perfectly valid, however it
> >> > causes clang to raise warnings like this:
> >> >
> >> > warning: value size does not match register size specified by the
> >> > constraint and modifier [-Wasm-operand-widths]
[...]
> >> > - asm volatile("strb %w0, [%1]" : : "rZ" (val), "r" (addr));
> >> > + asm volatile("strb %w0, [%x1]" : : "rZ" (val), "r" (addr));
> >>
> >> In general, the '[%xN]' pattern looks *very* suspicious to me. Any
> >> address must be 64-bit, so this would mask a legitimate warning.
> >>
> >> Given the prototype of this function the code if fine either way, but
> >> were we to refactor things (e.g. making this a macro), that might not be
> >> true.
> >>
> >> ... so I'm not sure it make sense to alter instances used for addresses.
> >
> > Good point, I'll leave instances dealing with addresses untouched for now.
> >
>
> OK, I am confused now. We started this thread under the assumption
> that all unqualified placeholders are warned about by Clang. Given
> that this appears not to be the case, could we please first find out
> what causes the warnings? Is it necessary at all to add the x
> modifiers for 64-bit types?
FWIW, I grabbed a clang 4.0.0 binary and had a play.
It looks like clang only warns when an operand is less than 64 bits
wide, and there is no 'x' or 'w' modifier. Pointers a 64 bits wide, so
never produce a warning.
As far as I can tell, applying to both integers and pointers:
* GCC and clang always treat %N as meaning xN for an r constraint, and
you need to use %wN to get wN.
* If an operand type is 64 bits in size, clang will not produce a warning
regarding the operand size.
* If an x or w modifier is used, clang will not produce a warning
regarding the operand size, regardless of whether it matches the
register size. Clang is happy for %wN to be used on a pointer type.
* If an operand type is less than 64 bits in size, and neither an x or
w modifier is used, clang will produce a warning as above.
* If an operand type is greater than 64 bits in size, clang encounters
an internal error.
Given that, I think we *should not* use the x modifier to suppress this
warning, as I think for those cases we have a potential bug as outlined
in my prior reply.
Instead, we should use a temporary 64-bit variable (or cast input
operands to 64-bit), which avoids that and makes clang happy.
I've included my test below. Note that clang will produce other errors for
invalid asm (e.g. for mov w0, x0).
Thanks,
Mark.
---->8----
#define TEST(t, w1, w2) \
t foo_##t##w1##_##w2(t a, t b) \
{ \
asm ( \
"mov %" #w1 "0, %" #w2 "1" \
: "=r" (a) : "r" (b) \
); \
\
return a; \
}
#define TEST_TYPE(t) \
TEST(t, , ) \
TEST(t, w, ) \
TEST(t, w, w) \
TEST(t, w, x) \
TEST(t, x, ) \
TEST(t, x, w) \
TEST(t, x, x) \
TEST_TYPE(int)
TEST_TYPE(long)
typedef long * longp;
TEST_TYPE(longp)
TEST_TYPE(__int128)
next prev parent reply other threads:[~2017-04-28 14:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-26 21:46 [PATCH v2] arm64: Add ASM modifier for xN register operands Matthias Kaehlcke
2017-04-27 11:02 ` Mark Rutland
2017-04-27 22:52 ` Matthias Kaehlcke
2017-04-28 7:18 ` Ard Biesheuvel
2017-04-28 9:53 ` Mark Rutland
2017-04-28 10:20 ` Ard Biesheuvel
2017-04-28 10:32 ` Mark Rutland
2017-04-28 14:33 ` Mark Rutland [this message]
2017-04-28 14:43 ` Ard Biesheuvel
2017-04-28 14:47 ` Will Deacon
2017-04-28 19:08 ` Matthias Kaehlcke
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=20170428143333.GA5292@leverpostej \
--to=mark.rutland@arm.com \
--cc=ard.biesheuvel@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@linaro.org \
--cc=cl@linux.com \
--cc=ghackmann@google.com \
--cc=grundler@chromium.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=md@google.com \
--cc=mka@chromium.org \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=tj@kernel.org \
--cc=vladimir.murzin@arm.com \
--cc=will.deacon@arm.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 NNTP newsgroup(s).