Bug 71709 - [6 Regression] powerpc64le: argument to strcpy() optimised out
Summary: [6 Regression] powerpc64le: argument to strcpy() optimised out
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Alan Modra
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-06-30 01:15 UTC by Anton Blanchard
Modified: 2016-09-28 21:37 UTC (History)
5 users (show)

See Also:
Host:
Target: powerpc64le-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-06-30 00:00:00


Attachments
fix (236 bytes, patch)
2016-06-30 13:37 UTC, Alan Modra
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Blanchard 2016-06-30 01:15:39 UTC
The following test case fails on ppc64le:

#include <string.h>

char boot_command_line[2048];
char *saved_command_line;
static char *static_command_line;
static char *initcall_command_line;
void *memblock_virt_alloc(int, int);

void setup_command_line(char *command_line)
{
        saved_command_line =
                memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
        initcall_command_line =
                memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
        static_command_line = memblock_virt_alloc(strlen(command_line) + 1, 0);
        strcpy(saved_command_line, boot_command_line);
        strcpy(static_command_line, command_line);
}

When built with:

# gcc -fno-common -mcmodel=medium -O2

The last strcpy() call has lost its first argument:

        bl memblock_virt_alloc
        nop
        addis 3,2,.LANCHOR0+2048@toc@ha         # gpr load fusion, type long
        ld 3,.LANCHOR0+2048@toc@l(3)
        mr 4,31
        bl strcpy
        nop
        mr 4,30
        bl strcpy
Comment 1 Andrew Pinski 2016-06-30 01:44:29 UTC
GCC must be thinking saved_command_line and static_command_line are the same ...

Because strcpy returns the 1st argument.
Comment 2 Segher Boessenkool 2016-06-30 02:15:46 UTC
Confirmed.
Comment 3 Segher Boessenkool 2016-06-30 02:22:28 UTC
Before the first RTL CSE, we have on that second strcpy:

(call_insn 47 46 0 2 (parallel [
            (set (reg:DI 3 3)
                (call (mem:SI (symbol_ref:DI ("strcpy") [flags 0x41]  <function_decl 0x3fff900ea000 strcpy>) [0 __builtin_strcpy S4 A8])
                    (const_int 64 [0x40])))
            (clobber (reg:DI 65 lr))
        ]) 71709.c:17 677 {*call_value_nonlocal_aixdi}
     (expr_list:REG_CALL_DECL (symbol_ref:DI ("strcpy") [flags 0x41]  <function_decl 0x3fff900ea000 strcpy>)
        (expr_list:REG_EH_REGION (const_int 0 [0])
            (nil)))
    (expr_list (use (reg:DI 2 2))
        (expr_list:DI (set (reg:DI 3 3)
                (reg:DI 3 3))
            (expr_list:DI (use (reg:DI 3 3))
                (expr_list:DI (use (reg:DI 4 4))
                    (nil))))))

but after cse1 it is:

(call_insn 47 46 0 2 (parallel [
            (set (reg:DI 3 3)
                (call (mem:SI (symbol_ref:DI ("strcpy") [flags 0x41]  <function_decl 0x3fff900ea000 strcpy>) [0 __builtin_strcpy S4 A8])
                    (const_int 64 [0x40])))
            (clobber (reg:DI 65 lr))
        ]) 71709.c:17 677 {*call_value_nonlocal_aixdi}
     (expr_list:REG_DEAD (reg:DI 4 4)
        (expr_list:REG_UNUSED (reg:DI 3 3)
            (expr_list:REG_CALL_DECL (symbol_ref:DI ("strcpy") [flags 0x41]  <function_decl 0x3fff900ea000 strcpy>)
                (expr_list:REG_EH_REGION (const_int 0 [0])
                    (nil)))))
    (expr_list (use (reg:DI 2 2))
        (expr_list:DI (set (reg:DI 3 3)
                (reg:DI 3 3))
            (expr_list:DI (use (reg:DI 3 3))
                (expr_list:DI (use (reg:DI 4 4))
                    (nil))))))

(note the REG_UNUSED).
Comment 4 Alan Modra 2016-06-30 13:17:16 UTC
Comment #3 isn't showing any real problem.  Where things go wrong is in ira where the first strcpy call gets a bad REG_RETURNED note.

I think it is a bug in ira-lives.c:find_call_crossed_cheap_reg where insn 50 below is not recognized as a single_set due to the unspec in the parallel (which is an rs6000 backend problem that needs fixing too), then tries to use reg_overlap_mentioned_p but that misses the set dest.  So find_call_crossed_cheap_reg thinks r3 hasn't been touched at that point and looks back at prior insns, finding the set from reg 168.


(insn 50 42 44 2 (parallel [
            (set (reg:DI 3 3)
                (mem/f/c:DI (unspec:DI [
                            (plus:DI (unspec:DI [
                                        (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
                                        (reg:DI 2 2)
                                    ] UNSPEC_TOCREL)
                                (const_int 2048 [0x800]))
                        ] UNSPEC_FUSION_ADDIS) [1 saved_command_line+0 S8 A64]))
            (unspec [
                    (const_int 0 [0])
                ] UNSPEC_FUSION_ADDIS)
            (use (reg:DI 2 2))
            (clobber (scratch:DI))
        ]) /home/alan/anton.c:18 875 {*toc_fusionload_di}
     (nil))

(call_insn 44 50 45 2 (parallel [
            (set (reg:DI 3 3)
                (call (mem:SI (symbol_ref:DI ("strcpy") [flags 0x41]  <function_decl 0x7f5bb162be00 strcpy>) [0 __builtin_strcpy S4 A8])
                    (const_int 0 [0])))
            (clobber (reg:DI 65 lr))
        ]) /home/alan/anton.c:18 711 {*call_value_nonlocal_aixdi}
     (expr_list:REG_RETURNED (reg/f:DI 168 [ _15 ])
        (expr_list:REG_DEAD (reg:DI 4 4)
            (expr_list:REG_UNUSED (reg:DI 3 3)
                (expr_list:REG_CALL_DECL (symbol_ref:DI ("strcpy") [flags 0x41]  <function_decl 0x7f5bb162be00 strcpy>)
                    (expr_list:REG_EH_REGION (const_int 0 [0])
                        (nil))))))
    (expr_list (use (reg:DI 2 2))
        (expr_list:DI (set (reg:DI 3 3)
                (reg:DI 3 3))
            (expr_list:DI (use (reg:DI 3 3))
                (expr_list:DI (use (reg:DI 4 4))
                    (nil))))))
Comment 5 Alan Modra 2016-06-30 13:37:38 UTC
Created attachment 38802 [details]
fix

Cures the error in find_call_crossed_cheap_reg
Comment 6 Segher Boessenkool 2016-06-30 18:52:30 UTC
Yeah that looks better, I'm still confused by REG_DEAD vs. REG_UNUSED, sigh.

The whole logic here seems somewhat confused.  Or at least it is confusing me :-)
Comment 7 Alan Modra 2016-07-01 06:34:35 UTC
find_call_crossed_cheap_reg is certainly confusing.  On looking at it again this morning, I can't see why it uses reg_overlap_mentioned_p to break out of the loop.  Who cares if the reg is referenced (except autoinc)?  I'm thinking now that the correct exit condition is simply reg_set_p (reg, prev).
Comment 8 Alan Modra 2016-07-01 11:15:54 UTC
Author: amodra
Date: Fri Jul  1 11:15:17 2016
New Revision: 237909

URL: https://gcc.gnu.org/viewcvs?rev=237909&root=gcc&view=rev
Log:
strcpy arg optimised out

For functions that return an argument unchanged, like strcat,
find_call_crossed_cheap_reg attempts to find an assignment between
a pseudo reg and the arg reg before the call, so that uses of the
pseudo after the call can instead use the return value.  The exit
condition on the loop looking at previous insns was wrong.  Uses of
the arg reg don't matter.  What matters is the insn setting the arg
reg as any assignment involving the arg reg prior to that insn is
likely a completely unrelated use of the hard reg.

	PR rtl-optimization/71709
	* ira-lives.c (find_call_crossed_cheap_reg): Exit loop on arg reg
	being set, not referenced.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ira-lives.c
Comment 9 Alan Modra 2016-07-01 12:42:06 UTC
Fixed.
Comment 10 Matthias Klose 2016-09-26 16:56:33 UTC
https://bugs.debian.org/838892 reports this for the gcc-6 branch too; could that be backported?
Comment 11 Bill Schmidt 2016-09-28 19:13:22 UTC
Working on the backports.  Stay tuned.
Comment 12 Bill Schmidt 2016-09-28 21:36:09 UTC
Author: wschmidt
Date: Wed Sep 28 21:35:37 2016
New Revision: 240598

URL: https://gcc.gnu.org/viewcvs?rev=240598&root=gcc&view=rev
Log:
2016-09-28  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Alan Modra  <amodra@gmail.com>

	Backport from mainline
	2016-07-01  Alan Modra  <amodra@gmail.com>

	PR rtl-optimization/71709
	* ira-lives.c (find_call_crossed_cheap_reg): Exit loop on arg reg
	being set, not referenced.


Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/ira-lives.c
Comment 13 Bill Schmidt 2016-09-28 21:37:31 UTC
Author: wschmidt
Date: Wed Sep 28 21:36:59 2016
New Revision: 240599

URL: https://gcc.gnu.org/viewcvs?rev=240599&root=gcc&view=rev
Log:
2016-09-28  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Alan Modra  <amodra@gmail.com>

	Backport from mainline
	2016-07-01  Alan Modra  <amodra@gmail.com>

	PR rtl-optimization/71709
	* ira-lives.c (find_call_crossed_cheap_reg): Exit loop on arg reg
	being set, not referenced.


Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/ira-lives.c