[Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
James Greenhalgh
james.greenhalgh@arm.com
Tue Aug 11 14:07:00 GMT 2015
On Tue, Aug 11, 2015 at 02:05:37PM +0100, Matthew Wahab wrote:
> The Aarch64 backend implements the atomic operations on memory with the same
> patterns iterated over logical and arithmetic operation. It also specifies
> constraints on an operand for the operations but the constraints used are only
> valid for the logical operations. This can lead to an ICE, with the compiler
> unable to generate a valid instruction.
>
> This patch reworks the atomic operation patterns to select the
> appropriate constraint for the operation. The logical operations take
> the constraints specified by the current lconst_atomic mode iterator
> while the arithmetic operations (plus, sub) take constraint "I".
>
> This patch also adds the test-case provided by Matthia Klose for the bugzilla
> entry https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67143, slightly modified to
> compile as plain C.
>
> Tested for arm-none-eabi with cross-compiled check-gcc and for
> arm-none-linux-gnueabihf with native bootstrap and make check.
>
> Ok for trunk?
>
> Matthew
> gcc/
> 2015-08-10 Matthew Wahab <matthew.wahab@arm.com>
>
> PR target/67143
> * config/aarch64/atomic.md (atomic_<optab><mode>): Replace
> 'lconst_atomic' with 'const_atomic'.
> (atomic_fetch_<optab><mode>): Likewise.
> (atomic_<optab>_fetch<mode>): Likewise.
> * config/aarch64/iterators.md (lconst-atomic): Move below
> 'const_atomic'.
> (const_atomic): New.
>
> gcc/testsuite/
> 2015-08-10 Matthew Wahab <matthew.wahab@arm.com>
> Matthias Klose <doko@debian.org>
>
> PR target/67143
> * gcc.target/aarch64/pr67143.c: New
> diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
> index 5d7966d..d3d6af7 100644
> --- a/gcc/config/aarch64/iterators.md
> +++ b/gcc/config/aarch64/iterators.md
> @@ -345,9 +345,6 @@
> ;; Attribute to describe constants acceptable in logical operations
> (define_mode_attr lconst [(SI "K") (DI "L")])
>
> -;; Attribute to describe constants acceptable in atomic logical operations
> -(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
> -
> ;; Map a mode to a specific constraint character.
> (define_mode_attr cmode [(QI "q") (HI "h") (SI "s") (DI "d")])
>
> @@ -843,6 +840,16 @@
> (plus "aarch64_plus_operand")
> (minus "aarch64_plus_operand")])
>
> +;; Constants acceptable for atomic operations.
> +;; This definition must appear in this file before the iterators it refers to.
> +(define_code_attr const_atomic
> + [(plus "I") (minus "I")
I'm worried this still gives us a mismatch between constraints and
predicates. The relevant predicates here are:
(define_predicate "aarch64_plus_operand"
(ior (match_operand 0 "register_operand")
(match_operand 0 "aarch64_plus_immediate")))
(define_predicate "aarch64_plus_immediate"
(and (match_code "const_int")
(ior (match_test "aarch64_uimm12_shift (INTVAL (op))")
(match_test "aarch64_uimm12_shift (-INTVAL (op))"))))
But our constraint only permits:
(define_constraint "I"
"A constant that can be used with an ADD operation."
(and (match_code "const_int")
(match_test "aarch64_uimm12_shift (ival)")))
Does this mean we are now loading constants we don't need to in to
registers? I don't think we could cause this to ICE - but are we
generating less good code than we would like?
Thanks,
James
More information about the Gcc-patches
mailing list