Re: Improve insert/emplace robustness to self insertion

[David Edelsohn]

On Sat, Jul 9, 2016 at 7:59 PM, Jonathan Wakely  wrote:
> On 09/07/16 13:47 -0400, David Edelsohn wrote:
>>
>> This patch has caused some new libstdc++ testsuite failures on AIX.
>
>
> Which patch?
>
> My last patch only added a new test, that can't have caused failures
> in unrelated tests.
>
> https://gcc.gnu.org/ml/libstdc++-cvs/2016-q3/msg00021.html

I thought that there were a recent set of libstdc++ patches related to
"insert'.  What else could have caused these regressions?  Recent
patches to C++ front-end?

Thanks, David

>
>
>
>
>> FAIL: 23_containers/list/debug/insert4_neg.cc (test for excess errors)
>>
>> Excess errors:
>>
>>
>> /tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7:
>> error: __gnu_debug::_Error_formatter&
>> __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
>> char*) [with _Iterator = __gnu_debug::_Safe_iterator
>> ] causes a
>> section type conflict with __gnu_debug::_Error_formatter&
>> __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
>> char*) [with _Iterator =
>> __gnu_debug::_Safe_iterator> std::__debug::list >]
>>
>> FAIL: 23_containers/deque/debug/insert4_neg.cc (test for excess errors)
>>
>> Excess errors:
>>
>>
>> /tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7:
>> error: __gnu_debug::_Error_formatter&
>> __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
>> char*) [with _Iterator = __gnu_debug::_Safe_iterator
>> , std::__debug::deque >]
>> causes a section type conflict with __gnu_debug::_Error_formatter&
>> __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
>> char*) [with _Iterator =
>> __gnu_debug::_Safe_iterator> const int*>, std::__debug::deque >]
>>
>> Thanks, David

Re: Improve insert/emplace robustness to self insertion

[Jonathan Wakely]

On 09/07/16 13:47 -0400, David Edelsohn wrote:

This patch has caused some new libstdc++ testsuite failures on AIX.


Which patch?

My last patch only added a new test, that can't have caused failures
in unrelated tests.

https://gcc.gnu.org/ml/libstdc++-cvs/2016-q3/msg00021.html




FAIL: 23_containers/list/debug/insert4_neg.cc (test for excess errors)

Excess errors:

/tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7:
error: __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator = __gnu_debug::_Safe_iterator
] causes a
section type conflict with __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator =
__gnu_debug::_Safe_iterator]

FAIL: 23_containers/deque/debug/insert4_neg.cc (test for excess errors)

Excess errors:

/tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7:
error: __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator = __gnu_debug::_Safe_iterator
, std::__debug::deque >]
causes a section type conflict with __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator =
__gnu_debug::_Safe_iterator, std::__debug::deque >]

Thanks, David

Re: [patch, fortran] Fix PR 71783

[Jerry DeLisle]

On 07/09/2016 01:16 PM, Thomas Koenig wrote:
> Hello world,
> 
> this patch fixes the regression by always allocating a charlen.
> 
> Why we still allocate a temporary even for obviously non-overlapping
> cases like the test case remains to be investigated. I'll open a
> separate PR for this.
> 
> Regression-tested. OK for all affected branches?
> 

Yes, OK and thanks.

Jerry

> Regards
> 
> Thomas
> 
> 2016-07-09  Thomas Koenig  
> 
> PR fortran/71783
> * frontend-passes.c (create_var):  Always allocate a charlen
> for character variables.
> 
> 2016-07-09  Thomas Koenig  
> 
> PR fortran/71783
> * gfortran.dg/dependency_46.f90:  New test.

[patch, fortran] Fix PR 71783

[Thomas Koenig]

Hello world,

this patch fixes the regression by always allocating a charlen.

Why we still allocate a temporary even for obviously non-overlapping
cases like the test case remains to be investigated. I'll open a
separate PR for this.

Regression-tested. OK for all affected branches?

Regards

Thomas

2016-07-09  Thomas Koenig  

PR fortran/71783
* frontend-passes.c (create_var):  Always allocate a charlen
for character variables.

2016-07-09  Thomas Koenig  

PR fortran/71783
* gfortran.dg/dependency_46.f90:  New test.
Index: frontend-passes.c
===
--- frontend-passes.c	(Revision 237949)
+++ frontend-passes.c	(Arbeitskopie)
@@ -665,12 +665,10 @@ create_var (gfc_expr * e, const char *vname)
 {
   gfc_expr *length;
 
+  symbol->ts.u.cl = gfc_new_charlen (ns, NULL);
   length = constant_string_length (e);
   if (length)
-	{
-	  symbol->ts.u.cl = gfc_new_charlen (ns, NULL);
-	  symbol->ts.u.cl->length = length;
-	}
+	symbol->ts.u.cl->length = length;
   else
 	symbol->attr.allocatable = 1;
 }
! { dg-do compile }
! PR 71783 - this used to ICE due to a missing charlen for the temporary.
! Test case by Toon Moene.

SUBROUTINE prtdata(ilen)
  INTEGER :: ilen
  character(len=ilen), allocatable :: cline(:)
  allocate(cline(2))
  cline(1) = 'a'
  cline(2) = cline(1)
END SUBROUTINE prtdata

Re: Improve insert/emplace robustness to self insertion

[David Edelsohn]

This patch has caused some new libstdc++ testsuite failures on AIX.

FAIL: 23_containers/list/debug/insert4_neg.cc (test for excess errors)

Excess errors:

/tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7:
error: __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator = __gnu_debug::_Safe_iterator
] causes a
section type conflict with __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator =
__gnu_debug::_Safe_iterator]

FAIL: 23_containers/deque/debug/insert4_neg.cc (test for excess errors)

Excess errors:

/tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7:
error: __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator = __gnu_debug::_Safe_iterator
, std::__debug::deque >]
causes a section type conflict with __gnu_debug::_Error_formatter&
__gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const
char*) [with _Iterator =
__gnu_debug::_Safe_iterator, std::__debug::deque >]

Thanks, David

Re: [PATCH FT32]: apply unbias to references to RAM symbols

[Georg-Johann Lay]

James Bowman schrieb:

The FT32 binutils use a bias to distinguish between RAM and flash
addresses.

This fix adds an ASM_OUTPUT_SYMBOL_REF() that unbiases references to
RAM symbols.

Only references to RAM objects have the bias applied. Flash objects
(that is, objects in ADDR SPACE 1) are not biased, so for these no bias
should be applied. Likewise references in the gdb section need to use
the biased address, so references in debug sections are not unbiased.

gcc/ChangeLog:

2016-07-08  James Bowman  

* config/ft32/ft32.c (ft32_elf_encode_section_info): New function.
* config/ft32/ft32.h (ASM_OUTPUT_SYMBOL_REF): New function.


Huh, ASM_OUTPUT_SYMBOL_REF it's a macro.



Index: gcc/config/ft32/ft32.c
===
--- gcc/config/ft32/ft32.c  (revision 237998)
+++ gcc/config/ft32/ft32.c  (working copy)
@@ -35,6 +35,7 @@
 #include "calls.h"
 #include "expr.h"
 #include "builtins.h"
+#include "print-tree.h"


Seems this include is no more needed as you commented-out below the used 
functions.  If the header is actually needed it should be mentioned in 
the ChangeLog.



 /* This file should be included last.  */
 #include "target-def.h"
@@ -895,6 +896,46 @@ yes:
   return 1;
 }
 
+#undef TARGET_ENCODE_SECTION_INFO

+#define TARGET_ENCODE_SECTION_INFO  ft32_elf_encode_section_info
+
+void
+ft32_elf_encode_section_info (tree decl, rtx rtl, int first)


Can't this function be static?


+{
+  enum tree_code code;
+  rtx symbol;
+
+  /* Careful not to prod global register variables.  */
+  if (!MEM_P (rtl))
+return;
+  symbol = XEXP (rtl, 0);
+  if (GET_CODE (symbol) != SYMBOL_REF)


We have SYMBOL_REF_P ==> if (!SYMBOL_REF_P (symbol)) ...


+return;
+
+  default_encode_section_info (decl, rtl, first);
+
+  code = TREE_CODE (decl);
+  switch (TREE_CODE_CLASS (code))
+{
+case tcc_declaration:
+  {
+   tree type = TREE_TYPE (decl);
+   int is_flash = (type && TYPE_P (type) && !ADDR_SPACE_GENERIC_P 
(TYPE_ADDR_SPACE (type)));


Line too long, should be wrapped, e.g. before the "&&".


+   if ((code == VAR_DECL) && !is_flash)
+ SYMBOL_REF_FLAGS (symbol) |= 0x1000;


"0x1000" is fragile.  If rtl.h::SYMBOL_FLAG_MACH_DEP_SHIFT is bumped in 
a future version because generic symbol_ref flags are added, hard-coded 
flags like these will break your code.



+  }
+  break;
+case tcc_constant:
+case tcc_exceptional:
+  if (code == STRING_CST)
+   SYMBOL_REF_FLAGS (symbol) |= 0x1000;


Same here.


+}
+
+  // debug_tree (decl);
+  // debug_rtx (rtl);
+  // printf("\n");


No code in comments.


+}
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-ft32.h"

Index: gcc/config/ft32/ft32.h
===
--- gcc/config/ft32/ft32.h  (revision 237998)
+++ gcc/config/ft32/ft32.h  (working copy)
@@ -506,4 +506,14 @@ do { \
 
 extern int ft32_is_mem_pm(rtx o);


Such prototypes usually live in ft32-protos.h, not in ft32.h.

 
+#define ASM_OUTPUT_SYMBOL_REF(stream, sym) \

+  do { \
+assemble_name (stream, XSTR (sym, 0)); \
+int section_debug = in_section && \
+  (SECTION_STYLE (in_section) == SECTION_NAMED) && \


If lines are wrapped, then before operators like "&&", not after them.


+  (in_section->named.common.flags & SECTION_DEBUG); \
+if (!section_debug && SYMBOL_REF_FLAGS (sym) & 0x1000) \


Same here with magic "0x1000".


+  asm_fprintf (stream, "-0x80"); \
+  } while (0)
+
 #endif /* GCC_FT32_H */


Johann

Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx

[Peter Bergner]

On 7/6/16 12:53 PM, David Edelsohn wrote:
> On Tue, Jul 5, 2016 at 10:26 PM, Peter Bergner  wrote:
>> The following patch fixes a bug where we do not disable POWER9 vector dform
>> addressing when we compile for POWER9 but without VSX support.  This 
>> manifested
>> itself with us trying to use dform addressing with altivec loads/stores
>> which is illegal, leading to an ICE.
> 
> Peter,
> 
> DFORM definitely should be disabled without VSX, but the patch seems
> incomplete.  If VSX and DFORM are enabled, and GCC chooses an Altivec
> instruction alternative in a pattern, what is to prevent the
> generation of a DFORM address?

Adding Uli and Alan to the CC since there are some reload questions. :-)

So I dug into this a little more.  With -mcpu=power9 (ie, no -mno-vsx),
I could not get an altivec pattern generated, without resorting to using
an altivec builtin.  The altivec builtins also seem to force reg+reg
addressing, since hey, that's all they support.  So I couldn't create
a test case to answer your question of what if VSX and DFORM are enabled
and we have an altivec pattern with a dform address.  Maybe there should
be a test case that generates one of the altivec load/store patterns,
but I couldn't do it.

To try and answer your question, I went back to using -mcpu=power9 -mno-vsx
with unpatched trunk (ie, without my patch to disable vector dform when
power9 vector is disabled).  Looking at the following simple load test
case, it seems reload is capable of fixing up dform addresses when they're
used in an altivec pattern:

bergner@genoa:~/gcc/BUGS/sawdey/altivec$ cat vec_load.i
typedef __attribute__((altivec(vector__))) __attribute__((aligned(16))) 
unsigned char vec_t;
vec_t
foo (vec_t *dst)
{
  return dst[1];
}
bergner@genoa:~/gcc/BUGS/sawdey/altivec$ 
/home/bergner/gcc/build/gcc-fsf-mainline-debug-3/gcc/xgcc 
-B/home/bergner/gcc/build/gcc-fsf-mainline-debug-3/gcc -S -O1 -mcpu=power9 
-mno-vsx vec_load.i -fdump-rtl-all-all

This gives us the following just before reload:

(insn 11 6 12 2 (set (reg/i:V16QI 79 2)
(mem:V16QI (plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10])) [0 MEM[(vec_tD.2411 *)dst_2(D) + 16B]+0 
S16 A128])) vec_load.i:6 1209 {*altivec_movv16qi}
 (expr_list:REG_DEAD (reg:DI 3 3 [ dstD.2412 ])
(nil)))

During reload, we see a reload for input:

Spilling for insn 11.

Reloads for insn # 11
Reload 0: reload_in (DI) = (plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10]))
BASE_REGS, RELOAD_FOR_INPUT (opnum = 1), inc by 16
reload_in_reg: (plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10]))
reload_reg_rtx: (reg:DI 3 3)
Insns emitted for these reloads:
(insn 18 16 11 2 (set (reg:DI 3 3)
(plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10]))) vec_load.i:6 75 {*adddi3}
 (nil))

...which fixes the address so it satisfies its constraints and leaves us
with the rtl we want:

(insn 18 6 11 2 (set (reg:DI 3 3)
(plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10]))) vec_load.i:6 75 {*adddi3}
 (nil))
(insn 11 18 12 2 (set (reg/i:V16QI 79 2)
(mem:V16QI (reg:DI 3 3) [0 MEM[(vec_tD.2411 *)dst_2(D) + 16B]+0 S16 
A128])) vec_load.i:6 1209 {*altivec_movv16qi}
 (nil))

So to answer your question, at least for the altivec load case, reload prevents
us from using reg+offset addressing.


The problem occurs when we have an altivec store, which is what the original
test case had:

bergner@genoa:~/gcc/BUGS/sawdey/altivec$ cat vec_store.i
typedef __attribute__((altivec(vector__))) __attribute__((aligned(16))) 
unsigned char vec_t;
void
foo (vec_t *dst, vec_t src)
{
  dst[1] = src;
}
bergner@genoa:~/gcc/BUGS/sawdey/altivec$ 
/home/bergner/gcc/build/gcc-fsf-mainline-debug-3/gcc/xgcc 
-B/home/bergner/gcc/build/gcc-fsf-mainline-debug-3/gcc -S -O1 -mcpu=power9 
-mno-vsx vec_store.i -fdump-rtl-all-all

This gives us the following just before reload:

(insn 7 4 0 2 (set (mem:V16QI (plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10])) [0 MEM[(vec_tD.2411 *)dst_1(D) + 16B]+0 
S16 A128])
(reg:V16QI 79 2 [ srcD.2413 ])) vec_store.i:5 1209 {*altivec_movv16qi}
 (expr_list:REG_DEAD (reg:V16QI 79 2 [ srcD.2413 ])
(expr_list:REG_DEAD (reg:DI 3 3 [ dstD.2412 ])
(nil


During reload, we see a reload for output:

Spilling for insn 7.
Using reg 9 for reload 1
Spilling for insn 7.
Using reg 9 for reload 1

Reloads for insn # 7
Reload 0: reload_out (V16QI) = (mem:V16QI (plus:DI (reg:DI 3 3 [ dstD.2412 ])
(const_int 16 [0x10])) 
[0 MEM[(vec_tD.2411 *)dst_1(D) + 16B]+0 S16 A128])
NO_REGS, RELOAD_FOR_OUTPUT (opnum = 0), optional
reload_out_reg: (mem:V16QI (plus:DI (reg:DI 3 3 [ dstD.2412 ])

Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx

[Peter Bergner]

On 7/6/16 6:29 PM, Michael Meissner wrote:

On Wed, Jul 06, 2016 at 05:01:38PM -0500, Peter Bergner wrote:

I had thought about adding the dform scalar flag, but it was already
correctly disabled and I wasn't sure whether we could have the p9
dform scalar without the vector part.  Probably not, so consider
the patch above as the latest.


Yes, you can have P9 dform scalar without P9 dform vector.


So you're saying my original patch is the correct change?  Ie:

-  rs6000_isa_flags &= ~OPTION_MASK_P9_VECTOR;
+  rs6000_isa_flags &= ~(OPTION_MASK_P9_VECTOR
+   | OPTION_MASK_P9_DFORM_VECTOR);


Peter

Re: [PATCH] Support running the selftests under valgrind

[David Malcolm]

On Fri, 2016-07-08 at 22:55 -0700, Andrew Pinski wrote:
> On Fri, Jul 8, 2016 at 12:46 PM, David Malcolm 
> wrote:
> > This patch adds a new phony target to gcc/Makefile.in to make it
> > easy
> > to run the selftests under valgrind, via "make selftest-valgrind".
> > This phony target isn't a dependency of anything; it's purely for
> > convenience (it takes about 4-5 seconds on my box).
> > 
> > Doing so uncovered a few leaks in the selftest suite, which the
> > patch also fixes, so that it runs cleanly under valgrind (on
> > x86_64-pc-linux-gnu, configured with --enable-valgrind-annotations,
> > at least).
> > 
> > Successfully bootstrapped on x86_64-pc-linux-gnu.
> > Manually verified that the valgrind output is "clean" on
> > x86_64-pc-linux-gnu [1].
> > 
> > OK for trunk?
> 
> I think this is a good idea.  I assume this not turned on by default.
> valgrind is still not fully working on aarch64 :).

Correct; that's what I was trying to express by saying it isn't a
dependency of anything.  I wouldn't want to make that a requirement.

FWIW, it's very useful when writing new selftests; I was using it when
writing the string literal location patch I just posted.  It caught a
memory leak when tracking string concatenation, for the case of
concatenating a string that's fully <= LINE_MAP_MAX_LOCATION_WITH_COLS
with a string that is at least partly beyond that boundary.  This leak
would have been hard to find using our traditional test approach. (the
version of the string literal patch that I posted is clean under "make
selftest-valgrind").

> Thanks,
> Andrew
> 
> > 
> > [1]:
> > 
> >  HEAP SUMMARY:
> >  in use at exit: 1,203,983 bytes in 2,114 blocks
> >total heap usage: 4,545 allocs, 2,431 frees, 3,212,841 bytes
> > allocated
> > 
> >  LEAK SUMMARY:
> > definitely lost: 0 bytes in 0 blocks
> > indirectly lost: 0 bytes in 0 blocks
> >   possibly lost: 0 bytes in 0 blocks
> > still reachable: 1,203,983 bytes in 2,114 blocks
> >  suppressed: 0 bytes in 0 blocks
> >  Reachable blocks (those to which a pointer was found) are not
> > shown.
> >  To see them, rerun with: --leak-check=full --show-leak-kinds=all
> > 
> >  For counts of detected and suppressed errors, rerun with: -v
> >  ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
> > 
> > gcc/ChangeLog:
> > * Makefile.in (selftest-valgrind): New phony target.
> > * function-tests.c (selftest::build_cfg): Delete pass
> > instances
> > created by the test.
> > (selftest::convert_to_ssa): Likewise.
> > (selftest::test_expansion_to_rtl): Likewise.
> > * tree-cfg.c (selftest::test_linear_chain): Release
> > dominator
> > vectors.
> > (selftest::test_diamond): Likewise.
> > ---
> >  gcc/Makefile.in  | 6 ++
> >  gcc/function-tests.c | 4 
> >  gcc/tree-cfg.c   | 6 ++
> >  3 files changed, 16 insertions(+)
> > 
> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> > index 5e7422d..1a4b5d7 100644
> > --- a/gcc/Makefile.in
> > +++ b/gcc/Makefile.in
> > @@ -1869,6 +1869,12 @@ s-selftest: $(GCC_PASSES) cc1$(exeext) stmp
> > -int-hdrs
> >  selftest-gdb: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
> > $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test -wrapper
> > gdb,--args
> > 
> > +# Convenience method for running selftests under valgrind:
> > +.PHONY: selftest-valgrind
> > +selftest-valgrind: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
> > +   $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test \
> > + -wrapper valgrind,--leak-check=full
> > +
> >  # Recompile all the language-independent object files.
> >  # This is used only if the user explicitly asks for it.
> >  compilations: $(BACKEND)
> > diff --git a/gcc/function-tests.c b/gcc/function-tests.c
> > index c8188e7..edd355f 100644
> > --- a/gcc/function-tests.c
> > +++ b/gcc/function-tests.c
> > @@ -296,6 +296,7 @@ build_cfg (tree fndecl)
> >push_cfun (fun);
> >lower_cf_pass->execute (fun);
> >pop_cfun ();
> > +  delete lower_cf_pass;
> > 
> >/* We can now convert to CFG form; for our trivial test function
> > this
> >   gives us:
> > @@ -310,6 +311,7 @@ build_cfg (tree fndecl)
> >push_cfun (fun);
> >build_cfg_pass->execute (fun);
> >pop_cfun ();
> > +  delete build_cfg_pass;
> >  }
> > 
> >  /* Convert a gimple+CFG function to SSA form.  */
> > @@ -325,6 +327,7 @@ convert_to_ssa (tree fndecl)
> >push_cfun (fun);
> >build_ssa_pass->execute (fun);
> >pop_cfun ();
> > +  delete build_ssa_pass;
> >  }
> > 
> >  /* Assuming we have a simple 3-block CFG like this:
> > @@ -594,6 +597,7 @@ test_expansion_to_rtl ()
> >init_function_start (fndecl);
> >expand_pass->execute (fun);
> >pop_cfun ();
> > +  delete expand_pass;
> > 
> >/* On x86_64, I get this:
> > (note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> > index 0fac49c..6d69435 100644
> > --- 

Re: [PATCH] PR target/71549: Convert V1TImode register to TImode in debug insn

[Markus Trippelsdorf]

On 2016.06.28 at 11:21 -0700, Gary Funck wrote:
> On 06/20/16 04:55:16, H.J. Lu wrote:
> > TImode register referenced in debug insn can be converted to V1TImode
> > by scalar to vector optimization.  We need to convert a debug insn if
> > it has a variable in a TImode register.
> 
> We have a situation on a few of the UPC tests, where they ICE on
> this gcc_assert().
> 
> 3820  gcc_assert (REG_P (loc)
> 3821  && GET_MODE (loc) == V1TImode);
> 
> (gdb) p val
> $2 = (rtx) 0x7fffef6d3978
> (gdb) pr
> warning: Expression is not an assignment (and might have no effect)
> (var_location:TI newval (subreg:TI (reg/v/f:V1TI 307 [ newval ]) 0))
> 
> (gdb) p loc
> $1 = (rtx) 0x7fffef409210
> (gdb) pr
> warning: Expression is not an assignment (and might have no effect)
> (subreg:TI (reg/v/f:V1TI 307 [ newval ]) 0)
> 
> As you can see, 'loc' is already a TI mode subreg based upon a V1TI mode reg.
> 
> I didn't try tracking down how we end up with 'loc' as a subreg, but will
> note that in UPC the pointer-to-shared representation is a 16 byte struct,
> aligned on 16 bytes, so the generated code will frequently deal with TImode
> values in registers.
> 
> Given the code that follows this assert,
> 
> 3822  /* Convert V1TImode register, which has been updated by a 
> SET
> 3823 insn before, to SUBREG TImode.  */
> 3824  PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 
> 0);
> 3825  df_insn_rescan (insn);
> 
> converts the V1TImode register into a TImode subreg, and we already have
> that situation, I tried the following patch:
> 
> --- /a/gcc-trunk/gcc/config/i386/i386.c 2016-06-26 19:01:12.099740515 -0700
> +++ config/i386/i386.c  2016-06-28 11:17:26.323396045 -0700
> @@ -3814,6 +3814,9 @@
> continue;
>   gcc_assert (GET_CODE (val) == VAR_LOCATION);
>   rtx loc = PAT_VAR_LOCATION_LOC (val);
> + /* If already a SUBREG, skip.  */
> + if (SUBREG_P (loc))
> +   continue;
>   gcc_assert (REG_P (loc)
>   && GET_MODE (loc) == V1TImode);
>   /* Convert V1TImode register, which has been updated by a SET
> 
> 
> Can the patch be amended to include this fix?  Let me know if you need
> additional information, or would like me to try something else.

See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71801, which has a
small testcase, that is fixed by Gary's patch, too.

-- 
Markus