Bug 67756 - [6 Regression] ICE compiling Linux Kernel fs/namei.c on ARM
Summary: [6 Regression] ICE compiling Linux Kernel fs/namei.c on ARM
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 6.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks: 67447
  Show dependency treegraph
 
Reported: 2015-09-29 07:16 UTC by Bernd Edlinger
Modified: 2015-10-04 16:03 UTC (History)
3 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: arm-linux-gnueabihf
Build:
Known to work: 5.1.0
Known to fail: 6.0
Last reconfirmed: 2015-09-29 00:00:00


Attachments
preprocessed source file (147.58 KB, application/bzip2)
2015-09-29 07:19 UTC, Bernd Edlinger
Details
Reduced testcase (510 bytes, text/plain)
2015-09-29 09:56 UTC, ktkachov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bernd Edlinger 2015-09-29 07:16:41 UTC
arm-linux-gnueabihf-gcc -v
Using built-in specs.
COLLECT_GCC=arm-linux-gnueabihf-gcc
COLLECT_LTO_WRAPPER=/home/ed/gnu/arm-linux-gnueabihf-linux64/libexec/gcc/arm-linux-gnueabihf/6.0.0/lto-wrapper
Target: arm-linux-gnueabihf
Configured with: ../gcc-6-20150927/configure --prefix=/home/ed/gnu/arm-linux-gnueabihf-linux64 --target=arm-linux-gnueabihf --enable-languages=c,c++,fortran,ada --with-arch=armv7-a --with-tune=cortex-a9 --with-fpu=vfpv3-d16 --with-float=hard
Thread model: posix
gcc version 6.0.0 20150927 (experimental) (GCC) 

arm-linux-gnueabihf-gcc -Wp,-MD,fs/.namei.o.d  -nostdinc -isystem /home/ed/gnu/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/6.0.0/include -I./arch/arm/include -Iarch/arm/include/generated  -Iinclude -I./arch/arm/include/uapi -Iarch/arm/include/generated/uapi -I./include/uapi -Iinclude/generated/uapi -include ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -std=gnu89 -fno-dwarf2-cfi-asm -fno-omit-frame-pointer -mapcs -mno-sched-prolog -mabi=aapcs-linux -mno-thumb-interwork -mfpu=vfp -funwind-tables -marm -D__LINUX_ARM_ARCH__=7 -march=armv7-a -msoft-float -Uarm -fno-delete-null-pointer-checks -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fstack-protector -Wno-unused-but-set-variable -fno-omit-frame-pointer -fno-optimize-sibling-calls -fno-var-tracking-assignments -g -pg -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -fconserve-stack -Werror=implicit-int -Werror=strict-prototypes -Werror=date-time -DCC_HAVE_ASM_GOTO    -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(namei)"  -D"KBUILD_MODNAME=KBUILD_STR(namei)" -c -o fs/namei.o fs/namei.c
fs/namei.c: In function 'vfs_link':
fs/namei.c:3934:1: error: insn does not satisfy its constraints:
 }
 ^
(insn 85 281 86 12 (parallel [
            (set (reg/v:SI 1 r1 [orig:113 max_links ] [113])
                (and:SI (ne:SI (reg/v:SI 1 r1 [orig:113 max_links ] [113])
                        (const_int 0 [0]))
                    (leu:SI (reg/v:SI 1 r1 [orig:113 max_links ] [113])
                        (reg:SI 2 r2 [orig:125 _25 ] [125]))))
            (clobber (reg:CC 100 cc))
        ]) fs/namei.c:3917 262 {*and_scc_scc_nodom}
     (nil))
fs/namei.c:3934:1: internal compiler error: in extract_constrain_insn, at recog.c:2200
0xa3db48 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
	../../gcc-6-20150927/gcc/rtl-error.c:109
0xa3db6f _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
	../../gcc-6-20150927/gcc/rtl-error.c:120
0xa1028d extract_constrain_insn(rtx_insn*)
	../../gcc-6-20150927/gcc/recog.c:2200
0x9efc9d reload_cse_simplify_operands
	../../gcc-6-20150927/gcc/postreload.c:408
0x9f29b5 reload_cse_simplify
	../../gcc-6-20150927/gcc/postreload.c:194
0x9f29b5 reload_cse_regs_1
	../../gcc-6-20150927/gcc/postreload.c:233
0x9f2acb reload_cse_regs
	../../gcc-6-20150927/gcc/postreload.c:81
0x9f2acb execute
	../../gcc-6-20150927/gcc/postreload.c:2350
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 1 Bernd Edlinger 2015-09-29 07:19:39 UTC
Created attachment 36411 [details]
preprocessed source file
Comment 2 ktkachov 2015-09-29 09:56:24 UTC
Created attachment 36412 [details]
Reduced testcase

Confirmed.
Attaching reduced testcase. Needs -O2 -mapcs
Comment 3 ktkachov 2015-09-29 09:57:11 UTC
Changing status...
Comment 4 Bernd Edlinger 2015-10-01 07:42:15 UTC
necessary compiler options to trigger the ICE: -O2 and -mapcs
arm-linux-gnueabihf-gcc -O2 -mapcs -S kernel-ice.c
Comment 5 Bernd Edlinger 2015-10-01 10:22:40 UTC
kernel-ice.c.231r.ira:
(insn 47 45 48 6 (parallel [
            (set (reg:SI 138)
                (and:SI (ne:SI (reg/v:SI 112 [ max_linksD.4175 ])
                        (const_int 0 [0]))
                    (leu:SI (reg/v:SI 112 [ max_linksD.4175 ])
                        (reg:SI 114 [ _15 ]))))
            (clobber (reg:CC 100 cc))
        ]) kernel-ice.c:49 262 {*and_scc_scc_nodom}
     (expr_list:REG_DEAD (reg:SI 114 [ _15 ])

which looks like alternative#0

kernel-ice.c.232r.reload:
(insn 47 118 48 6 (parallel [
            (set (reg/v:SI 1 r1 [orig:112 max_linksD.4175 ] [112])
                (and:SI (ne:SI (reg/v:SI 1 r1 [orig:112 max_linksD.4175 ] [112])
                        (const_int 0 [0]))
                    (leu:SI (reg/v:SI 1 r1 [orig:112 max_linksD.4175 ] [112])
                        (reg:SI 2 r2 [orig:114 _15 ] [114]))))
            (clobber (reg:CC 100 cc))
        ]) kernel-ice.c:49 262 {*and_scc_scc_nodom}
     (nil))

which looks wrong, because
(define_insn_and_split "*and_scc_scc_nodom"
  [(set (match_operand:SI 0 "s_register_operand" "=&Ts,&Ts,&Ts")
        (and:SI (match_operator:SI 3 "arm_comparison_operator"
                 [(match_operand:SI 1 "s_register_operand" "r,r,0")
                  (match_operand:SI 2 "arm_add_operand" "rIL,0,rIL")])
                (match_operator:SI 6 "arm_comparison_operator"
                 [(match_operand:SI 4 "s_register_operand" "r,r,r")
                  (match_operand:SI 5 "arm_add_operand" "rIL,rIL,rIL")])))

reload uses alternative#2 which defines op[0] as early clobber "&Ts",
and op[1] identical to op[0], but now we have op[4] clobbered.

This seems to be another LRA bug.  Right?

My patch from yesterday makes no difference here, but what's funny is,
that the set register was originally r138 but now the dump says
"set (reg/v:SI 1 r1 [orig:112 ...".
Comment 6 ktkachov 2015-10-01 10:24:51 UTC
Perhaps a bisection to the revision that started this could shed some light
Comment 7 Bernd Edlinger 2015-10-01 10:29:20 UTC
(In reply to ktkachov from comment #6)
> Perhaps a bisection to the revision that started this could shed some light

absolutely, but my computer powers are too limited for that.  could you help?
Comment 8 Markus Trippelsdorf 2015-10-01 10:34:13 UTC
(In reply to Bernd Edlinger from comment #7)
> (In reply to ktkachov from comment #6)
> > Perhaps a bisection to the revision that started this could shed some light
> 
> absolutely, but my computer powers are too limited for that.  could you help?

You could use the compile farm machines, e.g. building a cross on gcc112, which has 152 "cores".
Comment 9 ktkachov 2015-10-01 10:35:10 UTC
(In reply to Bernd Edlinger from comment #7)
> (In reply to ktkachov from comment #6)
> > Perhaps a bisection to the revision that started this could shed some light
> 
> absolutely, but my computer powers are too limited for that.  could you help?

I'll give it a shot
Comment 10 ktkachov 2015-10-01 11:24:58 UTC
The ICE started with:
Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Sep 1 19:37:52 2015 +0000

    2015-09-01  Vladimir Makarov  <vmakarov@redhat.com>
    
        PR target/61578
        * lra-lives.c (process_bb_lives): Process move pseudos with the
        same value for copies and preferences
        * lra-constraints.c (match_reload): Create match reload pseudo
        with the same value from single dying input pseudo.
    
    
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@227382 138bc75d-0d04-0410-961f-82ee72b054a4


So it seems like an LRA issue
Comment 11 Bernd Edlinger 2015-10-01 12:51:01 UTC
I must admit, that I don't know what I am doing here,
... but this (completely untested) patch seems to fix the ICE:
(and at least my linux kernel compiles without ICE now)

--- lra-assigns.c.jj	2015-07-16 17:26:35.000000000 +0200
+++ lra-assigns.c	2015-10-01 14:40:06.262300720 +0200
@@ -576,7 +576,7 @@ find_hard_regno_for_1 (int regno, int *c
 	/* Remember about multi-register pseudos.  For example, 2 hard
 	   register pseudos can start on the same hard register but can
 	   not start on HR and HR+1/HR-1.  */
-	for (hr = conflict_hr + 1;
+	for (hr = conflict_hr;
 	     hr < FIRST_PSEUDO_REGISTER && hr < conflict_hr + nregs;
 	     hr++)
 	  SET_HARD_REG_BIT (impossible_start_hard_regs, hr);



The problem starts when lra assigns r143 and r144 to r1
although both have obviously conflicting live ranges.
Comment 12 Vladimir Makarov 2015-10-01 15:27:27 UTC
(In reply to Bernd Edlinger from comment #0)
> 
> fs/namei.c:3934:1: internal compiler error: in extract_constrain_insn, at
> recog.c:2200
> 0xa3db48 _fatal_insn(char const*, rtx_def const*, char const*, int, char
> const*)
> 	../../gcc-6-20150927/gcc/rtl-error.c:109
> 0xa3db6f _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
> 	../../gcc-6-20150927/gcc/rtl-error.c:120
> 0xa1028d extract_constrain_insn(rtx_insn*)
> 	../../gcc-6-20150927/gcc/recog.c:2200
> 0x9efc9d reload_cse_simplify_operands
> 	../../gcc-6-20150927/gcc/postreload.c:408
> 0x9f29b5 reload_cse_simplify
> 	../../gcc-6-20150927/gcc/postreload.c:194
> 0x9f29b5 reload_cse_regs_1
> 	../../gcc-6-20150927/gcc/postreload.c:233
> 0x9f2acb reload_cse_regs
> 	../../gcc-6-20150927/gcc/postreload.c:81
> 0x9f2acb execute
> 	../../gcc-6-20150927/gcc/postreload.c:2350
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.

Thanks for reporting this.  I'll try to fix it this week.
Comment 13 Vladimir Makarov 2015-10-01 15:30:27 UTC
(In reply to Bernd Edlinger from comment #11)
> I must admit, that I don't know what I am doing here,
> ... but this (completely untested) patch seems to fix the ICE:
> (and at least my linux kernel compiles without ICE now)
> 

This is a wrong fix.  If the pseudos has the same value, they should not conflict if they start with the same register.  This part of code concerns exactly about the pseudos with the same value.

The root of the problem is in that the pseudos should not have the same value.

> --- lra-assigns.c.jj	2015-07-16 17:26:35.000000000 +0200
> +++ lra-assigns.c	2015-10-01 14:40:06.262300720 +0200
> @@ -576,7 +576,7 @@ find_hard_regno_for_1 (int regno, int *c
>  	/* Remember about multi-register pseudos.  For example, 2 hard
>  	   register pseudos can start on the same hard register but can
>  	   not start on HR and HR+1/HR-1.  */
> -	for (hr = conflict_hr + 1;
> +	for (hr = conflict_hr;
>  	     hr < FIRST_PSEUDO_REGISTER && hr < conflict_hr + nregs;
>  	     hr++)
>  	  SET_HARD_REG_BIT (impossible_start_hard_regs, hr);
> 
> 
> 
> The problem starts when lra assigns r143 and r144 to r1
> although both have obviously conflicting live ranges.
Comment 14 Vladimir Makarov 2015-10-01 15:41:13 UTC
(In reply to Bernd Edlinger from comment #5)

> 
> My patch from yesterday makes no difference here, but what's funny is,
> that the set register was originally r138 but now the dump says
> "set (reg/v:SI 1 r1 [orig:112 ...".

LRA creates a pseudo for matching reload operands and on the subsequent sub-pass assign a hard register to it.  When we create the pseudo it can be originated only from one pseudo p138 or p112 (matching pseudo operands).  Therefore we have such result.

It could be fixed but I don't see the efforts needed to spend on this is worth to fix this.  This discrepancy exists only between the insn and the corresponding output reload.
Comment 15 Bernd Edlinger 2015-10-01 18:41:53 UTC
(In reply to Vladimir Makarov from comment #14)
> (In reply to Bernd Edlinger from comment #5)
> 
> > 
> > My patch from yesterday makes no difference here, but what's funny is,
> > that the set register was originally r138 but now the dump says
> > "set (reg/v:SI 1 r1 [orig:112 ...".
> 
> LRA creates a pseudo for matching reload operands and on the subsequent
> sub-pass assign a hard register to it.  When we create the pseudo it can be
> originated only from one pseudo p138 or p112 (matching pseudo operands). 
> Therefore we have such result.
> 
> It could be fixed but I don't see the efforts needed to spend on this is
> worth to fix this.  This discrepancy exists only between the insn and the
> corresponding output reload.

I agree, that was a bit surprising deja-vu when I looked closely at the dump,
but this alias should not really hurt in deed.
Comment 16 Bernd Edlinger 2015-10-02 10:23:38 UTC
(In reply to Vladimir Makarov from comment #13)
> (In reply to Bernd Edlinger from comment #11)
> > I must admit, that I don't know what I am doing here,
> > ... but this (completely untested) patch seems to fix the ICE:
> > (and at least my linux kernel compiles without ICE now)
> > 
> 
> This is a wrong fix.  If the pseudos has the same value, they should not
> conflict if they start with the same register.  This part of code concerns
> exactly about the pseudos with the same value.
> 
> The root of the problem is in that the pseudos should not have the same
> value.
> 

OK, now I see the problem.

How about this?

--- lra-constraints.c.jj	2015-09-25 23:06:08.000000000 +0200
+++ lra-constraints.c	2015-10-02 12:18:51.346267777 +0200
@@ -160,6 +160,8 @@ static machine_mode curr_operand_mode[MA
    (e.g. constant) and whose subreg is given operand of the current
    insn.  VOIDmode in all other cases.  */
 static machine_mode original_subreg_reg_mode[MAX_RECOG_OPERANDS];
+/* True if the operand is early clobber.  */
+static bool goal_alt_early_clobber[MAX_RECOG_OPERANDS];
 
 

 
@@ -949,8 +951,10 @@ match_reload (signed char out, signed ch
 	      in the output, e.g. as an address part in memory,
 	      becuase output reload will actually extend the pseudo
 	      liveness.  We don't care about eliminable hard regs here
-	      as we are interesting only in pseudos.  */
-	   && (out < 0 || regno_use_in (REGNO (in_rtx), out_rtx) == NULL_RTX)
+	      as we are interesting only in pseudos.  This does not work
+	      for early clobber outputs.  */
+	   && (out < 0 || (regno_use_in (REGNO (in_rtx), out_rtx) == NULL_RTX
+			   && ! goal_alt_early_clobber[out]))
 	   ? lra_create_new_reg (inmode, in_rtx, goal_class, "")
 	   : lra_create_new_reg_with_unique_value (outmode, out_rtx,
 						   goal_class, ""));
@@ -1693,6 +1697,7 @@ process_alt_operands (int only_alternati
   bool curr_alt_win[MAX_RECOG_OPERANDS];
   bool curr_alt_offmemok[MAX_RECOG_OPERANDS];
   int curr_alt_matches[MAX_RECOG_OPERANDS];
+  bool curr_alt_early_clobber[MAX_RECOG_OPERANDS];
   /* The number of elements in the following array.  */
   int curr_alt_dont_inherit_ops_num;
   /* Numbers of operands whose reload pseudos should not be inherited.	*/
@@ -1803,6 +1808,7 @@ process_alt_operands (int only_alternati
 	      curr_alt_match_win[nop] = false;
 	      curr_alt_offmemok[nop] = false;
 	      curr_alt_matches[nop] = -1;
+	      curr_alt_early_clobber[nop] = false;
 	      continue;
 	    }
 
@@ -2483,6 +2489,7 @@ process_alt_operands (int only_alternati
 	  curr_alt_match_win[nop] = this_alternative_match_win;
 	  curr_alt_offmemok[nop] = this_alternative_offmemok;
 	  curr_alt_matches[nop] = this_alternative_matches;
+	  curr_alt_early_clobber[nop] = early_clobber_p;
 
 	  if (this_alternative_matches >= 0
 	      && !did_match && !this_alternative_win)
@@ -2642,6 +2649,7 @@ process_alt_operands (int only_alternati
 	      goal_alt_win[nop] = curr_alt_win[nop];
 	      goal_alt_match_win[nop] = curr_alt_match_win[nop];
 	      goal_alt_matches[nop] = curr_alt_matches[nop];
+	      goal_alt_early_clobber[nop] = curr_alt_early_clobber[nop];
 	      goal_alt[nop] = curr_alt[nop];
 	      goal_alt_offmemok[nop] = curr_alt_offmemok[nop];
 	    }
@@ -3366,6 +3374,7 @@ curr_insn_transform (bool check_only_p)
     {
       goal_alt_matched[i][0] = -1;
       goal_alt_matches[i] = -1;
+      goal_alt_early_clobber[i] = false;
     }
 
   commutative = curr_static_id->commutative;
Comment 17 Vladimir Makarov 2015-10-02 14:40:42 UTC
(In reply to Bernd Edlinger from comment #16)
> (In reply to Vladimir Makarov from comment #13)
> > (In reply to Bernd Edlinger from comment #11)
> > > I must admit, that I don't know what I am doing here,
> > > ... but this (completely untested) patch seems to fix the ICE:
> > > (and at least my linux kernel compiles without ICE now)
> > > 
> > 
> > This is a wrong fix.  If the pseudos has the same value, they should not
> > conflict if they start with the same register.  This part of code concerns
> > exactly about the pseudos with the same value.
> > 
> > The root of the problem is in that the pseudos should not have the same
> > value.
> > 
> 
> OK, now I see the problem.
> 
> How about this?
> 
The patch can be a solution.  It is a right direction.  Thanks for working on this.  I have a patch already too.  I am just testing it.  I'll commit it in a few hours.
Comment 18 Vladimir Makarov 2015-10-02 15:05:32 UTC
Author: vmakarov
Date: Fri Oct  2 15:04:59 2015
New Revision: 228396

URL: https://gcc.gnu.org/viewcvs?rev=228396&root=gcc&view=rev
Log:
2015-10-02  Vladimir Makarov  <vmakarov@redhat.com>

	PR rtl-optimization/67756
	* lra-constraints.c (match_reload): Add a new parameter.  Use it
	for creating a pseudo with the same value.
	(curr_insn_transform): Pass a new argument to match_reload.

2015-10-02  Vladimir Makarov  <vmakarov@redhat.com>

	PR rtl-optimization/67756
	* gcc.target/arm/pr67756.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/arm/pr67756.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra-constraints.c
    trunk/gcc/testsuite/ChangeLog
Comment 19 Bernd Edlinger 2015-10-02 20:27:29 UTC
ok, but now we have because of the warnings:

FAIL: gcc.target/arm/pr67756.c (test for excess errors)

I think something like this could fix it:

Index: pr67756.c
===================================================================
--- pr67756.c	(revision 228404)
+++ pr67756.c	(working copy)
@@ -2,6 +2,8 @@
 /* { dg-require-effective-target arm_hard_vfp_ok } */
 /* { dg-options "-O2 -mapcs -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16" } */
 
+int inode_permission (), try_break_deleg ();
+int mutex_lock (), mutex_unlock ();
 struct mutex
 {
 };
Comment 20 Vladimir Makarov 2015-10-02 20:33:44 UTC
(In reply to Bernd Edlinger from comment #19)
> ok, but now we have because of the warnings:
> 
> FAIL: gcc.target/arm/pr67756.c (test for excess errors)
> 
> I think something like this could fix it:
> 
> Index: pr67756.c
> ===================================================================
> --- pr67756.c	(revision 228404)
> +++ pr67756.c	(working copy)
> @@ -2,6 +2,8 @@
>  /* { dg-require-effective-target arm_hard_vfp_ok } */
>  /* { dg-options "-O2 -mapcs -march=armv7-a -mfloat-abi=hard
> -mfpu=vfpv3-d16" } */
>  
> +int inode_permission (), try_break_deleg ();
> +int mutex_lock (), mutex_unlock ();
>  struct mutex
>  {
>  };

Thanks for checking.  Could you commit it.  Thanks.
Comment 21 Bernd Edlinger 2015-10-03 06:30:08 UTC
(In reply to Vladimir Makarov from comment #20)
> Thanks for checking.  Could you commit it.  Thanks.

OK, done, r228445.