Re: [PATCH][RFC][rtlanal] Fix rtl-optimization/71295

[Bernd Schmidt]

On 05/31/2016 04:26 PM, Kyrill Tkachov wrote:

we're talking about (subreg:V2DI (reg/v:EI 111 [ b ]) 0).
EImode is a 24 byte arm-specific integer mode used to represent 3
D-registers.

[...]

I'm not familiar with the processes in that part of the code but my
suspicion is that we should be
rejecting that invalid subreg somewhere and this patch does that in
subreg_get_info a few lines before
the triggering assert. It rejects the subreg by setting
info->representable_p to false.



Is this the right way to go around fixing this?


I'm slightly uncertain, but I can't really see anything wrong with this 
approach. So, OK. There are things that could be cleaned up here, moving 
the duplicated !rknown tests to the outer if and maybe the declarations 
of regsize_[xy]mode into the if, but that's not something you have to do 
if you don't want to.



Bernd

[PATCH][RFC][rtlanal] Fix rtl-optimization/71295

[Kyrill Tkachov]

Hi all,

The ICE in this PR occurs when initialising the reginfo for the ira_costs for 
an instruction of the form:
(insn 6 15 16 2 (set (subreg:V2DI (reg/v:EI 111 [ b ]) 0)
(const_vector:V2DI [
(const_int 1 [0x1])
(const_int 1 [0x1])
])) mycrash.c:10 861 {*neon_movv2di}
 (nil))

we're talking about (subreg:V2DI (reg/v:EI 111 [ b ]) 0).
EImode is a 24 byte arm-specific integer mode used to represent 3 D-registers.

record_subregs_of_mode in reginfo.c analyses not just the subreg at offset 0 as 
requested
but also the adjacent V2DI subreg i.e. at offset 16. However, there cannot be a 
second V2DI 16-byte
subreg because EImode is only 24 bytes wide, so we get an ICE in 
subreg_get_info to that effect.

I'm not familiar with the processes in that part of the code but my suspicion 
is that we should be
rejecting that invalid subreg somewhere and this patch does that in 
subreg_get_info a few lines before
the triggering assert. It rejects the subreg by setting info->representable_p 
to false.

This fixes the ICE for me on ARM (reproducible at -O3) and doesn't show any 
regressions on aarch64 and x86_64.

Is this the right way to go around fixing this?

Thanks,
Kyrill

2016-05-31  Kyrylo Tkachov  

PR rtl-optimization/71295
* rtlanal.c (subreg_get_info): If taking a subreg at the requested
offset would go over the size of the inner mode reject it.

2016-05-31  Kyrylo Tkachov  

PR rtl-optimization/71295
* gcc.c-torture/compile/pr71295.c: New test.
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index bfefd968b47c530c77c50d72ce42f4a6d4955ea4..6ab96889408bc09815f5490532657ffcecf5c52e 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -3657,6 +3657,16 @@ subreg_get_info (unsigned int xregno, machine_mode xmode,
 	  info->offset = offset / regsize_xmode;
 	  return;
 	}
+  /* It's not valid to extract a subreg of mode YMODE at OFFSET that
+	 would go outside of XMODE.  */
+  if (!rknown
+	  && GET_MODE_SIZE (ymode) + offset > GET_MODE_SIZE (xmode))
+	{
+	  info->representable_p = false;
+	  info->nregs = nregs_ymode;
+	  info->offset = offset / regsize_xmode;
+	  return;
+	}
   /* Quick exit for the simple and common case of extracting whole
 	 subregisters from a multiregister value.  */
   /* ??? It would be better to integrate this into the code below,
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr71295.c b/gcc/testsuite/gcc.c-torture/compile/pr71295.c
new file mode 100644
index ..d2ec852fd08b307da50bfd993a6c43d42f303037
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr71295.c
@@ -0,0 +1,12 @@
+extern void fn2 (long long);
+int a;
+
+void
+fn1 ()
+{
+  long long b[3];
+  a = 0;
+  for (; a < 3; a++)
+b[a] = 1;
+  fn2 (b[1]);
+}