[Bug middle-end/44492] New: auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[Bug middle-end/44492] New: auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

struct T { const char *p; };
struct S { T a, b, c; unsigned d; };
void bar (const T &, const T &);

void
foo (S &s, T e)
{
  const char *a = e.p;
  const char *b = s.b.p;
  __asm__ volatile ("/* %0 %1 */" : : "rm" (a), "rm" (b));
  bar (e, s.b);
}

has PRE_MODIFY and PRE_INC in MEM operands of ASM_OPERANDS.  I can't see how
that can be right - there is no guarantee that the operand is used in the asm
at all, and even when it is used, it would have to be used in a load or store
with update insn.


-- 
           Summary: auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm
                    operands
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
GCC target triplet: powerpc64-linux


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2010-06-10 09:33 -------
I had saw this before and it was declared this was correct behavior but I
cannot find the bug any more.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #2 from jakub at gcc dot gnu dot org  2010-06-10 09:44 -------
struct T { unsigned long p; };
struct S { T a, b, c; unsigned d; };

__attribute__((noinline))
void
bar (const T &x, const T &y)
{
  if (x.p != 0x2348 || y.p != 0x2346)
    __builtin_abort ();
}

__attribute__((noinline))
void
foo (S &s, T e)
{
  unsigned long a = e.p;
  unsigned long b = s.b.p;
  __asm__ volatile ("" : : "rm" (a), "rm" (b));
  bar (e, s.b);
}

int
main ()
{
  S s = { { 0x2345 }, { 0x2346 }, { 0x2347 }, 6 };
  T t = { 0x2348 };
  foo (s, t);
}

Updated testcase, this one I believe (don't have a ppc64 around to runtest it)
should crash at runtime.
If PRE_INC/PRE_MODIFY is allowed in asm operands with "m" constraint, then "m"
couldn't be actually ever safely used in inline asm, at least on ppc64.
(mem:DI (plus:DI (reg:DI 4) (const_int 8))) is printed as 8(4) identically to
what (mem:DI (pre_inc:DI (reg:DI 4))) prints - 8(4).  How could the asm find
out which one it is?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2010-06-10 09:47 -------
I think the real issue is that m is too generic from the point of the
documentation.

Reading:
http://gcc.gnu.org/onlinedocs/gcc-4.5.0/gcc/Simple-Constraints.html#Simple-Constraints

Makes it sound like it could accept an auto increment/decrement pattern.  While
using something like "o" would not.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #4 from schwab at linux-m68k dot org  2010-06-10 10:12 -------
You need to use the %U and %X modifiers together with "m".  If the insn does
not support them (no update and/or indexed form) you cannot use "m".  Basically
this means you can use "m" only with the normal load/store insns.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2010-06-10 10:25 -------
And ensure that it is used exactly once in an inline asm pattern?  Even for "g"
constraint?  That just can't be true.
Just look at how many of "m" and "=m" constraints e.g. glibc uses.
Only 8 occurrences of %U, but e.g.:
./iconvdata/iso-2022-kr.c:      asm ("" : "=m" (buf));                         
                    \
./iconvdata/iso-2022-cn.c:      asm ("" : "=m" (buf));                         
                      \
./nscd/nscd_helper.c:  asm ("" : "=m" (tvend));
./nptl/sysdeps/unix/sysv/linux/powerpc/pthread_once.c:                  : "=&r"
(oldval), "=&r" (tmp), "=m" (*once_control)
./nptl/sysdeps/unix/sysv/linux/powerpc/pthread_once.c:                  : "r"
(once_control), "r" (newval), "m" (*once_control)
./nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h:                 : "=&r"
(__val), "=m" (*futex)                         \
./nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h:                 : "r"
(futex), "r" (id), "m" (*futex)                  \
./string/test-memcmp.c:   asm ("" : "=g" (r) : "0" (r));
./string/test-strncmp.c:          asm ("" : "=g" (r) : "0" (r));
./string/test-strcmp.c:   asm ("" : "=g" (r) : "0" (r));
./debug/tst-longjmp_chk.c:  asm volatile ("" : "=m" (buf));
./math/math_private.h:#define math_force_eval(x) __asm __volatile ("" : : "m"
(x))
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "=&r"
(__val), "=m" (*mem)                          \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "b" (mem),
"r" (value), "m" (*mem)                  \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "=&r"
(__val), "=m" (*mem)                          \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "b" (mem),
"r" (value), "m" (*mem)                  \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "=&b"
(__val), "=&r" (__tmp), "=m" (*mem)           \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "b" (mem),
"r" (value), "m" (*mem)                  \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "=&b"
(__val), "=m" (*mem)                          \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "b" (mem),
"m" (*mem)                               \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "=&b"
(__val), "=m" (*mem)                          \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                      : "b" (mem),
"m" (*mem)                               \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                     : "=&b" (__val),
"=&r" (__tmp), "=m" (*mem)            \
./sysdeps/powerpc/powerpc64/bits/atomic.h:                     : "b" (mem), "m"
(*mem)                                \
./sysdeps/powerpc/bits/atomic.h:                      : "=&r" (__val), "=m"
(*mem)                            \
./sysdeps/powerpc/bits/atomic.h:                      : "b" (mem), "r" (value),
"m" (*mem)                    \
./sysdeps/powerpc/bits/atomic.h:                      : "=&r" (__val), "=m"
(*mem)                            \
./sysdeps/powerpc/bits/atomic.h:                      : "b" (mem), "r" (value),
"m" (*mem)                    \
./sysdeps/powerpc/bits/atomic.h:                      : "=&b" (__val), "=&r"
(__tmp), "=m" (*mem)             \
./sysdeps/powerpc/bits/atomic.h:                      : "b" (mem), "r" (value),
"m" (*mem)                    \
./sysdeps/powerpc/bits/atomic.h:                      : "=&b" (__val), "=m"
(*mem)                            \
./sysdeps/powerpc/bits/atomic.h:                      : "b" (mem), "m" (*mem)  
                              \
./sysdeps/powerpc/bits/atomic.h:                      : "=&b" (__val), "=m"
(*mem)                            \
./sysdeps/powerpc/bits/atomic.h:                      : "b" (mem), "m" (*mem)  
                              \
./sysdeps/powerpc/bits/atomic.h:                       : "=&b" (__val), "=&r"
(__tmp), "=m" (*mem)            \
./sysdeps/powerpc/bits/atomic.h:                       : "b" (mem), "m" (*mem) 
                              \

Guess the Linux kernel has many more of these.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #6 from schwab at linux-m68k dot org  2010-06-10 10:59 -------
You can use them as many times as needed.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #7 from schwab at linux-m68k dot org  2010-06-10 11:06 -------
A matched constraint can never have side effects, and the powerpc specific uses
are already correct (though they could take advantage of the "es" or "Z"
constraints).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #8 from jakub at gcc dot gnu dot org  2010-06-10 11:36 -------
Don't understand the last comment.
If matched constraint can't have side-effects, then this bug is valid and
auto-inc-dec.c should just do if (asm_noperands (insn) >= 0) continue;
similarly how it refuses to auto-inc into JUMP_INSNs.
If this bug is not valid, then even the powerpc specific uses can't be correct,
they don't mention many of the "m" or "=m" operands at all.
If it would be ok to use "m" constraint only together with matching %U on ppc,
use it at least once, but it would be fine to use it more than once, then
"lwz%U2%X2 %0,%2
lwz%U2%X2 %1,%2" would increment the reg in "m" operand 2 twice if it contains
pre_inc/pre_modify in it.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #9 from schwab at linux-m68k dot org  2010-06-10 11:50 -------
You cannot use an "m" operand more than once, since it can include side
effects.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[fche at redhat dot com]

------- Comment #10 from fche at redhat dot com  2010-06-10 12:11 -------
(In reply to comment #9)
> You cannot use an "m" operand more than once, since it can include side
> effects.

Nor less than once, apparently.


-- 

fche at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fche at redhat dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #11 from jakub at gcc dot gnu dot org  2010-06-10 12:24 -------
I believe for GCC it shouldn't be hard to at least easily detect the used zero
times case which happens very often in lost of code.
asm ("... %2 ... " : "=m" (*p) : "m" (*p), "r" (p));
is just very common, the "=m" and "m" just tells gcc that the memory is used
and might change.
It would be even possible using a target hook to check whether it is used
exactly once, furthermore in target specific proper way (say, on powerpc64 the
check would
be that there is exactly one use of the operand with U modifier, at most one
with X modifier and exactly one another use, on ia64 it could be that there is
exactly one use with P modifier and exactly one use another use, on the targets
which print incdec in *print_*address it could be by default check for exactly
one use of the operand.
This is not going to be perfect, it would count uses in comments and also
doesn't cover e.g. a single use in not always executed code (say the asm has
conditional jump around a few insns and uses the mem in there), but could fix
up significant amount of code in the wild.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[matz at gcc dot gnu dot org]

------- Comment #12 from matz at gcc dot gnu dot org  2010-06-10 12:26 -------
I don't think it ever was intended that 'm' includes operands with embedded
side-effects.  I don't think so because making this work reliably is
relatively difficult.  In particular the tests that Jakub mentions would need
implementation (and probably other changes too), and the point is that such
things never were implemented.  Hence with enough work it's probably possible
to construct testcases also for much older versions of GCC that fail in
similar ways.

If that means to slightly change the definition of 'm' compared to what is
documented currently, well, so be it.  The other definition is unreliable
anyway, so any inline asm that uses 'm' and expects a side-effect is fishy
at best.

It is fishy because if the compiler forwards a side-effect into the operands
it would have to rewrite the inline asm string to actually contain an
instruction to calculate this side-effect, which obviously is bollocks.

So, yes, auto-inc-dec should of course _not_ push side-effects into inline
asm.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #13 from schwab at linux-m68k dot org  2010-06-10 12:39 -------
"m" is defined to be the most general memory constraint, and a pre/post
modified memory operand is still a memory operand.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #14 from schwab at linux-m68k dot org  2010-06-10 12:42 -------
> asm ("... %2 ... " : "=m" (*p) : "m" (*p), "r" (p));

In this case the compiler should never use a side effect.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #15 from schwab at linux-m68k dot org  2010-06-10 12:46 -------
The %X modifier has nothing to do with side effects, it is used for indexed
addressing modes.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[fche at redhat dot com]

------- Comment #16 from fche at redhat dot com  2010-06-10 13:24 -------
(In reply to comment #13)
> "m" is defined to be the most general memory constraint, and a pre/post
> modified memory operand is still a memory operand.

If this is to stand, please amend the documentation to warn about this.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[matz at gcc dot gnu dot org]

------- Comment #17 from matz at gcc dot gnu dot org  2010-06-10 13:34 -------
> "m" is defined to be the most general memory constraint, and a pre/post
> modified memory operand is still a memory operand.

I know that this is the case, which is why I said: "If that means to slightly
change the definition of 'm' compared to what is documented currently, well,
so be it."

I.e. I'm arguing that the documentation should be amended to state something
that can actually be implemented in a reliable way.  I.e. include "without
side-effects" at the appropriate place.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #18 from schwab at linux-m68k dot org  2010-06-10 15:09 -------
One option is to add a sequence point before every asm statement.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[schwab at linux-m68k dot org]

------- Comment #19 from schwab at linux-m68k dot org  2010-06-10 15:19 -------
Of course, there _is_ already a sequence point before every statement.  Doh.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #20 from jakub at gcc dot gnu dot org  2010-06-10 15:22 -------
Another option is to disallow side-effects in inline asm memory operands and
let the user explicitly request that the side-effects are possible, through
some new constraint modifier (e.g. _ used as "_mi" or "=_g" would tell
auto-inc-dec it can put side-effects into it).  When _ is present, the user
would promise that the mem operand is used exactly once in an insn that is
executed exactly once in that inline asm and that any target dependent
modifiers
that are needed for auto-update insns are used too (%P on ia64, %U on ppc,
etc.).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #21 from jakub at gcc dot gnu dot org  2010-06-10 17:06 -------
Created an attachment (id=20883)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20883&action=view)
gcc46-pr44494.patch

WIP patch that implements that.  Still missing documentation, testcases and
likely also doing similar test in constrain_operands, for asm_noperands > 0
there only (just not sure how to find
out cheaply from constrain_operands that we are in inline asm).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #22 from jakub at gcc dot gnu dot org  2010-06-11 13:22 -------
Created an attachment (id=20888)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20888&action=view)
gcc46-pr44492.patch

Updated patch.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20883|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #23 from jakub at gcc dot gnu dot org  2010-06-24 17:48 -------
Subject: Bug 44492

Author: jakub
Date: Thu Jun 24 17:48:16 2010
New Revision: 161328

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161328
Log:
        PR middle-end/44492
        * recog.h (struct recog_data): Add is_asm field.
        * recog.c (asm_operand_ok, constrain_operands): If neither < nor > is
        present in constraints of inline-asm operand and memory operand
        contains {PRE,POST}_{INC,DEC,MODIFY}, return 0.
        (extract_insn): Initialize recog_data.is_asm.
        * doc/md.texi (Constraints): Document operand side-effect rules.

        * g++.dg/torture/pr44492.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/torture/pr44492.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/doc/md.texi
    trunk/gcc/recog.c
    trunk/gcc/recog.h
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492

[Bug middle-end/44492] auto-inc-dec pushes PRE_MODIFY/PRE_INC into inline asm operands

[jakub at gcc dot gnu dot org]

------- Comment #24 from jakub at gcc dot gnu dot org  2010-08-13 12:47 -------
Fixed.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44492