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 >