Re: [PATCH] Fix combiner move_deaths DEBUG_INSN handling (PR target/84614)

2018-03-02 Thread Jeff Law
On 03/01/2018 04:13 PM, Jakub Jelinek wrote:
> Hi!
> 
> prev_real_insn doesn't skip over DEBUG_INSNs, on aarch64 on this
> testcase we get both -fcompare-debug failure and wrong-code with -g.
> 
> Instead of adding yet another two:
>   do
> insn = prev_real_insn (insn);
>   while (DEBUG_INSN_P (insn));
> loops, this patch introduces new functions that stop only at
> NONDEBUG_INSN_P.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
> cross to aarch64-linux on the testcase, ok for trunk?
> 
> 2018-03-02  Jakub Jelinek  
> 
>   PR target/84614
>   * rtl.h (prev_real_nondebug_insn, next_real_nondebug_insn): New
>   prototypes.
>   * emit-rtl.c (next_real_insn, prev_real_insn): Fix up function
>   comments.
>   (next_real_nondebug_insn, prev_real_nondebug_insn): New functions.
>   * cfgcleanup.c (try_head_merge_bb): Use prev_real_nondebug_insn
>   instead of a loop around prev_real_insn.
>   * combine.c (move_deaths): Use prev_real_nondebug_insn instead of
>   prev_real_insn.
> 
>   * gcc.dg/pr84614.c: New test.
THanks.  We're going to need the {next,prev}_real_nondebug_insn routines
in reorg/resource as well.  MIPS can't currently build a linux kernel
because we fail all over the place with DEBUG_INSNs within reorg.

I've got a patch that's enough to get mips to bootstrap and build
kernels, but haven't posted it yet.

jeff


Re: [PATCH] Fix combiner move_deaths DEBUG_INSN handling (PR target/84614)

2018-03-01 Thread Segher Boessenkool
Hi Jakub,

On Fri, Mar 02, 2018 at 12:13:51AM +0100, Jakub Jelinek wrote:
> prev_real_insn doesn't skip over DEBUG_INSNs, on aarch64 on this
> testcase we get both -fcompare-debug failure and wrong-code with -g.
> 
> Instead of adding yet another two:
>   do
> insn = prev_real_insn (insn);
>   while (DEBUG_INSN_P (insn));
> loops, this patch introduces new functions that stop only at
> NONDEBUG_INSN_P.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
> cross to aarch64-linux on the testcase, ok for trunk?

Looks good to me, thanks!  (The non-combine parts are arguably obvious
and trivial cleanup).


Segher


> 2018-03-02  Jakub Jelinek  
> 
>   PR target/84614
>   * rtl.h (prev_real_nondebug_insn, next_real_nondebug_insn): New
>   prototypes.
>   * emit-rtl.c (next_real_insn, prev_real_insn): Fix up function
>   comments.
>   (next_real_nondebug_insn, prev_real_nondebug_insn): New functions.
>   * cfgcleanup.c (try_head_merge_bb): Use prev_real_nondebug_insn
>   instead of a loop around prev_real_insn.
>   * combine.c (move_deaths): Use prev_real_nondebug_insn instead of
>   prev_real_insn.
> 
>   * gcc.dg/pr84614.c: New test.