C++ PATCH for c++/81671, nullptr_t template parameter

2017-08-10 Thread Jason Merrill
We were wrongly checking for nullptr_node rather than nullptr_type.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 325d154928a702116006f1ca40806cee600d6a8b
Author: Jason Merrill 
Date:   Thu Aug 10 22:05:38 2017 -0700

PR c++/81671 - nullptr_t template parameter

* pt.c (convert_nontype_argument): Fix nullptr_t check.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 0f899b9..bf1f75d 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6879,7 +6879,7 @@ convert_nontype_argument (tree type, tree expr, 
tsubst_flags_t complain)
 }
   else if (NULLPTR_TYPE_P (type))
 {
-  if (expr != nullptr_node)
+  if (!NULLPTR_TYPE_P (TREE_TYPE (expr)))
{
  if (complain & tf_error)
error ("%qE is not a valid template argument for type %qT "
diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr39.C 
b/gcc/testsuite/g++.dg/cpp0x/nullptr39.C
new file mode 100644
index 000..a34a6af
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nullptr39.C
@@ -0,0 +1,15 @@
+// PR c++/81671
+// { dg-do compile { target c++11 } }
+
+namespace std { typedef decltype(nullptr) nullptr_t; }
+
+template struct Bar
+{};
+template struct Bar
+{
+template struct Bind { constexpr static int const cb = 0; 
};
+};
+int foo()
+{
+  return Bar::Bind::cb;
+}


Re: [PING] [PATCH] [AArch64] Add addr_type attribute

2017-08-10 Thread Hurugalawadi, Naveen
Hi,  

Please consider this as a personal reminder to review the patch
at following link and let me know your comments on the same.  

https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01634.html

Thanks,
Naveen


Re: [PING] [PATCH] [AArch64] vec_pack_trunc_ should split after register allocator

2017-08-10 Thread Hurugalawadi, Naveen
Hi,  

Please consider this as a personal reminder to review the patch
at following link and let me know your comments on the same.  

https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01529.html

Thanks,
Naveen



  

[PING 5] [PATCH][AArch64] Add neon_pairwise_add & neon_pairwise_add_q types

2017-08-10 Thread Hurugalawadi, Naveen
Hi,  

Please consider this as a personal reminder to review the patch
at following link and let me know your comments on the same.  

https://gcc.gnu.org/ml/gcc-patches/2017-03/msg00505.html

Thanks,
Naveen





    

[RS6000] PR 80938, Don't emit eh_frame for regs that don't need saving

2017-08-10 Thread Alan Modra
It is possible when using out-of-line register saves or store multiple
to save some registers unnecessarily, for example one reg in the block
saved might be unused.  We don't need to emit eh_frame info for those
registers as that just bloats the eh_frame info, and also can result
in an ICE when shrink-wrap gives multiple paths through the function
saving different sets of registers.  All the join points need to have
identical eh_frame register save state.

This patch reverts the previous fix for PR80939 "Use SAVE_MULTIPLE
only if we restore what it saves (PR80938)" and instead fixes the PR
by correcting the eh_frame info.  The change to rs6000_savres_strategy
is an optimization, but note that it hides the underlying problem in
the PR testcase.

Bootstrapped and regression tested powerpc64-linux (-m32 too) and
powerpc64le-linux, with
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00774.html and
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00775.html applied.
OK to apply?

PR target/80938
* config/rs6000/rs6000.c (rs6000_savres_strategy): Revert 2017-08-09.
Don't use store multiple if only one reg needs saving.
(rs6000_frame_related): Don't emit eh_frame for regs that
don't need saving.
(rs6000_emit_epilogue): Likewise.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2070648..abc55bd 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24432,20 +24432,37 @@ rs6000_savres_strategy (rs6000_stack_t *info,
   && flag_shrink_wrap_separate
   && optimize_function_for_speed_p (cfun)))
 {
-  /* Prefer store multiple for saves over out-of-line routines,
-since the store-multiple instruction will always be smaller.  */
-  strategy |= SAVE_INLINE_GPRS | SAVE_MULTIPLE;
-
-  /* The situation is more complicated with load multiple.  We'd
-prefer to use the out-of-line routines for restores, since the
-"exit" out-of-line routines can handle the restore of LR and the
-frame teardown.  However if doesn't make sense to use the
-out-of-line routine if that is the only reason we'd need to save
-LR, and we can't use the "exit" out-of-line gpr restore if we
-have saved some fprs; In those cases it is advantageous to use
-load multiple when available.  */
-  if (info->first_fp_reg_save != 64 || !lr_save_p)
-   strategy |= REST_INLINE_GPRS | REST_MULTIPLE;
+  int count;
+
+  for (count = 0, i = info->first_gp_reg_save; i < 32; i++)
+   if (save_reg_p (i))
+ count += 1;
+
+  if (count <= 1)
+   /* Don't use store multiple if only one reg needs to be
+  saved.  This can occur for example when the ABI_V4 pic reg
+  (r30) needs to be saved to make calls, but r31 is not
+  used.  */
+   strategy |= SAVE_INLINE_GPRS | REST_INLINE_GPRS;
+  else
+   {
+ /* Prefer store multiple for saves over out-of-line
+routines, since the store-multiple instruction will
+always be smaller.  */
+ strategy |= SAVE_INLINE_GPRS | SAVE_MULTIPLE;
+
+ /* The situation is more complicated with load multiple.
+We'd prefer to use the out-of-line routines for restores,
+since the "exit" out-of-line routines can handle the
+restore of LR and the frame teardown.  However if doesn't
+make sense to use the out-of-line routine if that is the
+only reason we'd need to save LR, and we can't use the
+"exit" out-of-line gpr restore if we have saved some
+fprs; In those cases it is advantageous to use load
+multiple when available.  */
+ if (info->first_fp_reg_save != 64 || !lr_save_p)
+   strategy |= REST_INLINE_GPRS | REST_MULTIPLE;
+   }
 }
 
   /* Using the "exit" out-of-line routine does not improve code size
@@ -24454,21 +24471,6 @@ rs6000_savres_strategy (rs6000_stack_t *info,
   else if (!lr_save_p && info->first_gp_reg_save > 29)
 strategy |= SAVE_INLINE_GPRS | REST_INLINE_GPRS;
 
-  /* We can only use save multiple if we need to save all the registers from
- first_gp_reg_save.  Otherwise, the CFI gets messed up (we save some
- register we do not restore).  */
-  if (strategy & SAVE_MULTIPLE)
-{
-  int i;
-
-  for (i = info->first_gp_reg_save; i < 32; i++)
-   if (fixed_reg_p (i) || !save_reg_p (i))
- {
-   strategy &= ~SAVE_MULTIPLE;
-   break;
- }
-}
-
   /* Don't ever restore fixed regs.  */
   if ((strategy & (REST_INLINE_GPRS | REST_MULTIPLE)) != REST_INLINE_GPRS)
 for (i = info->first_gp_reg_save; i < 32; i++)
@@ -25681,9 +25683,15 @@ rs6000_frame_related (rtx_insn *insn, rtx reg, 
HOST_WIDE_INT val,
 register save functions, or store multiple, then omit
 eh_frame info for any user-defined global regs.  If
 eh_frame 

[RS6000] Merge rs6000_reg_live_or_pic_offset_p into save_reg_p

2017-08-10 Thread Alan Modra
rs6000_reg_live_or_pic_offset_p is just save_reg_p with special
handling for the pic register and eh_return.  This merge also
simplifies the eh_return handling.  The intent of
https://gcc.gnu.org/ml/gcc-patches/2010-09/msg01838.html was to say
the PIC reg needed to be saved for eh_return, not all gprs.  And that
is already done without the crtl->calls_eh_return test on the return
statement.  Of course, it doesn't hurt to say all gprs need to be
saved for eh_return (the target-independent code does that by setting
DF live), but it's unnecessary in the backend.

Bootstrapped and regression tested powerpc64-linux (-m32 too) and
powerpc64le-linux.  OK?

* config/rs6000/rs6000.c (rs6000_reg_live_or_pic_offset_p): Merge..
(save_reg_p): ..into this.  Update all callers.
(first_reg_to_save): Simplify.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index b628808..2070648 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24055,22 +24055,19 @@ rs6000_split_multireg_move (rtx dst, rtx src)
 /* This page contains routines that are used to determine what the
function prologue and epilogue code will do and write them out.  */
 
-static inline bool
-save_reg_p (int r)
-{
-  return !call_used_regs[r] && df_regs_ever_live_p (r);
-}
-
-/* Determine whether the gp REG is really used.  */
+/* Determine whether the REG is really used.  */
 
 static bool
-rs6000_reg_live_or_pic_offset_p (int reg)
+save_reg_p (int reg)
 {
   /* We need to mark the PIC offset register live for the same conditions
  as it is set up, or otherwise it won't be saved before we clobber it.  */
 
   if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
 {
+  /* When calling eh_return, we must return true for all the cases
+where conditional_register_usage marks the PIC offset reg
+call used.  */
   if (TARGET_TOC && TARGET_MINIMAL_TOC
  && (crtl->calls_eh_return
  || df_regs_ever_live_p (reg)
@@ -24082,11 +24079,7 @@ rs6000_reg_live_or_pic_offset_p (int reg)
return true;
 }
 
-  /* If the function calls eh_return, claim used all the registers that would
- be checked for liveness otherwise.  */
-
-  return ((crtl->calls_eh_return || df_regs_ever_live_p (reg))
- && !call_used_regs[reg]);
+  return !call_used_regs[reg] && df_regs_ever_live_p (reg);
 }
 
 /* Return the first fixed-point register that is required to be
@@ -24102,13 +24095,6 @@ first_reg_to_save (void)
 if (save_reg_p (first_reg))
   break;
 
-  if (first_reg > RS6000_PIC_OFFSET_TABLE_REGNUM
-  && ((DEFAULT_ABI == ABI_V4 && flag_pic != 0)
- || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
- || (TARGET_TOC && TARGET_MINIMAL_TOC))
-  && rs6000_reg_live_or_pic_offset_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
-first_reg = RS6000_PIC_OFFSET_TABLE_REGNUM;
-
 #if TARGET_MACHO
   if (flag_pic
   && crtl->uses_pic_offset_table
@@ -26290,7 +26276,7 @@ rs6000_get_separate_components (void)
   for (unsigned regno = info->first_gp_reg_save; regno < 32; regno++)
{
  if (IN_RANGE (offset, -0x8000, 0x7fff)
- && rs6000_reg_live_or_pic_offset_p (regno))
+ && save_reg_p (regno))
bitmap_set_bit (components, regno);
 
  offset += reg_size;
@@ -27031,7 +27017,7 @@ rs6000_emit_prologue (void)
   int offset = info->gp_save_offset + frame_off;
   for (int i = info->first_gp_reg_save; i < 32; i++)
{
- if (rs6000_reg_live_or_pic_offset_p (i)
+ if (save_reg_p (i)
  && !cfun->machine->gpr_is_wrapped_separately[i])
emit_frame_save (frame_reg_rtx, reg_mode, i, offset,
 sp_off - frame_off);
@@ -28481,7 +28467,7 @@ rs6000_emit_epilogue (int sibcall)
   int offset = info->gp_save_offset + frame_off;
   for (i = info->first_gp_reg_save; i < 32; i++)
{
- if (rs6000_reg_live_or_pic_offset_p (i)
+ if (save_reg_p (i)
  && !cfun->machine->gpr_is_wrapped_separately[i])
{
  rtx reg = gen_rtx_REG (reg_mode, i);
@@ -28524,13 +28510,9 @@ rs6000_emit_epilogue (int sibcall)
cfa_restores = add_crlr_cfa_restore (info, cfa_restores);
 
   for (i = info->first_gp_reg_save; i < 32; i++)
-   if (!restoring_GPRs_inline
-   || using_load_multiple
-   || rs6000_reg_live_or_pic_offset_p (i))
+   if (save_reg_p (i)
+   && !cfun->machine->gpr_is_wrapped_separately[i])
  {
-   if (cfun->machine->gpr_is_wrapped_separately[i])
- continue;
-
rtx reg = gen_rtx_REG (reg_mode, i);
cfa_restores = alloc_reg_note (REG_CFA_RESTORE, reg, cfa_restores);
  }

-- 
Alan Modra
Australia Development Lab, IBM


[RS6000] Don't restore fixed regs

2017-08-10 Thread Alan Modra
As noted in fixed_reg_p comment

/* Return whether REG is a global user reg or has been specifed by
   -ffixed-REG.  We should not restore these, and so cannot use
   lmw or out-of-line restore functions if there are any.  We also
   can't save them (well, emit frame notes for them), because frame
   unwinding during exception handling will restore saved registers.  */

* config/rs6000/rs6000.c (rs6000_savres_strategy): Don't restore
fixed regs.

Bootstrapped and regression tested powerpc64-linux (-m32 too) and
powerpc64le-linux.  OK?

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 74158cd..b628808 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24309,6 +24309,7 @@ rs6000_savres_strategy (rs6000_stack_t *info,
bool using_static_chain_p)
 {
   int strategy = 0;
+  int i;
 
   /* Select between in-line and out-of-line save and restore of regs.
  First, all the obvious cases where we don't use out-of-line.  */
@@ -24383,6 +24384,17 @@ rs6000_savres_strategy (rs6000_stack_t *info,
 | SAVE_INLINE_GPRS
 | SAVE_INLINE_VRS);
 
+  /* Don't ever restore fixed regs.  That means we can't use the
+ out-of-line register restore functions if a fixed reg is in the
+ range of regs restored.   */
+  if (!(strategy & REST_INLINE_FPRS))
+for (i = info->first_fp_reg_save; i < 64; i++)
+  if (fixed_regs[i])
+   {
+ strategy |= REST_INLINE_FPRS;
+ break;
+   }
+
   /* We can only use the out-of-line routines to restore fprs if we've
  saved all the registers from first_fp_reg_save in the prologue.
  Otherwise, we risk loading garbage.  Of course, if we have saved
@@ -24390,10 +24402,8 @@ rs6000_savres_strategy (rs6000_stack_t *info,
   if ((strategy & SAVE_INLINE_FPRS)
   && !(strategy & REST_INLINE_FPRS))
 {
-  int i;
-
   for (i = info->first_fp_reg_save; i < 64; i++)
-   if (fixed_regs[i] || !save_reg_p (i))
+   if (!save_reg_p (i))
  {
strategy |= REST_INLINE_FPRS;
break;
@@ -24401,13 +24411,19 @@ rs6000_savres_strategy (rs6000_stack_t *info,
 }
 
   /* Similarly, for altivec regs.  */
+  if (!(strategy & REST_INLINE_VRS))
+for (i = info->first_altivec_reg_save; i < LAST_ALTIVEC_REGNO + 1; i++)
+  if (fixed_regs[i])
+   {
+ strategy |= REST_INLINE_VRS;
+ break;
+   }
+
   if ((strategy & SAVE_INLINE_VRS)
   && !(strategy & REST_INLINE_VRS))
 {
-  int i;
-
   for (i = info->first_altivec_reg_save; i < LAST_ALTIVEC_REGNO + 1; i++)
-   if (fixed_regs[i] || !save_reg_p (i))
+   if (!save_reg_p (i))
  {
strategy |= REST_INLINE_VRS;
break;
@@ -24467,6 +24483,16 @@ rs6000_savres_strategy (rs6000_stack_t *info,
  }
 }
 
+  /* Don't ever restore fixed regs.  */
+  if ((strategy & (REST_INLINE_GPRS | REST_MULTIPLE)) != REST_INLINE_GPRS)
+for (i = info->first_gp_reg_save; i < 32; i++)
+  if (fixed_reg_p (i))
+   {
+ strategy |= REST_INLINE_GPRS;
+ strategy &= ~REST_MULTIPLE;
+ break;
+   }
+
   /* We can only use load multiple or the out-of-line routines to
  restore gprs if we've saved all the registers from
  first_gp_reg_save.  Otherwise, we risk loading garbage.
@@ -24475,10 +24501,8 @@ rs6000_savres_strategy (rs6000_stack_t *info,
   if ((strategy & (SAVE_INLINE_GPRS | SAVE_MULTIPLE)) == SAVE_INLINE_GPRS
   && (strategy & (REST_INLINE_GPRS | REST_MULTIPLE)) != REST_INLINE_GPRS)
 {
-  int i;
-
   for (i = info->first_gp_reg_save; i < 32; i++)
-   if (fixed_reg_p (i) || !save_reg_p (i))
+   if (!save_reg_p (i))
  {
strategy |= REST_INLINE_GPRS;
strategy &= ~REST_MULTIPLE;

-- 
Alan Modra
Australia Development Lab, IBM


[RS6000] linux startfile/endfile

2017-08-10 Thread Alan Modra
These need to match the gnu-user.h definitions to support
--enable-default-pie.  Otherwise we end up linking the wrong startup
files when defaulting to PIE.

I've just copied the HAVE_LD_PIE variants as we don't need the
!HAVE_LD_PIE variant.  (In fact, gnu-user.h doesn't need them either.)

Bootstrapped and regression tested powerpc64-linux and
powerpc64le-linux.  OK?

PR target/81170
PR target/81295
* config/rs6000/sysv4.h (STARTFILE_LINUX_SPEC): Upgrade to
match gnu-user.h startfile.
(ENDFILE_LINUX_SPEC): Similarly.

diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
index de38629..cbee891 100644
--- a/gcc/config/rs6000/sysv4.h
+++ b/gcc/config/rs6000/sysv4.h
@@ -757,24 +757,34 @@ ENDIAN_SELECT(" -mbig", " -mlittle", DEFAULT_ASM_ENDIAN)
 #define CRTOFFLOADEND ""
 #endif
 
-#ifdef HAVE_LD_PIE
-#defineSTARTFILE_LINUX_SPEC "\
-%{!shared: %{pg|p|profile:gcrt1.o%s;pie:Scrt1.o%s;:crt1.o%s}} \
-%{mnewlib:ecrti.o%s;:crti.o%s} \
-%{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s} \
-" CRTOFFLOADBEGIN
-#else
-#defineSTARTFILE_LINUX_SPEC "\
-%{!shared: %{pg|p|profile:gcrt1.o%s;:crt1.o%s}} \
-%{mnewlib:ecrti.o%s;:crti.o%s} \
-%{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s} \
-" CRTOFFLOADBEGIN
-#endif
-
-#defineENDFILE_LINUX_SPEC "\
-%{shared|pie:crtendS.o%s;:crtend.o%s} \
-%{mnewlib:ecrtn.o%s;:crtn.o%s} \
-" CRTOFFLOADEND
+/* STARTFILE_LINUX_SPEC should be the same as GNU_USER_TARGET_STARTFILE_SPEC
+   but with the mnewlib ecrti.o%s selection substituted for crti.o%s.  */
+#defineSTARTFILE_LINUX_SPEC \
+  "%{shared:; \
+ pg|p|profile:gcrt1.o%s; \
+ static:crt1.o%s; \
+ " PIE_SPEC ":Scrt1.o%s; \
+ :crt1.o%s} \
+   %{mnewlib:ecrti.o%s;:crti.o%s} \
+   %{static:crtbeginT.o%s; \
+ shared|" PIE_SPEC ":crtbeginS.o%s; \
+ :crtbegin.o%s} \
+   %{fvtable-verify=none:%s; \
+ fvtable-verify=preinit:vtv_start_preinit.o%s; \
+ fvtable-verify=std:vtv_start.o%s} \
+   " CRTOFFLOADBEGIN
+
+/* ENDFILE_LINUX_SPEC should be the same as GNU_USER_TARGET_ENDFILE_SPEC
+   but with the mnewlib ecrtn.o%s selection substituted for crtn.o%s.  */
+#define ENDFILE_LINUX_SPEC \
+  "%{fvtable-verify=none:%s; \
+ fvtable-verify=preinit:vtv_end_preinit.o%s; \
+ fvtable-verify=std:vtv_end.o%s} \
+   %{static:crtend.o%s; \
+ shared|" PIE_SPEC ":crtendS.o%s; \
+ :crtend.o%s} \
+   %{mnewlib:ecrtn.o%s;:crtn.o%s} \
+   " CRTOFFLOADEND
 
 #define LINK_START_LINUX_SPEC ""
 

-- 
Alan Modra
Australia Development Lab, IBM


Re: [PATCH] PR libstdc++/53984 handle exceptions in basic_istream::sentry

2017-08-10 Thread Jonathan Wakely

On 26/07/17 23:06 +0100, Jonathan Wakely wrote:

On 26/07/17 20:14 +0200, Paolo Carlini wrote:

Hi again,

On 26/07/2017 16:27, Paolo Carlini wrote:

Hi,

On 26/07/2017 16:21, Andreas Schwab wrote:
ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: 
dg-require-file-io 18 {} for " dg-require-file-io 18 "" "

Should be already fixed, a trivial typo.
... but now the new test simply fails for me. If I don't spot 
something else trivial over the next few hours I guess better 
waiting for Jon to look into that.


Sorry about that, I must have only checked for FAILs and missed the
ERRORs.

It should have been an ifstream not fstream, otherwise the filebuf
can't even open the file. Fixed like so, committed to trunk.


This FAILs on AIX, because reading from a directory with fread doesn't
fail on AIX. This makes the test check whether reading fails, before
trying to check the behaviour of formatted input functions that fail.

Tested powerpc64le-linux and powerpc-aix, committed to trunk.

commit bb7f3e33c3442f2d93134555bb74cf3ea2991710
Author: Jonathan Wakely 
Date:   Thu Aug 10 22:02:17 2017 +0100

PR libstdc++/81808 skip test if reading directory doesn't fail

	PR libstdc++/81808
	* testsuite/27_io/basic_fstream/53984.cc: Adjust test for targets
	that allow opening a directory as a FILE and reading from it.

diff --git a/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc b/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
index e49d2b1..a319aff 100644
--- a/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
@@ -17,6 +17,8 @@
 
 // { dg-require-fileio "" }
 
+// PR libstdc++/53984
+
 #include 
 #include 
 
@@ -26,9 +28,32 @@ test01()
   std::ifstream in(".");
   if (in)
   {
+char c;
+if (in.get(c))
+{
+  // Reading a directory doesn't produce an error on this target
+  // so the formatted input functions below wouldn't fail anyway
+  // (see PR libstdc++/81808).
+  return;
+}
 int x;
+in.clear();
+// Formatted input function should set badbit, but not throw:
 in >> x;
 VERIFY( in.bad() );
+
+in.clear();
+in.exceptions(std::ios::badbit);
+try
+{
+  // Formatted input function should set badbit, and throw:
+  in >> x;
+  VERIFY( false );
+}
+catch (const std::exception&)
+{
+  VERIFY( in.bad() );
+}
   }
 }
 


Re: [PATCH] Do not instrument void variables with MPX (PR tree-opt/79987).

2017-08-10 Thread Ilya Enkovich
2017-08-10 10:40 GMT+03:00 Martin Liška :
> Hello.
>
> In order to prevent the ICE, CHKP should not isntrument variables of void 
> type.

Hi,

There was another thread for this PR where I proposed a way to handle such vars
via size relocations. But there was no feedback in that thread for two
months and
skipping void var is better than ICE. The patch is OK then.

Thanks,
Ilya

>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?
> Martin
>
> gcc/ChangeLog:
>
> 2017-08-09  Martin Liska  
>
> PR tree-opt/79987
> * tree-chkp.c (chkp_get_bounds_for_decl_addr): Do not instrument
> variables of void type.
>
> gcc/testsuite/ChangeLog:
>
> 2017-08-09  Martin Liska  
>
> PR tree-opt/79987
> * gcc.target/i386/mpx/pr79987.c: New test.
> ---
>  gcc/testsuite/gcc.target/i386/mpx/pr79987.c | 5 +
>  gcc/tree-chkp.c | 3 +++
>  2 files changed, 8 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/mpx/pr79987.c
>
>


Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

2017-08-10 Thread Joseph Myers
On Thu, 10 Aug 2017, Ximin Luo wrote:

> Normally, system-wide CFLAGS etc are a static expression of policy. What I 
> mean
> by that is, writing logic to determine CFLAGS for particular package, would
> "look like" the actual wording of that policy. So for example, "all packages
> should have debug" => "CFLAGS+=-g", "packages that match property X should 
> have
> flag Y" => "if X then CFLAGS += Y". OTOH if your policy is "packages should be
> reproducible" and we have to add these prefix-remapping flags to CFLAGS, 
> you're
> adding a dependency from the value of CFLAGS itself, onto the build-time
> filesystem. So CFLAGS is no longer a static expression of policy, it depends 
> on
> information not part of the policy.
> 
> So, prefix-map is really different from other CFLAGS, it adds extra dependency
> relationships that other flags don't add.

I think the difference is essentially that these options belong in CC not 
CFLAGS (CFLAGS is for non-semantic options, CC should include any options 
that are required in every compilation).

(FWIW, I support having the environment variable, but think there should 
be some command-line option exactly equivalent to it.)

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH] Factor out division by squares and remove division around comparisons (1/2)

2017-08-10 Thread Joseph Myers
On Thu, 10 Aug 2017, Jackson Woodruff wrote:

> > We also change:
> > 
> >   x / (- y) -> (-x) / y
> > 
> > Which requires -fno-trapping-math.

I don't see why that requires -fno-trapping-math.  The exceptions should 
be identical from both variants, as should the result, as far as defined 
by C or IEEE 754 (it's possible it might affect the sign of a NaN result 
on some architectures, but only where that sign is unspecified in C and 
IEEE 754 terms, so such a change is OK).

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

2017-08-10 Thread Joseph Myers
On Wed, 9 Aug 2017, Martin Sebor wrote:

> The problem isn't that the declarations aren't merged at the call
> site but rather that the middle-end gives const precedence over
> pure so when both attributes are provided the former wins.

But that precedence is correct.  Any function with the semantics of const 
also has the semantics of pure.  The problem is that the function doesn't 
have the semantics of the attribute it's declared with.  And any 
diagnostic based purely on the presence of both attributes would be 
essentially a style diagnostic - for the style principle "if you have 
multiple declarations of a function, they should have the same 
information" - rather than "these attributes are inconsistent".

(An optional warning for different information in different declarations 
is reasonable enough.  Actually in glibc it would be useful to have a 
warning for a different but related case - two functions both declared, 
later one defined as an alias of another, but the declarations don't have 
the same attributes.  I'm pretty sure there are cases where the internal 
declaration of __foo is missing attributes present on the public 
declaration of foo.  But such a warning for aliases is only appropriate 
for attributes that are properties of the function, not attributes that 
are properties of particular names for it - __foo may well be a hidden 
symbol where foo isn't.)

-- 
Joseph S. Myers
jos...@codesourcery.com


[PATCH, rs6000] More diagnostic cleanup, addressing PR79845

2017-08-10 Thread Bill Schmidt
Hi,

This continues the cleanup of diagnostic messages in the rs6000 back end.  The
primary focus is to make sure that we use quoted strings with %qs, %<, %> as
appropriate, and that option strings are separated from translatable strings
to make things easier on the internationalization folks, as requested in
PR79845.  While working on this, I noticed a couple of places where the
diagnostic strings result in excessively long lines, and cleaned these up as
well.

One peculiarity I noticed, but did not attempt to address, is that a small
handful of diagnostic strings are tagged with _N ().  There doesn't seem to
be any rhyme or reason to this.  I expect it's a result of copy/paste from
somewhere and most of these strings should be translated.  But that's for
another day (and probably another person).

Bootstrapped and tested on powerpc64le-linux-gnu (POWER8 64-bit) and on
powerpc64-linux-gnu (POWER7 32- and 64-bit) with no regressions.  Is this
okay for trunk?

Thanks,
Bill


[gcc]

2017-08-10  Bill Schmidt  

PR target/79845
* config/rs6000/linux64.h (INVALID_64BIT): Use quoted strings.
* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
Likewise.
* config/rs6000/rs6000.c (rs6000_init_hard_regno_mode_ok): Use
quoted strings, and make more translator-friendly.
(darwin_rs6000_override_options): Likewise.
(rs6000_option_override_internal): Likewise.
(rs6000_return_in_memory): Fix overlong line.
(init_cmulative_args): Use quoted strings, and make more
translator-friendly.
(rs6000_pass_by_reference): Fix overlong line.
(def_builtin): Use quoted strings.
(altivec_expand_predicate_builtin): Use quoted strings, and make
more translator-friendly.
(htm_expand_builtin): Use quoted strings.
(cpu_expand_builtin): Use quoted strings, and make more
translator-friendly.
(altivec_expand_builtin): Likewise.
(paired_expand_predicate_builtin): Likewise.
(rs6000_invalid_builtin): Likewise.
(builtin_function_type): Use quoted strings.
(rs6000_expand_split_stack_prologue): Use quoted strings, and make
more translator-friendly.
(rs6000_trampoline_init): Likewise.
(rs6000_handle_altivec_attribute): Likewise.
(rs6000_inner_target_options): Use quoted strings.
(rs6000_disable_incompatible_switches): Likewise.
* config/rs6000/sysv4.h (SUBTARGET_OVERRIDE_OPTIONS): Use quoted
strings, and make more translator-friendly.
(SUBSUBTARGET_OVERRIDE_OPTIONS): Use quoted strings.

[gcc/testsuite]

2017-08-10  Bill Schmidt  

PR target/79845
* g++.dg/ext/altivec-cell-5.C: Adjust diagnostic strings.
* gcc.target/powerpc/altivec-cell-5.c: Likewise.
* gcc.target/powerpc/bfp/scalar-cmp-exp-eq-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-cmp-exp-gt-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-cmp-exp-lt-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-cmp-exp-unordered-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-exp-1.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-exp-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-exp-4.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-1.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-4.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-1.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-10.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-4.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-5.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-7.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-8.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-data-class-11.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-data-class-6.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-data-class-7.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-neg-2.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-neg-3.c: Likewise.
* gcc.target/powerpc/bfp/scalar-test-neg-5.c: Likewise.
* gcc.target/powerpc/bfp/vec-extract-exp-2.c: Likewise.
* gcc.target/powerpc/bfp/vec-extract-exp-3.c: Likewise.
* gcc.target/powerpc/bfp/vec-extract-sig-2.c: Likewise.
* gcc.target/powerpc/bfp/vec-extract-sig-3.c: Likewise.
* gcc.target/powerpc/bfp/vec-insert-exp-2.c: Likewise.
* 

[PATCH][compare-elim] Merge zero-comparisons with normal ops

2017-08-10 Thread Michael Collison
Hi all,

One issue that we keep encountering on aarch64 is GCC not making good use of 
the flag-setting arithmetic instructions
like ADDS, SUBS, ANDS etc. that perform an arithmetic operation and compare the 
result against zero.
They are represented in a fairly standard way in the backend as PARALLEL 
patterns:
(parallel [(set (reg x1) (op (reg x2) (reg x3)))
   (set (reg cc) (compare (op (reg x2) (reg x3)) (const_int 0)))])

GCC isn't forming these from separate arithmetic and comparison instructions as 
aggressively as it could.
A particular pain point is when the result of the arithmetic insn is used 
before the comparison instruction.
The testcase in this patch is one such example where we have:
(insn 7 35 33 2 (set (reg/v:SI 0 x0 [orig:73  ] [73])
(plus:SI (reg:SI 0 x0 [ x ])
(reg:SI 1 x1 [ y ]))) "comb.c":3 95 {*addsi3_aarch64}
 (nil))
(insn 33 7 34 2 (set (reg:SI 1 x1 [77])
(plus:SI (reg/v:SI 0 x0 [orig:73  ] [73])
(const_int 2 [0x2]))) "comb.c":4 95 {*addsi3_aarch64}
 (nil))
(insn 34 33 17 2 (set (reg:CC 66 cc)
(compare:CC (reg/v:SI 0 x0 [orig:73  ] [73])
(const_int 0 [0]))) "comb.c":4 391 {cmpsi}
 (nil))

This scares combine away as x0 is used in insn 33 as well as the comparison in 
insn 34.
I think the compare-elim pass can help us here.

This patch extends it by handling comparisons against zero, finding the 
defining instruction of the compare
and merging the comparison with the defining instruction into a PARALLEL that 
will hopefully match the form
described above. If between the comparison and the defining insn we find an 
instruction that uses the condition
registers or any control flow we bail out, and we don't cross basic blocks.
This simple technique allows us to catch cases such as this example.

Bootstrapped and tested on arm-none-linux-gnueabihf, aarch64-none-linux-gnu and 
x86_64.

Ok for trunk?

2017-08-05  Kyrylo Tkachov  
Michael Collison 

* compare-elim.c: Include emit-rtl.h.
(can_merge_compare_into_arith): New function.
(try_validate_parallel): Likewise.
(try_merge_compare): Likewise.
(try_eliminate_compare): Call the above when no previous clobber
is available.
(execute_compare_elim_after_reload): Add DF_UD_CHAIN and DF_DU_CHAIN
dataflow problems.

2017-08-05  Kyrylo Tkachov  
Michael Collison 

* gcc.target/aarch64/cmpelim_mult_uses_1.c: New test.


pr5198v1.patch
Description: pr5198v1.patch


Re: [PATCH, i386]: Remove UNSPEC_STACK_CHECK ...

2017-08-10 Thread Uros Bizjak
On Thu, Aug 10, 2017 at 8:32 PM, Uros Bizjak  wrote:
> ... and use address spaces to load boundary value from TLS block.
>
> The patch removes special handling for boundary value location.

I have renamed ix86_split_stack_boundary to ix86_split_stack_guard
(and added missing function comment), so the new ChangeLog reads:

2017-08-10  Uros Bizjak  

* config/i386/i386-protos.h (ix86_split_stack_guard): New prototype.
* config/i386/i386.c (ix86_split_stack_guard): New function.
(ix86_xpand_split_stack_prologue): Call ix86_split_stack_guard.
(ix86_legitimate_address_p) : Remove.
(i386_asm_output_addr_const_extra) : Ditto.
(optput_pic_addr_const): Remove UNSPEC_STACK_CHECK handling.
* config/i386/i386.md (unspec): Remove UNSPEC_STACK_CHECK.
(split_stack_space_check): Call ix86_split_stack_guard.

Uros.


Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

2017-08-10 Thread Ximin Luo
Yury Gribov:
> On Thu, Aug 3, 2017 at 12:45 PM, Ximin Luo  wrote:
>> Yury Gribov:
>>> [..]
>>>
>>> Shouldn't -fdebug-prefix-map be updated to use the same syntax as 
>>> BUILD_PATH_PREFIX_MAP?
>>>
>>
>> -fdebug-prefix-map is a CLI option and can be given multiple times, each 
>> flag given is in the form of $from=$to where $from can't contain a '='.
>>
>> BUILD_PATH_PREFIX_MAP is a single envvar that encodes a list-of-pairs of the 
>> form $to=$from:$to=$from with some escaping for flexibility and to support 
>> things like windows paths. Since it's a new envvar, Ian Jackson suggested 
>> $to=$from to emphasise the reproducible ($to) part. I liked the idea so I 
>> implemented it like that. (We did a lot of bikeshedding over on the 
>> rb-general mailing list about the exact format and this is what we settled 
>> on, I'd like to avoid getting into that again but would nevertheless do it, 
>> if it's necessary to get this patch accepted.)
>>
>> Because -fdebug-prefix-map currently only encodes one $from=$to pair, it 
>> would be a very disruptive and highly backward-incompatible change to make 
>> it use the same syntax as B_P_P_M. A slightly less disruptive but still 
>> backward-incompatible change would be to make it encode a single $to=$from 
>> pair, but I don't really see the advantage to doing so - what were your 
>> thoughts on this?
> 
> I believe it would much easier to reason about environment variable
> behavior when it boils down to "prepend some standard flag to
> command-line flags".  It would also simplify maintenance of local
> compiler patch as core functionality can be merged to mainline GCC
> whereas debatable environment variable part stays in the distro.
> 

[answered in another email together with other related points]

>> If by "first class option" you meant a command-line flag, GCC *already has* 
>> that (-fdebug-prefix-map) and it wasn't enough to achieve reproducibility in 
>> many cases we tested.
>> dpkg-buildflags actually already adds these flags to CFLAGS CXXFLAGS etc on 
>> Debian. However, with this patch using the environment variable, we are able 
>> to reproduce 1800 more packages out of 26000.
> 
> Just curious, why -fdebug-prefix-map (maybe modified to support
> multiple renames) was not enough for these packages (and why they
> can't be fixed instead)?
> 

One important reason is that some packages embed CFLAGS/CXXFLAGS in build
output such as pkg-config files or Makefiles to be installed as examples. To
fix this, we'd have to add buildsystem-specific logic to strip out
-fdebug-prefix-map when it was writing such output. This does not affect all of
these 1800 packages, but I saw enough cases that I was convinced that the use
of a new envvar was a better approach - I don't think buildsystems should be
burdened with having to know "which flag values are reproducible vs not", this
is not the case with other CFLAGS.

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git


Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

2017-08-10 Thread Ximin Luo
Jakub Jelinek:
> On Fri, Aug 04, 2017 at 08:32:33AM -0400, Matthias Klose wrote:
 GCC already supports a similar environment variable SOURCE_DATE_EPOCH, 
 which was accepted about 2 years ago in a patch written by one of our GSoC 
 students. We are not planning any more environment variables like this, 
 and are committed to fixing other sources of non-determinism by patching 
 the relevant build scripts.
>>> I would have rejected that as well :-)  One of the few times I would
>>> have disagreed with Bernd.
>>
>> You can argue that gcc has command line options to set these, but most build
>> systems can be influenced by well documented environment variables like 
>> CFLAGS,
>> CXXFLAGS, LDFLAGS, so adding one more variable like SOURCE_DATE_EPOCH makes
>> sense, considering that many tools dealing with timestamps don't even have
>> command line options to do these (and there it's not just about compilers).
> 
> Unlike SOURCE_DATE_EPOCH, the other env vars are autoconf/cmake etc.
> related, we really shouldn't be adding more of these.
> If some package has messed up build system, you can use
> CXX='g++ -fwhatever'
> or whatever other way to pass flags you want to the compiler or pick the
> compiler you prefer to use.
> 

As I said, this is the only envvar that we're planning to add, we're not
planning to add any more. Please also see my point in my other email, about
how this is really more of a "cancel-out already-existing environment" piece of
data, different from other first-class options to GCC.

It would be really nice not to have to patch 1800 packages to make them
reproducible.

X

>> [..]

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git


Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

2017-08-10 Thread Ximin Luo
Yury Gribov:
> [..]
> 
> In general any distro provides some way to set default parameters for
> every package.  Adding hacks to compiler to work around limitations of
> particular build system does not sound right.
> 
> Here's a relevant discussion from binutils ML:
> https://sourceware.org/ml/binutils/2009-05/msg00086.html
> 

I would argue that what is being worked around, is not a limitation of a
particular buildsystem but rather a limitation of the compiler and the
language. It is GCC itself that generates the unreproducible output, and this
issues affects all buildsystems. They do not explicitly request unreproducible
output, so the fix should not be the responsibility of buildsystem.

GCC takes the input from an environment-like object (the filesystem). Part of
this input (some prefix) differs between different builds, through no fault of
any particular buildsystem but rather through the "fault" of the user running
the build - they put the source and/or build directory under that path.

Since no buildsystem is responsible for this part of the filesystem layout, it
does not seem appropriate to me, to require buildsystems to add logic to deal
with this - e.g. to pass this information down through layers of the tooling
stack, to be eventually consumed by the GCC command line. Instead, it seems
much cleaner for the "fix" to be supplied by the user (the same entity that
controls the filesystem) via some other environment-like object (here, a UNIX
envvar), and for GCC to read this directly just like it reads the abspath() /
realpath() from the filesystem environment.

CFLAGS/CXXFLAGS is also not appropriate because these flags are meant to
directly affect how GCC works. By contrast, these prefix-map options are meant
to *cancel out* already-existing sources of unreproducibility from the
filesystem environment; they do not affect the output pro-actively like other
first-class options do. Also, adding -fdebug-prefix-map to CFLAGS/CXXFLAGS
causes unreproducibility elsewhere.

(from another email)
> I believe it would much easier to reason about environment variable
> behavior when it boils down to "prepend some standard flag to
> command-line flags".  It would also simplify maintenance of local
> compiler patch as core functionality can be merged to mainline GCC
> whereas debatable environment variable part stays in the distro.

To re-iterate my point: I don't think it's right to think of these prefix maps
as the same as other command line flags. They *cancel out* already-existing
sources of unreproducibility from the filesystem environment that GCC is
*already reading*, that already makes the output hard-to-reason-about (i.e.
unreproducible). So I think it is more appropriate to pass them in an
environment-like object, rather than via command-line flags.

Normally, system-wide CFLAGS etc are a static expression of policy. What I mean
by that is, writing logic to determine CFLAGS for particular package, would
"look like" the actual wording of that policy. So for example, "all packages
should have debug" => "CFLAGS+=-g", "packages that match property X should have
flag Y" => "if X then CFLAGS += Y". OTOH if your policy is "packages should be
reproducible" and we have to add these prefix-remapping flags to CFLAGS, you're
adding a dependency from the value of CFLAGS itself, onto the build-time
filesystem. So CFLAGS is no longer a static expression of policy, it depends on
information not part of the policy.

So, prefix-map is really different from other CFLAGS, it adds extra dependency
relationships that other flags don't add.

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git


Re: C++ PATCH for c++/81359, Unparsed NSDMI error in SFINAE context

2017-08-10 Thread Jason Merrill
On Thu, Aug 10, 2017 at 12:13 AM, Markus Trippelsdorf
 wrote:
> On 2017.08.09 at 14:30 -0400, Jason Merrill wrote:
>> The issue here is that we try to determine the EH specification of
>> B::C::C() from within SFINAE context, and we can't determine it yet
>> because the NSDMI for B::C::i hasn't been parsed yet.  This patch
>> allows that determination to fail quietly in SFINAE context; we'll try
>> again the next time it is needed.
>
> Thanks.
>
> Unfortunately it breaks the following testcase:

Fixed thus:
commit 98a57ff77c97446c0477bea2aed831b62a0726a6
Author: Jason Merrill 
Date:   Thu Aug 10 12:23:12 2017 -0700

Fix regression from 81359 patch.

* method.c (synthesized_method_walk): Don't diagnose lack of
operator delete.

diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index bff9605..809ebc8 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1693,12 +1693,18 @@ synthesized_method_walk (tree ctype, 
special_function_kind sfk, bool const_p,
 
   if (check_vdtor && type_has_virtual_destructor (BINFO_TYPE (base_binfo)))
{
- fn = locate_fn_flags (ctype, cp_operator_id (DELETE_EXPR),
-   ptr_type_node, flags, complain);
  /* Unlike for base ctor/op=/dtor, for operator delete it's fine
 to have a null fn (no class-specific op delete).  */
- if (fn && fn == error_mark_node && deleted_p)
-   *deleted_p = true;
+ fn = locate_fn_flags (ctype, cp_operator_id (DELETE_EXPR),
+   ptr_type_node, flags, tf_none);
+ if (fn && fn == error_mark_node)
+   {
+ if (complain & tf_error)
+   locate_fn_flags (ctype, cp_operator_id (DELETE_EXPR),
+ptr_type_node, flags, complain);
+ if (deleted_p)
+   *deleted_p = true;
+   }
  check_vdtor = false;
}
 }
diff --git a/gcc/testsuite/g++.dg/inherit/vdtor1.C 
b/gcc/testsuite/g++.dg/inherit/vdtor1.C
new file mode 100644
index 000..caba17f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/inherit/vdtor1.C
@@ -0,0 +1,7 @@
+struct A {
+  void operator delete(void *, unsigned long);
+};
+struct B : A {
+  virtual ~B();
+};
+struct C : B {};


Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.

2017-08-10 Thread Jason Merrill

On 07/14/2017 01:35 AM, Martin Liška wrote:

On 05/01/2017 09:13 PM, Jason Merrill wrote:

On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška  wrote:

On 04/25/2017 01:58 PM, Jakub Jelinek wrote:

On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:

Hello.

This is patch that was originally installed by Jason and later reverted due to 
PR70422.
In the later PR Richi suggested a fix for that and Segher verified that it 
helped him
to survive regression tests. That's reason why I'm resending that.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin



>From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 20 Apr 2017 14:56:30 +0200
Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
  symbol entry.

gcc/cp/ChangeLog:

2017-04-20  Jason Merrill  
  Martin Liska  
  Segher Boessenkool  

  PR c++/64266
  PR c++/70353
  PR bootstrap/70422
  Core issue 1962
  * decl.c (cp_fname_init): Decay the initializer to pointer.
  (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
  * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
  DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
  DECL_IGNORED_P.  Don't call cp_finish_decl.


If we don't emit those into the debug info, will the debugger be
able to handle __FUNCTION__ etc. properly?


No, debugger with the patch can't handled these. Similar to how clang
behaves currently. Maybe it can be conditionally enabled with -g3, or -g?


Admittedly, right now we emit it into debug info only if those decls
are actually used, say on:
const char * foo () { return __FUNCTION__; }
const char * bar () { return ""; }
we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
has to have some handling of it anyway.  But while in functions
that don't refer to __FUNCTION__ it is always the debugger that needs
to synthetize those and thus they will be always pointer-equal,
if there are some uses of it and for other uses the debugger would
synthetize it, there is the possibility that the debugger synthetized
string will not be the same object as actually used in the function.


You're right, currently one has to use a special function to be able to
print it in debugger. I believe we've already discussed that, according
to spec, the strings don't have to point to a same string.

Suggestions what we should do with the patch?


We need to emit debug information for these variables.  From Jim's
description in 70422 it seems that the problem is that the reference
to the string from the debug information is breaking
function_mergeable_rodata_prefix, which relies on
current_function_decl.  It seems to me that its callers should pass
along their decl parameter so that f_m_r_p can use the decl's
DECL_CONTEXT rather than rely on current_function_decl being set
properly>
Jason



Ok, after some time I returned back to it. I followed your advises and
changed the function function_mergeable_rodata_prefix. Apart from a small
rebase was needed.

May I ask Jim to test the patch?
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.



+  DECL_IGNORED_P (decl) = 1;


As I said before, we do need to emit debug information for these 
variables, so this is wrong.



-  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
+  tree decl = current_function_decl;
+  if (decl && DECL_CONTEXT (decl)
+  && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+decl = DECL_CONTEXT (decl);


I don't see how this would help; it still relies on 
current_function_decl being set correctly, which was the problem we were 
running into before.


Jason


Re: [PATCH 3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

2017-08-10 Thread Martin Sebor

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 016f68d..1aa9e22 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c

[ ... ]

+
+  if (TREE_CODE (type) == ARRAY_TYPE)
+{
+  /* Return the constant size unless it's zero (that's a zero-length
+array likely at the end of a struct).  */
+  tree size = TYPE_SIZE_UNIT (type);
+  if (size && TREE_CODE (size) == INTEGER_CST
+ && !integer_zerop (size))
+   return size;
+}

Q. Do we have a canonical test for the trailing array idiom?   In some
contexts isn't it size 1?  ISTM This test needs slight improvement.
Ideally we'd use some canonical test for detect the trailing array idiom
rather than open-coding it here.  You might look at the array index
warnings in tree-vrp.c to see if it's got a canonical test you can call
or factor and use.


You're right, there is an API for this (array_at_struct_end_p,
as Richard pointed out).  I didn't want to use it because it
treats any array at the end of a struct as a flexible array
member, but simple tests show that that's what -Wstringop-
overflow does now, and it wasn't my intention to tighten up
the checking under this change.  It surprises me that no tests
exposed this. Let me relax the check and think about proposing
to tighten it up separately.


@@ -3883,6 +3920,30 @@ expand_builtin_strncat (tree exp, rtx)
   return NULL_RTX;
 }

+/* Helper to check the sizes of sequences and the destination of calls
+   to __builtin_strncpy (DST, SRC, LEN) and __builtin___strncpy_chk.
+   Returns true on success (no overflow warning), false otherwise.  */
+
+static bool
+check_strncpy_sizes (tree exp, tree dst, tree src, tree len)
+{
+  tree dstsize = compute_objsize (dst, warn_stringop_overflow - 1);
+
+  if (!check_sizes (OPT_Wstringop_overflow_,
+   exp, len, /*maxlen=*/NULL_TREE, src, dstsize))
+return false;
+
+  if (!dstsize || TREE_CODE (len) != INTEGER_CST)
+return true;
+
+  if (tree_int_cst_lt (dstsize, len))
+warning_at (EXPR_LOCATION (exp), OPT_Wstringop_truncation,
+   "%K%qD specified bound %E exceeds destination size %E",
+   exp, get_callee_fndecl (exp), len, dstsize);
+
+  return true;

So in the case where you issue the warning, what should the return value
be?  According to the comment it should be false.  It looks like you got
the wrong return value for the tree_int_cst_lt (dstsize, len) test.


Corrected.  The return value is unused by the only caller so
there is no test to exercise it.


   return false;
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index b0563fe..ac6503f 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c



+
+/* A helper of handle_builtin_stxncpy.  Check to see if the specified
+   bound is a) equal to the size of the destination DST and if so, b)
+   if it's immediately followed by DST[LEN - 1] = '\0'.  If a) holds
+   and b) does not, warn.  Otherwise, do nothing.  Return true if
+   diagnostic has been issued.
+
+   The purpose is to diagnose calls to strncpy and stpncpy that do
+   not nul-terminate the copy while allowing for the idiom where
+   such a call is immediately followed by setting the last element
+   to nul, as in:
+ char a[32];
+ strncpy (a, s, sizeof a);
+ a[sizeof a - 1] = '\0';
+*/

So using gsi_next to find the next statement could make the heuristic
fail to find the a[sizeof a - 1] = '\0'; statement when debugging is
enabled.

gsi_next_nondebug would be better as it would skip over any debug insns.


Thanks.  I'll have to remember this.


What might be even better would be to use the immediate uses of the
memory tag.  For your case there should be only one immediate use and it
should point to the statement which NUL terminates the destination.  Or
maybe that would be worse in that you only want to allow this exception
when the statements are consecutive.


I'll have to try this to better understand how it might work.


+  /* Look for dst[i] = '\0'; after the stxncpy() call and if found
+ avoid the truncation warning.  */
+  gsi_next ();
+  gimple *next_stmt = gsi_stmt (gsi);

Here's the gsi_next I'm referring to.



+  else
+{
+  /* The source length is uknown.  Try to determine the destination

s/uknown/unknown/




 /* Handle a memcpy-like ({mem{,p}cpy,__mem{,p}cpy_chk}) call.
If strlen of the second argument is known and length of the third argument
is that plus one, strlen of the first argument is the same after this
@@ -2512,6 +2789,19 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)

You still need to rename strlen_optimize_stmt since after your changes
it does both optimizations and warnings.


I'm not sure I understand why.  It's a pre-existing function that
just dispatches to the built-in handlers.  We don't rename function
callers each time we improve error/warning detection in some
function they call (case in point: all the expanders in builtins.c)
Why do it here?  And what would be a suitable name?  All that 

C++ PATCH for c++/80452, Core 1579: implicit move semantics on return/throw

2017-08-10 Thread Jason Merrill
Jon's earlier patch expanded our existing implicit move semantics
support to allow other types, but this testcase demonstrates a hole in
that support: it didn't affect function template argument deduction.
Rather than try to tweak deduction as well as reference binding, this
patch actually does overload resolution twice, as specified in the
standard.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 26828c017926fc2b32f6bb399463f0525b08ebac
Author: Jason Merrill 
Date:   Thu Aug 10 11:47:27 2017 -0400

PR c++/80452 - Core 1579, implicit move semantics on return/throw

* cp-tree.h (LOOKUP_PREFER_RVALUE): Now means that we've already
tentatively changed the lvalue to an rvalue.
* call.c (reference_binding): Remove LOOKUP_PREFER_RVALUE handling.
(build_over_call): If LOOKUP_PREFER_RVALUE, check that the first
parameter is an rvalue reference.
* except.c (build_throw): Do maybe-rvalue overload resolution twice.
* typeck.c (check_return_expr): Likewise.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 4903119..3790299 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -101,7 +101,7 @@ struct conversion {
   /* If KIND is ck_ref_bind, true when either an lvalue reference is
  being bound to an lvalue expression or an rvalue reference is
  being bound to an rvalue expression.  If KIND is ck_rvalue,
- true when we should treat an lvalue as an rvalue (12.8p33).  If
+ true when we are treating an lvalue as an rvalue (12.8p33).  If
  KIND is ck_base, always false.  */
   BOOL_BITFIELD rvaluedness_matches_p: 1;
   BOOL_BITFIELD check_narrowing: 1;
@@ -1161,6 +1161,7 @@ standard_conversion (tree to, tree from, tree expr, bool 
c_cast_p,
}
   conv = build_conv (ck_rvalue, from, conv);
   if (flags & LOOKUP_PREFER_RVALUE)
+   /* Tell convert_like_real to set LOOKUP_PREFER_RVALUE.  */
conv->rvaluedness_matches_p = true;
 }
 
@@ -1629,11 +1630,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool 
c_cast_p, int flags,
   conv = build_identity_conv (tfrom, expr);
   conv = direct_reference_binding (rto, conv);
 
-  if (flags & LOOKUP_PREFER_RVALUE)
-   /* The top-level caller requested that we pretend that the lvalue
-  be treated as an rvalue.  */
-   conv->rvaluedness_matches_p = TYPE_REF_IS_RVALUE (rto);
-  else if (TREE_CODE (rfrom) == REFERENCE_TYPE)
+  if (TREE_CODE (rfrom) == REFERENCE_TYPE)
/* Handle rvalue reference to function properly.  */
conv->rvaluedness_matches_p
  = (TYPE_REF_IS_RVALUE (rto) == TYPE_REF_IS_RVALUE (rfrom));
@@ -1659,8 +1656,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool 
c_cast_p, int flags,
   /* Don't allow binding of lvalues (other than function lvalues) to
 rvalue references.  */
   if (is_lvalue && TYPE_REF_IS_RVALUE (rto)
- && TREE_CODE (to) != FUNCTION_TYPE
-  && !(flags & LOOKUP_PREFER_RVALUE))
+ && TREE_CODE (to) != FUNCTION_TYPE)
conv->bad_p = true;
 
   /* Nor the reverse.  */
@@ -6917,6 +6913,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, 
int argnum,
   else
flags |= LOOKUP_ONLYCONVERTING;
   if (convs->rvaluedness_matches_p)
+   /* standard_conversion got LOOKUP_PREFER_RVALUE.  */
flags |= LOOKUP_PREFER_RVALUE;
   if (TREE_CODE (expr) == TARGET_EXPR
  && TARGET_EXPR_LIST_INIT_P (expr))
@@ -7716,6 +7713,19 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
  ++arg_index;
  parm = TREE_CHAIN (parm);
}
+
+  if (flags & LOOKUP_PREFER_RVALUE)
+   {
+ /* The implicit move specified in 15.8.3/3 fails "...if the type of
+the first parameter of the selected constructor is not an rvalue
+reference to the object’s type (possibly cv-qualified)" */
+ gcc_assert (!(complain & tf_error));
+ tree ptype = convs[0]->type;
+ if (TREE_CODE (ptype) != REFERENCE_TYPE
+ || !TYPE_REF_IS_RVALUE (ptype)
+ || CONVERSION_RANK (convs[0]) > cr_exact)
+   return error_mark_node;
+   }
 }
   /* Bypass access control for 'this' parameter.  */
   else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3a0bd16..6c4153d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5296,7 +5296,7 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, 
TYPENAME_FLAG };
(Normally, these entities are registered in the symbol table, but
not found by lookup.)  */
 #define LOOKUP_HIDDEN (LOOKUP_PREFER_NAMESPACES << 1)
-/* Prefer that the lvalue be treated as an rvalue.  */
+/* We're trying to treat an lvalue as an rvalue.  */
 #define LOOKUP_PREFER_RVALUE (LOOKUP_HIDDEN << 1)
 /* We're inside an init-list, so narrowing conversions are ill-formed.  */
 

[PATCH] Fix always true condition in gimplify_adjust_omp_clauses

2017-08-10 Thread Marek Polacek
My new warning triggered here and said "bitwise comparison always evaluates to
true" and I believe it's right: GOVD_WRITTEN = 131072 which is 10... in
binary, so n->value & GOVD_WRITTEN can never be 1; I suppose 0 was meant to
be used here instead.  Too bad Jakub's away, so he can't confirm.

Bootstrapped/regtested on x86_64-linux and ppc64le-linux, ok for trunk?

2017-08-10  Marek Polacek  

* gimplify.c (gimplify_adjust_omp_clauses): Compare with 0 instead of
1.

diff --git gcc/gimplify.c gcc/gimplify.c
index 86623e09f5d..e52d7dcddaf 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -8915,7 +8915,7 @@ gimplify_adjust_omp_clauses (gimple_seq *pre_p, 
gimple_seq body, tree *list_p,
OMP_CLAUSE_SHARED_READONLY (c) = 1;
  else if (DECL_P (decl)
   && ((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_SHARED
-   && (n->value & GOVD_WRITTEN) != 1)
+   && (n->value & GOVD_WRITTEN) != 0)
   || (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LINEAR
   && !OMP_CLAUSE_LINEAR_NO_COPYOUT (c)))
   && omp_shared_to_firstprivate_optimizable_decl_p (decl))

Marek


[PATCH, i386]: Remove UNSPEC_STACK_CHECK ...

2017-08-10 Thread Uros Bizjak
... and use address spaces to load boundary value from TLS block.

The patch removes special handling for boundary value location.

2017-08-10  Uros Bizjak  

* config/i386/i386-protos.h (ix86_split_stack_boundary): New prototype.
* config/i386/i386.c (ix86_split_stack_boundary): New function.
(ix86_xpand_split_stack_prologue): Call ix86_split_stack_boundary.
(ix86_legitimate_address_p) : Remove.
(i386_asm_output_addr_const_extra) : Ditto.
(optput_pic_addr_const): Remove UNSPEC_STACK_CHECK handling.
* config/i386/i386.md (unspec): Remove UNSPEC_STACK_CHECK.
(split_stack_space_check): Call ix86_split_stack_boundary.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32} with
all default languages plus go.

Committed to mainline SVN.

Uros.
Index: config/i386/i386-protos.h
===
--- config/i386/i386-protos.h   (revision 251027)
+++ config/i386/i386-protos.h   (working copy)
@@ -201,6 +201,8 @@ extern void ix86_expand_truncdf_32 (rtx, rtx);
 
 extern void ix86_expand_vecop_qihi (enum rtx_code, rtx, rtx, rtx);
 
+extern rtx ix86_split_stack_boundary (void);
+
 #ifdef TREE_CODE
 extern void init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);
 #endif /* TREE_CODE  */
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 251027)
+++ config/i386/i386.c  (working copy)
@@ -15784,6 +15784,28 @@ static GTY(()) rtx split_stack_fn;
 
 static GTY(()) rtx split_stack_fn_large;
 
+/*  */
+
+rtx
+ix86_split_stack_boundary (void)
+{
+  int offset;
+  addr_space_t as = DEFAULT_TLS_SEG_REG;
+  rtx r;
+
+#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
+  offset = TARGET_THREAD_SPLIT_STACK_OFFSET;
+#else
+  gcc_unreachable ();
+#endif
+
+  r = GEN_INT (offset);
+  r = gen_const_mem (Pmode, r);
+  set_mem_addr_space (r, as);
+
+  return r;
+}
+
 /* Handle -fsplit-stack.  These are the first instructions in the
function, even before the regular prologue.  */
 
@@ -15815,10 +15837,8 @@ ix86_expand_split_stack_prologue (void)
  us SPLIT_STACK_AVAILABLE bytes, so if we need less than that we
  can compare directly.  Otherwise we need to do an addition.  */
 
-  limit = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, const0_rtx),
- UNSPEC_STACK_CHECK);
-  limit = gen_rtx_CONST (Pmode, limit);
-  limit = gen_rtx_MEM (Pmode, limit);
+  limit = ix86_split_stack_boundary ();
+
   if (allocate < SPLIT_STACK_AVAILABLE)
 current = stack_pointer_rtx;
   else
@@ -16891,10 +16911,6 @@ ix86_legitimate_address_p (machine_mode, rtx addr,
  case UNSPEC_DTPOFF:
break;
 
- case UNSPEC_STACK_CHECK:
-   gcc_assert (flag_split_stack);
-   break;
-
  default:
/* Invalid address unspec.  */
return false;
@@ -17984,17 +18000,10 @@ output_pic_addr_const (FILE *file, rtx x, int code
putc (ASSEMBLER_DIALECT == ASM_INTEL ? ')' : ']', file);
   break;
 
- case UNSPEC:
-   if (XINT (x, 1) == UNSPEC_STACK_CHECK)
-{
-  bool f = i386_asm_output_addr_const_extra (file, x);
-  gcc_assert (f);
-  break;
-}
-
-   gcc_assert (XVECLEN (x, 0) == 1);
-   output_pic_addr_const (file, XVECEXP (x, 0, 0), code);
-   switch (XINT (x, 1))
+case UNSPEC:
+  gcc_assert (XVECLEN (x, 0) == 1);
+  output_pic_addr_const (file, XVECEXP (x, 0, 0), code);
+  switch (XINT (x, 1))
{
case UNSPEC_GOT:
  fputs ("@GOT", file);
@@ -19690,22 +19699,6 @@ i386_asm_output_addr_const_extra (FILE *file, rtx
   break;
 #endif
 
-case UNSPEC_STACK_CHECK:
-  {
-   int offset;
-
-   gcc_assert (flag_split_stack);
-
-#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
-   offset = TARGET_THREAD_SPLIT_STACK_OFFSET;
-#else
-   gcc_unreachable ();
-#endif
-
-   fprintf (file, "%s:%d", TARGET_64BIT ? "%fs" : "%gs", offset);
-  }
-  break;
-
 default:
   return false;
 }
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 251027)
+++ config/i386/i386.md (working copy)
@@ -86,7 +86,6 @@
   UNSPEC_SET_RIP
   UNSPEC_SET_GOT_OFFSET
   UNSPEC_MEMORY_BLOCKAGE
-  UNSPEC_STACK_CHECK
   UNSPEC_PROBE_STACK
 
   ;; TLS support
@@ -12630,21 +12629,18 @@
   [(set (pc) (if_then_else
  (ltu (minus (reg SP_REG)
  (match_operand 0 "register_operand"))
-  (unspec [(const_int 0)] UNSPEC_STACK_CHECK))
+  (match_dup 2))
  (label_ref (match_operand 1))
  (pc)))]
   ""
 {
-  rtx reg, size, limit;
+  rtx reg = gen_reg_rtx (Pmode);
 
-  reg = gen_reg_rtx (Pmode);
-  size = force_reg (Pmode, operands[0]);
-  emit_insn (gen_sub3_insn (reg, stack_pointer_rtx, size));
-  limit = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, 

Re: PR81738: Split vect-alias-check-6.c

2017-08-10 Thread Richard Biener
On August 10, 2017 3:37:55 PM GMT+02:00, Richard Sandiford 
 wrote:
>The second loop in the testcase only vectorises if we can reverse
>a vector and if aligned loads aren't required.
>
>Sanity-checked on aarch64-linux-gnu, x86_64-linux-gnu and
>powerpc64le-linux-gnu (although all three were unaffected).
>OK to install?

OK.

Richard.

>Thanks,
>Richard
>
>
>2017-08-10  Richard Sandiford  
>
>gcc/testsuite/
>   PR testsuite/81738
>   * gcc.dg/vect/vect-alias-check-6.c: Move second function to...
>   * gcc.dg/vect/vect-alias-check-7.c: ...this new file.  Require
>   vect_perm and vect_element_align for vectorization.
>
>Index: gcc/testsuite/gcc.dg/vect/vect-alias-check-6.c
>===
>--- gcc/testsuite/gcc.dg/vect/vect-alias-check-6.c 2017-08-04
>11:40:26.372205514 +0100
>+++ gcc/testsuite/gcc.dg/vect/vect-alias-check-6.c 2017-08-10
>14:36:24.201888108 +0100
>@@ -12,12 +12,5 @@ f1 (struct s *a, struct s *b)
> a->x[i + 1] += b->x[i];
> }
> 
>-void
>-f2 (struct s *a, struct s *b)
>-{
>-  for (int i = 0; i < N; ++i)
>-a->x[i] += b->x[N - i - 1];
>-}
>-
>-/* { dg-final { scan-tree-dump-times {checking that [^\n]* and [^\n]*
>have different addresses} 2 "vect" } } */
>-/* { dg-final { scan-tree-dump-times "LOOP VECTORIZED" 2 "vect" } } */
>+/* { dg-final { scan-tree-dump {checking that [^\n]* and [^\n]* have
>different addresses} "vect" } } */
>+/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
>Index: gcc/testsuite/gcc.dg/vect/vect-alias-check-7.c
>===
>--- /dev/null  2017-08-09 18:16:39.535015779 +0100
>+++ gcc/testsuite/gcc.dg/vect/vect-alias-check-7.c 2017-08-10
>14:36:24.201888108 +0100
>@@ -0,0 +1,16 @@
>+/* { dg-do compile } */
>+/* { dg-require-effective-target vect_int } */
>+
>+#define N 16
>+
>+struct s { int x[N]; };
>+
>+void
>+f1 (struct s *a, struct s *b)
>+{
>+  for (int i = 0; i < N; ++i)
>+a->x[i] += b->x[N - i - 1];
>+}
>+
>+/* { dg-final { scan-tree-dump {checking that [^\n]* and [^\n]* have
>different addresses} "vect" } } */
>+/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" { target {
>vect_perm && vect_element_align } } } } */



[Patch][AARCH64] Fix PR 81643 by modifying test case.

2017-08-10 Thread Steve Ellcey
I would like to modify gcc.target/aarch64/long_branch_1.c to no longer
look for an Ltb label.  After a patch to c-typeck.c to add a predict
statement to GOTO statements, GCC no longer generates one of the types
of long branch that this test is checking for.

I tried modifying the test and creating a new test to generate this label
but was unsucessful, I also added in an abort statement that would trigger
when GCC tried to generate this label and then ran a bootstrap GCC build
and a full testsuite run.  The abort never triggered. There may still
be some way to generate this label but I don't think it is worth the time
to find it and so I would like to just remove the check so that the
testcase no longer fails.

OK for checkin?  Tested on aarch64. 

Steve Ellcey
sell...@cavium.com



2017-08-10  Steve Ellcey  

PR target/81643
* gcc.target/aarch64/long_branch_1.c: Remove Ltb check.



diff --git a/gcc/testsuite/gcc.target/aarch64/long_branch_1.c 
b/gcc/testsuite/gcc.target/aarch64/long_branch_1.c
index 46f500d..9e0f423 100644
--- a/gcc/testsuite/gcc.target/aarch64/long_branch_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/long_branch_1.c
@@ -88,4 +88,3 @@ start3:
 
 /* { dg-final { scan-assembler "Lbcond" } } */
 /* { dg-final { scan-assembler "Lcb" } } */
-/* { dg-final { scan-assembler "Ltb" } } */


[PATCH] -fftz-math: assume that denorms _must_ be flushed to zero optimizations

2017-08-10 Thread Pekka Jääskeläinen
Hi,

The attached patch adds a new switch -fftz-math which makes certain
optimizations
assume that "flush to zero" behavior of denormal inputs and outputs is
not an optimization
hint, but required behavior for semantical correctness.

The need for this was initiated by HSAIL (BRIG). With HSAIL, flush to
zero handling is required,
(not only "allowed") in case an HSAIL instruction is marked with the
'ftz' modifier (all HSA Base
profile instructions are).

The patch is not complete and likely misses many optimizations.
However, it is a starting point
that fixes a few cases brought out by the HSAIL conformance suite. We
plan to extend this
as new cases come up.

OK for trunk?

BR,
Pekka
Index: gcc/common.opt
===
--- gcc/common.opt	(revision 251026)
+++ gcc/common.opt	(working copy)
@@ -2281,6 +2281,11 @@
 Common Report Var(flag_single_precision_constant) Optimization
 Convert floating point constants to single precision constants.
 
+fftz-math
+Common Report Var(flag_ftz_math) Optimization
+Optimizations handle floating-point operations as they must flush
+subnormal floating-point values to zero.
+
 fsplit-ivs-in-unroller
 Common Report Var(flag_split_ivs_in_unroller) Init(1) Optimization
 Split lifetimes of induction variables when loops are unrolled.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi	(revision 251026)
+++ gcc/doc/invoke.texi	(working copy)
@@ -9458,6 +9458,17 @@
 This option is experimental and does not currently guarantee to
 disable all GCC optimizations that affect signaling NaN behavior.
 
+@item -fftz-math
+@opindex ftz-math
+This option is experimental. With this flag on GCC treats
+floating-point operations (except abs, class, copysign and neg) as
+they must flush subnormal input operands and results to zero
+(FTZ). The FTZ rules are derived from HSA Programmers Reference Manual
+for the base profile. This alters optimizations that would break the
+rules, for example X * 1 -> X simplification. The option assumes the
+target supports FTZ in hardware and has it enabled - either by default
+or set by the user.
+
 @item -fno-fp-int-builtin-inexact
 @opindex fno-fp-int-builtin-inexact
 Do not allow the built-in functions @code{ceil}, @code{floor},
Index: gcc/fold-const-call.c
===
--- gcc/fold-const-call.c	(revision 251026)
+++ gcc/fold-const-call.c	(working copy)
@@ -697,7 +697,7 @@
 	  && do_mpfr_arg1 (result, mpfr_y1, arg, format));
 
 CASE_CFN_FLOOR:
-  if (!REAL_VALUE_ISNAN (*arg) || !flag_errno_math)
+  if ((!REAL_VALUE_ISNAN (*arg) || !flag_errno_math) && !flag_ftz_math)
 	{
 	  real_floor (result, format, arg);
 	  return true;
@@ -705,7 +705,7 @@
   return false;
 
 CASE_CFN_CEIL:
-  if (!REAL_VALUE_ISNAN (*arg) || !flag_errno_math)
+  if ((!REAL_VALUE_ISNAN (*arg) || !flag_errno_math) && !flag_ftz_math)
 	{
 	  real_ceil (result, format, arg);
 	  return true;
Index: gcc/match.pd
===
--- gcc/match.pd	(revision 251026)
+++ gcc/match.pd	(working copy)
@@ -143,6 +143,7 @@
 (simplify
  (mult @0 real_onep)
  (if (!HONOR_SNANS (type)
+  && !flag_ftz_math
   && (!HONOR_SIGNED_ZEROS (type)
   || !COMPLEX_FLOAT_TYPE_P (type)))
   (non_lvalue @0)))
@@ -151,6 +152,7 @@
 (simplify
  (mult @0 real_minus_onep)
   (if (!HONOR_SNANS (type)
+   && !flag_ftz_math
&& (!HONOR_SIGNED_ZEROS (type)
|| !COMPLEX_FLOAT_TYPE_P (type)))
(negate @0)))
@@ -332,13 +334,13 @@
 /* In IEEE floating point, x/1 is not equivalent to x for snans.  */
 (simplify
  (rdiv @0 real_onep)
- (if (!HONOR_SNANS (type))
+ (if (!HONOR_SNANS (type) && !flag_ftz_math)
   (non_lvalue @0)))
 
 /* In IEEE floating point, x/-1 is not equivalent to -x for snans.  */
 (simplify
  (rdiv @0 real_minus_onep)
- (if (!HONOR_SNANS (type))
+ (if (!HONOR_SNANS (type) && !flag_ftz_math)
   (negate @0)))
 
 (if (flag_reciprocal_math)
Index: gcc/simplify-rtx.c
===
--- gcc/simplify-rtx.c	(revision 251026)
+++ gcc/simplify-rtx.c	(working copy)
@@ -2565,8 +2565,10 @@
 	return op1;
 
   /* In IEEE floating point, x*1 is not equivalent to x for
-	 signalling NaNs.  */
+	 signalling NaNs.
+	 For -fftz-math, x*1 is not equivalent to x for subnormals. */
   if (!HONOR_SNANS (mode)
+	  && (FLOAT_MODE_P (mode) && !flag_ftz_math)
 	  && trueop1 == CONST1_RTX (mode))
 	return op0;
 


[PATCH, DWARF] Add DW_CFA_AARCH64_negate_ra_state to dwarf2.def/h and dwarfnames.c

2017-08-10 Thread Jiong Wang

Hi,

  A new vendor CFA DW_CFA_AARCH64_negate_ra_state was introduced for ARMv8.3-A
return address signing, it is multiplexing DW_CFA_GNU_window_save in CFA vendor
extension space.

  This patch adds necessary code to make it available to external, the GDB
patch (https://sourceware.org/ml/gdb-patches/2017-08/msg00215.html) is intended
to use it.

  A new DW_CFA_DUP for it is added in dwarf2.def.  The use of DW_CFA_DUP is to
avoid duplicated case value issue when included in libiberty/dwarfnames.

  Native x86 builds OK to make sure no macro expanding errors.

  OK for trunk?

2017-08-10  Jiong Wang  

include/
* dwarf2.def (DW_CFA_AARCH64_negate_ra_state): New DW_CFA_DUP.
* dwarf2.h (DW_CFA_DUP): New define.

libiberty/
* dwarfnames.c (DW_CFA_DUP): New define.

diff --git a/include/dwarf2.def b/include/dwarf2.def
index a91e9439cd82f3bb9fdddc14904114e5490c1af6..2a3b23fef873db9bb2498cd28c4fafc72e6a234f 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -778,6 +778,7 @@ DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
 /* GNU extensions.
NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64.  */
 DW_CFA (DW_CFA_GNU_window_save, 0x2d)
+DW_CFA_DUP (DW_CFA_AARCH64_negate_ra_state, 0x2d)
 DW_CFA (DW_CFA_GNU_args_size, 0x2e)
 DW_CFA (DW_CFA_GNU_negative_offset_extended, 0x2f)
 
diff --git a/include/dwarf2.h b/include/dwarf2.h
index 14b6f22e39e2f2f8cadb05009bfd10fafa9ea07c..a2e022dbdb35c18bb591e0f00930978846b82c01 100644
--- a/include/dwarf2.h
+++ b/include/dwarf2.h
@@ -52,6 +52,7 @@
 #define DW_ATE(name, value) , name = value
 #define DW_ATE_DUP(name, value) , name = value
 #define DW_CFA(name, value) , name = value
+#define DW_CFA_DUP(name, value) , name = value
 #define DW_IDX(name, value) , name = value
 #define DW_IDX_DUP(name, value) , name = value
 
@@ -104,6 +105,7 @@
 #undef DW_ATE
 #undef DW_ATE_DUP
 #undef DW_CFA
+#undef DW_CFA_DUP
 #undef DW_IDX
 #undef DW_IDX_DUP
 
diff --git a/libiberty/dwarfnames.c b/libiberty/dwarfnames.c
index e58d03c3a3d814f3a271edb4689c6306a2f958f0..dacd78dbaa9b33d6e9fdf35330cdc446dcf4f76c 100644
--- a/libiberty/dwarfnames.c
+++ b/libiberty/dwarfnames.c
@@ -75,6 +75,7 @@ Boston, MA 02110-1301, USA.  */
 #define DW_ATE(name, value) case name: return # name ;
 #define DW_ATE_DUP(name, value)
 #define DW_CFA(name, value) case name: return # name ;
+#define DW_CFA_DUP(name, value)
 #define DW_IDX(name, value) case name: return # name ;
 #define DW_IDX_DUP(name, value)
 
@@ -105,5 +106,6 @@ Boston, MA 02110-1301, USA.  */
 #undef DW_ATE
 #undef DW_ATE_DUP
 #undef DW_CFA
+#undef DW_CFA_DUP
 #undef DW_IDX
 #undef DW_IDX_DUP


[PATCH] i386: Require MMX for builtins with MMX register

2017-08-10 Thread H.J. Lu
When MMX is disabled, MMX registers aren't available.  We shouldn't
allow builtins with MMX registers when MMX registers aren't available.
This applies to MMX and 3DNow builtins as well as SSE builtins with
MMX registers.

Also we should always declare all builtins and ix86_expand_builtin
will give an error if the builtin isn't available.

gcc.target/i386/crc32-4.c is updated to scan __builtin_ia32_crc32di only
for ia32 since __builtin_ia32_crc32di is a normal function for ia32 and
a builtin function for x86-64.

Tested on i686 and x86-64.  OK for trunk?

H.J.
---
gcc/

PR target/79565
* config/i386/i386-builtin.def: Add OPTION_MASK_ISA_MMX to all
builtins with MMX register.
* config/i386/i386.c (def_builtin): Always declare all builtins.
(ix86_expand_builtin): Check if MMX is enabled for builtins with
MMX register.
* config/i386/sse.md: Require TARGET_MMX for all patterns with
MMX register.

gcc/testsuite/

PR target/79565
* gcc.target/i386/crc32-4.c: Scan __builtin_ia32_crc32di only
for ia32 target.
(crc32d): Check for "needs isa option" error for non-ia32
targets.
* gcc.target/i386/pr70325.c (f): Check for "needs isa option"
error instead.
* gcc.target/i386/pr79565.c: New test.
---
 gcc/config/i386/i386-builtin.def| 116 
 gcc/config/i386/i386.c  |  48 +++--
 gcc/config/i386/sse.md  |  30 +
 gcc/testsuite/gcc.target/i386/crc32-4.c |   4 +-
 gcc/testsuite/gcc.target/i386/pr70325.c |   2 +-
 gcc/testsuite/gcc.target/i386/pr79565.c |  51 ++
 6 files changed, 139 insertions(+), 112 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr79565.c

diff --git a/gcc/config/i386/i386-builtin.def b/gcc/config/i386/i386-builtin.def
index e91468a1a2e..be73d7e6181 100644
--- a/gcc/config/i386/i386-builtin.def
+++ b/gcc/config/i386/i386-builtin.def
@@ -102,7 +102,7 @@ BDESC (0, CODE_FOR_fnclex, "__builtin_ia32_fnclex", 
IX86_BUILTIN_FNCLEX, UNKNOWN
 BDESC (OPTION_MASK_ISA_MMX, CODE_FOR_mmx_emms, "__builtin_ia32_emms", 
IX86_BUILTIN_EMMS, UNKNOWN, (int) VOID_FTYPE_VOID)
 
 /* 3DNow! */
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_femms, "__builtin_ia32_femms", 
IX86_BUILTIN_FEMMS, UNKNOWN, (int) VOID_FTYPE_VOID)
+BDESC (OPTION_MASK_ISA_3DNOW | OPTION_MASK_ISA_MMX, CODE_FOR_mmx_femms, 
"__builtin_ia32_femms", IX86_BUILTIN_FEMMS, UNKNOWN, (int) VOID_FTYPE_VOID)
 
 /* FXSR, XSAVE, XSAVEOPT, XSAVEC and XSAVES.  */
 BDESC (OPTION_MASK_ISA_FXSR, CODE_FOR_nothing, "__builtin_ia32_fxsave", 
IX86_BUILTIN_FXSAVE, UNKNOWN, (int) VOID_FTYPE_PVOID)
@@ -136,8 +136,8 @@ BDESC (OPTION_MASK_ISA_SSE, CODE_FOR_sse_storehps, 
"__builtin_ia32_storehps", IX
 BDESC (OPTION_MASK_ISA_SSE, CODE_FOR_sse_storelps, "__builtin_ia32_storelps", 
IX86_BUILTIN_STORELPS, UNKNOWN, (int) VOID_FTYPE_PV2SF_V4SF)
 
 /* SSE or 3DNow!A  */
-BDESC (OPTION_MASK_ISA_SSE | OPTION_MASK_ISA_3DNOW_A, CODE_FOR_sse_sfence, 
"__builtin_ia32_sfence", IX86_BUILTIN_SFENCE, UNKNOWN, (int) VOID_FTYPE_VOID)
-BDESC (OPTION_MASK_ISA_SSE | OPTION_MASK_ISA_3DNOW_A, CODE_FOR_sse_movntq, 
"__builtin_ia32_movntq", IX86_BUILTIN_MOVNTQ, UNKNOWN, (int) 
VOID_FTYPE_PULONGLONG_ULONGLONG)
+BDESC (OPTION_MASK_ISA_SSE | OPTION_MASK_ISA_3DNOW_A | OPTION_MASK_ISA_MMX, 
CODE_FOR_sse_sfence, "__builtin_ia32_sfence", IX86_BUILTIN_SFENCE, UNKNOWN, 
(int) VOID_FTYPE_VOID)
+BDESC (OPTION_MASK_ISA_SSE | OPTION_MASK_ISA_3DNOW_A | OPTION_MASK_ISA_MMX, 
CODE_FOR_sse_movntq, "__builtin_ia32_movntq", IX86_BUILTIN_MOVNTQ, UNKNOWN, 
(int) VOID_FTYPE_PULONGLONG_ULONGLONG)
 
 /* SSE2 */
 BDESC (OPTION_MASK_ISA_SSE2, CODE_FOR_sse2_lfence, "__builtin_ia32_lfence", 
IX86_BUILTIN_LFENCE, UNKNOWN, (int) VOID_FTYPE_VOID)
@@ -469,34 +469,34 @@ BDESC (OPTION_MASK_ISA_MMX, CODE_FOR_mmx_ashrv4hi3, 
"__builtin_ia32_psraw", IX86
 BDESC (OPTION_MASK_ISA_MMX, CODE_FOR_mmx_ashrv2si3, "__builtin_ia32_psrad", 
IX86_BUILTIN_PSRAD, UNKNOWN, (int) V2SI_FTYPE_V2SI_V2SI_COUNT)
 
 /* 3DNow! */
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_pf2id, "__builtin_ia32_pf2id", 
IX86_BUILTIN_PF2ID, UNKNOWN, (int) V2SI_FTYPE_V2SF)
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_floatv2si2, "__builtin_ia32_pi2fd", 
IX86_BUILTIN_PI2FD, UNKNOWN, (int) V2SF_FTYPE_V2SI)
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_rcpv2sf2, "__builtin_ia32_pfrcp", 
IX86_BUILTIN_PFRCP, UNKNOWN, (int) V2SF_FTYPE_V2SF)
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_rsqrtv2sf2, 
"__builtin_ia32_pfrsqrt", IX86_BUILTIN_PFRSQRT, UNKNOWN, (int) V2SF_FTYPE_V2SF)
-
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_uavgv8qi3, 
"__builtin_ia32_pavgusb", IX86_BUILTIN_PAVGUSB, UNKNOWN, (int) 
V8QI_FTYPE_V8QI_V8QI)
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_haddv2sf3, "__builtin_ia32_pfacc", 
IX86_BUILTIN_PFACC, UNKNOWN, (int) V2SF_FTYPE_V2SF_V2SF)
-BDESC (OPTION_MASK_ISA_3DNOW, CODE_FOR_mmx_addv2sf3, "__builtin_ia32_pfadd", 
IX86_BUILTIN_PFADD, UNKNOWN, 

Re: [PATCH][GCC][AArch64] Inline calls to lrint when possible

2017-08-10 Thread Szabolcs Nagy
On 07/06/17 12:38, Tamar Christina wrote:
> Hi All,
> 
> This patch allows the inlining of lrint when -fno-math-errno
> assuming that errno does not need to be set when the rounded value
> is not representable as a long.
> 

turns out emitting frintx+fcvtzs is wrong for ilp32
because spurious inexact may be raised when the final
result is out-of-bound for 32bit long.

i opened
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81800

> The case
> 
> void f(double *a, long *b, double x)
> {
>   *a = __builtin_rint(x);
>   *b = __builtin_lrint(x);
> }
> 
> now generates with -fno-math-errno:
> 
> f:
>   frintx  d0, d0
>   fcvtzs  x2, d0
>   str d0, [x0]
>   str x2, [x1]
>   ret
> 
> When the flag is not used the same function call is emitted as before:
> 
> f:
>   stp x29, x30, [sp, -32]!
>   frintx  d1, d0
>   add x29, sp, 0
>   str x19, [sp, 16]
>   mov x19, x1
>   str d1, [x0]
>   bl  lrint
>   str x0, [x19]
>   ldr x19, [sp, 16]
>   ldp x29, x30, [sp], 32
>   ret
> 
> Bootstrapped and regtested on aarch64-none-linux-gnu and no regressions.
> The patch also has no regressions on Spec2006.
> 
> Ok for trunk?
> 
> gcc/
> 2017-06-07  Tamar Christina  
> 
>   * config/aarch64/aarch64.md (lrint2): New.
> 
> gcc/testsuite/
> 2017-06-07  Tamar Christina  
> 
>   * gcc.target/aarch64/lrint-matherr.h: New.
>   * gcc.target/aarch64/inline-lrint_1.c: New.
>   * gcc.target/aarch64/inline-lrint_2.c: New.
>   * gcc.target/aarch64/no-inline-lrint_1.c: New.
>   * gcc.target/aarch64/no-inline-lrint_2.c: New.
> 



[PATCH] Factor out division by squares and remove division around comparisons (2/2)

2017-08-10 Thread Jackson Woodruff

Hi all,

The patch implements the some of the division optimizations discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026 .

We now reassociate (as discussed in the bug report):

x / (y * y) -> x  * (1 / y) * (1 / y)

If it is reasonable to do so. This is done with
-funsafe-math-optimizations.

Bootstrapped and regtested with part (1/2). OK for trunk?

Jackson

gcc/

2017-08-03  Jackson Woodruff  

PR 71026/tree-optimization
* tree-ssa-math-opts (is_division_by_square,
is_square_of, insert_sqaure_reciprocals): New.
(insert_reciprocals): Change to insert reciprocals
before a division by a square.
(execute_cse_reciprocals_1): Change to consider
division by a square.


gcc/testsuite

2017-08-03  Jackson Woodruff  

PR 71026/tree-optimization
* gcc.dg/associate_division_1.c: New.

diff --git a/gcc/testsuite/gcc.dg/associate_division_1.c 
b/gcc/testsuite/gcc.dg/associate_division_1.c
new file mode 100644
index 
..187d3597af8825a6a43f29bfaa68b089d2d5bbfe
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/associate_division_1.c
@@ -0,0 +1,46 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-optimized" } */
+
+float
+extract_square (float x, float y, float *a, float *b)
+{
+  *a = 3 / (y * y);
+  *b = 5 / (y * y);
+
+  return x / (y * y);
+}
+
+/* Don't expect the 'powmult' (calculation of y * y)
+   to be deleted until a later pass, so look for one
+   more multiplication than strictly necessary.  */
+float
+extract_recip (float *w, float x, float y, float z)
+{
+  *w = 7 / y;
+
+  return x / (y * y) + z / y;
+}
+
+float
+neg_recip (float x, float y, float z)
+{
+  return (x / y) + (z / -y);
+}
+
+float
+const_divisor (float *w, float x, float y, float z)
+{
+  *w = 5 / y;
+  return x / (y * 3.0f) + z / y;
+}
+
+/* 4 For the pointers to a, b, w and w, 4 multiplications in 'extract_square',
+   5 multiplications in 'extract_recip', 0 multiplications
+   in 'neg_recip', 3 multiplcations in 'const_divisor' expected.  */
+/* { dg-final { scan-tree-dump-times " \\* " 14 "optimized" } } */
+
+/* 1 division in 'extract_square', 1 division in 'extract_recip',
+   1 division in 'neg_recip', 1 division in 'const_divisor'.  */
+/* { dg-final { scan-tree-dump-times " / " 4 "optimized" } } */
+
+
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 
7ac1659fa0670b7080685f3f9513939807073a63..0db160f68001ffb90942c4002703625430128b9f
 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -340,6 +340,38 @@ is_division_by (gimple *use_stmt, tree def)
 && gimple_assign_rhs1 (use_stmt) != def;
 }
 
+/* Return whether USE_STMT is DEF * DEF.  */
+static inline bool
+is_square_of (gimple *use_stmt, tree def)
+{
+  if (gimple_code (use_stmt) == GIMPLE_ASSIGN
+  && gimple_assign_rhs_code (use_stmt) == MULT_EXPR)
+{
+  tree op0 = gimple_assign_rhs1 (use_stmt);
+  tree op1 = gimple_assign_rhs2 (use_stmt);
+
+  return op0 == op1 && op0 == def;
+}
+  return 0;
+}
+
+/* Return whether USE_STMT is a floating-point division by
+   DEF * DEF.  */
+static inline bool
+is_division_by_square (gimple *use_stmt, tree def)
+{
+  if (gimple_code (use_stmt) == GIMPLE_ASSIGN
+  && gimple_assign_rhs_code (use_stmt) == RDIV_EXPR)
+{
+  tree denominator = gimple_assign_rhs2 (use_stmt);
+  if (TREE_CODE (denominator) == SSA_NAME)
+   {
+ return is_square_of (SSA_NAME_DEF_STMT (denominator), def);
+   }
+}
+  return 0;
+}
+
 /* Walk the subset of the dominator tree rooted at OCC, setting the
RECIP_DEF field to a definition of 1.0 / DEF that can be used in
the given basic block.  The field may be left NULL, of course,
@@ -369,14 +401,16 @@ insert_reciprocals (gimple_stmt_iterator *def_gsi, struct 
occurrence *occ,
  build_one_cst (type), def);
 
   if (occ->bb_has_division)
-{
-  /* Case 1: insert before an existing division.  */
-  gsi = gsi_after_labels (occ->bb);
-  while (!gsi_end_p (gsi) && !is_division_by (gsi_stmt (gsi), def))
+   {
+ /* Case 1: insert before an existing division.  */
+ gsi = gsi_after_labels (occ->bb);
+ while (!gsi_end_p (gsi)
+&& (!is_division_by (gsi_stmt (gsi), def))
+&& (!is_division_by_square (gsi_stmt (gsi), def)))
gsi_next ();
 
-  gsi_insert_before (, new_stmt, GSI_SAME_STMT);
-}
+ gsi_insert_before (, new_stmt, GSI_SAME_STMT);
+   }
   else if (def_gsi && occ->bb == def_gsi->bb)
 {
   /* Case 2: insert right after the definition.  Note that this will
@@ -403,6 +437,80 @@ insert_reciprocals (gimple_stmt_iterator *def_gsi, struct 
occurrence *occ,
 }
 
 
+/* Walk the tree until we find the first division by a square.  Insert
+   the definition of 

Re: [PATCH] Factor out division by squares and remove division around comparisons (1/2)

2017-08-10 Thread Jackson Woodruff

And have now attached that patch.

Jackson


On 08/10/2017 03:09 PM, Jackson Woodruff wrote:

Hi all,

The patch implements the division opitmizations discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026 .

The implemented change differs slightly from the
proposed one in that we re-associate:

C / x comparison 0.0 -> x comparison' 0.0

Where C is any constant and comparison' is changed as
appropriate if C is negative.

The implementations also removes the division from:

x / C comparison 0.0 -> x comparison' 0.0

Where again, comparison' is changed as appropriate if C is
negative.

We also change the association of

 x / (y * C) -> (x / C) / y

If C is a constant. All of the above require
-funsafe-math-optimizations.

We also change:

  x / (- y) -> (-x) / y

Which requires -fno-trapping-math.

Bootstrapped and regtested (with part 2 of this patch) on aarch64.

OK for trunk?

(Apologies if the recipients in the 'to' field received this twice,
I accidentally sent this from an email gcc-patches doesn't accept)

Jackson

gcc/

2017-08-03  Jackson Woodruff  

PR 71026/tree-optimization
* match.pd: New patterns.


gcc/testsuite

2017-08-03  Jackson Woodruff  

PR 71026/tree-optimization
* gcc.dg/associate_comparison_1.c: New.
* gcc.dg/associate_division_2.c: New.



diff --git a/gcc/match.pd b/gcc/match.pd
index 
e98db52af84946cf579c6434e06d450713a47162..7abfd11601a956ecb59c945256c9f30dc0b6f6c9
 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -342,16 +342,42 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (negate @0)))
 
 (if (flag_reciprocal_math)
- /* Convert (A/B)/C to A/(B*C)  */
+ /* Convert (A/B)/C to A/(B*C) where B is not a constant.  */
  (simplify
   (rdiv (rdiv:s @0 @1) @2)
-   (rdiv @0 (mult @1 @2)))
+   (if (TREE_CODE (@1) != REAL_CST)
+(rdiv @0 (mult @1 @2
+
+ /* Simplify x / (C * y) to (x / C) / y where C is a constant.  */
+ (simplify
+  (rdiv @0
+   (mult @1 REAL_CST@2))
+  (rdiv (rdiv @0 @2) @1))
 
  /* Convert A/(B/C) to (A/B)*C  */
  (simplify
   (rdiv @0 (rdiv:s @1 @2))
(mult (rdiv @0 @1) @2)))
 
+(if (!flag_trapping_math)
+  /* Simplify x / (- y) to -x / y.  */
+  (simplify
+(rdiv @0 (negate @1))
+(rdiv (negate @0) @1)))
+
+(if (flag_unsafe_math_optimizations)
+  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
+  (for op (lt le gt ge)
+  neg_op (gt ge lt le)
+(simplify
+  (op (rdiv REAL_CST@0 @1) real_zerop@2)
+  (switch
+   (if (!real_equal (TREE_REAL_CST_PTR (@0), )
+   && !REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
+   (op @1 @2))
+   (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
+   (neg_op @1 @2))
+
 /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
 (for div (trunc_div ceil_div floor_div round_div exact_div)
  (simplify
@@ -3446,6 +3472,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (if (!HONOR_SNANS (type))
@0))
 
+ /* Simplify (x * C1) cmp C2 -> x cmp C2 / C1
+and simplify (x / C1) cmp C2 -> x cmp C2 * C1.  */
+ (for cmp (lt le gt ge)
+  neg_cmp (gt ge lt le)
+  (for op (mult rdiv)
+   inv_op (rdiv mult)
+  (simplify
+   (cmp (op @0 REAL_CST@1) REAL_CST@2)
+   (switch
+   (if (!real_equal (TREE_REAL_CST_PTR (@1), )
+&& !REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
+(cmp @0 (inv_op @2 @1)))
+   (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
+(neg_cmp @0 (inv_op @2 @1)))
+
  /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
  (for root (SQRT CBRT)
   (simplify
diff --git a/gcc/testsuite/gcc.dg/associate_comparison_1.c 
b/gcc/testsuite/gcc.dg/associate_comparison_1.c
new file mode 100644
index 
..c12e5cfb2a8388068fa1731cc2305f9fe5139fd6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-funsafe-math-optimizations -fdump-tree-optimized" } */
+
+int
+lt_cmp (float x, float y)
+{
+  return x / 2 <= 0;
+}
+
+int
+inv_cmp (float x, float y)
+{
+  return 5 / x >= 0;
+}
+
+
+/* { dg-final { scan-tree-dump-not " / " "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/associate_division_2.c 
b/gcc/testsuite/gcc.dg/associate_division_2.c
new file mode 100644
index 
..1f9c1741ce980f1148016e37399134aa8a619b1d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/associate_division_2.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-optimized" } */
+
+float
+neg_mov (float w, float x, float *y, float *z)
+{
+  *z = x / (- w);
+  *y = 3 / w;
+
+  return 5 / w;
+}
+
+/* { dg-final { scan-tree-dump-times " / " 1 "optimized" } } */


Re: [PATCH, Fortran] Support for legacy %FILL fields in STRUCTUREs

2017-08-10 Thread Fritz Reese
I find them strange too... But as you say, they are needed for old
code, and some such code bases are quite large and/or have
uncooperative users or maintainers.

Committed with %qs throughout, thanks for review.
---
Fritz Reese


On Tue, Aug 1, 2017 at 11:29 AM, Thomas Koenig  wrote:
> Hi Fritz,
>
>
>
>> Regtests on x86_64-redhat-linux. OK for trunk?
>
>
> Patch looks good in principle; I really find all these DEC extensions
> strange, but if they are needed for old code, why not?
>
> Just one point:
>
>> +   gfc_error ("%s not allowed outside STRUCTURE at %C", "%FILL");
>
>
> This should have %qs (throughout the patch).
>
> OK for trunk with that.
>
> Regards
>
> Thomas


[PATCH] Factor out division by squares and remove division around comparisons (1/2)

2017-08-10 Thread Jackson Woodruff

Hi all,

The patch implements the division opitmizations discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026 .

The implemented change differs slightly from the
proposed one in that we re-associate:

C / x comparison 0.0 -> x comparison' 0.0

Where C is any constant and comparison' is changed as
appropriate if C is negative.

The implementations also removes the division from:

x / C comparison 0.0 -> x comparison' 0.0

Where again, comparison' is changed as appropriate if C is
negative.

We also change the association of

 x / (y * C) -> (x / C) / y

If C is a constant. All of the above require
-funsafe-math-optimizations.

We also change:

  x / (- y) -> (-x) / y

Which requires -fno-trapping-math.

Bootstrapped and regtested (with part 2 of this patch) on aarch64.

OK for trunk?

(Apologies if the recipients in the 'to' field received this twice,
I accidentally sent this from an email gcc-patches doesn't accept)

Jackson

gcc/

2017-08-03  Jackson Woodruff  

PR 71026/tree-optimization
* match.pd: New patterns.


gcc/testsuite

2017-08-03  Jackson Woodruff  

PR 71026/tree-optimization
* gcc.dg/associate_comparison_1.c: New.
* gcc.dg/associate_division_2.c: New.



Re: [PATCH, Fortran] Correctly set -fd-lines-as-comments with -fdec

2017-08-10 Thread Fritz Reese
Yes I did. Committed, thanks.

---
Fritz Reese


On Tue, Aug 1, 2017 at 11:11 AM, Thomas Koenig  wrote:
> Hi Fritz,
>
>> This is a simple patch. The original intent was for -fdec to set
>> -fd-lines-as-comments by default if flag_d_lines was unspecified by
>> the user. However, currently flag_d_lines is interrogated in
>> set_dec_flags(), usually before its final value is determined. The
>> attached patch fixes this behavior to work as intended.
>>
>> Any objections for trunk? If not I think it is safe to commit.
>
>
> Did you regression-test?  OK for trunk if yes.
>
> Regards
>
> Thomas


Re: [PATCH] Fix ifunc and resolver (PR ipa/81213).

2017-08-10 Thread Jan Hubicka
> On 06/30/2017 10:47 AM, Martin Liška wrote:
> >Hello.
> >
> >Following patch does refactoring of make_resolver_func where ifunc
> >alias and resolver were probably confused.
> >
> >Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> >i386.exp tests work on x86_64-linux-gnu.
> >
> >Ready to be installed?
> >Martin
> >
> >gcc/ChangeLog:
> >
> >2017-06-29  Martin Liska  
> >
> > PR ipa/81213
> > * config/i386/i386.c (make_resolver_func): Do complete
> > refactoring of the function.

OK,
thanks!

Honza


Re: [PATCH, rs6000] fold-vec testcase fix-ups

2017-08-10 Thread Segher Boessenkool
On Thu, Aug 10, 2017 at 08:27:17AM -0500, Will Schmidt wrote:
> A testcase coverage issue and an obvious typo fix.
> 
> Mostly obvious,..  OK for trunk? 

Yes; one comment:

> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c 
> b/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c
> index d8acc3c..73131bb 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c
> @@ -1,9 +1,9 @@
>  /* Verify that overloaded built-ins for vec_pack with long long
> inputs produce the right results.  */
>  
> -/* { dg-do compile } */
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
>  /* { dg-require-effective-target powerpc_p8vector_ok } */
>  /* { dg-options "-mvsx -mpower8-vector -O2" } */

You can just say

/* { dg-do compile { target lp64 } } */

(make sure to test before you commit :-) )


Segher


Re: [PATCH] i386: Don't use frame pointer without stack access

2017-08-10 Thread H.J. Lu
On Thu, Aug 10, 2017 at 12:39 AM, Uros Bizjak  wrote:
> On Thu, Aug 10, 2017 at 9:09 AM, Richard Sandiford
>  wrote:
>> "H.J. Lu"  writes:
>>> On Wed, Aug 9, 2017 at 10:28 AM, H.J. Lu  wrote:
 On Wed, Aug 9, 2017 at 8:26 AM, Andi Kleen  wrote:
>> This must be much more specific.  How does it impact:
>>
>> 1. LTO
>> 2. Function inlining.
>> 3. Partial function inlining.
>> 4. Shrink-wrapping.
>>
>> Any of them can impact function stack frame.
>
> It doesn't. It's just to get back to the previous state.
>
> Also these others already have explicit options to disable them.
>

 How about

 item -fkeep-frame-pointer
 @opindex fkeep-frame-pointer
 Keep the frame pointer in a register for functions.  This option always
 forces a new stack frame for all functions regardless of whether
 @option{-fomit-frame-pointer} is enabled or not.  Disabled by default.

>>>
>>> Here is the updated patch with -fkeep-frame-pointer.
>>
>> It sounds like there's still some disagreement about what this option
>> should mean; Andi's and Arjan's replies didn't seem to be asking for
>> the same thing.
>>
>> I think as implemented the patch just retains the GCC 7 x86 behaviour of
>> -fno-omit-frame-pointer, i.e. forces a frame to be created *somewhere*
>> between the start and end addresses of the function, but makes no
>> guarantee where.  It could be a bundle of three instructions somewhere
>> in the middle of a basic block, and the code might not be executed for
>> all paths through the function (e.g. it might only be executed on
>> error paths).
>>
>> I think even if we think that's useful, it should be documented clearly.
>> Otherwise people might assume that a function f is guaranteed to set up
>> a frame every time it's called.
>
> As said earlier, I think we should proceed with the previous patch
> (that documents -fno-omit-frame-pointer limitations), It was
> demonstrated that the patch does not make current situation worse.
>

I will check in my previous patch today.

Thanks.


-- 
H.J.


Re: [PATCH][v2] Introduce TARGET_SUPPORTS_ALIASES

2017-08-10 Thread Jan Hubicka
> On 07/31/2017 11:57 AM, Yuri Gribov wrote:
> > On Mon, Jul 31, 2017 at 9:04 AM, Martin Liška  wrote:
> >> Hi.
> >>
> >> Doing the transformation suggested by Honza.
> >>
> >> Patch can bootstrap on ppc64le-redhat-linux and x86_64-linux-gnu and 
> >> survives regression tests.
> >> And I also verified that works on hppa2.0w-hp-hpux11.11 (target w/o 
> >> aliasing support).
> >>
> >> Ready to be installed?
> > 
> > A nit - you can probly get rid of ATTRIBUTE_UNUSED in note_mangling_alias 
> > now.
> > 
> > -Y
> > 
> 
> Sure.
> 
> Done in v2.
> 
> Martin

> >From 78ee08b25d22125cb1fa248bac98ef1e84504761 Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Tue, 25 Jul 2017 13:11:28 +0200
> Subject: [PATCH] Introduce TARGET_SUPPORTS_ALIASES
> 
> gcc/c-family/ChangeLog:
> 
> 2017-07-25  Martin Liska  
> 
>   * c-opts.c (c_common_post_options): Replace ASM_OUTPUT_DEF with
>   TARGET_SUPPORTS_ALIASES.
> 
> gcc/ChangeLog:
> 
> 2017-07-25  Martin Liska  
> 
>   * asan.c (asan_protect_global): Replace ASM_OUTPUT_DEF with
>   TARGET_SUPPORTS_ALIASES.
>   * cgraph.c (cgraph_node::create_same_body_alias): Likewise.
>   * ipa-visibility.c (can_replace_by_local_alias): Likewise.
>   (optimize_weakref): Likewise.
>   * symtab.c (symtab_node::noninterposable_alias): Likewise.
>   * varpool.c (varpool_node::create_extra_name_alias): Likewise.
>   * defaults.h: Introduce TARGET_SUPPORTS_ALIASES.
> 
> gcc/cp/ChangeLog:
> 
> 2017-07-25  Martin Liska  
> 
>   * decl2.c (get_tls_init_fn): Replace ASM_OUTPUT_DEF with
>   TARGET_SUPPORTS_ALIASES.
>   (handle_tls_init): Likewise.
>   (note_mangling_alias): Likewise.  Remove ATTRIBUTE_UNUSED for
>   both arguments.
>   * optimize.c (can_alias_cdtor): Likewise.

OK,
thanks!

Honza


Re: [PATCH] rs6000: Use SAVE_MULTIPLE only if we restore what it saves (PR80938)

2017-08-10 Thread Segher Boessenkool
On Thu, Aug 10, 2017 at 02:17:40PM +0930, Alan Modra wrote:
> > > Ick, looks like papering over the real problem to me, and will no
> > > doubt cause -Os size regressions.
> > 
> > I think it is very directly solving the real problem.  It isn't likely
> > to cause size regressions (look how long it took for this PR to show
> > up, so this cannot happen often).
> > 
> > This only happens if r30 (the PIC reg) is used but r31 isn't; which means
> > that a) there are no other registers to save, or b) the function is marked
> > as needing a hard frame pointer but eventually doesn't need one.
> > 
> > (RA picks the registers r31, r30, ... in sequence).
> > 
> > In the case in the PR, this patch replaces one insn by one (cheaper)
> > insn.
> 
> And in other cases your patch will prevent stmw when it should be
> used.  Testcase attached.  It shows the wrong use of lmw too.

I dunno...  If you change r25 to r14 in that testcase it will use stmw 14
with my patch reverted.  Not very reasonable imho (but then again, people
using register asm like this get what they deserve anyway).


Segher


Re: [PATCH] Make inlining consistent in LTO and non-LTO mode (PR target/71991).

2017-08-10 Thread Jan Hubicka
> > -  /* If callee has no option attributes, then it is ok to inline.  */
> > -  if (!callee_tree)
> > +  /* If callee has no option attributes (or default),
> > + then it is ok to inline.  */
> > +  if (!callee_tree || callee_tree == target_option_default_node)
> 
>  I am not sure this actually makes sense, because 
>  target_option_default_node is not very
>  meaningful for LTO (it contains whatever was passed to LTO driver). 
> >>>
> >>> I see!
> >>>
> >>>  Perhaps one can check
>  for explicit optimization/machine attribute and whether caller and 
>  callee come from
> > 
> > I'm not sure what you mean by 'for explicit optimization/machine attribute' 
> > ?

You can simply do lookup_attribute and see if callee_tree was set because of 
attribute
or because of LTO.  In current patch the comparsion with 
target_option_default_node
still does not make much of sense.  But perhaps just 
lookup_attribute ("optimize", DECL_ATTRIBUTES (...)) would do the job. This is 
already
done in ipa-inline.

Honza
> > 
> > I'm attaching a new patch, is it closer?
> > 
> > Martin
> > 
>  same compilation unit, though this is quite hackish and will do 
>  unexpected things with COMDATs.
> >>>
> >>> That's quite cumbersome. Any other idea than marking the PR as won't fix?
> >>
> >> Yep, it is not prettiest. The problem is that the concept that callee can 
> >> change semantics
> >> when no explicit attribute is present is sloppy.  I am not sure how many 
> >> programs rely on it
> >> (it is kind of surprising to see functions not being inlined into your 
> >> target attribute annotated
> >> function I guess).
> >> Note that we check for original file in inliner already - this can be done 
> >> by comparing lto_file_data
> >> of corresponding cgraph nodes.
> >>
> >> Honza
> >>
> > 


PR81738: Split vect-alias-check-6.c

2017-08-10 Thread Richard Sandiford
The second loop in the testcase only vectorises if we can reverse
a vector and if aligned loads aren't required.

Sanity-checked on aarch64-linux-gnu, x86_64-linux-gnu and
powerpc64le-linux-gnu (although all three were unaffected).
OK to install?

Thanks,
Richard


2017-08-10  Richard Sandiford  

gcc/testsuite/
PR testsuite/81738
* gcc.dg/vect/vect-alias-check-6.c: Move second function to...
* gcc.dg/vect/vect-alias-check-7.c: ...this new file.  Require
vect_perm and vect_element_align for vectorization.

Index: gcc/testsuite/gcc.dg/vect/vect-alias-check-6.c
===
--- gcc/testsuite/gcc.dg/vect/vect-alias-check-6.c  2017-08-04 
11:40:26.372205514 +0100
+++ gcc/testsuite/gcc.dg/vect/vect-alias-check-6.c  2017-08-10 
14:36:24.201888108 +0100
@@ -12,12 +12,5 @@ f1 (struct s *a, struct s *b)
 a->x[i + 1] += b->x[i];
 }
 
-void
-f2 (struct s *a, struct s *b)
-{
-  for (int i = 0; i < N; ++i)
-a->x[i] += b->x[N - i - 1];
-}
-
-/* { dg-final { scan-tree-dump-times {checking that [^\n]* and [^\n]* have 
different addresses} 2 "vect" } } */
-/* { dg-final { scan-tree-dump-times "LOOP VECTORIZED" 2 "vect" } } */
+/* { dg-final { scan-tree-dump {checking that [^\n]* and [^\n]* have different 
addresses} "vect" } } */
+/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
Index: gcc/testsuite/gcc.dg/vect/vect-alias-check-7.c
===
--- /dev/null   2017-08-09 18:16:39.535015779 +0100
+++ gcc/testsuite/gcc.dg/vect/vect-alias-check-7.c  2017-08-10 
14:36:24.201888108 +0100
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_int } */
+
+#define N 16
+
+struct s { int x[N]; };
+
+void
+f1 (struct s *a, struct s *b)
+{
+  for (int i = 0; i < N; ++i)
+a->x[i] += b->x[N - i - 1];
+}
+
+/* { dg-final { scan-tree-dump {checking that [^\n]* and [^\n]* have different 
addresses} "vect" } } */
+/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" { target { vect_perm 
&& vect_element_align } } } } */


[PATCH, rs6000] fold-vec testcase fix-ups

2017-08-10 Thread Will Schmidt
Hi, 
A testcase coverage issue and an obvious typo fix.

Mostly obvious,..  OK for trunk? 
Thanks,
-Will

[gcc/testsuite]

2017-08-10  Will Schmidt  

* gcc.target/powerpc/fold-vec-msum-short.c: Fix typo.
* gcc.target/powerpc/fold-vec/pack-longlong.c: Mark for 64-bit only.


diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-msum-short.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-msum-short.c
index 61e1d35..2e590ab 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-msum-short.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-msum-short.c
@@ -13,11 +13,11 @@ test_msum_si (vector signed short vss2, vector signed short 
vss3,
 {
   return vec_msum (vss2, vss3, vsi2);
 }
 
 vector unsigned int
-test_msum)ui (vector unsigned short vus2, vector unsigned short vus3,
+test_msum_ui (vector unsigned short vus2, vector unsigned short vus3,
   vector unsigned int vui2)
 {
   return vec_msum (vus2, vus3, vui2);
 }
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c
index d8acc3c..73131bb 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-pack-longlong.c
@@ -1,9 +1,9 @@
 /* Verify that overloaded built-ins for vec_pack with long long
inputs produce the right results.  */
 
-/* { dg-do compile } */
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
 /* { dg-require-effective-target powerpc_p8vector_ok } */
 /* { dg-options "-mvsx -mpower8-vector -O2" } */
 
 #include 
 




[AArch64, PATCH] Improve Neon store of zero

2017-08-10 Thread Jackson Woodruff

Hi all,

This patch changes patterns in aarch64-simd.md to replace

moviv0.4s, 0
strq0, [x0, 16]

With:

stp xzr, xzr, [x0, 16]

When we are storing zeros to vectors like this:

void f(uint32x4_t *p) {
  uint32x4_t x = { 0, 0, 0, 0};
  p[1] = x;
}

Bootstrapped and regtested on aarch64 with no regressions.
OK for trunk?

Jackson

gcc/

2017-08-09  Jackson Woodruff  

* aarch64-simd.md (mov): No longer force zero
immediate into register.
(*aarch64_simd_mov): Add new case for stp
using zero immediate.


gcc/testsuite

2017-08-09  Jackson Woodruff  

* gcc.target/aarch64/simd/neon_str_zero.c: New.

diff --git a/gcc/config/aarch64/aarch64-simd.md 
b/gcc/config/aarch64/aarch64-simd.md
index 
74de9b8c89dd5e4e3d87504594c969de0e0128ce..0149a742d34ae4fd5b3fd705b03c845f94aa1d59
 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -23,7 +23,10 @@
(match_operand:VALL_F16 1 "general_operand" ""))]
   "TARGET_SIMD"
   "
-if (GET_CODE (operands[0]) == MEM)
+if (GET_CODE (operands[0]) == MEM
+   && !(aarch64_simd_imm_zero (operands[1], mode)
+&& aarch64_legitimate_address_p (mode, operands[0],
+ PARALLEL, 1)))
   operands[1] = force_reg (mode, operands[1]);
   "
 )
@@ -94,63 +97,70 @@
 
 (define_insn "*aarch64_simd_mov"
   [(set (match_operand:VD 0 "nonimmediate_operand"
-   "=w, m,  w, ?r, ?w, ?r, w")
+   "=w, m,  m,  w, ?r, ?w, ?r, w")
(match_operand:VD 1 "general_operand"
-   "m,  w,  w,  w,  r,  r, Dn"))]
+   "m,  Dz, w,  w,  w,  r,  r, Dn"))]
   "TARGET_SIMD
-   && (register_operand (operands[0], mode)
-   || register_operand (operands[1], mode))"
+   && ((register_operand (operands[0], mode)
+   || register_operand (operands[1], mode))
+  || (memory_operand (operands[0], mode)
+ && immediate_operand (operands[1], mode)))"
 {
switch (which_alternative)
  {
  case 0: return "ldr\\t%d0, %1";
- case 1: return "str\\t%d1, %0";
- case 2: return "mov\t%0., %1.";
- case 3: return "umov\t%0, %1.d[0]";
- case 4: return "fmov\t%d0, %1";
- case 5: return "mov\t%0, %1";
- case 6:
+ case 1: return "str\\txzr, %0";
+ case 2: return "str\\t%d1, %0";
+ case 3: return "mov\t%0., %1.";
+ case 4: return "umov\t%0, %1.d[0]";
+ case 5: return "fmov\t%d0, %1";
+ case 6: return "mov\t%0, %1";
+ case 7:
return aarch64_output_simd_mov_immediate (operands[1],
  mode, 64);
  default: gcc_unreachable ();
  }
 }
-  [(set_attr "type" "neon_load1_1reg, neon_store1_1reg,\
+  [(set_attr "type" "neon_load1_1reg, neon_stp, neon_store1_1reg,\
 neon_logic, neon_to_gp, f_mcr,\
 mov_reg, neon_move")]
 )
 
 (define_insn "*aarch64_simd_mov"
   [(set (match_operand:VQ 0 "nonimmediate_operand"
-   "=w, m,  w, ?r, ?w, ?r, w")
+   "=w, Ump,  m,  w, ?r, ?w, ?r, w")
(match_operand:VQ 1 "general_operand"
-   "m,  w,  w,  w,  r,  r, Dn"))]
+   "m,  Dz, w,  w,  w,  r,  r, Dn"))]
   "TARGET_SIMD
-   && (register_operand (operands[0], mode)
-   || register_operand (operands[1], mode))"
+   && ((register_operand (operands[0], mode)
+   || register_operand (operands[1], mode))
+   || (memory_operand (operands[0], mode)
+  && immediate_operand (operands[1], mode)))"
 {
   switch (which_alternative)
 {
 case 0:
return "ldr\\t%q0, %1";
 case 1:
-   return "str\\t%q1, %0";
+   return "stp\\txzr, xzr, %0";
 case 2:
-   return "mov\t%0., %1.";
+   return "str\\t%q1, %0";
 case 3:
+   return "mov\t%0., %1.";
 case 4:
 case 5:
-   return "#";
 case 6:
+   return "#";
+case 7:
return aarch64_output_simd_mov_immediate (operands[1], mode, 128);
 default:
gcc_unreachable ();
 }
 }
   [(set_attr "type" "neon_load1_1reg, neon_store1_1reg,\
- neon_logic, multiple, multiple, multiple,\
- neon_move")
-   (set_attr "length" "4,4,4,8,8,8,4")]
+neon_stp, neon_logic, multiple, multiple,\
+multiple, neon_move")
+   (set_attr "length" "4,4,4,4,8,8,8,4")]
 )
 
 ;; When storing lane zero we can use the normal STR and its more permissive
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/neon_str_zero.c 
b/gcc/testsuite/gcc.target/aarch64/simd/neon_str_zero.c
new file mode 100644
index 
..07198de109432b530745cc540790303ae0245efb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/neon_str_zero.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+#include 
+
+void
+f (uint32x4_t *p)
+{

Re: [PATCH 6/6] qsort comparator consistency checking

2017-08-10 Thread Alexander Monakov
On Wed, 9 Aug 2017, Jeff Law wrote:
> >> The _5th macro isn't that bad either, appart from using reserved namespace
> >> identifiers (it really should be something like qsort_5th and the arguments
> >> shouldn't start with underscores).
> > 
> > I didn't understand what Jeff found "ugly" about it; I wonder what epithets
> > would be applied then to more, ahem, unusual parts of GCC.
> I doubt anyone would want to hear what I might say about other code.
> I'm sure I wouldn't want my kids reading how I might refer to certain
> parts of GCC.

Imho it's no good to just say "ugly" in patch review without any further
elaboration, it only serves to provide a minor chilling effect, telling
the contributor they haven't done good enough (for your personal taste)
without informing them where/how they could have improved.

More importantly, am I correct in understanding that at this point all
remaining changes are reviewed and approved, and I can go ahead with
preparing vec::qsort -> vec::sort mass-renaming patch and rebasing the
others on top of it?  Or is the original approach with argument-counting
trick still under consideration?

Thanks.
Alexander


Re: [PATCH] Add libgcc support for cache clearing on ARM VxWorks

2017-08-10 Thread Richard Earnshaw (lists)
On 01/08/17 15:31, Olivier Hainque wrote:
> Hello,
> 
> On top of previous changes reworking the arm-vxworks support
> 
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00085.html
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00075.html
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00078.html
> 
> This patch adds a variant implementation of _clear_cache
> for arm-vxworks*, needed for proper functioning of trampolines
> on targets with separate instruction/data caches.
> 
> This allows, among other things, Ada tasking programs to
> run on variety of configurations which we are exercising with
> at least ACATS nightly with gcc-6 based compilers.
> 
> As this is touching a common ARM implementation file (lib1funcs.S),
> this presumably requires approval from an ARM port maintainer. CC
> Richard for this, as he participated in recent discussions regarding
> VxWorks for ARM.
> 
> OK to commit ?
> 

I'm not going to object to this patch - it's ok at a GCC level; but I'm
not sure that architecturally this is going to work.  The implementation
of cache clearing is very specific to the implementation and I don't
think it is possible to write a portable single implementation.

I must admit, that not being a kernel expert, I'm not completely
familiar with all the details of cache maintenance, but I don't think a
single sequence like this will work in general.

Your code seems to focus only on the Data cache clearing.  Documentation
in extend.texi seems to suggest that __builtin___clear_cache is supposed
to clear the _instruction_ cache (which may of course involve also
synchronizing the Dcache to memory first).  Similar comments in md.texi
refer to the clear_cache instruction pattern flushing the Icache.  Note
that arm processors of any architecture revision do not automatically
synchronize I and D caches.

Before ARMv7 cache maintenance operations were not available in user
mode (I don't know if vxworks is expected to run only in a privileged mode).

On a system without caches the instructions will just abort

I believe that on v7 you also need ISB and DMB operations at the end of
the sequence.  But such instructions aren't available on older
architectures.

I will not object to this being committed (it's your port); but I'd
strongly recommend that you don't at this stage.

R.

> Thanks much in advance for your feedback,
> 
> With Kind Regards,
> 
> Olivier
> 
> 2017-08-01  Doug Rupp  
> Olivier Hainque  
> 
>   libgcc/
>   * config/arm/lib1funcs.S (_clear_cache): Add variant for __vxworks.
>   * config/arm/t-vxworks: New file, add _clear_cache to LIB1ASMFUNCS.
>   * config.host (arm-wrs-vxworks, arm-wrs-vxworks7): Add arm/t-vxworks
>   to tmake_file.
> 
>   gcc/
>   * config/arm/vxworks.h (CLEAR_INSN_CACHE): Define.
> 
> 
> 0004-Add-libgcc-support-for-insn-cache-clearing-on-ARM-Vx.patch
> 
> 
> --- a/gcc/config/arm/vxworks.h
> +++ b/gcc/config/arm/vxworks.h
> @@ -154,6 +154,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively. 
>  If not, see
>  #undef TARGET_DEFAULT_WORD_RELOCATIONS
>  #define TARGET_DEFAULT_WORD_RELOCATIONS 1
>  
> +/* Clear the instruction cache from `beg' to `end'.  This is
> +   implemented in lib1funcs.S, so ensure an error if this definition
> +   is used.  */
> +#undef  CLEAR_INSN_CACHE
> +#define CLEAR_INSN_CACHE(BEG, END) not_used
> +
>  /* Define this to be nonzero if static stack checking is supported.  */
>  #define STACK_CHECK_STATIC_BUILTIN 1
>  
> diff --git a/libgcc/config.host b/libgcc/config.host
> index 9556c77..a35c8fb7 100644
> --- a/libgcc/config.host
> +++ b/libgcc/config.host
> @@ -389,7 +389,7 @@ arc*-*-linux*)
>   extra_parts="$extra_parts crttls.o"
>   ;;
>  arm-wrs-vxworks|arm-wrs-vxworks7)
> - tmake_file="$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl 
> arm/t-softfp t-softfp"
> + tmake_file="$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl 
> arm/t-softfp t-softfp arm/t-vxworks"
>   extra_parts="$extra_parts crti.o crtn.o"
>   case ${host} in
>   *-*-vxworks7)
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index 8d8c3ce..4b21c02 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -1572,8 +1572,27 @@ LSYM(Lover12):
>   do_pop  {r7}
>   RET
>   FUNC_END clear_cache
> +#elif defined __vxworks
> + ARM_FUNC_START clear_cache
> +.L1:
> + mcr p15, 0, r0, c7, c11, 1  @ Clean data cache by MVA to PoU
> + mcr p15, 0, r0, c7, c10, 4  @ Ensure visibility of the data
> + @ cleaned from the cache
> + mcr p15, 0, r0, c7, c5, 1   @ Invalidate instruction cache by
> + @ MVA to PoU
> + mcr p15, 0, r0, c7, c5, 7   @ Invalidate branch predictor by
> + @ MVA to PoU
> + mcr p15, 0, r0, c7, c10, 4  @ Ensure 

Re: [nvptx, PR 81662, committed] Error out on nvptx for fpatchable-function-entry

2017-08-10 Thread Thomas Schwinge
Hi Tom!

On Thu, 3 Aug 2017 13:23:16 +0200, Tom de Vries  wrote:
> [ was: Re: [testsuite, PR81662, committed] Skip 
> fpatchable-function-entry tests for nvptx ]
> 
> On 08/03/2017 09:17 AM, Tom de Vries wrote:
> > fpatchable-function-entry requires nop, which nvptx does not have.

Generally, should we perhaps use something like the following to at least
make it obvious in the generated PTX code that the compiler tried to
generate a "nop"?

--- gcc/config/nvptx/nvptx.md
+++ gcc/config/nvptx/nvptx.md
@@ -989,10 +989,11 @@
 
 ;; Miscellaneous
 
+;; PTX doesn't define a real "nop" instruction.
 (define_insn "nop"
   [(const_int 0)]
   ""
-  "")
+  "/* nop */")
 
 (define_insn "return"
   [(return)]

I have not tested this, have not verified whether we need to set the
"predicable" attribute to "false" here (existing problem maybe?), have
not thought about any other implications.  But given that "comments in
PTX are treated as whitespace", that should be fine?


> > I've disabled the corresponding test for nvptx.

Conceptually ACK.  Not useful on PTX.


> This patch errors out on nvptx for fpatchable-function-entry.

> --- a/gcc/config/nvptx/nvptx.c
> +++ b/gcc/config/nvptx/nvptx.c
> @@ -180,6 +180,10 @@ nvptx_option_override (void)
>if (!global_options_set.x_flag_no_common)
>  flag_no_common = 1;
>  
> +  /* The patch area requires nops, which we don't have.  */
> +  if (function_entry_patch_area_size > 0)
> +sorry ("not generating patch area, nops not supported");
> +
>/* Assumes that it will see only hard registers.  */
>flag_var_tracking = 0;

I noticed that this doesn't trigger if using "-fpatchable-function-entry"
during offloading compilation (but it does trigger for
"-foffload=-fpatchable-function-entry").  Is this a) intentional or by
accident, and/or b) desired?


> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
> +
> +extern int a;
> +
> +int f3 (void);
> +
> +int
> +__attribute__((noinline))
> +f3 (void)
> +{
> +  return 5*a;
> +}
> +
> +/* { dg-excess-errors "sorry, unimplemented: not generating patch area, nops 
> not supported" } */

Given that the first argument of "dg-excess-errors" is just a comment,
shouldn't we instead explicitly scan for this specific diagnostic, while
also still watching for other excess errors?

--- gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
+++ gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-message "sorry, unimplemented: not generating patch area, nops not 
supported" "" { target *-*-* } 0 } */
 
 extern int a;
 
@@ -11,5 +12,3 @@ f3 (void)
 {
   return 5*a;
 }
-
-/* { dg-excess-errors "sorry, unimplemented: not generating patch area, nops 
not supported" } */


Grüße
 Thomas


Re: *ping* [patch, fortran] Fix PR 60355, missing error for BIND(C) outside module scope

2017-08-10 Thread Paul Richard Thomas
Hi Thomas,

It looks fine to me. OK for trunk.

Thanks

Paul

PS What about PR34640 ?


On 9 August 2017 at 20:44, Thomas Koenig  wrote:
> Am 02.08.2017 um 15:19 schrieb Thomas Koenig:
>>
>> the attached patch is a bit smaller than it looks, because most of
>> it is due to reformatting a large comment.  It is rather simple -
>> checking for an incorrectly placed BIND(C) variable was sometimes not
>> done because the test was mixed in with other tests where implicitly
>> typed variables were excluded.
>>
>> Regression-tested. OK for trunk?
>
>
> Ping.
>
> Patch at:
>
> https://gcc.gnu.org/ml/fortran/2017-08/msg00010.html
>
> Regards
>
> Thomas



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


[PATCH] Fix failing objc.dg/proto-lossage-4.m

2017-08-10 Thread Marek Polacek
With -m32 intptr_t can be just 'int', not 'long int', so accept that too.

Tested on x86_64-linux, applying to trunk.

2017-08-10  Marek Polacek  

* objc.dg/proto-lossage-4.m: Accept int/long int as intptr_t.

diff --git gcc/testsuite/objc.dg/proto-lossage-4.m 
gcc/testsuite/objc.dg/proto-lossage-4.m
index 4c6b560bab4..c9c80b7c427 100644
--- gcc/testsuite/objc.dg/proto-lossage-4.m
+++ gcc/testsuite/objc.dg/proto-lossage-4.m
@@ -28,13 +28,13 @@ long foo(void) {
   receiver += [receiver anotherValue]; /* { dg-warning "invalid receiver type 
.intptr_t." } */
 
   receiver += [(Obj *)receiver someValue]; /* { dg-warning ".Obj. may not 
respond to .\\-someValue." } */
-/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
integer from pointer without a cast" "" { target *-*-* } .-1 } */
+/* { dg-warning "assignment to 'intptr_t {aka (long )?int}' from 'id' makes 
integer from pointer without a cast" "" { target *-*-* } .-1 } */
 
   receiver += [(Obj *)receiver anotherValue];
   receiver += [(Obj  *)receiver someValue];
   receiver += [(Obj  *)receiver anotherValue];
   receiver += [objrcvr someValue]; /* { dg-warning ".Obj. may not respond to 
.\\-someValue." } */
-/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
integer from pointer without a cast" "" { target *-*-* } .-1 } */
+/* { dg-warning "assignment to 'intptr_t {aka (long )?int}' from 'id' makes 
integer from pointer without a cast" "" { target *-*-* } .-1 } */
 
   receiver += [objrcvr anotherValue];
   receiver += [(Obj  *)objrcvr someValue];
@@ -42,7 +42,7 @@ long foo(void) {
   receiver += [objrcvr2 someValue];
   receiver += [objrcvr2 anotherValue];
   receiver += [(Obj *)objrcvr2 someValue]; /* { dg-warning ".Obj. may not 
respond to .\\-someValue." } */
-/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
integer from pointer without a cast" "" { target *-*-* } .-1 } */
+/* { dg-warning "assignment to 'intptr_t {aka (long )?int}' from 'id' makes 
integer from pointer without a cast" "" { target *-*-* } .-1 } */
 
   receiver += [(Obj *)objrcvr2 anotherValue];
 

Marek


Re: [PATCH][www] Amend bugs/management.html with worklist items

2017-08-10 Thread Martin Liška
PING^1

On 05/30/2017 02:52 PM, Richard Biener wrote:
> 
> This patch (in the attempt to find a place to put a link to
> http://gcc.opensuse.org/gcc_bugzilla/ to) adds a list of suggestions
> where to look for work in bugzilla.
> 
> Ok?
> 
> Thanks,
> Richard.
> 
> 2017-05-30  Richard Biener  
> 
>   * management.html: Add list of suggestions where to look for work.
> 
> Index: management.html
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/bugs/management.html,v
> retrieving revision 1.32
> diff -u -r1.32 management.html
> --- management.html   3 Mar 2017 16:20:01 -   1.32
> +++ management.html   30 May 2017 12:50:46 -
> @@ -51,6 +51,18 @@
>  Work fields to indicate which versions still have the bug.
>  
>  
> +How to Find Something to Work On
> +
> +Use the bugzilla searches on the
> +https://gcc.gnu.org/;>main page to work on current
> +regressions.
> +Look for bugs in state UNCONFIRMED or listed on
> + href="http://gcc.opensuse.org/gcc_bugzilla/;>http://gcc.opensuse.org/gcc_bugzilla/
>  and do first
> +hand analysis.
> +Look for bugs with specific keywords, like easy-hack or
> +diagnostic.
> +
> +
>  Maintainer's View of Fields
>  
>  Bugzilla offers a number of different fields.  From a maintainer's
> 



Re: C PATCH to display types when printing a conversion warning (PR c/81233)

2017-08-10 Thread Marek Polacek
On Thu, Aug 10, 2017 at 10:52:54AM +0200, Andreas Schwab wrote:
> On Jul 13 2017, Marek Polacek <pola...@redhat.com> wrote:
> 
> > diff --git gcc/testsuite/objc.dg/proto-lossage-4.m 
> > gcc/testsuite/objc.dg/proto-lossage-4.m
> > index e72328b3703..4c6b560bab4 100644
> > --- gcc/testsuite/objc.dg/proto-lossage-4.m
> > +++ gcc/testsuite/objc.dg/proto-lossage-4.m
> > @@ -28,13 +28,13 @@ long foo(void) {
> >receiver += [receiver anotherValue]; /* { dg-warning "invalid receiver 
> > type .intptr_t." } */
> >  
> >receiver += [(Obj *)receiver someValue]; /* { dg-warning ".Obj. may not 
> > respond to .\\-someValue." } */
> > -/* { dg-warning "assignment makes integer from pointer without a cast" "" 
> > { target *-*-* } .-1 } */
> > +/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
> > integer from pointer without a cast" "" { target *-*-* } .-1 } */
> >  
> >receiver += [(Obj *)receiver anotherValue];
> >receiver += [(Obj  *)receiver someValue];
> >receiver += [(Obj  *)receiver anotherValue];
> >receiver += [objrcvr someValue]; /* { dg-warning ".Obj. may not respond 
> > to .\\-someValue." } */
> > -/* { dg-warning "assignment makes integer from pointer without a cast" "" 
> > { target *-*-* } .-1 } */
> > +/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
> > integer from pointer without a cast" "" { target *-*-* } .-1 } */
> >  
> >receiver += [objrcvr anotherValue];
> >receiver += [(Obj  *)objrcvr someValue];
> > @@ -42,7 +42,7 @@ long foo(void) {
> >receiver += [objrcvr2 someValue];
> >receiver += [objrcvr2 anotherValue];
> >receiver += [(Obj *)objrcvr2 someValue]; /* { dg-warning ".Obj. may not 
> > respond to .\\-someValue." } */
> > -/* { dg-warning "assignment makes integer from pointer without a cast" "" 
> > { target *-*-* } .-1 } */
> > +/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
> > integer from pointer without a cast" "" { target *-*-* } .-1 } */
> >  
> >receiver += [(Obj *)objrcvr2 anotherValue];
> >  
> 
> FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime  (test for warnings, line 30)
> FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime  (test for warnings, line 36)
> FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime  (test for warnings, line 44)
> FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20170810/gcc/testsuite/objc.dg/proto-lossage-4.m:30:12: 
> warning: assignment to 'intptr_t {aka int}' from 'id' makes integer from 
> pointer without a cast [-Wint-conversion]
> /daten/aranym/gcc/gcc-20170810/gcc/testsuite/objc.dg/proto-lossage-4.m:36:12: 
> warning: assignment to 'intptr_t {aka int}' from 'id' makes integer from 
> pointer without a cast [-Wint-conversion]
> /daten/aranym/gcc/gcc-20170810/gcc/testsuite/objc.dg/proto-lossage-4.m:44:12: 
> warning: assignment to 'intptr_t {aka int}' from 'id' makes integer from 
> pointer without a cast [-Wint-conversion]

Argh.  Sorry, will fix now.

Marek


Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)

2017-08-10 Thread Marek Polacek
On Thu, Aug 10, 2017 at 09:35:07AM +0200, Andreas Schwab wrote:
> FAIL: gcc.dg/compare2.c case 20 (test for warnings, line 49)
> FAIL: gcc.dg/compare2.c case 24 (test for warnings, line 57)
> FAIL: gcc.dg/compare2.c (test for excess errors)
> Excess errors:
> /usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:49:12: warning: 
> operand of ?: changes signedness from 'int' to 'unsigned int' due to 
> unsignedness of other operand [-Wsign-compare]
> /usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:57:12: warning: 
> operand of ?: changes signedness from 'int' to 'unsigned int' due to 
> unsignedness of other operand [-Wsign-compare]

Fixed.

Marek


[PATCH] Fix failing gcc.dg/compare2.c

2017-08-10 Thread Marek Polacek
Not sure how I missed this, but a few dg-warning and dg-bogus need updating.

Tested on x86_64-linux and ppc64le-linux, applying to trunk.

2017-08-10  Marek Polacek  

PR testsuite/81784
* gcc.dg/compare2.c: Update dg-bogus and dg-warning.

diff --git gcc/testsuite/gcc.dg/compare2.c gcc/testsuite/gcc.dg/compare2.c
index cfadaccb8af..f742e95f605 100644
--- gcc/testsuite/gcc.dg/compare2.c
+++ gcc/testsuite/gcc.dg/compare2.c
@@ -41,18 +41,18 @@ void f(int x, unsigned int y)
   y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different signedness" "case 
16" } */
 
   /* ?: branches are constants.  */
-  tf ? x : (tf?64:32); /* { dg-bogus "conditional expression" "case 17" } */
-  tf ? y : (tf?64:32); /* { dg-bogus "conditional expression" "case 18" } */
+  tf ? x : (tf?64:32); /* { dg-bogus "changes signedness" "case 17" } */
+  tf ? y : (tf?64:32); /* { dg-bogus "changes signedness" "case 18" } */
 
   /* ?: branches are signed constants.  */
-  tf ? x : (tf?64:-1); /* { dg-bogus "conditional expression" "case 19" } */
-  tf ? y : (tf?64:-1); /* { dg-warning "conditional expression" "case 20" } */
+  tf ? x : (tf?64:-1); /* { dg-bogus "changes signedness" "case 19" } */
+  tf ? y : (tf?64:-1); /* { dg-warning "changes signedness" "case 20" } */
 
   /* ?: branches are (recursively) constants.  */
-  tf ? x : (tf?64:(tf?128:256)); /* { dg-bogus "conditional expression" "case 
21" } */
-  tf ? y : (tf?64:(tf?128:256)); /* { dg-bogus "conditional expression" "case 
22" } */
+  tf ? x : (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 21" 
} */
+  tf ? y : (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 22" 
} */
 
   /* ?: branches are (recursively) signed constants.  */
-  tf ? x : (tf?64:(tf?128:-1)); /* { dg-bogus "conditional expression" "case 
23" } */
-  tf ? y : (tf?64:(tf?128:-1)); /* { dg-warning "conditional expression" "case 
24" } */
+  tf ? x : (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case 23" } 
*/
+  tf ? y : (tf?64:(tf?128:-1)); /* { dg-warning "changes signedness" "case 24" 
} */
 }

Marek


Re: C PATCH to display types when printing a conversion warning (PR c/81233)

2017-08-10 Thread Andreas Schwab
On Jul 13 2017, Marek Polacek <pola...@redhat.com> wrote:

> diff --git gcc/testsuite/objc.dg/proto-lossage-4.m 
> gcc/testsuite/objc.dg/proto-lossage-4.m
> index e72328b3703..4c6b560bab4 100644
> --- gcc/testsuite/objc.dg/proto-lossage-4.m
> +++ gcc/testsuite/objc.dg/proto-lossage-4.m
> @@ -28,13 +28,13 @@ long foo(void) {
>receiver += [receiver anotherValue]; /* { dg-warning "invalid receiver 
> type .intptr_t." } */
>  
>receiver += [(Obj *)receiver someValue]; /* { dg-warning ".Obj. may not 
> respond to .\\-someValue." } */
> -/* { dg-warning "assignment makes integer from pointer without a cast" "" { 
> target *-*-* } .-1 } */
> +/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
> integer from pointer without a cast" "" { target *-*-* } .-1 } */
>  
>receiver += [(Obj *)receiver anotherValue];
>receiver += [(Obj  *)receiver someValue];
>receiver += [(Obj  *)receiver anotherValue];
>receiver += [objrcvr someValue]; /* { dg-warning ".Obj. may not respond to 
> .\\-someValue." } */
> -/* { dg-warning "assignment makes integer from pointer without a cast" "" { 
> target *-*-* } .-1 } */
> +/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
> integer from pointer without a cast" "" { target *-*-* } .-1 } */
>  
>receiver += [objrcvr anotherValue];
>receiver += [(Obj  *)objrcvr someValue];
> @@ -42,7 +42,7 @@ long foo(void) {
>receiver += [objrcvr2 someValue];
>receiver += [objrcvr2 anotherValue];
>receiver += [(Obj *)objrcvr2 someValue]; /* { dg-warning ".Obj. may not 
> respond to .\\-someValue." } */
> -/* { dg-warning "assignment makes integer from pointer without a cast" "" { 
> target *-*-* } .-1 } */
> +/* { dg-warning "assignment to 'intptr_t {aka long int}' from 'id' makes 
> integer from pointer without a cast" "" { target *-*-* } .-1 } */
>  
>receiver += [(Obj *)objrcvr2 anotherValue];
>  

FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime  (test for warnings, line 30)
FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime  (test for warnings, line 36)
FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime  (test for warnings, line 44)
FAIL: objc.dg/proto-lossage-4.m -fgnu-runtime (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20170810/gcc/testsuite/objc.dg/proto-lossage-4.m:30:12: 
warning: assignment to 'intptr_t {aka int}' from 'id' makes integer from 
pointer without a cast [-Wint-conversion]
/daten/aranym/gcc/gcc-20170810/gcc/testsuite/objc.dg/proto-lossage-4.m:36:12: 
warning: assignment to 'intptr_t {aka int}' from 'id' makes integer from 
pointer without a cast [-Wint-conversion]
/daten/aranym/gcc/gcc-20170810/gcc/testsuite/objc.dg/proto-lossage-4.m:44:12: 
warning: assignment to 'intptr_t {aka int}' from 'id' makes integer from 
pointer without a cast [-Wint-conversion]

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.

2017-08-10 Thread Martin Liška
PING^2

On 07/27/2017 02:56 PM, Martin Liška wrote:
> PING^1
> 
> On 07/14/2017 10:35 AM, Martin Liška wrote:
>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška  wrote:
 On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>> Hello.
>>
>> This is patch that was originally installed by Jason and later reverted 
>> due to PR70422.
>> In the later PR Richi suggested a fix for that and Segher verified that 
>> it helped him
>> to survive regression tests. That's reason why I'm resending that.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression 
>> tests.
>>
>> Ready to be installed?
>> Martin
>
>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>> From: marxin 
>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>  symbol entry.
>>
>> gcc/cp/ChangeLog:
>>
>> 2017-04-20  Jason Merrill  
>>  Martin Liska  
>>  Segher Boessenkool  
>>
>>  PR c++/64266
>>  PR c++/70353
>>  PR bootstrap/70422
>>  Core issue 1962
>>  * decl.c (cp_fname_init): Decay the initializer to pointer.
>>  (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>  * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>  DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>  DECL_IGNORED_P.  Don't call cp_finish_decl.
>
> If we don't emit those into the debug info, will the debugger be
> able to handle __FUNCTION__ etc. properly?

 No, debugger with the patch can't handled these. Similar to how clang
 behaves currently. Maybe it can be conditionally enabled with -g3, or -g?

> Admittedly, right now we emit it into debug info only if those decls
> are actually used, say on:
> const char * foo () { return __FUNCTION__; }
> const char * bar () { return ""; }
> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
> has to have some handling of it anyway.  But while in functions
> that don't refer to __FUNCTION__ it is always the debugger that needs
> to synthetize those and thus they will be always pointer-equal,
> if there are some uses of it and for other uses the debugger would
> synthetize it, there is the possibility that the debugger synthetized
> string will not be the same object as actually used in the function.

 You're right, currently one has to use a special function to be able to
 print it in debugger. I believe we've already discussed that, according
 to spec, the strings don't have to point to a same string.

 Suggestions what we should do with the patch?
>>>
>>> We need to emit debug information for these variables.  From Jim's
>>> description in 70422 it seems that the problem is that the reference
>>> to the string from the debug information is breaking
>>> function_mergeable_rodata_prefix, which relies on
>>> current_function_decl.  It seems to me that its callers should pass
>>> along their decl parameter so that f_m_r_p can use the decl's
>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>> properly> 
>>> Jason
>>>
>>
>> Ok, after some time I returned back to it. I followed your advises and
>> changed the function function_mergeable_rodata_prefix. Apart from a small
>> rebase was needed.
>>
>> May I ask Jim to test the patch?
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Martin
>>
> 



Re: [PATCH][RFC] Make expansion of balanced binary trees of switches on tree level.

2017-08-10 Thread Martin Liška
On 08/02/2017 01:51 PM, Richard Biener wrote:
> On Wed, Aug 2, 2017 at 1:20 PM, Martin Liška  wrote:
>> Hello.
>>
>> After some discussions with Honza, I've decided to convert current code in 
>> stmt.c that
>> is responsible for switch expansion. More precisely, I would like to convert 
>> the code
>> to expand gswitch statements on tree level. Currently the newly created pass 
>> is executed
>> at the end of tree optimizations.
>>
>> My plan for future is to inspire in [1] and come up with some more 
>> sophisticated switch
>> expansions. For that I've been working on a paper where I'll summarize 
>> statistics based
>> on what I've collected in openSUSE distribution with specially instrumented 
>> GCC. If I'll be
>> happy I can also fit in to schedule of this year's Cauldron with a talk.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Thoughts?
> 
> First of all thanks.
> 
> I think part of switch expansion moved to switch-conversion some time ago
> (emit_case_bit_tests).  So maybe the full lowering should be in at least
> the same source file and it should maybe applied earlier for a subset of
> cases (very low number of cases for example).

Hello.

Agree! Eventually we should merge both files into a single one. I would prefer 
to start
with new generic name of file (gimple-switch-low.c), which will eventually eat 
content
of tree-switch-conversion.c. Is it acceptable approach?

As Jeff already accepted the patch, I will install it as soon as an agreement 
in the question
will be done.

Martin

> 
> Did you base the code on the RTL expansion code or did you re-write it from
> scratch?
> 
> No further comments sofar.
> 
> Richard.
> 
>> Martin
>>
>> [1] https://www.nextmovesoftware.com/technology/SwitchOptimization.pdf
>>
>> gcc/ChangeLog:
>>
>> 2017-07-31  Martin Liska  
>>
>> * Makefile.in: Add gimple-switch-low.o.
>> * passes.def: Include pass_lower_switch.
>> * stmt.c (dump_case_nodes): Remove and move to
>> gimple-switch-low.c.
>> (case_values_threshold): Likewise.
>> (expand_switch_as_decision_tree_p): Likewise.
>> (emit_case_decision_tree): Likewise.
>> (expand_case): Likewise.
>> (balance_case_nodes): Likewise.
>> (node_has_low_bound): Likewise.
>> (node_has_high_bound): Likewise.
>> (node_is_bounded): Likewise.
>> (emit_case_nodes): Likewise.
>> * timevar.def: Add TV_TREE_SWITCH_LOWERING.
>> * tree-pass.h: Add make_pass_lower_switch.
>> * gimple-switch-low.c: New file.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-07-31  Martin Liska  
>>
>> * gcc.dg/tree-prof/update-loopch.c: Scan patterns in
>> switchlower.
>> * gcc.dg/tree-ssa/vrp104.c: Likewise.
>> ---
>>  gcc/Makefile.in|1 +
>>  gcc/gimple-switch-low.c| 1226 
>> 
>>  gcc/passes.def |1 +
>>  gcc/stmt.c |  861 -
>>  gcc/testsuite/gcc.dg/tree-prof/update-loopch.c |   10 +-
>>  gcc/testsuite/gcc.dg/tree-ssa/vrp104.c |2 +-
>>  gcc/timevar.def|1 +
>>  gcc/tree-pass.h|1 +
>>  8 files changed, 1236 insertions(+), 867 deletions(-)
>>  create mode 100644 gcc/gimple-switch-low.c
>>
>>



[PATCH] Do not instrument void variables with MPX (PR tree-opt/79987).

2017-08-10 Thread Martin Liška
Hello.

In order to prevent the ICE, CHKP should not isntrument variables of void type.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2017-08-09  Martin Liska  

PR tree-opt/79987
* tree-chkp.c (chkp_get_bounds_for_decl_addr): Do not instrument
variables of void type.

gcc/testsuite/ChangeLog:

2017-08-09  Martin Liska  

PR tree-opt/79987
* gcc.target/i386/mpx/pr79987.c: New test.
---
 gcc/testsuite/gcc.target/i386/mpx/pr79987.c | 5 +
 gcc/tree-chkp.c | 3 +++
 2 files changed, 8 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/mpx/pr79987.c


diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr79987.c b/gcc/testsuite/gcc.target/i386/mpx/pr79987.c
new file mode 100644
index 000..b3ebda95694
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr79987.c
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
+
+extern void foo;
+void *bar =  /* { dg-warning "taking address of expression of type .void." } */
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 12af458fb90..951aec10b3a 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -3197,6 +3197,9 @@ chkp_get_bounds_for_decl_addr (tree decl)
   && !flag_chkp_incomplete_type)
   return chkp_get_zero_bounds ();
 
+  if (VOID_TYPE_P (TREE_TYPE (decl)))
+return chkp_get_zero_bounds ();
+
   if (flag_chkp_use_static_bounds
   && VAR_P (decl)
   && (TREE_STATIC (decl)



Re: [PATCH] i386: Don't use frame pointer without stack access

2017-08-10 Thread Uros Bizjak
On Thu, Aug 10, 2017 at 9:09 AM, Richard Sandiford
 wrote:
> "H.J. Lu"  writes:
>> On Wed, Aug 9, 2017 at 10:28 AM, H.J. Lu  wrote:
>>> On Wed, Aug 9, 2017 at 8:26 AM, Andi Kleen  wrote:
> This must be much more specific.  How does it impact:
>
> 1. LTO
> 2. Function inlining.
> 3. Partial function inlining.
> 4. Shrink-wrapping.
>
> Any of them can impact function stack frame.

 It doesn't. It's just to get back to the previous state.

 Also these others already have explicit options to disable them.

>>>
>>> How about
>>>
>>> item -fkeep-frame-pointer
>>> @opindex fkeep-frame-pointer
>>> Keep the frame pointer in a register for functions.  This option always
>>> forces a new stack frame for all functions regardless of whether
>>> @option{-fomit-frame-pointer} is enabled or not.  Disabled by default.
>>>
>>
>> Here is the updated patch with -fkeep-frame-pointer.
>
> It sounds like there's still some disagreement about what this option
> should mean; Andi's and Arjan's replies didn't seem to be asking for
> the same thing.
>
> I think as implemented the patch just retains the GCC 7 x86 behaviour of
> -fno-omit-frame-pointer, i.e. forces a frame to be created *somewhere*
> between the start and end addresses of the function, but makes no
> guarantee where.  It could be a bundle of three instructions somewhere
> in the middle of a basic block, and the code might not be executed for
> all paths through the function (e.g. it might only be executed on
> error paths).
>
> I think even if we think that's useful, it should be documented clearly.
> Otherwise people might assume that a function f is guaranteed to set up
> a frame every time it's called.

As said earlier, I think we should proceed with the previous patch
(that documents -fno-omit-frame-pointer limitations), It was
demonstrated that the patch does not make current situation worse.

-fkeep-frame-pointer is an orthogonal issue, and this option should
guarantee frame formation in *all* situations.This means that the
option should disable (partial) inlining, shrink-wrapping, etc.
Actually, it would disable so much optimizations, that its usefullnes
is questioned. OTOH, nobody ever complained about limited FP debugging
"experience" when mentioned optimizations were activated.

BTW: The option should be called -fforce-frame-pointer, but I really
doubt about its usefullnes.

Uros.


Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)

2017-08-10 Thread Andreas Schwab
FAIL: gcc.dg/compare2.c case 20 (test for warnings, line 49)
FAIL: gcc.dg/compare2.c case 24 (test for warnings, line 57)
FAIL: gcc.dg/compare2.c (test for excess errors)
Excess errors:
/usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:49:12: warning: 
operand of ?: changes signedness from 'int' to 'unsigned int' due to 
unsignedness of other operand [-Wsign-compare]
/usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:57:12: warning: 
operand of ?: changes signedness from 'int' to 'unsigned int' due to 
unsignedness of other operand [-Wsign-compare]

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH 3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

2017-08-10 Thread Richard Biener
On August 10, 2017 7:26:33 AM GMT+02:00, Jeff Law  wrote:
>On 08/06/2017 02:07 PM, Martin Sebor wrote:
>> Part 3 of the series contains the meat of the patch: the new
>> -Wstringop-truncation option, and enhancements to -Wstringop-
>> overflow, and -Wpointer-sizeof-memaccess to detect misuses of
>> strncpy and strncat.
>> 
>> Martin
>> 
>> gcc-81117-3.diff
>> 
>> 
>> PR c/81117 - Improve buffer overflow checking in strncpy
>> 
>> gcc/ChangeLog:
>> 
>>  PR c/81117
>>  * builtins.c (compute_objsize): Handle arrays that
>>  compute_builtin_object_size likes to fail for.  Make extern.
>>  * builtins.h (compute_objsize): Declare.
>>  (check_strncpy_sizes): New function.
>>  (expand_builtin_strncpy): Call check_strncpy_sizes.
>>  * gimple-fold.c (gimple_fold_builtin_strncpy): Implement
>>  -Wstringop-truncation.
>>  (gimple_fold_builtin_strncat): Same.
>>  * gimple.c (gimple_build_call_from_tree): Set call location.
>>  * tree-ssa-strlen.c (strlen_to_stridx): New global variable.
>>  (maybe_diag_bound_equal_length, is_strlen_related_p): New functions.
>>  (handle_builtin_stxncpy, handle_builtin_strncat): Same.
>>  (handle_builtin_strlen): Use strlen_to_stridx.
>>  (strlen_optimize_stmt): Handle flavors of strncat, strncpy, and
>>  stpncpy.
>>  Use strlen_to_stridx.
>>  (pass_strlen::execute): Release strlen_to_stridx.
>>  * doc/invoke.texi (-Wsizeof-pointer-memaccess): Document
>enhancement.
>>  (-Wstringop-truncation): Document new option.
>> 
>> gcc/c-family/ChangeLog:
>> 
>>  PR c/81117
>>  * c-warn.c (sizeof_pointer_memaccess_warning): Handle arrays.
>>  * c.opt (-Wstriingop-truncation): New option.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>  PR c/81117
>>  * c-c++-common/Wsizeof-pointer-memaccess3.c: New test.
>>  * c-c++-common/Wstringop-overflow.c: Same.
>>  * c-c++-common/Wstringop-truncation.c: Same.
>>  * c-c++-common/Wsizeof-pointer-memaccess2.c: Adjust.
>>  * c-c++-common/attr-nonstring-2.c: New test.
>>  * g++.dg/torture/Wsizeof-pointer-memaccess1.C: Adjust.
>>  * g++.dg/torture/Wsizeof-pointer-memaccess2.C: Same.
>>  * gcc.dg/torture/pr63554.c: Same.
>>  * gcc.dg/Walloca-1.c: Disable macro tracking.
>> 
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index 016f68d..1aa9e22 100644
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>[ ... ]
>> +
>> +  if (TREE_CODE (type) == ARRAY_TYPE)
>> +{
>> +  /* Return the constant size unless it's zero (that's a
>zero-length
>> + array likely at the end of a struct).  */
>> +  tree size = TYPE_SIZE_UNIT (type);
>> +  if (size && TREE_CODE (size) == INTEGER_CST
>> +  && !integer_zerop (size))
>> +return size;
>> +}
>Q. Do we have a canonical test for the trailing array idiom?  

Array_at_struct_end_p

 In some
>contexts isn't it size 1?  ISTM This test needs slight improvement.
>Ideally we'd use some canonical test for detect the trailing array
>idiom
>rather than open-coding it here.  You might look at the array index
>warnings in tree-vrp.c to see if it's got a canonical test you can call
>or factor and use.
>
>
>
>
>> @@ -3883,6 +3920,30 @@ expand_builtin_strncat (tree exp, rtx)
>>return NULL_RTX;
>>  }
>>  
>> +/* Helper to check the sizes of sequences and the destination of
>calls
>> +   to __builtin_strncpy (DST, SRC, LEN) and __builtin___strncpy_chk.
>> +   Returns true on success (no overflow warning), false otherwise. 
>*/
>> +
>> +static bool
>> +check_strncpy_sizes (tree exp, tree dst, tree src, tree len)
>> +{
>> +  tree dstsize = compute_objsize (dst, warn_stringop_overflow - 1);
>> +
>> +  if (!check_sizes (OPT_Wstringop_overflow_,
>> +exp, len, /*maxlen=*/NULL_TREE, src, dstsize))
>> +return false;
>> +
>> +  if (!dstsize || TREE_CODE (len) != INTEGER_CST)
>> +return true;
>> +
>> +  if (tree_int_cst_lt (dstsize, len))
>> +warning_at (EXPR_LOCATION (exp), OPT_Wstringop_truncation,
>> +"%K%qD specified bound %E exceeds destination size %E",
>> +exp, get_callee_fndecl (exp), len, dstsize);
>> +
>> +  return true;
>So in the case where you issue the warning, what should the return
>value
>be?  According to the comment it should be false.  It looks like you
>got
>the wrong return value for the tree_int_cst_lt (dstsize, len) test.
>
>
>
>
>
>>  
>>return false;
>> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
>> index b0563fe..ac6503f 100644
>> --- a/gcc/tree-ssa-strlen.c
>> +++ b/gcc/tree-ssa-strlen.c
>
>> +
>> +/* A helper of handle_builtin_stxncpy.  Check to see if the
>specified
>> +   bound is a) equal to the size of the destination DST and if so,
>b)
>> +   if it's immediately followed by DST[LEN - 1] = '\0'.  If a) holds
>> +   and b) does not, warn.  Otherwise, do nothing.  Return true if
>> +   diagnostic has been issued.
>> +
>> +   The purpose is to diagnose calls to strncpy and stpncpy that do

Re: [PATCH] i386: Don't use frame pointer without stack access

2017-08-10 Thread Richard Sandiford
"H.J. Lu"  writes:
> On Wed, Aug 9, 2017 at 10:28 AM, H.J. Lu  wrote:
>> On Wed, Aug 9, 2017 at 8:26 AM, Andi Kleen  wrote:
 This must be much more specific.  How does it impact:

 1. LTO
 2. Function inlining.
 3. Partial function inlining.
 4. Shrink-wrapping.

 Any of them can impact function stack frame.
>>>
>>> It doesn't. It's just to get back to the previous state.
>>>
>>> Also these others already have explicit options to disable them.
>>>
>>
>> How about
>>
>> item -fkeep-frame-pointer
>> @opindex fkeep-frame-pointer
>> Keep the frame pointer in a register for functions.  This option always
>> forces a new stack frame for all functions regardless of whether
>> @option{-fomit-frame-pointer} is enabled or not.  Disabled by default.
>>
>
> Here is the updated patch with -fkeep-frame-pointer.

It sounds like there's still some disagreement about what this option
should mean; Andi's and Arjan's replies didn't seem to be asking for
the same thing.

I think as implemented the patch just retains the GCC 7 x86 behaviour of
-fno-omit-frame-pointer, i.e. forces a frame to be created *somewhere*
between the start and end addresses of the function, but makes no
guarantee where.  It could be a bundle of three instructions somewhere
in the middle of a basic block, and the code might not be executed for
all paths through the function (e.g. it might only be executed on
error paths).

I think even if we think that's useful, it should be documented clearly.
Otherwise people might assume that a function f is guaranteed to set up
a frame every time it's called.

Thanks,
Richard


Re: C++ PATCH for c++/81359, Unparsed NSDMI error in SFINAE context

2017-08-10 Thread Markus Trippelsdorf
On 2017.08.09 at 14:30 -0400, Jason Merrill wrote:
> The issue here is that we try to determine the EH specification of
> B::C::C() from within SFINAE context, and we can't determine it yet
> because the NSDMI for B::C::i hasn't been parsed yet.  This patch
> allows that determination to fail quietly in SFINAE context; we'll try
> again the next time it is needed.

Thanks.

Unfortunately it breaks the following testcase:

 ~ % cat asm-js.ii
struct A {
  void operator delete(void *, unsigned long);
};
struct B : A {
  virtual ~B();
};
struct C : B {};

 ~ % g++ -c asm-js.ii
asm-js.ii:7:8: error: no matching function for call to ‘C::operator 
delete(void*)’
 struct C : B {};
^
asm-js.ii:2:8: note: candidate: ‘static void A::operator delete(void*, long 
unsigned int)’
   void operator delete(void *, unsigned long);
^~~~
asm-js.ii:2:8: note:   candidate expects 2 arguments, 1 provided


-- 
Markus


Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

2017-08-10 Thread Richard Sandiford
Segher Boessenkool  writes:
> On Wed, Aug 09, 2017 at 05:54:40PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool  writes:
>> > We need it, for example, to properly cost the various define_insn_and_split
>> > (for which "type" is only an approximation, and is woefully inadequate
>> > for determining cost).
>> 
>> But define_insn_and_splits could override the cost explicitly if they
>> need to.  That seems neater than testing for them in C.
>
> All 190 of them?  Not counting those that are define_insn+define_split
> (we still have way too many of those).
>
> Neat, indeed, but not altogether practical :-(

Are there really 190 separate cases though?  If not, then it's
possible to have separate attributes that describe various forms of
multi-instruction pattern.  These can then get mapped automatically
to the appropriate "type" and often can also be used to set a
conservatively-correct "length".

Thanks,
Richard