Hi Jakub,
> On Mon, Jul 16, 2018 at 09:24:10AM +0200, Jakub Jelinek wrote:
>> On Sun, Jul 15, 2018 at 11:21:56PM +0200, Tom de Vries wrote:
>> > 2018-07-15 Tom de Vries <tdevr...@suse.de>
>> >
>> > * var-tracking.c (vt_initialize): Fix pre_dec handling. Print adjusted
>> > insn slim if dump_flags request TDF_SLIM.
>> >
>> > * gcc.target/i386/vartrack-1.c: New test.
>> >
>> > ---
>> > --- a/gcc/var-tracking.c
>> > +++ b/gcc/var-tracking.c
>> > @@ -115,6 +115,7 @@
>> > #include "tree-pretty-print.h"
>> > #include "rtl-iter.h"
>> > #include "fibonacci_heap.h"
>> > +#include "print-rtl.h"
>> >
>> > typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>> > typedef fibonacci_node <long, basic_block_def> bb_heap_node_t;
>> > @@ -10208,12 +10209,17 @@ vt_initialize (void)
>> > log_op_type (PATTERN (insn), bb, insn,
>> > MO_ADJUST, dump_file);
>> > VTI (bb)->mos.safe_push (mo);
>> > - VTI (bb)->out.stack_adjust += pre;
>> > }
>> > }
>> >
>> > cselib_hook_called = false;
>> > adjust_insn (bb, insn);
>> > +
>> > + if (!frame_pointer_needed)
>> > + {
>> > + if (pre)
>> > + VTI (bb)->out.stack_adjust += pre;
>> > + }
>>
>> That is certainly unexpected. For the pre side-effects, they should be
>> applied before adjusting the insn, not after that.
>> I'll want to see this under the debugger.
>
> You're right, adjust_mems takes the PRE_INC/PRE_DEC/PRE_MODIFY into account
> too, so by adjusting stack_adjust before adjust_insn the effects happen
> twice:
> case PRE_INC:
> case PRE_DEC:
> size = GET_MODE_SIZE (amd->mem_mode);
> addr = plus_constant (GET_MODE (loc), XEXP (loc, 0),
> GET_CODE (loc) == PRE_INC ? size : -size);
> Unless we have instructions where we e.g. pre_dec sp on the lhs and
> use some mem based on sp on the rhs, but I'd hope that should be invalid
> RTL, because it is ambiguous what value would sp on the rhs have.
>
> Please change the above patch to do:
> adjust_insn (bb, insn);
> +
> + if (!frame_pointer_needed && pre)
> + VTI (bb)->out.stack_adjust += pre;
>
> Ok for trunk with that change.
it turns out the test FAILs on i386-pc-soalris2.11 with -m64: the dump
doesn't even contain argp: However, adding -fomit-frame-pointer makes
it work, and still PASSes on x86_64-pc-linux-gnu.
Ok for for mainline?
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-07-17 Rainer Orth <r...@cebitec.uni-bielefeld.de>
* gcc.target/i386/vartrack-1.c (dg-options): Add
-fomit-frame-pointer.
diff --git a/gcc/testsuite/gcc.target/i386/vartrack-1.c b/gcc/testsuite/gcc.target/i386/vartrack-1.c
--- a/gcc/testsuite/gcc.target/i386/vartrack-1.c
+++ b/gcc/testsuite/gcc.target/i386/vartrack-1.c
@@ -1,5 +1,5 @@
/* { dg-require-effective-target lp64 } */
-/* { dg-options "-O1 -g -fdump-rtl-vartrack-details-slim" } */
+/* { dg-options "-O1 -g -fomit-frame-pointer -fdump-rtl-vartrack-details-slim" } */
static volatile int vv = 1;
