All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Steven J. Hill" <Steven.Hill@imgtec.com>
To: "Maciej W. Rozycki" <macro@codesourcery.com>
Cc: <linux-mips@linux-mips.org>
Subject: Re: [PATCH 5/7] MIPS: Fix microMIPS LL/SC immediate offsets
Date: Mon, 17 Nov 2014 10:38:26 -0600	[thread overview]
Message-ID: <546A2482.2040902@imgtec.com> (raw)
In-Reply-To: <alpine.DEB.1.10.1411152113210.2881@tp.orcam.me.uk>

On 11/15/2014 04:08 PM, Maciej W. Rozycki wrote:
> In the microMIPS encoding some memory access instructions have their 
> immediate offset reduced to 12 bits only.  That does not match the GCC 
> `R' constraint we use in some places to satisfy the requirement, 
> resulting in build failures like this:
> 
> {standard input}: Assembler messages:
> {standard input}:720: Error: macro used $at after ".set noat"
> {standard input}:720: Warning: macro instruction expanded into multiple instructions
> 
> Fix the problem by defining a macro, `GCC_OFF12_ASM', that expands to 
> the right constraint depending on whether microMIPS or standard MIPS 
> code is produced.  Also apply the fix to where `m' is used as in the 
> worst case this change does nothing, e.g. where the pointer was already 
> in a register such as a function argument and no further offset was 
> requested, and in the best case it avoids an extraneous sequence of up 
> to two instructions to load the high 20 bits of the address in the LL/SC 
> loop.  This reduces the risk of lock contention that is the higher the 
> more instructions there are in the critical section between LL and SC.
> 
> Strictly speaking we could just bulk-replace `R' with `ZC' as the latter 
> constraint adjusts automatically depending on the ISA selected.  
> However it was only introduced with GCC 4.9 and we keep supporing older 
> compilers for the standard MIPS configuration, hence the slightly more 
> complicated approach I chose.
> 
> The choice of a zero-argument function-like rather than an object-like 
> macro was made so that it does not look like a function call taking the 
> C expression used for the constraint as an argument.  This is so as not 
> to confuse the reader or formatting checkers like `checkpatch.pl' and 
> follows previous practice.
> 
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
> 
I am extremely pleased with this patch. We had discussed this problem
and various potential solutions a year or two ago, but I was never able
to find time to look at it again. Thanks for figuring this out.


Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>

WARNING: multiple messages have this Message-ID (diff)
From: "Steven J. Hill" <Steven.Hill@imgtec.com>
To: "Maciej W. Rozycki" <macro@codesourcery.com>
Cc: linux-mips@linux-mips.org
Subject: Re: [PATCH 5/7] MIPS: Fix microMIPS LL/SC immediate offsets
Date: Mon, 17 Nov 2014 10:38:26 -0600	[thread overview]
Message-ID: <546A2482.2040902@imgtec.com> (raw)
Message-ID: <20141117163826.EG4Di1XSetk1ngN-o19jXkwEbkRKq-UbMyYFBVLjCJ0@z> (raw)
In-Reply-To: <alpine.DEB.1.10.1411152113210.2881@tp.orcam.me.uk>

On 11/15/2014 04:08 PM, Maciej W. Rozycki wrote:
> In the microMIPS encoding some memory access instructions have their 
> immediate offset reduced to 12 bits only.  That does not match the GCC 
> `R' constraint we use in some places to satisfy the requirement, 
> resulting in build failures like this:
> 
> {standard input}: Assembler messages:
> {standard input}:720: Error: macro used $at after ".set noat"
> {standard input}:720: Warning: macro instruction expanded into multiple instructions
> 
> Fix the problem by defining a macro, `GCC_OFF12_ASM', that expands to 
> the right constraint depending on whether microMIPS or standard MIPS 
> code is produced.  Also apply the fix to where `m' is used as in the 
> worst case this change does nothing, e.g. where the pointer was already 
> in a register such as a function argument and no further offset was 
> requested, and in the best case it avoids an extraneous sequence of up 
> to two instructions to load the high 20 bits of the address in the LL/SC 
> loop.  This reduces the risk of lock contention that is the higher the 
> more instructions there are in the critical section between LL and SC.
> 
> Strictly speaking we could just bulk-replace `R' with `ZC' as the latter 
> constraint adjusts automatically depending on the ISA selected.  
> However it was only introduced with GCC 4.9 and we keep supporing older 
> compilers for the standard MIPS configuration, hence the slightly more 
> complicated approach I chose.
> 
> The choice of a zero-argument function-like rather than an object-like 
> macro was made so that it does not look like a function call taking the 
> C expression used for the constraint as an argument.  This is so as not 
> to confuse the reader or formatting checkers like `checkpatch.pl' and 
> follows previous practice.
> 
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
> 
I am extremely pleased with this patch. We had discussed this problem
and various potential solutions a year or two ago, but I was never able
to find time to look at it again. Thanks for figuring this out.


Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>

  reply	other threads:[~2014-11-17 16:38 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-15 22:06 [PATCH 0/7] MIPS: Assorted microMIPS fixes Maciej W. Rozycki
2014-11-15 22:06 ` Maciej W. Rozycki
2014-11-15 22:07 ` [PATCH 1/7] MIPS: Kconfig: Enable microMIPS support for Malta Maciej W. Rozycki
2014-11-15 22:07   ` Maciej W. Rozycki
2014-11-17 16:38   ` Steven J. Hill
2014-11-17 16:38     ` Steven J. Hill
2014-11-15 22:07 ` [PATCH 2/7] MIPS: mm: Only build one microassembler that is suitable Maciej W. Rozycki
2014-11-15 22:07   ` Maciej W. Rozycki
2014-11-15 22:08 ` [PATCH 3/7] MIPS: signal.c: Fix an invalid cast in ISA mode bit handling Maciej W. Rozycki
2014-11-15 22:08   ` Maciej W. Rozycki
2014-11-15 22:08 ` [PATCH 4/7] MIPS: Kconfig: Only allow 32-bit microMIPS builds Maciej W. Rozycki
2014-11-15 22:08   ` Maciej W. Rozycki
2014-11-15 22:08 ` [PATCH 5/7] MIPS: Fix microMIPS LL/SC immediate offsets Maciej W. Rozycki
2014-11-15 22:08   ` Maciej W. Rozycki
2014-11-17 16:38   ` Steven J. Hill [this message]
2014-11-17 16:38     ` Steven J. Hill
2014-11-15 22:09 ` [PATCH 6/7] MIPS: Apply `.insn' to fixup labels throughout Maciej W. Rozycki
2014-11-15 22:09   ` Maciej W. Rozycki
2014-11-17 16:38   ` Steven J. Hill
2014-11-17 16:38     ` Steven J. Hill
2014-11-17 16:57     ` Maciej W. Rozycki
2014-11-17 16:57       ` Maciej W. Rozycki
2014-11-15 22:09 ` [PATCH 7/7] MIPS: atomic.h: Reformat to fit in 79 columns Maciej W. Rozycki
2014-11-15 22:09   ` Maciej W. Rozycki
2014-11-17 16:37 ` [PATCH 0/7] MIPS: Assorted microMIPS fixes Steven J. Hill
2014-11-17 16:37   ` Steven J. Hill

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=546A2482.2040902@imgtec.com \
    --to=steven.hill@imgtec.com \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@codesourcery.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.