[Bug target/65689] New: [AArch64] S constraint fails for inline asm at -O0

[Bug target/65689] New: [AArch64] S constraint fails for inline asm at -O0

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

            Bug ID: 65689
           Summary: [AArch64] S constraint fails for inline asm at -O0
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: alalaw01 at gcc dot gnu.org

Starting with r221532
(https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01064.html),

void
test (void)
{
    __asm__ ("@ %c0" : : "S" (&test + 4));
}

fails to compile at -O0 on all aarch64 targets with:

c-output-template-3.c: In function 'test':
c-output-template-3.c:7:5: error: impossible constraint in 'asm'
     __asm__ ("@ %c0" : : "S" (&test + 4));

(This is gcc.target/aarch64/c-output-template-3.c, without the -O added in
r221905, as that leads to successful compilation - however, the testcase should
compile without -O too.)

[Bug target/65689] [AArch64] S constraint fails for inline asm at -O0

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #1 from alalaw01 at gcc dot gnu.org ---
Problem stems from parse_input_constraint (in stmt.c):

    if (reg_class_for_constraint (cn) != NO_REGS
        || insn_extra_address_constraint (cn))
      *allows_reg = true;
    else if (insn_extra_memory_constraint (cn))
      *allows_mem = true;
    else
      {
        /* Otherwise we can't assume anything about the nature of
           the constraint except that it isn't purely registers.
           Treat it like "g" and hope for the best.  */
        *allows_reg = true;
        *allows_mem = true;
      }

which causes expand_asm_operands to use (reg/f:DI ...), which fails the
definition of the S constraint. If instead parse_input_constraint set both
allows_reg and allows_mem to false (as it does for e.g. an "i" constraint, via
a special-case), expand_asm_operands would follow the register to its
definition:

(const:DI (plus:DI (symbol_ref:DI ("test") [flags 0x3] <function_decl
0x7fb7c60300 test>)
        (const_int 4 [0x4])))

(as also happens with -O), which satisfies the S constraint.

One solution could be to generalize the special case in parse_input_constraint.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[jgreenhalgh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

James Greenhalgh <jgreenhalgh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |aarch64*-*-*
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-07
                 CC|                            |jgreenhalgh at gcc dot gnu.org
      Known to work|                            |4.9.3
   Target Milestone|---                         |6.0
            Summary|[AArch64] S constraint      |[5 Regression][AArch64] S
                   |fails for inline asm at -O0 |constraint fails for inline
                   |                            |asm at -O0
     Ever confirmed|0                           |1
      Known to fail|                            |5.0

--- Comment #2 from James Greenhalgh <jgreenhalgh at gcc dot gnu.org> ---
Filling in the blank fields, and confirmed.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |rejects-valid
           Priority|P3                          |P1
                 CC|                            |rguenth at gcc dot gnu.org
   Target Milestone|6.0                         |5.0

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

alalaw01 at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P2

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I don't see why this should compile with -O0 actually, it assumes optimization
being performed at -O0.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 35258
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35258&action=edit
gcc5-pr65689.patch

Untested fix.  For aarch64, there are lots of constraints determined by this
patch to not allow either reg or mem:
S, Y, Ush, Uss, Usn, Usd, Usf, Ui3, Up3, Ufc, Dn, Dh, Dq, Dl, Dr, Dz, Dd
and then
V, <, >
determined as only *allows_mem.  But '<' and '>' are already handled in
process_*_constraint earlier, so it is just V.  The rest are determined
(conservatively) to allow either.
On x86_64, G is determined to allow neither, V, <, > similarly to aarch64 to
only *allows_mem, the rest maybe both.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #6 from alalaw01 at gcc dot gnu.org ---
Whilst I think this probably would fix the problem - surely this will change
the meaning of loads of constraints, on loads of platforms? I will of course
defer to the release manager(s) (!), but IMHO this feels rather risky to do at
this late stage, i.e. potentially "the cure is worse than the disease"...?

Secondly, do I understand correctly, that the constraint-parsing mechanism will
only come into play for plain ol' define_constraints, whereeas
define_register_constraint / define_memory_constraint would provide/override
with their own values? Does this still leave us with consistent meaning for all
three kinds of define...constraint?

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to alalaw01 from comment #6)
> Whilst I think this probably would fix the problem - surely this will change
> the meaning of loads of constraints, on loads of platforms? I will of course
> defer to the release manager(s) (!), but IMHO this feels rather risky to do
> at this late stage, i.e. potentially "the cure is worse than the disease"...?

The patch doesn't change meaning of any constraints, just attempts to provide
more precise answers on what constraints allow registers and what allow memory.

> Secondly, do I understand correctly, that the constraint-parsing mechanism
> will only come into play for plain ol' define_constraints, whereeas
> define_register_constraint / define_memory_constraint would provide/override
> with their own values? Does this still leave us with consistent meaning for
> all three kinds of define...constraint?

The new generated function is only used for define_constraint, sure,
define_register_constraint registered constraints are always handled as
*allows_reg = true;, define_memory_constraint are always handled as *allows_mem
= true;, define_address_constraint is handled also like *allows_reg = true;,
all 3 already before and still after the patch unchanged.  Similarly most of
the hardcoded well known architecture independent constraints.  The patch is
only about the case where it previously just made a very conservative last bet,
with the patch it still handles a couple of obvious constraints right and falls
back just for the rest where even genpreds.c doesn't safely know.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[alalaw01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #8 from alalaw01 at gcc dot gnu.org ---
Well, meaning/behaviour. But thanks for the patch - I've bootstrapped and
check-gcc'd on AArch64 and arm hf (Cortex-A15 + Neon) with no regressions.

[Bug target/65689] [5/6 Regression][AArch64] S constraint fails for inline asm at -O0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Fri Apr 17 16:43:28 2015
New Revision: 222186

URL: https://gcc.gnu.org/viewcvs?rev=222186&root=gcc&view=rev
Log:
    PR target/65689
    * genpreds.c (struct constraint_data): Add maybe_allows_reg and
    maybe_allows_mem bitfields.
    (maybe_allows_none_start, maybe_allows_none_end,
    maybe_allows_reg_start, maybe_allows_reg_end, maybe_allows_mem_start,
    maybe_allows_mem_end): New variables.
    (compute_maybe_allows): New function.
    (add_constraint): Use it to initialize maybe_allows_reg and
    maybe_allows_mem fields.
    (choose_enum_order): Sort the non-is_register/is_const_int/is_memory/
    is_address constraints such that those that allow neither mem nor
    reg come first, then those that only allow reg but not mem, then
    those that only allow mem but not reg, then the rest.
    (write_allows_reg_mem_function): New function.
    (write_tm_preds_h): Call it.
    * stmt.c (parse_output_constraint, parse_input_constraint): Use
    the generated insn_extra_constraint_allows_reg_mem function
    instead of always setting *allows_reg = true; *allows_mem = true;
    for unknown extra constraints.

    * gcc.target/aarch64/c-output-template-4.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/aarch64/c-output-template-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/genpreds.c
    trunk/gcc/stmt.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[5/6 Regression][AArch64] S |[5 Regression][AArch64] S
                   |constraint fails for inline |constraint fails for inline
                   |asm at -O0                  |asm at -O0

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed on the trunk now, queued for 5.2 if it is fine on the trunk.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.

[Bug target/65689] [5 Regression][AArch64] S constraint fails for inline asm at -O0

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65689

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.2                         |5.3

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.