On Fri, Sep 30, 2011 at 8:17 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> GCC on the following testcase warns
> warning: use of memory input without lvalue in asm operand 0 is deprecated
> [enabled by default]
> starting with 4.6, but the source actually had an lvalue there (I don't
> think we should forbid for input operands const qualified memory).
> On "m" (1) in the source we would error out early, but here we
> fold that "m" (*(int *) var) during gimple folding into
> "m" (1). The following patch makes sure that we don't fold an lvalue
> into non-lvalue for asm operands that allow memory, but not reg.
>
> This has been originally reported on Linux kernel bitops, which IMHO
> are invalid too, they say to the compiler that "m" (*(unsigned long *)array)
> is being read, but in fact the inline asm relies on the whole array to
> be after that too. IMHO one should tell the compiler about that,
> through "m" (*(struct { unsigned long __l[0x10000000]; } *)array)
> or similar.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?
Hmm, I don't think this change is ok. We rely on maybe_fold_reference
to re-fold mem-refs to valid gimple form (from propagating say
&a.b.c to MEM[p, 4] which first gives the invalid MEM[&a.b.c, 4] and
then the folding changes this to MEM[&a, 12]). You need to preserve that
and only disable constant folding.
Thus I suggest to add a parameter to maybe_fold_reference that says
whether to try constant folding (we already have is_lhs, so why not
use that?)
Thanks,
Richard.
> 2011-09-30 Jakub Jelinek <ja...@redhat.com>
>
> PR inline-asm/50571
> * gimple-fold.c (fold_stmt_1) <case GIMPLE_ASM>: If
> constraints allow mem and not reg, don't optimize lvalues
> into non-lvalues.
>
> * gcc.dg/pr50571.c: New test.
>
> --- gcc/gimple-fold.c.jj 2011-09-29 14:25:46.000000000 +0200
> +++ gcc/gimple-fold.c 2011-09-29 21:25:00.000000000 +0200
> @@ -1201,28 +1201,55 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>
> case GIMPLE_ASM:
> /* Fold *& in asm operands. */
> - for (i = 0; i < gimple_asm_noutputs (stmt); ++i)
> - {
> - tree link = gimple_asm_output_op (stmt, i);
> - tree op = TREE_VALUE (link);
> - if (REFERENCE_CLASS_P (op)
> - && (op = maybe_fold_reference (op, true)) != NULL_TREE)
> - {
> - TREE_VALUE (link) = op;
> - changed = true;
> - }
> - }
> - for (i = 0; i < gimple_asm_ninputs (stmt); ++i)
> - {
> - tree link = gimple_asm_input_op (stmt, i);
> - tree op = TREE_VALUE (link);
> - if (REFERENCE_CLASS_P (op)
> - && (op = maybe_fold_reference (op, false)) != NULL_TREE)
> - {
> - TREE_VALUE (link) = op;
> - changed = true;
> - }
> - }
> + {
> + size_t noutputs;
> + const char **oconstraints;
> + const char *constraint;
> + bool allows_mem, allows_reg, is_inout;
> +
> + noutputs = gimple_asm_noutputs (stmt);
> + oconstraints = XALLOCAVEC (const char *, noutputs);
> +
> + for (i = 0; i < gimple_asm_noutputs (stmt); ++i)
> + {
> + tree link = gimple_asm_output_op (stmt, i);
> + tree op = TREE_VALUE (link);
> + constraint
> + = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> + oconstraints[i] = constraint;
> + parse_output_constraint (&constraint, i, 0, 0, &allows_mem,
> + &allows_reg, &is_inout);
> + if (REFERENCE_CLASS_P (op)
> + && (op = maybe_fold_reference (op, true)) != NULL_TREE
> + && (allows_reg
> + || !allows_mem
> + || is_gimple_lvalue (op)
> + || !is_gimple_lvalue (TREE_VALUE (link))))
> + {
> + TREE_VALUE (link) = op;
> + changed = true;
> + }
> + }
> + for (i = 0; i < gimple_asm_ninputs (stmt); ++i)
> + {
> + tree link = gimple_asm_input_op (stmt, i);
> + tree op = TREE_VALUE (link);
> + constraint
> + = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> + parse_input_constraint (&constraint, 0, 0, noutputs, 0,
> + oconstraints, &allows_mem, &allows_reg);
> + if (REFERENCE_CLASS_P (op)
> + && (op = maybe_fold_reference (op, false)) != NULL_TREE
> + && (allows_reg
> + || !allows_mem
> + || is_gimple_lvalue (op)
> + || !is_gimple_lvalue (TREE_VALUE (link))))
> + {
> + TREE_VALUE (link) = op;
> + changed = true;
> + }
> + }
> + }
> break;
>
> case GIMPLE_DEBUG:
> --- gcc/testsuite/gcc.dg/pr50571.c.jj 2011-09-29 21:28:05.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr50571.c 2011-09-29 21:30:08.000000000 +0200
> @@ -0,0 +1,11 @@
> +/* PR inline-asm/50571 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +static const int var[4] = { 1, 2, 3, 4 };
> +
> +void
> +foo (void)
> +{
> + __asm volatile ("" : : "m" (*(int *) var));
> +}
>
> Jakub
>
