Re: [PATCH] Fix another ICE with C++ addressable bitsize 0 return value (PR c++/82159)

[Jason Merrill]

OK.

On Wed, Oct 11, 2017 at 4:54 PM, Jakub Jelinek  wrote:
> Hi!
>
> Another case where we ICE because we optimize away store to bitsize 0
> addressable object from call.  We can't optimize them away, otherwise
> we'd try to create the temporaries again and fail to do so.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2017-10-11  Jakub Jelinek  
>
> PR c++/82159
> * expr.c (store_field): Don't optimize away bitsize == 0 store
> from CALL_EXPR with addressable return type.
>
> * g++.dg/opt/pr82159-2.C: New test.
>
> --- gcc/expr.c.jj   2017-10-10 22:04:06.0 +0200
> +++ gcc/expr.c  2017-10-11 16:48:45.428536126 +0200
> @@ -6749,8 +6749,11 @@ store_field (rtx target, HOST_WIDE_INT b
>  return const0_rtx;
>
>/* If we have nothing to store, do nothing unless the expression has
> - side-effects.  */
> -  if (bitsize == 0)
> + side-effects.  Don't do that for zero sized addressable lhs of
> + calls.  */
> +  if (bitsize == 0
> +  && (!TREE_ADDRESSABLE (TREE_TYPE (exp))
> + || TREE_CODE (exp) != CALL_EXPR))
>  return expand_expr (exp, const0_rtx, VOIDmode, EXPAND_NORMAL);
>
>if (GET_CODE (target) == CONCAT)
> --- gcc/testsuite/g++.dg/opt/pr82159-2.C.jj 2017-10-11 17:27:18.050861346 
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr82159-2.C2017-10-11 17:27:27.081753330 
> +0200
> @@ -0,0 +1,65 @@
> +// PR c++/82159
> +// { dg-do compile }
> +// { dg-options "" }
> +
> +template  struct D { T e; };
> +struct F : D {
> +  F(const F &);
> +};
> +struct G : F {
> +  template  G operator-(T);
> +};
> +template  struct I {
> +  typedef typename T::template J ak;
> +};
> +template  struct K { typename I::ak an; };
> +struct H {
> +  G l;
> +};
> +struct C {
> +  ~C();
> +};
> +template  struct M : T {
> +  template  M(U, V);
> +  H h;
> +  virtual void foo() { T::bar(); }
> +};
> +template  class A;
> +template  struct B {
> +  typedef int BT;
> +  struct BC {};
> +  template  struct BD {
> +G g;
> +BD(BT, T n) : g(n.l - 0) {}
> +  };
> +  B(BT, BC);
> +};
> +template  struct O;
> +template 
> +struct O > > : public B >::BC {};
> +struct L : B > {
> +  struct P : C {
> +void bar(H *x) {
> +  BT a;
> +  BD(a, *x);
> +}
> +  };
> +  template  L(U x, V n) : B(x, n) {}
> +  int ll;
> +  virtual int baz() { M(this, ll); }
> +};
> +template  class Q {
> +  O > > q;
> +  virtual L baz() { L(0, q); }
> +};
> +template  class T> struct R {
> +  R() { T(); }
> +};
> +struct S {
> +  template  class J : R {};
> +};
> +void foo() { K c; }
> +
> +int main() {
> +  return 0;
> +}
>
> Jakub

[PATCH 2/2] PR libgcc/59714 complex division is surprising on aarch64

[vladimir . mezentsev]

From: Vladimir Mezentsev 

FMA (floating-point multiply-add) instructions are supported on aarch64.
These instructions can produce different result if two operations executed 
separately.
-ffp-contract=off doesn't allow the FMA instructions.

Tested on aarch64-linux-gnu.
No regression. Two failed tests now passed.

ChangeLog:
2017-10-11  Vladimir Mezentsev  

PR libgcc/59714
* libgcc/config/aarch64/t-aarch64 (HOST_LIBGCC2_CFLAGS): Add -ffp-contract=off
---
 libgcc/config/aarch64/t-aarch64 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64
index 3af933c..e33bef0 100644
--- a/libgcc/config/aarch64/t-aarch64
+++ b/libgcc/config/aarch64/t-aarch64
@@ -18,4 +18,5 @@
 # along with GCC; see the file COPYING3.  If not see
 # .
 
+HOST_LIBGCC2_CFLAGS += -ffp-contract=off
 LIB2ADD += $(srcdir)/config/aarch64/sync-cache.c
-- 
1.8.3.1

Re: [PATCH 2/2] C/C++: add fix-it hints for various missing symbols (v3)

[Jason Merrill]

On Thu, Oct 5, 2017 at 12:08 PM, David Malcolm  wrote:
> Here's a slight update to this patch, since v2 was made invalid by
> r253411 ("C: underline parameters in mismatching function calls").
>
> Both v2 and r253411 added code to c-parser.c/h to track the location_t
> of the last consumed token (although I somehow managed to name the new
> field in c_parser differently between the two versions...)
>
> This version (v3) is the same as v2, but removes the copy of the
> above code, updating the usage sites to use the field name from
> r253411.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> in conjunction with patch 1 of the kit:
>   https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01745.html
>
> OK for trunk?

OK.

Jason

Re: Transform (x >> cst) != 0 to x >= (1 << cst) and (x >> cst) == 0 to x < (1 << cst)

[Jeff Law]

On 10/04/2017 12:34 PM, Prathamesh Kulkarni wrote:

>>> At a late stage, maybe during an RTL pass or expansion (or just before
>>> expansion) it would indeed be good to generate a shift for such
>>> comparisons, on targets where that sets a cc. The lack of this
>>> transformation could be considered a blocker for the other one, to avoid
>>> regressing on bar.
>> Right.  In fact, I think Jakub's test ought to be added to this work as
>> part of its basic testing.
> Um, how to write the above-test case for bar() in DejaGNU format ?
> Is it possible to test for number of insns or to use nm to test for
> size of a function with dejagnu directive ?
> Or should I scan for "cmpq" since the pattern transforms a right shift
> and cmp against 0 to cmp between operands ?
I've used a variety of approaches.  The difficulty in the x86 world is
that there's enough tuning variants that change the resulting code which
can make scanning problematical.

So one approach is to look at the total object size if that's a reliable
indicator of the issue at hand.

As an example see gcc.target/m68k/pr52076-?.c.  I'm sure there's others
if you were to search for "object-size" in the testsuite.

You could try to count the insns or search for specific key insns that
indicate we generated desirable or undesirable code.

But what might ultimately be best would be to scan the rtl at expansion
time.  That catches things at the earliest point we can observe them.

Another alternative would be to include some dumping code to indicate
when we transform the canonical gimple form into the form we want for
the x86 backend at rtl expansion.  You could then scan the debugging
dumps for those annotations.

jeff

Re: [patch] generate TAGS for params.def file

[Jeff Law]

On 10/09/2017 04:23 AM, Aldy Hernandez wrote:
> I hate grepping around for those --param definitions, so I've fixed it.
> 
> I also merged all the *.def files into one pattern.
> 
> OK for trunk?
> 
> curr
> 
> 
> gcc/
> 
>   * Makefile.in (TAGS): Merge all the *.def files into one pattern.
>   Handle params.def.
OK.
jeff

Re: [PATCH] Fix bitmap_bit_in_range_p (PR tree-optimization/82493).

[Jeff Law]

On 10/11/2017 12:13 AM, Martin Liška wrote:
> Hi.
> 
> This fixes some implementations mistakes in sbitmap.c 
> (bitmap_bit_in_range_p). There's reference
> to implementation one can take inspiration from:
> https://www.cs.umd.edu/class/sum2003/cmsc311/Notes/BitOp/bitRange.html
> 
> Problem with our implementation is that one can't do:
> (SBITMAP_ELT_TYPE)1 << SBITMAP_ELT_BITS (that would overflow)
> Thus I do conditionally ~(SBITMAP_ELT_TYPE)0 at some places in the code.
> 
> I also added quite some unit tests for the method. But another questions pop 
> up:
> 1) there are missing boundary asserts (or checking asserts) in sbitmap.c
> 2) we should probably include test-cases also for other functions
> 
> I can work on that (probably later) if desired?
> 
> And my patch breaks ssa-dse-26.c test-case, because now it properly returns 
> true for:
> 
> #0  bitmap_bit_in_range_p (bmap=0x21c4940, start=0, end=8) at 
> ../../gcc/sbitmap.c:326
> #1  0x00d618f5 in live_bytes_read (use_ref=..., ref=0x7fffd480, 
> live=0x21c4940) at ../../gcc/tree-ssa-dse.c:496
> #2  0x00d61c4d in dse_classify_store (ref=0x7fffd480, 
> stmt=0x13ea7d70, use_stmt=0x7fffd470, byte_tracking_enabled=true, 
> live_bytes=0x21c4940) at ../../gcc/tree-ssa-dse.c:594
> #3  0x00d6235b in dse_dom_walker::dse_optimize_stmt 
> (this=0x7fffd5c0, gsi=0x7fffd530) at ../../gcc/tree-ssa-dse.c:820
> #4  0x00d62461 in dse_dom_walker::before_dom_children 
> (this=0x7fffd5c0, bb=0x13d76270) at ../../gcc/tree-ssa-dse.c:852
> #5  0x013b1698 in dom_walker::walk (this=0x7fffd5c0, 
> bb=0x13d76270) at ../../gcc/domwalk.c:308
> #6  0x00d625ac in (anonymous namespace)::pass_dse::execute 
> (this=0x21d58c0, fun=0x13eac0b0) at ../../gcc/tree-ssa-dse.c:906
> #7  0x00b27441 in execute_one_pass (pass=pass@entry=0x21d58c0) at 
> ../../gcc/passes.c:2495
> #8  0x00b27d01 in execute_pass_list_1 (pass=0x21d58c0) at 
> ../../gcc/passes.c:2584
> #9  0x00b27d13 in execute_pass_list_1 (pass=0x21d5480) at 
> ../../gcc/passes.c:2585
> #10 0x00b27d55 in execute_pass_list (fn=, 
> pass=) at ../../gcc/passes.c:2595
> #11 0x00b26681 in do_per_function_toporder 
> (callback=callback@entry=0xb27d40 , 
> data=0x21d5300) at ../../gcc/passes.c:1737
> #12 0x00b283d7 in execute_ipa_pass_list (pass=0x21d52a0) at 
> ../../gcc/passes.c:2935
> #13 0x007d29d2 in ipa_passes () at ../../gcc/cgraphunit.c:2399
> #14 symbol_table::compile (this=this@entry=0x13d6e100) at 
> ../../gcc/cgraphunit.c:2534
> #15 0x007d5277 in symbol_table::compile (this=0x13d6e100) at 
> ../../gcc/cgraphunit.c:2695
> #16 symbol_table::finalize_compilation_unit (this=0x13d6e100) at 
> ../../gcc/cgraphunit.c:2692
> #17 0x00c118ac in compile_file () at ../../gcc/toplev.c:481
> #18 0x00c13eee in do_compile () at ../../gcc/toplev.c:2037
> #19 0x00c141da in toplev::main (this=0x7fffd85e, argc=21, 
> argv=0x7fffd958) at ../../gcc/toplev.c:2172
> #20 0x0061aeab in main (argc=21, argv=0x7fffd958) at 
> ../../gcc/main.c:39
> 
> (gdb) p debug(bmap)
> n_bits = 256, set = {8 9 10 11 12 13 14 15 }
> 
> Jeff can you please help me?
> Apart from that the patch can bootstrap on ppc64le-redhat-linux and survives 
> regression tests.
I think it's an off-by-1 error in the call to in_range_p.  It's an
inclusive check so it's checking 0, 1, 2, 3, 4, 5, 6, 7, 8 -- which
covers 9 bytes.  We really just wanted to cover 8 bytes.  I want to look
at the rest of tree-ssa-dse.c so see if there are other instances before
checking in that fix.


bitmap_bit_in_range_p is almost a direct copy from bitmap_set_range.
You might want to peek bitmap_set_range and see if it has the same set
of bugs.

jeff

Re: [PATCH 3/3] Add targetm.insn_cost hook

[Segher Boessenkool]

Hi!

On Wed, Oct 11, 2017 at 05:39:25PM -0600, Sandra Loosemore wrote:
> On 10/09/2017 01:35 PM, Segher Boessenkool wrote:
> >This adds a new hook that the insn_cost function uses if a target has
> >implemented it (it uses the old pattern_cost nee insn_rtx_cost if not).

> As a target maintainer, I'm kind of confused by this patch, and I don't 
> think the tm.texi change gives sufficient guidance about the default 
> hook behavior, how it interacts with TARGET_RTX_COSTS and/or 
> TARGET_ADDRESS_COST, or the different contexts the three hooks are used 
> in.  Do target maintainers need to do something to define this new hook 
> to prevent performance regressions?

No, everything works as it did if you don't do anything.  Should I add
a little bit of documentation what the hook does by default?  It may be
good as a scary story, to get more ports to use their own hook
implementation instead ;-)

> I could try to write up some advice about cost models and tuning for the 
> internals manual, but at present I don't feel like I have any 
> understanding of what motivated this change or how it changed the 
> recommended practices for back end tuning.  :-(

insn_rtx_cost did not work with instructions: it worked with instruction
_patterns_.  It also does not work for anything but single-set and similar
patterns (not the same as single_set, that requires an instruction!)

It also is really hard to write a TARGET_RTX_COSTS that properly describes
the cost of the machine instructions you have (and to keep it updated!)

With these patches combine can make better decisions already (no longer
do many instructions have "unknown" cost).  I am planning to move more
users of TARGET_RTX_COSTS over to insn_cost, or even get the needed info
in a more direct way.  This will take time; all help is welcome :-)


Segher

[PATCH,AIX] rs6000 output_aligned_decl_common fix

[David Edelsohn]

GCC toplev.c uses ASM_OUTPUT_ALIGNED_DECL_COMMON with a NULL decl to
emit the LTO marker, so the rs6000 implementation needs to test for
that situation.  config/darwin.c tests similarly.  With this patch,
gcc -flto produces correct output on AIX.

Bootstrapped on powerpc-ibm-aix7.2.0.0

Thanks, David

* config/rs6000/rs6000.c
(rs6000_xcoff_asm_output_aligned_decl_common): Test for NULL decl.

Index: rs6000.c
===
--- rs6000.c(revision 253666)
+++ rs6000.c(working copy)
@@ -34375,7 +34375,8 @@ rs6000_xcoff_asm_output_aligned_decl_common (FILE
   size, align2);

 #ifdef HAVE_GAS_HIDDEN
-  fputs (rs6000_xcoff_visibility (decl), stream);
+  if (decl != NULL)
+fputs (rs6000_xcoff_visibility (decl), stream);
 #endif
   putc ('\n', stream);
 }

[PATCH] rs6000: Remove TARGET_ISEL64

[Segher Boessenkool]

TARGET_ISEL64 just means TARGET_ISEL && TARGET_POWERPC64.  Since
everywhere it is used uses :GPR already, we can just as well use
TARGET_ISEL always.

Tested as usual, committing.


Segher


2017-10-11  Segher Boessenkool  

* config/rs6000/rs6000.h (TARGET_ISEL64): Delete.
* config/rs6000/rs6000.md (sel): Delete mode attribute.
(movcc, isel_signed_, isel_unsigned_,
*isel_reversed_signed_, *isel_reversed_unsigned_): Use
TARGET_ISEL instead of TARGET_ISEL.

---
 gcc/config/rs6000/rs6000.h  |  2 --
 gcc/config/rs6000/rs6000.md | 13 +
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 21e536b..5a5244a 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -565,8 +565,6 @@ extern int rs6000_vector_align[];
 #define TARGET_ALTIVEC_ABI rs6000_altivec_abi
 #define TARGET_LDBRX (TARGET_POPCNTD || rs6000_cpu == PROCESSOR_CELL)
 
-#define TARGET_ISEL64 (TARGET_ISEL && TARGET_POWERPC64)
-
 /* ISA 2.01 allowed FCFID to be done in 32-bit, previously it was 64-bit only.
Enable 32-bit fcfid's on any of the switches for newer ISA machines or
XILINX.  */
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 7e1566a..611aa9d 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -578,9 +578,6 @@ (define_mode_attr bits [(QI "8") (HI "16") (SI "32") (DI 
"64")])
 ; DImode bits
 (define_mode_attr dbits [(QI "56") (HI "48") (SI "32")])
 
-;; ISEL/ISEL64 target selection
-(define_mode_attr sel [(SI "") (DI "64")])
-
 ;; Bitmask for shift instructions
 (define_mode_attr hH [(SI "h") (DI "H")])
 
@@ -4915,7 +4912,7 @@ (define_expand "movcc"
 (if_then_else:GPR (match_operand 1 "comparison_operator" "")
   (match_operand:GPR 2 "gpc_reg_operand" "")
   (match_operand:GPR 3 "gpc_reg_operand" "")))]
-  "TARGET_ISEL"
+  "TARGET_ISEL"
   "
 {
   if (rs6000_emit_cmove (operands[0], operands[1], operands[2], operands[3]))
@@ -4940,7 +4937,7 @@ (define_insn "isel_signed_"
  (const_int 0)])
 (match_operand:GPR 2 "reg_or_zero_operand" "O,b")
 (match_operand:GPR 3 "gpc_reg_operand" "r,r")))]
-  "TARGET_ISEL"
+  "TARGET_ISEL"
   "isel %0,%2,%3,%j1"
   [(set_attr "type" "isel")])
 
@@ -4952,7 +4949,7 @@ (define_insn "isel_unsigned_"
  (const_int 0)])
 (match_operand:GPR 2 "reg_or_zero_operand" "O,b")
 (match_operand:GPR 3 "gpc_reg_operand" "r,r")))]
-  "TARGET_ISEL"
+  "TARGET_ISEL"
   "isel %0,%2,%3,%j1"
   [(set_attr "type" "isel")])
 
@@ -4968,7 +4965,7 @@ (define_insn "*isel_reversed_signed_"
  (const_int 0)])
 (match_operand:GPR 2 "gpc_reg_operand" "r,r")
 (match_operand:GPR 3 "reg_or_zero_operand" "O,b")))]
-  "TARGET_ISEL"
+  "TARGET_ISEL"
 {
   PUT_CODE (operands[1], reverse_condition (GET_CODE (operands[1])));
   return "isel %0,%3,%2,%j1";
@@ -4983,7 +4980,7 @@ (define_insn "*isel_reversed_unsigned_"
  (const_int 0)])
 (match_operand:GPR 2 "gpc_reg_operand" "r,r")
 (match_operand:GPR 3 "reg_or_zero_operand" "O,b")))]
-  "TARGET_ISEL"
+  "TARGET_ISEL"
 {
   PUT_CODE (operands[1], reverse_condition (GET_CODE (operands[1])));
   return "isel %0,%3,%2,%j1";
-- 
1.8.3.1

Re: [PATCH 3/3] Add targetm.insn_cost hook

[Sandra Loosemore]

On 10/09/2017 01:35 PM, Segher Boessenkool wrote:

This adds a new hook that the insn_cost function uses if a target has
implemented it (it uses the old pattern_cost nee insn_rtx_cost if not).

I'll commit this now; it was okayed by Jeff at
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00204.html .


Segher


2017-10-09  Segher Boessenkool  

* target.def (insn_cost): New hook.
* doc/tm.texi.in (TARGET_INSN_COST): New hook.
* doc/tm.texi: Regenerate.
* rtlanal.c (insn_cost): Use the new hook.


As a target maintainer, I'm kind of confused by this patch, and I don't 
think the tm.texi change gives sufficient guidance about the default 
hook behavior, how it interacts with TARGET_RTX_COSTS and/or 
TARGET_ADDRESS_COST, or the different contexts the three hooks are used 
in.  Do target maintainers need to do something to define this new hook 
to prevent performance regressions?


I could try to write up some advice about cost models and tuning for the 
internals manual, but at present I don't feel like I have any 
understanding of what motivated this change or how it changed the 
recommended practices for back end tuning.  :-(


-Sandra the missing-some-brain-cells

Re: [PATCH,AIX] Fix issue with PRI*64 on AIX.

[Ian Lance Taylor]

On Tue, Oct 10, 2017 at 2:09 AM, REIX, Tony  wrote:
> Description:
>  * This patch enables to build on AIX.
>
> Tests:
>  * AIX: Build: SUCCESS
>- build made by means of gmake within GCC 8 trunk.
>
> ChangeLog:
>   * go-system.h : Enable to build on AIX.
>   (fix issue with PRIx64 and PRIu64)

Thanks.  Committed with this ChangeLog entry.

2017-10-11  Tony Reix  

* go-system.h (__STDC_FORMAT_MACROS): Define before including any
system header files, as is done in ../system.h.

Ian

Re: patch to fix PR82353

[Vladimir Makarov]

On 10/11/2017 05:11 PM, Jakub Jelinek wrote:

On Wed, Oct 11, 2017 at 03:39:21PM -0400, Vladimir Makarov wrote:

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353

LRA did not update hard reg liveness on bb borders for hard regs which are
part of insn patterns like CFLAGS reg.  It was ok for inheritance in EBB
which creates only moves and they usually have no embedded hard regs in the
patterns.  But LRA rematerialization needs this.  So this patch implements
such hard reg liveness updates.

The patch was successfully bootstrapped and tested on x86-64.

Committed as rev. 253656.

Thanks for the fix, however I believe we do not want C++ tests in
gcc.target/*/, those are historically handled in g++.dg/ (and if we wanted
to have them, we'd introduce g++.target/*/), furthermore the test requires
working -fsanitize=undefined and such tests belog into g++.dg/ubsan/
if they are in C++.  And finally, the test fails on i386, where we
rematerialize something else, so I think we should just limit it to lp64.

I just followed arch64 which has C++ tests in gcc.target/arch64.

I think we will have more and more target tests on C++ in the future, so 
probably having g++.target is a good idea.

Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
change, ok for trunk?



Sure, Jakub.

[PATCH] rs6000: Improve isel

[Segher Boessenkool]

This removes output_isel.  Instead, the define_insn's now output the
isel instructions directly.

It adds a reg_or_zero operand predicate, too, because the reg_or_cint
predicate is too lax here.  Also use it in the "reversed" variants of
the instructions.

Tested on powerpc64-linux {-m32,-m64}; committing to trunk.


Segher


2017-10-11  Segher Boessenkool  

* config/rs6000/predicates.md (zero_constant, all_ones_constant):
Move up in file.
(reg_or_cint_operand): Fix comment.
(reg_or_zero_operand): New predicate.
* config/rs6000/rs6000-protos.h (output_isel): Delete.
* config/rs6000/rs6000.c (output_isel): Delete.
* config/rs6000/rs6000.md (isel_signed_): Use reg_or_zero_operand
instead of reg_or_cint_operand.  Output instruction directly (not via
output_isel).
(isel_unsigned_): Ditto.
(*isel_reversed_signed_): Use reg_or_zero_operand instead of
gpc_reg_operand.  Add an instruction alternative for this.  Output
instruction directly.
(*isel_reversed_unsigned_): Ditto.

---
 gcc/config/rs6000/predicates.md   | 28 --
 gcc/config/rs6000/rs6000-protos.h |  1 -
 gcc/config/rs6000/rs6000.c| 18 --
 gcc/config/rs6000/rs6000.md   | 50 +++
 4 files changed, 40 insertions(+), 57 deletions(-)

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index 237b432..569158f 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -199,6 +199,16 @@ (define_predicate "ca_operand"
   return CA_REGNO_P (REGNO (op));
 })
 
+;; Return 1 if operand is constant zero (scalars and vectors).
+(define_predicate "zero_constant"
+  (and (match_code "const_int,const_double,const_wide_int,const_vector")
+   (match_test "op == CONST0_RTX (mode)")))
+
+;; Return 1 if operand is constant -1 (scalars and vectors).
+(define_predicate "all_ones_constant"
+  (and (match_code "const_int,const_double,const_wide_int,const_vector")
+   (match_test "op == CONSTM1_RTX (mode) && !FLOAT_MODE_P (mode)")))
+
 ;; Return 1 if op is a signed 5-bit constant integer.
 (define_predicate "s5bit_cint_operand"
   (and (match_code "const_int")
@@ -543,12 +553,16 @@ (define_predicate "reg_or_u_short_operand"
 (match_operand 0 "u_short_cint_operand")
 (match_operand 0 "gpc_reg_operand")))
 
-;; Return 1 if op is any constant integer 
-;; or non-special register.
+;; Return 1 if op is any constant integer or a non-special register.
 (define_predicate "reg_or_cint_operand"
   (ior (match_code "const_int")
(match_operand 0 "gpc_reg_operand")))
 
+;; Return 1 if op is constant zero or a non-special register.
+(define_predicate "reg_or_zero_operand"
+  (ior (match_operand 0 "zero_constant")
+   (match_operand 0 "gpc_reg_operand")))
+
 ;; Return 1 if op is a constant integer valid for addition with addis, addi.
 (define_predicate "add_cint_operand"
   (and (match_code "const_int")
@@ -744,16 +758,6 @@ (define_predicate "easy_vector_constant_vsldoi"
(and (match_test "easy_altivec_constant (op, mode)")
 (match_test "vspltis_shifted (op) != 0")
 
-;; Return 1 if operand is constant zero (scalars and vectors).
-(define_predicate "zero_constant"
-  (and (match_code "const_int,const_double,const_wide_int,const_vector")
-   (match_test "op == CONST0_RTX (mode)")))
-
-;; Return 1 if operand is constant -1 (scalars and vectors).
-(define_predicate "all_ones_constant"
-  (and (match_code "const_int,const_double,const_wide_int,const_vector")
-   (match_test "op == CONSTM1_RTX (mode) && !FLOAT_MODE_P (mode)")))
-
 ;; Return 1 if operand is a vector int register or is either a vector constant
 ;; of all 0 bits of a vector constant of all 1 bits.
 (define_predicate "vector_int_reg_or_same_bit"
diff --git a/gcc/config/rs6000/rs6000-protos.h 
b/gcc/config/rs6000/rs6000-protos.h
index c6be5b1..db0e692 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -209,7 +209,6 @@ extern void rs6000_emit_epilogue (int);
 extern void rs6000_expand_split_stack_prologue (void);
 extern void rs6000_split_stack_space_check (rtx, rtx);
 extern void rs6000_emit_eh_reg_restore (rtx, rtx);
-extern const char * output_isel (rtx *);
 extern void rs6000_call_aix (rtx, rtx, rtx, rtx);
 extern void rs6000_sibcall_aix (rtx, rtx, rtx, rtx);
 extern void rs6000_aix_asm_output_dwarf_table_ref (char *);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 8b014e7..e868482 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -23255,24 +23255,6 @@ rs6000_emit_int_cmove (rtx dest, rtx op, rtx 
true_cond, rtx false_cond)
   return 1;
 }
 
-const char *
-output_isel (rtx *operands)
-{
-  enum rtx_code code;
-
-  code = GET_CODE (operands[1]);
-
-  if (code == GE || code == GEU || code == LE || code == LEU || 

Re: [PATCH] C++: show location of problematic extern "C" specifications

[David Malcolm]

On Wed, 2017-10-11 at 17:20 -0400, Jason Merrill wrote:
> On Wed, Oct 11, 2017 at 4:50 PM, David Malcolm 
> wrote:
> > On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote:
> > > On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm  > > om>
> > > wrote:
> > > > * cp-tree.h (struct saved_scope): Add "location" field.
> > > 
> > > saved_scope seems like the wrong place for this; it's only
> > > interesting
> > > at parse time, so having it saved during template instantiation
> > > doesn't seem useful.  I'd prefer to put it in cp_parser and have
> > > maybe_show_extern_c_location look in the_parser.
> > 
> > Thanks.
> > 
> > I have a new version of the patch *mostly* working that way, but
> > one of
> > the uses of maybe_show_extern_c_location is within
> > decl.c:grokfndecl
> > (when complaining about user-defined literal operators within C
> > linkage), and there doesn't seem to be access to the cp_parser *
> > from
> > there.
> > 
> > I could fix this by adding a cp_parser * for this to grokfndecl,
> > which
> > would mean adding it to grokdeclarator, but I get the impression
> > that
> > the code is structured so that the decl-handling isn't meant to
> > know
> > about the parser.
> 
> That's why I was thinking to look at "the_parser", which functions in
> parser.c can get at directly.
> 
> Jason

Aha - thanks: I missed the underscore and read it as "the parser". 
Sorry.

Am working on an updated patch.
Dave

[PATCH, i386]: Re-enable FICOM instruction with IX86_FPCMP_SAHF FP comparison strategy

[Uros Bizjak]

Hello!

"*cmp__cc_i387" is never matched by
combine. The order of operands in x87 ficom compare is forced by
combine in simplify_comparison () function. Float operator is treated
as RTX_OBJ with a precedence over other operators and is always put in
the first place.

Patch defines TARGET_CANONICALIZE_COMPARISON target hook to swap
condition and operands to match ficom instruction.

2017-10-11  Uros Bizjak  

* config/i386/i386.c (ix86_canonicalize_comparison): New function.
(TARGET_CANONICALIZE_COMPARISON): Define.

testsuite/ChangeLog:

2017-10-11  Uros Bizjak  

* gcc.target/i386/387-ficom-2.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 253653)
+++ config/i386/i386.c  (working copy)
@@ -4792,6 +4792,30 @@ ix86_conditional_register_usage (void)
   fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
 }
 
+/* Canonicalize a comparison from one we don't have to one we do have.  */
+
+static void
+ix86_canonicalize_comparison (int *code, rtx *op0, rtx *op1,
+ bool op0_preserve_value)
+{
+  /* The order of operands in x87 ficom compare is forced by combine in
+ simplify_comparison () function. Float operator is treated as RTX_OBJ
+ with a precedence over other operators and is always put in the first
+ place. Swap condition and operands to match ficom instruction.  */
+  if (!op0_preserve_value
+  && GET_CODE (*op0) == FLOAT && MEM_P (XEXP (*op0, 0)) && REG_P (*op1))
+{
+  enum rtx_code scode = swap_condition ((enum rtx_code) *code);
+
+  /* We are called only for compares that are split to SAHF instruction.
+Ensure that we have setcc/jcc insn for the swapped condition.  */
+  if (ix86_fp_compare_code_to_integer (scode) != UNKNOWN)
+   {
+ std::swap (*op0, *op1);
+ *code = (int) scode;
+   }
+}
+}
 
 /* Save the current options */
 
@@ -49649,6 +49673,9 @@ ix86_run_selftests (void)
 #undef TARGET_CONDITIONAL_REGISTER_USAGE
 #define TARGET_CONDITIONAL_REGISTER_USAGE ix86_conditional_register_usage
 
+#undef TARGET_CANONICALIZE_COMPARISON
+#define TARGET_CANONICALIZE_COMPARISON ix86_canonicalize_comparison
+
 #undef TARGET_LOOP_UNROLL_ADJUST
 #define TARGET_LOOP_UNROLL_ADJUST ix86_loop_unroll_adjust
 
Index: testsuite/gcc.target/i386/387-ficom-2.c
===
--- testsuite/gcc.target/i386/387-ficom-2.c (nonexistent)
+++ testsuite/gcc.target/i386/387-ficom-2.c (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "-march=i386" } } */
+/* { dg-options "-Os -march=i386 -ffast-math -masm=att" } */
+
+#include "387-ficom-1.c"
+
+/* { dg-final { scan-assembler-times "ficomps" 3 } } */
+/* { dg-final { scan-assembler-times "ficompl" 3 } } */

Re: [PATCH] C++: show location of problematic extern "C" specifications

[Jason Merrill]

On Wed, Oct 11, 2017 at 4:50 PM, David Malcolm  wrote:
> On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote:
>> On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm 
>> wrote:
>> > * cp-tree.h (struct saved_scope): Add "location" field.
>>
>> saved_scope seems like the wrong place for this; it's only
>> interesting
>> at parse time, so having it saved during template instantiation
>> doesn't seem useful.  I'd prefer to put it in cp_parser and have
>> maybe_show_extern_c_location look in the_parser.
>
> Thanks.
>
> I have a new version of the patch *mostly* working that way, but one of
> the uses of maybe_show_extern_c_location is within decl.c:grokfndecl
> (when complaining about user-defined literal operators within C
> linkage), and there doesn't seem to be access to the cp_parser * from
> there.
>
> I could fix this by adding a cp_parser * for this to grokfndecl, which
> would mean adding it to grokdeclarator, but I get the impression that
> the code is structured so that the decl-handling isn't meant to know
> about the parser.

That's why I was thinking to look at "the_parser", which functions in
parser.c can get at directly.

Jason

Re: [PATCH 1/2] C++: avoid partial duplicate implementation of cp_parser_error

[Jason Merrill]

On Tue, Sep 26, 2017 at 9:56 AM, David Malcolm  wrote:
> In r251026 (aka 3fe34694f0990d1d649711ede0326497f8a849dc,
> "C/C++: show pertinent open token when missing a close token")
> I copied part of cp_parser_error into cp_parser_required_error,
> leading to duplication of code.
>
> This patch eliminates this duplication by merging the two copies of the
> code into a new cp_parser_error_1 subroutine.
>
> Doing so removes an indentation level, making the patch appear to have
> more churn than it really does.

FWIW, you could also attach a patch generated with -w to ignore
whitespace changes.

The patch is OK.

Jason

libgo patch committed: fix uintptr(_t)? issues

[Ian Lance Taylor]

This patch by Tony Reix fixes a couple of issues in libgo on systems
that distinguish between libgo's uintptr and uintptr_t.  Bootstrapped
on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 253468)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-adc6eb826f156d0980f0ad9f9efc5c919ec4905e
+af46ad16dc34773877068393d331ac8ae34b2219
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/go-caller.c
===
--- libgo/runtime/go-caller.c   (revision 253311)
+++ libgo/runtime/go-caller.c   (working copy)
@@ -159,7 +159,7 @@ syminfo_callback (void *data, uintptr_t
 /* Set *VAL to the value of the symbol for PC.  */
 
 static _Bool
-__go_symbol_value (uintptr_t pc, uintptr_t *val)
+__go_symbol_value (uintptr pc, uintptr *val)
 {
   *val = 0;
   backtrace_syminfo (__go_get_backtrace_state (), pc, syminfo_callback,
Index: libgo/runtime/proc.c
===
--- libgo/runtime/proc.c(revision 253311)
+++ libgo/runtime/proc.c(working copy)
@@ -179,7 +179,7 @@ fixcontext(ucontext_t* c)
 // So we make the field larger in runtime2.go and pick an appropriate
 // offset within the field here.
 static ucontext_t*
-ucontext_arg(uintptr* go_ucontext)
+ucontext_arg(uintptr_t* go_ucontext)
 {
uintptr_t p = (uintptr_t)go_ucontext;
size_t align = __alignof__(ucontext_t);

Re: patch to fix PR82353

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 03:39:21PM -0400, Vladimir Makarov wrote:
> The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
> 
> LRA did not update hard reg liveness on bb borders for hard regs which are
> part of insn patterns like CFLAGS reg.  It was ok for inheritance in EBB
> which creates only moves and they usually have no embedded hard regs in the
> patterns.  But LRA rematerialization needs this.  So this patch implements
> such hard reg liveness updates.
> 
> The patch was successfully bootstrapped and tested on x86-64.
> 
> Committed as rev. 253656.

Thanks for the fix, however I believe we do not want C++ tests in
gcc.target/*/, those are historically handled in g++.dg/ (and if we wanted
to have them, we'd introduce g++.target/*/), furthermore the test requires
working -fsanitize=undefined and such tests belog into g++.dg/ubsan/
if they are in C++.  And finally, the test fails on i386, where we
rematerialize something else, so I think we should just limit it to lp64.

Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
change, ok for trunk?

2017-10-11  Jakub Jelinek  

PR target/82353
* gcc.target/i386/i386.exp (tests): Revert the '.C' extension change.
* gcc.target/i386/pr82353.C: Moved to ...
* g++.dg/ubsan/pr82353.C: ... here.  Restrict to i?86/x86_64 && lp64.

--- gcc/testsuite/gcc.target/i386/i386.exp.jj   2017-10-11 22:37:51.0 
+0200
+++ gcc/testsuite/gcc.target/i386/i386.exp  2017-10-11 22:59:47.455746874 
+0200
@@ -445,7 +445,7 @@ if [runtest_file_p $runtests $srcdir/$su
 }
 
 # Everything else.
-set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]]
+set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]]
 set tests [prune $tests $srcdir/$subdir/vect-args.c]
 
 # Main loop.
--- gcc/testsuite/gcc.target/i386/pr82353.C.jj  2017-10-11 22:37:51.0 
+0200
+++ gcc/testsuite/gcc.target/i386/pr82353.C 2017-10-11 23:01:37.618390302 
+0200
@@ -1,60 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -std=c++11 -fsanitize=undefined 
-fno-sanitize-recover=undefined -w -fdump-rtl-reload" } */
-
-extern unsigned long tf_2_var_1, tf_2_var_21;
-extern bool tf_2_var_2, tf_2_var_24, tf_2_var_6, tf_2_var_5;
-extern unsigned char tf_2_var_16, tf_2_var_31;
-extern short tf_2_var_69;
-extern unsigned tf_2_var_233;
-struct tf_2_struct_1 {
-  short member_1_0 : 27;
-  long member_1_1 : 10;
-};
-struct a {
-  int member_2_0 : 5;
-};
-struct tf_2_struct_3 {
-  static tf_2_struct_1 member_3_0;
-};
-struct tf_2_struct_4 {
-  static unsigned member_4_0;
-  a member_4_1;
-};
-struct tf_2_struct_5 {
-  tf_2_struct_1 member_5_2;
-  tf_2_struct_4 member_5_4;
-};
-struct tf_2_struct_6 {
-  tf_2_struct_5 member_6_2;
-  short member_6_4;
-} extern tf_2_struct_obj_2;
-extern tf_2_struct_3 tf_2_struct_obj_8;
-tf_2_struct_1 a;
-tf_2_struct_5 b;
-tf_2_struct_1 tf_2_struct_3::member_3_0;
-unsigned tf_2_struct_4::member_4_0;
-void tf_2_init() {
-  a.member_1_1 = tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1 = 5;
-}
-void tf_2_foo() {
-  int c = tf_2_struct_obj_2.member_6_2.member_5_4.member_4_1.member_2_0 -
-  -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
-  tf_2_struct_obj_8.member_3_0.member_1_0 =
-  tf_2_var_24 >
-  tf_2_var_21 * a.member_1_0 * tf_2_var_2 - tf_2_var_5 % a.member_1_1;
-  if ((~(tf_2_var_31 * tf_2_var_6) &&
-   -~tf_2_struct_obj_2.member_6_4 * 90284000534361) %
-  ~tf_2_var_31 * tf_2_var_6)
-b.member_5_2.member_1_0 << tf_2_var_16 << tf_2_var_1;
-  tf_2_var_233 = -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
-  int d(tf_2_struct_obj_2.member_6_4);
-  if (b.member_5_2.member_1_0)
-b.member_5_2.member_1_1 = c;
-  bool e(~-~tf_2_struct_obj_2.member_6_4);
-  a.member_1_1 % e;
-  if (tf_2_var_5 / tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1)
-b.member_5_4.member_4_0 = tf_2_var_21 * a.member_1_0 * tf_2_var_2;
-  tf_2_var_69 = tf_2_var_6;
-}
-
-/* { dg-final { scan-rtl-dump-not "Inserting rematerialization insn" "reload" 
} } */
--- gcc/testsuite/g++.dg/ubsan/pr82353.C.jj 2017-10-11 23:00:59.442860406 
+0200
+++ gcc/testsuite/g++.dg/ubsan/pr82353.C2017-10-11 23:04:27.264301238 
+0200
@@ -0,0 +1,60 @@
+/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-O2 -std=c++11 -fsanitize=undefined 
-fno-sanitize-recover=undefined -w -fdump-rtl-reload" } */
+
+extern unsigned long tf_2_var_1, tf_2_var_21;
+extern bool tf_2_var_2, tf_2_var_24, tf_2_var_6, tf_2_var_5;
+extern unsigned char tf_2_var_16, tf_2_var_31;
+extern short tf_2_var_69;
+extern unsigned tf_2_var_233;
+struct tf_2_struct_1 {
+  short member_1_0 : 27;
+  long member_1_1 : 10;
+};
+struct a {
+  int member_2_0 : 5;
+};
+struct tf_2_struct_3 {
+  static tf_2_struct_1 member_3_0;
+};
+struct tf_2_struct_4 {
+  static unsigned member_4_0;
+  a member_4_1;
+};
+struct tf_2_struct_5 {
+  tf_2_struct_1 member_5_2;
+  

[PATCH] Improve x86 and + rotate (PR target/82498)

[Jakub Jelinek]

Hi!

As can be seen on the testcase below, the *3_mask
insn/splitter is able to optimize only the case when the and is
performed in SImode and then the result subreged into QImode,
while if the computation is already in QImode, we don't handle it.

Fixed by adding another pattern, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

2017-10-11  Jakub Jelinek  

PR target/82498
* config/i386/i386.md (*3_mask_1): New
define_insn_and_split.

* gcc.target/i386/pr82498.c: New test.

--- gcc/config/i386/i386.md.jj  2017-10-10 11:54:11.0 +0200
+++ gcc/config/i386/i386.md 2017-10-11 19:24:27.673606778 +0200
@@ -11187,6 +11187,26 @@ (define_insn_and_split "*3_mask_1"
+  [(set (match_operand:SWI48 0 "nonimmediate_operand")
+   (any_rotate:SWI48
+ (match_operand:SWI48 1 "nonimmediate_operand")
+ (and:QI
+   (match_operand:QI 2 "register_operand")
+   (match_operand:QI 3 "const_int_operand"
+   (clobber (reg:CC FLAGS_REG))]
+  "ix86_binary_operator_ok (, mode, operands)
+   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (mode)-1))
+  == GET_MODE_BITSIZE (mode)-1
+   && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(parallel
+ [(set (match_dup 0)
+  (any_rotate:SWI48 (match_dup 1)
+(match_dup 2)))
+  (clobber (reg:CC FLAGS_REG))])])
+
 ;; Implement rotation using two double-precision
 ;; shift instructions and a scratch register.
 
--- gcc/testsuite/gcc.target/i386/pr82498.c.jj  2017-10-11 20:21:10.677088306 
+0200
+++ gcc/testsuite/gcc.target/i386/pr82498.c 2017-10-11 20:22:31.569101564 
+0200
@@ -0,0 +1,52 @@
+/* PR target/82498 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic -masm=att" } */
+/* { dg-final { scan-assembler-not {\mand[bwlq]\M} } } */
+
+unsigned
+f1 (unsigned x, unsigned char y)
+{
+  if (y == 0)
+return x;
+  y &= __CHAR_BIT__ * __SIZEOF_INT__ - 1;
+  return (x << y) | (x >> (__CHAR_BIT__ * __SIZEOF_INT__ - y));
+}
+
+unsigned
+f2 (unsigned x, unsigned y)
+{
+  if (y == 0)
+return x;
+  y &= __CHAR_BIT__ * __SIZEOF_INT__ - 1;
+  return (x << y) | (x >> (__CHAR_BIT__ * __SIZEOF_INT__ - y));
+}
+
+unsigned
+f3 (unsigned x, unsigned short y)
+{
+  if (y == 0)
+return x;
+  y &= __CHAR_BIT__ * __SIZEOF_INT__ - 1;
+  return (x << y) | (x >> (__CHAR_BIT__ * __SIZEOF_INT__ - y));
+}
+
+unsigned
+f4 (unsigned x, unsigned char y)
+{
+  y &= __CHAR_BIT__ * __SIZEOF_INT__ - 1;
+  return (x << y) | (x >> (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1)));
+}
+
+unsigned
+f5 (unsigned x, unsigned int y)
+{
+  y &= __CHAR_BIT__ * __SIZEOF_INT__ - 1;
+  return (x << y) | (x >> (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1)));
+}
+
+unsigned
+f6 (unsigned x, unsigned short y)
+{
+  y &= __CHAR_BIT__ * __SIZEOF_INT__ - 1;
+  return (x << y) | (x >> (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1)));
+}

Jakub

[PATCH] Add further VEC_SELECT verification

[Jakub Jelinek]

Hi!

This patch adds verification that vec_select in *.md files
doesn't have any out of bounds indices in the selector.

Bootstrapped/regtested on x86_64-linux and i686-linux, tested
by building cc1 of aarch64, arm, powerpc64le and s390x cross,
and tested by hacking up sse.md to have an out of bounds access there,
ok for trunk?

2017-10-11  Jakub Jelinek  

* genrecog.c (validate_pattern): For VEC_SELECT verify that
CONST_INT selectors are 0 to GET_MODE_NUNITS (imode) - 1.

--- gcc/genrecog.c.jj   2017-09-01 09:25:40.0 +0200
+++ gcc/genrecog.c  2017-10-11 17:53:20.107198630 +0200
@@ -751,6 +751,21 @@ validate_pattern (rtx pattern, md_rtx_in
error_at (info->loc,
  "vec_select parallel with %d elements, expected %d",
  XVECLEN (XEXP (pattern, 1), 0), expected);
+ else if (VECTOR_MODE_P (imode))
+   {
+ unsigned int nelems = GET_MODE_NUNITS (imode);
+ int i;
+ for (i = 0; i < expected; ++i)
+   if (CONST_INT_P (XVECEXP (XEXP (pattern, 1), 0, i))
+   && (UINTVAL (XVECEXP (XEXP (pattern, 1), 0, i))
+   >= nelems))
+ error_at (info->loc,
+   "out of bounds selector %u in vec_select, "
+   "expected at most %u",
+   (unsigned)
+   UINTVAL (XVECEXP (XEXP (pattern, 1), 0, i)),
+   nelems - 1);
+   }
}
  if (imode != VOIDmode && !VECTOR_MODE_P (imode))
error_at (info->loc, "%smode of first vec_select operand is not a "

Jakub

[PATCH] Fix another ICE with C++ addressable bitsize 0 return value (PR c++/82159)

[Jakub Jelinek]

Hi!

Another case where we ICE because we optimize away store to bitsize 0
addressable object from call.  We can't optimize them away, otherwise
we'd try to create the temporaries again and fail to do so.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2017-10-11  Jakub Jelinek  

PR c++/82159
* expr.c (store_field): Don't optimize away bitsize == 0 store
from CALL_EXPR with addressable return type.

* g++.dg/opt/pr82159-2.C: New test.

--- gcc/expr.c.jj   2017-10-10 22:04:06.0 +0200
+++ gcc/expr.c  2017-10-11 16:48:45.428536126 +0200
@@ -6749,8 +6749,11 @@ store_field (rtx target, HOST_WIDE_INT b
 return const0_rtx;
 
   /* If we have nothing to store, do nothing unless the expression has
- side-effects.  */
-  if (bitsize == 0)
+ side-effects.  Don't do that for zero sized addressable lhs of
+ calls.  */
+  if (bitsize == 0
+  && (!TREE_ADDRESSABLE (TREE_TYPE (exp))
+ || TREE_CODE (exp) != CALL_EXPR))
 return expand_expr (exp, const0_rtx, VOIDmode, EXPAND_NORMAL);
 
   if (GET_CODE (target) == CONCAT)
--- gcc/testsuite/g++.dg/opt/pr82159-2.C.jj 2017-10-11 17:27:18.050861346 
+0200
+++ gcc/testsuite/g++.dg/opt/pr82159-2.C2017-10-11 17:27:27.081753330 
+0200
@@ -0,0 +1,65 @@
+// PR c++/82159
+// { dg-do compile }
+// { dg-options "" }
+
+template  struct D { T e; };
+struct F : D {
+  F(const F &);
+};
+struct G : F {
+  template  G operator-(T);
+};
+template  struct I {
+  typedef typename T::template J ak;
+};
+template  struct K { typename I::ak an; };
+struct H {
+  G l;
+};
+struct C {
+  ~C();
+};
+template  struct M : T {
+  template  M(U, V);
+  H h;
+  virtual void foo() { T::bar(); }
+};
+template  class A;
+template  struct B {
+  typedef int BT;
+  struct BC {};
+  template  struct BD {
+G g;
+BD(BT, T n) : g(n.l - 0) {}
+  };
+  B(BT, BC);
+};
+template  struct O;
+template 
+struct O > > : public B >::BC {};
+struct L : B > {
+  struct P : C {
+void bar(H *x) {
+  BT a;
+  BD(a, *x);
+}
+  };
+  template  L(U x, V n) : B(x, n) {}
+  int ll;
+  virtual int baz() { M(this, ll); }
+};
+template  class Q {
+  O > > q;
+  virtual L baz() { L(0, q); }
+};
+template  class T> struct R {
+  R() { T(); }
+};
+struct S {
+  template  class J : R {};
+};
+void foo() { K c; }
+
+int main() {
+  return 0;
+}

Jakub

[PATCH] Add some further testcases

[Jakub Jelinek]

On Tue, Oct 10, 2017 at 10:21:22PM +0200, Jakub Jelinek wrote:
> While going through still open [5 Regression] bugs manually, I've gathered
> various testcases from PRs that were fixed by other changes and thus
> IMHO the tests are worth being added.  I have some further PRs to go through
> tomorrow, so I might add some further ones.
> 
> Regtested on x86_64-linux and i686-linux, committed to trunk. 

So, here is the remainder from 5 branch closing, regtested on x86_64-linux
and i686-linux, committed to trunk:

2017-10-11  Jakub Jelinek  

PR c++/80194
* g++.dg/cpp1y/pr80194.C: New test.

PR c++/78523
* g++.dg/cpp1y/pr78523.C: New test.

PR c++/82414
* g++.dg/lto/pr82414_0.C: New test.

PR tree-optimization/78558
* gcc.dg/vect/pr78558.c: New test.

PR middle-end/80421
* gcc.c-torture/execute/pr80421.c: New test.

--- gcc/testsuite/g++.dg/cpp1y/pr80194.C.jj 2017-10-11 11:24:28.294661793 
+0200
+++ gcc/testsuite/g++.dg/cpp1y/pr80194.C2017-10-11 11:24:21.039750326 
+0200
@@ -0,0 +1,17 @@
+// PR c++/80194
+// { dg-do compile { target c++14 } }
+
+int fn1 ();
+
+template 
+void
+fn2 (Fn &)
+{
+  fn (42);
+}
+
+void fn2 ()
+{
+  auto const x = fn1 ();
+  fn2 ([&](auto) { x; });
+}
--- gcc/testsuite/g++.dg/cpp1y/pr78523.C.jj 2017-10-11 10:51:51.608551082 
+0200
+++ gcc/testsuite/g++.dg/cpp1y/pr78523.C2017-10-11 10:51:43.175654055 
+0200
@@ -0,0 +1,12 @@
+// PR c++/78523
+// { dg-do compile { target c++14 } }
+
+int bar ();
+
+void
+foo ()
+{
+  const int t = bar ();
+  auto f = [=] (auto x) { return t; };
+  f (0);
+}
--- gcc/testsuite/g++.dg/lto/pr82414_0.C.jj 2017-10-11 11:56:56.0 
+0200
+++ gcc/testsuite/g++.dg/lto/pr82414_0.C2017-10-11 11:59:16.0 
+0200
@@ -0,0 +1,13 @@
+// PR c++/82414
+// { dg-lto-do link }
+// { dg-lto-options { { -flto -g } } }
+
+typedef __attribute__ ((__aligned__ (16))) struct S { __extension__ unsigned 
long long Part[2]; } T; // bogus warning "violates one definition rule"
+
+int
+main ()
+{
+  T tf;
+  asm volatile ("" : : "g" (__alignof__(tf)), "g" (__alignof__ (struct S)), 
"g" (__alignof__ (T)));
+  return 0;
+}
--- gcc/testsuite/gcc.dg/vect/pr78558.c.jj  2017-10-11 11:04:59.940924884 
+0200
+++ gcc/testsuite/gcc.dg/vect/pr78558.c 2017-10-11 11:06:20.584940340 +0200
@@ -0,0 +1,44 @@
+/* PR tree-optimization/78558 */
+
+#include "tree-vect.h"
+
+struct S
+{
+  char p[48];
+  unsigned long long q, r, s;
+} s[50];
+
+struct D
+{
+  unsigned long long q, r;
+} d[50];
+
+void
+foo (void)
+{
+  unsigned long i;
+  for (i = 0; i < 50; ++i)
+{
+  d[i].q = s[i].q;
+  d[i].r = s[i].r;
+}
+}
+
+int
+main ()
+{
+  check_vect ();
+  unsigned long i;
+  for (i = 0; i < 50; ++i)
+{
+  s[i].q = i;
+  s[i].r = 50 * i;
+}
+  asm volatile ("" : : "g" (s), "g" (d) : "memory");
+  foo ();
+  asm volatile ("" : : "g" (s), "g" (d) : "memory");
+  for (i = 0; i < 50; ++i)
+if (d[i].q != i || d[i].r != 50 * i)
+  abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr80421.c.jj2017-10-11 
11:36:45.070670843 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr80421.c   2017-10-11 
11:41:29.670198693 +0200
@@ -0,0 +1,121 @@
+/* PR middle-end/80421 */
+
+__attribute__ ((noinline, noclone)) void
+baz (const char *t, ...)
+{
+  asm volatile (""::"r" (t):"memory");
+  if (*t == 'T')
+__builtin_abort ();
+}
+
+unsigned int
+foo (char x)
+{
+  baz ("x %c\n", x);
+  switch (x)
+{
+default:
+  baz ("case default\n");
+  if (x == 'D' || x == 'I')
+   baz ("This should never be reached.\n");
+  return 0;
+case 'D':
+  baz ("case 'D'\n");
+  return 0;
+case 'I':
+  baz ("case 'I'\n");
+  return 0;
+}
+}
+
+void
+bar (void)
+{
+  int a = 2;
+  int b = 5;
+  char c[] = {
+2, 4, 1, 2, 5, 5, 2, 4, 4, 0, 0, 0, 0, 0, 0, 3, 4, 4, 2, 4,
+1, 2, 5, 5, 2, 4, 1, 0, 0, 0, 2, 4, 4, 3, 4, 3, 3, 5, 1, 3,
+5, 5, 2, 4, 4, 2, 4, 1, 3, 5, 3, 3, 5, 1, 3, 5, 1, 2, 4, 4,
+2, 4, 2, 3, 5, 1, 3, 5, 1, 3, 5, 5, 2, 4, 1, 2, 4, 2, 3, 5,
+3, 3, 5, 1, 3, 5, 5, 2, 4, 1, 2, 4, 1, 3, 5, 3, 3, 5, 1, 3,
+5, 5, 2, 4, 4, 2, 4, 1, 3, 5, 3, 3, 5, 1, 3, 5, 1, 2, 4, 1,
+2, 4, 2, 3, 5, 1, 3, 5, 1, 3, 5, 1, 2, 4, 1, 2, 4, 1, 3, 5,
+1, 3, 5, 1, 3, 5, 1, 2, 4, 4, 2, 4, 1, 3, 5, 1, 3, 5, 1, 3,
+5, 5, 2, 4, 4, 2, 4, 2, 3, 5, 3, 3, 5, 1, 3, 5, 5, 2, 4, 4,
+2, 4, 1, 3, 5, 3, 3, 5, 1, 3, 5, 1, 2, 5, 5, 2, 4, 2, 3, 5,
+1, 3, 4, 1, 3, 5, 1, 2, 5, 5, 2, 4, 1, 2, 5, 1, 3, 5, 3, 3,
+5, 1, 2, 5, 5, 2, 4, 2, 2, 5, 1, 3, 5, 3, 3, 5, 1, 2, 5, 1,
+2, 4, 1, 2, 5, 2, 3, 5, 1, 3, 5, 1, 2, 5, 1, 2, 4, 2, 2, 5,
+1, 3, 5, 1, 3, 5, 1, 2, 5, 5, 2, 4, 2, 2, 5, 2, 3, 5, 3, 3,
+5, 1, 2, 5, 5, 2, 4, 2, 2, 5, 2, 3, 5, 3, 3, 5, 1, 2, 5, 5,
+2, 4, 2, 2, 5, 1, 3, 5, 3, 3, 5, 1, 2, 5, 5, 2, 4, 2, 2, 5,
+1, 3, 5, 3, 3, 5, 1, 2, 5, 1, 2, 4, 1, 2, 5, 2, 3, 5, 1, 3,
+ 

Re: [PATCH] C++: show location of problematic extern "C" specifications

[David Malcolm]

On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote:
> On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm 
> wrote:
> > * cp-tree.h (struct saved_scope): Add "location" field.
> 
> saved_scope seems like the wrong place for this; it's only
> interesting
> at parse time, so having it saved during template instantiation
> doesn't seem useful.  I'd prefer to put it in cp_parser and have
> maybe_show_extern_c_location look in the_parser.

Thanks.

I have a new version of the patch *mostly* working that way, but one of
the uses of maybe_show_extern_c_location is within decl.c:grokfndecl
(when complaining about user-defined literal operators within C
linkage), and there doesn't seem to be access to the cp_parser * from
there.

I could fix this by adding a cp_parser * for this to grokfndecl, which
would mean adding it to grokdeclarator, but I get the impression that
the code is structured so that the decl-handling isn't meant to know
about the parser.

Thoughts?
Dave

Re: [PATCH] C++: show location of problematic extern "C" specifications

[Jason Merrill]

On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm  wrote:
> * cp-tree.h (struct saved_scope): Add "location" field.

saved_scope seems like the wrong place for this; it's only interesting
at parse time, so having it saved during template instantiation
doesn't seem useful.  I'd prefer to put it in cp_parser and have
maybe_show_extern_c_location look in the_parser.

Jason

patch to fix PR82353

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353

LRA did not update hard reg liveness on bb borders for hard regs which 
are part of insn patterns like CFLAGS reg.  It was ok for inheritance in 
EBB which creates only moves and they usually have no embedded hard regs 
in the patterns.  But LRA rematerialization needs this.  So this patch 
implements such hard reg liveness updates.


The patch was successfully bootstrapped and tested on x86-64.

Committed as rev. 253656.


Index: ChangeLog
===
--- ChangeLog	(revision 253655)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@
+2017-10-11  Vladimir Makarov  
+
+	PR sanitizer/82353
+	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
+	locations.
+	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
+	(make_hard_regno_born, make_hard_regno_dead): Update
+	bb_killed_pseudos and bb_gen_pseudos.
+
 2017-10-11  Nathan Sidwell  
 
 	* incpath.h (enum incpath_kind): Name enum, prefix values.
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 253655)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2017-10-11  Vladimir Makarov  
+
+	PR sanitizer/82353
+	* gcc.target/i386/i386.exp (tests): Permit '.C' extension.
+	* gcc.target/i386/pr82353.C: New.
+
 2017-10-11  Uros Bizjak  
 
 	* gcc.target/i386/387-ficom-1.c: New test.
Index: lra-lives.c
===
--- lra-lives.c	(revision 253253)
+++ lra-lives.c	(working copy)
@@ -220,6 +220,9 @@ lra_intersected_live_ranges_p (lra_live_
   return false;
 }
 
+/* The corresponding bitmaps of BB currently being processed.  */
+static bitmap bb_killed_pseudos, bb_gen_pseudos;
+
 /* The function processing birth of hard register REGNO.  It updates
living hard regs, START_LIVING, and conflict hard regs for living
pseudos.  Conflict hard regs for the pic pseudo is not updated if
@@ -243,6 +246,7 @@ make_hard_regno_born (int regno, bool ch
 	|| i != REGNO (pic_offset_table_rtx))
 #endif
   SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno);
+  bitmap_set_bit (bb_gen_pseudos, regno);
 }
 
 /* Process the death of hard register REGNO.  This updates
@@ -255,6 +259,8 @@ make_hard_regno_dead (int regno)
 return;
   sparseset_set_bit (start_dying, regno);
   CLEAR_HARD_REG_BIT (hard_regs_live, regno);
+  bitmap_clear_bit (bb_gen_pseudos, regno);
+  bitmap_set_bit (bb_killed_pseudos, regno);
 }
 
 /* Mark pseudo REGNO as living at program point POINT, update conflicting
@@ -299,9 +305,6 @@ mark_pseudo_dead (int regno, int point)
 }
 }
 
-/* The corresponding bitmaps of BB currently being processed.  */
-static bitmap bb_killed_pseudos, bb_gen_pseudos;
-
 /* Mark register REGNO (pseudo or hard register) in MODE as live at
program point POINT.  Update BB_GEN_PSEUDOS.
Return TRUE if the liveness tracking sets were modified, or FALSE
Index: lra.c
===
--- lra.c	(revision 253253)
+++ lra.c	(working copy)
@@ -820,7 +820,8 @@ collect_non_operand_hard_regs (rtx *x, l
   const char *fmt = GET_RTX_FORMAT (code);
 
   for (i = 0; i < data->insn_static_data->n_operands; i++)
-if (x == data->operand_loc[i])
+if (! data->insn_static_data->operand[i].is_operator
+	&& x == data->operand_loc[i])
   /* It is an operand loc. Stop here.  */
   return list;
   for (i = 0; i < data->insn_static_data->n_dups; i++)
Index: testsuite/gcc.target/i386/i386.exp
===
--- testsuite/gcc.target/i386/i386.exp	(revision 253253)
+++ testsuite/gcc.target/i386/i386.exp	(working copy)
@@ -445,7 +445,7 @@ if [runtest_file_p $runtests $srcdir/$su
 }
 
 # Everything else.
-set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]]
+set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]]
 set tests [prune $tests $srcdir/$subdir/vect-args.c]
 
 # Main loop.
Index: testsuite/gcc.target/i386/pr82353.C
===
--- testsuite/gcc.target/i386/pr82353.C	(nonexistent)
+++ testsuite/gcc.target/i386/pr82353.C	(working copy)
@@ -0,0 +1,60 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -std=c++11 -fsanitize=undefined -fno-sanitize-recover=undefined -w -fdump-rtl-reload" } */
+
+extern unsigned long tf_2_var_1, tf_2_var_21;
+extern bool tf_2_var_2, tf_2_var_24, tf_2_var_6, tf_2_var_5;
+extern unsigned char tf_2_var_16, tf_2_var_31;
+extern short tf_2_var_69;
+extern unsigned tf_2_var_233;
+struct tf_2_struct_1 {
+  short member_1_0 : 27;
+  long member_1_1 : 10;
+};
+struct a {
+  int member_2_0 : 5;
+};
+struct tf_2_struct_3 {
+  static tf_2_struct_1 member_3_0;
+};
+struct tf_2_struct_4 {
+  static unsigned member_4_0;
+  a 

[PATCH] C: detect more missing semicolons (PR c/7356)

[David Malcolm]

[This patch assumes the two patches here:
 https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00274.html ]

c_parser_declaration_or_fndef has logic for parsing what might be
either a declaration or a function definition.

This patch adds a test to detect cases where a semicolon would have
terminated the decls as a declaration, where the token that follows
would start a new declaration specifier, and updates the error message
accordingly, with a fix-it hint.

This addresses PR c/7356, fixing the case of a stray token before a
#include which previously gave inscrutable output, and improving e.g.:

  int i
  int j;

from:

  t.c:2:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int'
   int j;
   ^~~

to:

  t.c:1:6: error: expected ';' before 'int'
   int i
^
;
   int j;
   ~~~

The patch also adds a test for PR c/44515 as a baseline.

gcc.dg/noncompile/920923-1.c needs a slight update, as the output for
the first line changes from:

  920923-1.c:2:14: error: expected '=', ',', ';', 'asm' or
  '__attribute__' before 'unsigned'
   typedef BYTE unsigned char; /* { dg-error "expected" } */
^~~~

to:
  920923-1.c:2:13: error: expected ';' before 'unsigned'
   typedef BYTE unsigned char; /* { dg-error "expected" } */
   ^
   ;
  920923-1.c:2:1: warning: useless type name in empty declaration
   typedef BYTE unsigned char; /* { dg-error "expected" } */
   ^~~

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?

Thanks
Dave

gcc/c/ChangeLog:
PR c/7356
* c-parser.c (c_parser_declaration_or_fndef): Detect missing
semicolons.

gcc/testsuite/ChangeLog:
PR c/7356
PR c/44515
* c-c++-common/pr44515.c: New test case.
* gcc.dg/pr7356-2.c: New test case.
* gcc.dg/pr7356.c: New test case.
* gcc.dg/spellcheck-typenames.c: Update the "singed" char "TODO"
case to reflect changes to output.
* gcc.dg/noncompile/920923-1.c: Add dg-warning to reflect changes
to output.
---
 gcc/c/c-parser.c| 36 +
 gcc/testsuite/c-c++-common/pr44515.c| 14 +++
 gcc/testsuite/gcc.dg/noncompile/920923-1.c  |  1 +
 gcc/testsuite/gcc.dg/pr7356-2.c | 33 ++
 gcc/testsuite/gcc.dg/pr7356.c   | 17 ++
 gcc/testsuite/gcc.dg/spellcheck-typenames.c |  5 ++--
 6 files changed, 99 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/pr44515.c
 create mode 100644 gcc/testsuite/gcc.dg/pr7356-2.c
 create mode 100644 gcc/testsuite/gcc.dg/pr7356.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a5e3ec4..7c3b834 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -2241,11 +2241,37 @@ c_parser_declaration_or_fndef (c_parser *parser, bool 
fndef_ok,
}
   if (!start_function (specs, declarator, all_prefix_attrs))
{
- /* This can appear in many cases looking nothing like a
-function definition, so we don't give a more specific
-error suggesting there was one.  */
- c_parser_error (parser, "expected %<=%>, %<,%>, %<;%>, % "
- "or %<__attribute__%>");
+ /* At this point we've consumed:
+  declaration-specifiers declarator
+and the next token isn't CPP_EQ, CPP_COMMA, CPP_SEMICOLON,
+RID_ASM, RID_ATTRIBUTE, or RID_IN,
+but the
+  declaration-specifiers declarator
+aren't grokkable as a function definition, so we have
+an error.  */
+ gcc_assert (!c_parser_next_token_is (parser, CPP_SEMICOLON));
+ if (c_parser_next_token_starts_declspecs (parser))
+   {
+ /* If we have
+  declaration-specifiers declarator decl-specs
+then assume we have a missing semicolon, which would
+give us:
+  declaration-specifiers declarator  decl-specs
+   ^
+   ;
+  <~ declaration ~~>
+Use c_parser_require to get an error with a fix-it hint.  */
+ c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>");
+ parser->error = false;
+   }
+ else
+   {
+ /* This can appear in many cases looking nothing like a
+function definition, so we don't give a more specific
+error suggesting there was one.  */
+ c_parser_error (parser, "expected %<=%>, %<,%>, %<;%>, % "
+ "or %<__attribute__%>");
+   }
  if (nested)
c_pop_function_context ();
  break;
diff --git a/gcc/testsuite/c-c++-common/pr44515.c 
b/gcc/testsuite/c-c++-common/pr44515.c
new file mode 100644

Re: [PATCH 00/17] RFC: New source-location representation; Language Server Protocol

[David Malcolm]

On Mon, 2017-07-24 at 16:04 -0400, David Malcolm wrote:
> We currently capture some source location information in the
> frontends, but there are many kinds of source entity for which we
> *don't*
> retain the location information after the initial parse.
> 
> For example, in the C/C++ frontends:
> 
> * we don't capture the locations of the individual parameters
>   within e.g. an extern function declaration, so we can't underline
>   the pertinent param when there's a mismatching type in a call to
>   that function decl.
> 
> * we don't capture the locations of attributes of a function,
>   so we can't underline these if they're wrong (e.g. a "noreturn" on
> a
>   function that does in fact return).
> 
> * we don't retain the locations of things like close parens and
>   semicolons for after parsing, so we can't offer fix-it hints for
>   adding new attributes, or, say the C++11 "override" feature.
> 
> * we can't at present implement many kinds of useful "cousins" of a
>   compiler on top of the GCC codebase (e.g. code refactoring tools,
>   code reformatting tools, IDE support daemons, etc), since much of
> the
>   useful location information is discarded at parse time.
> 
> This patch kit implements:
> 
> (a) a new, optional, representation of this location information,
> enabled by a command-line flag
> 
> (b) improvements to various diagnostics to use this location
> information
> if it's present, falling back to the status-quo (less accurate)
> source locations otherwise
> 
> (b) a gcc-based implementation of Microsoft's Language Server
> Protocol,
>   https://github.com/Microsoft/language-server-protocol
> allowing IDEs to connect to a gcc-based LSP server, making RPC
> calls to query it for things like "where is this struct
> declared?".
> This last part is very much just a proof-of-concept.
> 
> 
> 
> (a) The new location information
> 
> 
> Our existing "tree" type represents a node within an abstract syntax
> tree,
> but this is sometimes too abstract - sometimes we want the locations
> of the clauses and tokens that were abstracted away by the frontends.
> 
> In theory we could generate the full parse tree ("concrete syntax
> tree"),
> showing every production followed to parse the input, but it is
> likely
> to be unwieldy: large and difficult to navigate.
> 
> (aside: I found myself re-reading the Dragon book to refresh my mind
> on exactly what an AST vs a CST is; I also found this blog post to be
> very useful:
>   http://eli.thegreenplace.net/2009/02/16/abstract-vs-concrete-syntax
> -trees )
> 
> So the patch kit implements a middle-ground: an additional tree of
> parse
> information, much more concrete than our "tree" type, but not quite
> the
> full parse tree.
> 
> My working title for this system is "BLT" (and hence "class
> blt_node").
> I could claim that this is a acronym for "bonus location tree" (but
> it's actually a reference to a sandwich) - it needs a name, and that
> name needs to not clash with anything else in the source tree.
> "Parse Tree" would be "PT" which clashes with "points-to", and
> "Concrete Syntax Tree" would be "CST" which clashes with our
> abbreviation
> for "constant".  ("BLT" popped into my mind somewhere between "AST"
> and "CST"; ideas for better names welcome).
> 
> blt_nodes form a tree-like structure; a blt_node has a "kind",
> identifying the terminal or nonterminal it corresponds to
> (e.g. BLT_TRANSLATION_UNIT or BLT_DECLARATION_SPECIFIERS).
> This is just an enum, but allows for language-specific traversals,
> without introducing significant language-specific features in
> the shared "gcc" dir (it's just an enum of IDs).
> 
> There is a partial mapping between "tree" and blt_node: a blt_node
> can reference a tree, and a tree can reference a blt_node, though
> typically the mapping is very sparse; most don't.  This allows us
> to go from e.g. a function_decl in the "tree" world and navigate to
> pertinent parts of the syntax that was used to declare it.
> 
> All of this is enabled by a new "-fblt" command-line option; in the
> absense of -fblt, almost all of it becomes close to no-ops, and the
> relevant diagnostics fall back to using less accurate location
> information.
> 
> So it's a kind of optional, "on-the-side" record of how we parsed
> the source, with a sparse relationship to our tree type.
> 
> The patch kit implements it for the C and C++ frontends.
> 
> An example of a BLT dump for a C file can be seen here:
>   https://dmalcolm.fedorapeople.org/gcc/2017-07-24/fdump-blt.html
> It shows the tree structure using indentation (and colorization);
> source locations are printed, and, for each node where the
> location is different from the parent, the pertinent source range
> is printed and underlined inline.
> (BTW, does the colorization of dumps look useful for other
> dump formats?  similarly for the ASCII art for printing hierarchies)
> 
> 
> 

Re: [PATCH 1/2] add unique_ptr header

[David Malcolm]

On Sat, 2017-08-05 at 01:39 -0400, Trevor Saunders wrote:
> On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> > On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > > aiui C++03 is C++98 with a few additions to the stl.
> > 
> > Again, STL != C++ Standard Library.
> > 
> > C++03 made some important changes to the core language,
> > particularly
> > the value-initialization rules.
> > 
> > Most of the library changes were small bug fixes, and most were to
> > locales (which didn't originate in the STL and aren't even
> > templates)
> > and iostreams (which didn't originate in the STL).
> > 
> > There were also changes to std::auto_ptr (also not from the STL)
> > which
> > this might rely on (I haven't checked).
> 
> I doubt it, as Pedro said in his email it originally copied code from
> std::auto_ptr, but it doesn't use the standard libraries definition
> of
> std::auto_ptr anywhere.  However please do feel free to look at the
> implementation.
> 
> Trev
> 

Trevor: did this go anywhere?

Do we have an unique_ptr class we can use within gcc's own
implementation?

(I was hoping to use it for
  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html
  "[PATCH 1/3] c-family: add name_hint/deferred_diagnostic";
see the discussion at:
  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00123.html
onwards)

Thanks
Dave

[PATCH, i386]: Remove dead x87 cbranch helpers

[Uros Bizjak]

Hello!

Remove dead code, obsoleted by cbranch rewrite years ago.

2017-10-11  Uros Bizjak  

* config/i386/i386.md (*cmp__i387):
Do not use float_operator operator predicate.
(*cmp__cc_i387): Ditto.
* config/i386/predicates.md (float_operator): Remove predicate.

2017-10-11  Uros Bizjak  

* config/i386/i386.md (*jcc_0_i387): Remove insn pattern.
(*jccxf_i387): Ditto.
(*jcc_i387): Ditto.
(*jccu_i387): Ditto.
(*jcc__i387): Ditto.
(*jcc_*_i387 splitters): Remove.
* config/i386/i386-protos.h (ix86_split_fp_branch): Remove prototype.
* config/i386/i386.c (ix86_split_fp_branch): Remove.
* config/i386/predicates.md (ix86_swapped_fp_comparison_operator):
Remove predicate.

testsuite/ChangeLog:

2017-10-11  Uros Bizjak  

* gcc.target/i386/387-ficom-1.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/i386-protos.h
===
--- config/i386/i386-protos.h   (revision 253645)
+++ config/i386/i386-protos.h   (working copy)
@@ -165,9 +165,6 @@ extern void ix86_asm_output_function_label (FILE *
 extern void ix86_call_abi_override (const_tree);
 extern int ix86_reg_parm_stack_space (const_tree);
 
-extern void ix86_split_fp_branch (enum rtx_code code, rtx, rtx,
- rtx, rtx, rtx);
-
 extern bool ix86_libc_has_function (enum function_class fn_class);
 
 extern void x86_order_regs_for_local_alloc (void);
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 253645)
+++ config/i386/i386.c  (working copy)
@@ -24339,32 +24339,7 @@ ix86_expand_branch (enum rtx_code code, rtx op0, r
 }
 }
 
-/* Split branch based on floating point condition.  */
 void
-ix86_split_fp_branch (enum rtx_code code, rtx op1, rtx op2,
- rtx target1, rtx target2, rtx tmp)
-{
-  rtx condition;
-  rtx_insn *i;
-
-  if (target2 != pc_rtx)
-{
-  std::swap (target1, target2);
-  code = reverse_condition_maybe_unordered (code);
-}
-
-  condition = ix86_expand_fp_compare (code, op1, op2,
- tmp);
-
-  i = emit_jump_insn (gen_rtx_SET
- (pc_rtx,
-  gen_rtx_IF_THEN_ELSE (VOIDmode,
-condition, target1, target2)));
-  if (split_branch_probability.initialized_p ())
-add_reg_br_prob_note (i, split_branch_probability);
-}
-
-void
 ix86_expand_setcc (rtx dest, enum rtx_code code, rtx op0, rtx op1)
 {
   rtx ret;
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 253645)
+++ config/i386/i386.md (working copy)
@@ -1612,8 +1612,8 @@
(unspec:HI
  [(compare:CCFP
 (match_operand:X87MODEF 1 "register_operand" "f")
-(match_operator:X87MODEF 3 "float_operator"
-  [(match_operand:SWI24 2 "memory_operand" "m")]))]
+(float:X87MODEF
+  (match_operand:SWI24 2 "memory_operand" "m")))]
  UNSPEC_FNSTSW))]
   "TARGET_80387
&& (TARGET_USE_MODE_FIOP
@@ -1628,8 +1628,8 @@
   [(set (reg:CCFP FLAGS_REG)
(compare:CCFP
  (match_operand:X87MODEF 1 "register_operand" "f")
- (match_operator:X87MODEF 3 "float_operator"
-   [(match_operand:SWI24 2 "memory_operand" "m")])))
+ (float:X87MODEF
+   (match_operand:SWI24 2 "memory_operand" "m"
(clobber (match_operand:HI 0 "register_operand" "=a"))]
   "TARGET_80387 && TARGET_SAHF && !TARGET_CMOVE
&& (TARGET_USE_MODE_FIOP
@@ -1640,7 +1640,7 @@
(unspec:HI
  [(compare:CCFP
 (match_dup 1)
-(match_op_dup 3 [(match_dup 2)]))]
+(float:X87MODEF (match_dup 2)))]
UNSPEC_FNSTSW))
(set (reg:CC FLAGS_REG)
(unspec:CC [(match_dup 0)] UNSPEC_SAHF))]
@@ -12032,142 +12032,6 @@
   if (! ix86_comparison_operator (operands[0], VOIDmode))
 FAIL;
 })
-
-;; Define combination compare-and-branch fp compare instructions to help
-;; combine.
-
-(define_insn "*jcc_0_i387"
-  [(set (pc)
-   (if_then_else (match_operator:CCFP 0 "ix86_fp_comparison_operator"
-   [(match_operand:X87MODEF 1 "register_operand" "f")
-(match_operand:X87MODEF 2 "const0_operand")])
- (label_ref (match_operand 3))
- (pc)))
-   (clobber (reg:CCFP FPSR_REG))
-   (clobber (reg:CCFP FLAGS_REG))
-   (clobber (match_scratch:HI 4 "=a"))]
-  "TARGET_80387 && !TARGET_CMOVE"
-  "#")
-
-(define_insn "*jccxf_i387"
-  [(set (pc)
-   (if_then_else (match_operator:CCFP 0 "ix86_fp_comparison_operator"
-   [(match_operand:XF 1 "register_operand" "f")
-(match_operand:XF 2 "register_operand" "f")])
-  

[PATCH] Include path enumeration

[Nathan Sidwell]

We have an enumeration to indicate particular fragments of the include 
path.  But


1) it's unnamed.  So we pass it as int or size_t(!), which is stupid.

2) It's members have rather generic names (QUOTE, BRACKET, SYSTEM, AFTER)

3) There's no MAX value, so we use a magic constant '4'.

This patch fixes these by (a) naming it incpath_kind, prefixing its 
members with INC_ and providing an INC_MAX member.


Applying to trunk as sufficiently obvious and close to C++FE and cpplib.

nathan

--
Nathan Sidwell
2017-10-11  Nathan Sidwell  

	gcc/
	* incpath.h (enum incpath_e): Name enum, prefix values.
	(add_path, add_cpp_dir_path, get_added_cpp_dirs): Use incpath_e type.
	* incpath.c (heads, tails): Use INC_MAX.
	(add_env_var_paths, add_standard_paths): Use incpath_e type.
	(merge_include_chains, split_quote_chain,
	register_include_chains): Update incpath_e names.
	(add_cpp_dir_path, add_path, get_added_cpp_dirs): Use incpath_e type.
	* config/darwin-c.c (add_system_framework_path): Update incpath_e
	names.
	(add_framework_path, darwin_register_objc_includes ): Likewise.
	* config/vms/vms-c.c (vms_c_register_includes): Likewise.

	c-family/
	* c-opts.c (add_prefixed_path): Change chain to incpath_e type.
	(c_common_handle_option): Update incpath_e names.

	fortran/
	* cpp.c (gfc_cpp_add_include_path): Update incpath_e names.
	(gfc_cpp_add_include_path_after): Likewise.

Index: incpath.h
===
--- incpath.h	(revision 253649)
+++ incpath.h	(working copy)
@@ -18,13 +18,22 @@
 #ifndef GCC_INCPATH_H
 #define GCC_INCPATH_H
 
+/* Various fragments of include path.  */
+enum incpath_kind {
+  INC_QUOTE = 0, /* include "foo" */
+  INC_BRACKET,   /* include  */
+  INC_SYSTEM,/* sysinclude */
+  INC_AFTER,	/* post-sysinclude.  */
+  INC_MAX
+};
+
 extern void split_quote_chain (void);
-extern void add_path (char *, int, int, bool);
+extern void add_path (char *, incpath_kind, int, bool);
 extern void register_include_chains (cpp_reader *, const char *,
  const char *, const char *,
  int, int, int);
-extern void add_cpp_dir_path (struct cpp_dir *, int);
-extern struct cpp_dir *get_added_cpp_dirs (int);
+extern void add_cpp_dir_path (struct cpp_dir *, incpath_kind);
+extern struct cpp_dir *get_added_cpp_dirs (incpath_kind);
 
 struct target_c_incpath_s {
   /* Do extra includes processing.  STDINC is false iff -nostdinc was given.  */
@@ -34,6 +43,4 @@ struct target_c_incpath_s {
 
 extern struct target_c_incpath_s target_c_incpath;
 
-enum { QUOTE = 0, BRACKET, SYSTEM, AFTER };
-
 #endif /* GCC_INCPATH_H */
Index: c-family/c-opts.c
===
--- c-family/c-opts.c	(revision 253649)
+++ c-family/c-opts.c	(working copy)
@@ -118,7 +118,7 @@ static void set_std_c11 (int);
 static void check_deps_environment_vars (void);
 static void handle_deferred_opts (void);
 static void sanitize_cpp_opts (void);
-static void add_prefixed_path (const char *, size_t);
+static void add_prefixed_path (const char *, incpath_kind);
 static void push_command_line_include (void);
 static void cb_file_change (cpp_reader *, const line_map_ordinary *);
 static void cb_dir_change (cpp_reader *, const char *);
@@ -316,7 +316,7 @@ c_common_handle_option (size_t scode, co
 
 case OPT_I:
   if (strcmp (arg, "-"))
-	add_path (xstrdup (arg), BRACKET, 0, true);
+	add_path (xstrdup (arg), INC_BRACKET, 0, true);
   else
 	{
 	  if (quote_chain_split)
@@ -550,7 +550,7 @@ c_common_handle_option (size_t scode, co
   break;
 
 case OPT_idirafter:
-  add_path (xstrdup (arg), AFTER, 0, true);
+  add_path (xstrdup (arg), INC_AFTER, 0, true);
   break;
 
 case OPT_imacros:
@@ -567,7 +567,7 @@ c_common_handle_option (size_t scode, co
   break;
 
 case OPT_iquote:
-  add_path (xstrdup (arg), QUOTE, 0, true);
+  add_path (xstrdup (arg), INC_QUOTE, 0, true);
   break;
 
 case OPT_isysroot:
@@ -575,15 +575,15 @@ c_common_handle_option (size_t scode, co
   break;
 
 case OPT_isystem:
-  add_path (xstrdup (arg), SYSTEM, 0, true);
+  add_path (xstrdup (arg), INC_SYSTEM, 0, true);
   break;
 
 case OPT_iwithprefix:
-  add_prefixed_path (arg, SYSTEM);
+  add_prefixed_path (arg, INC_SYSTEM);
   break;
 
 case OPT_iwithprefixbefore:
-  add_prefixed_path (arg, BRACKET);
+  add_prefixed_path (arg, INC_BRACKET);
   break;
 
 case OPT_lang_asm:
@@ -1326,7 +1326,7 @@ sanitize_cpp_opts (void)
 
 /* Add include path with a prefix at the front of its name.  */
 static void
-add_prefixed_path (const char *suffix, size_t chain)
+add_prefixed_path (const char *suffix, incpath_kind chain)
 {
   char *path;
   const char *prefix;
Index: config/darwin-c.c
===
--- config/darwin-c.c	(revision 253649)
+++ config/darwin-c.c	(working copy)
@@ -433,7 +433,7 @@ 

Re: [PATCH] Add a warning for invalid function casts

[Martin Sebor]

On 10/11/2017 11:26 AM, Joseph Myers wrote:

On Tue, 10 Oct 2017, Martin Sebor wrote:


The ideal solution for 1) would be a function pointer that can
never be used to call a function (i.e., the void* equivalent
for functions).[X]


I don't think that's relevant.  The normal idiom for this in modern C
code, if not just using void *, is void (*) (void), and since the warning
is supposed to be avoiding excessive false positives and detecting the
cases that are likely to be used for ABI-incompatible calls, the warning
should allow void (*) (void) there.


I think we'll just have to agree to disagree.  I'm not convinced
that using void(*)(void) for this is idiomatic or pervasive enough
to drive design decisions.  Bernd mentioned just libgo and libffi
as the code bases that do, and both you and I have noted that any
pointer type works equally well for this purpose.  The problem
with almost any type, including void(*) (void), is that they can
be misused to call the incompatible function.  Since the sole
purpose of this new warning is to help find those misuses,
excluding void(*)(void) from the checking is directly at odds
with its goal.

I would prefer not to design an unnecessary back door into
the implementation and compromise the effectiveness of the warning
for what's clearly an inferior choice made without fully considering
the risk of misusing the result.  Instead I hope the warning will
drive improvements to code to make its intent explicit.  In my view
that's a good thing even if the code works correctly today.

I don't know how much code there is out that uses void (*)(void)
as a generic function pointer that's never intended to be called.
but I wouldn't expect there to be so much of it to make my
suggestion unfeasible.  I could of course be wrong.  If I am,
we'd find out pretty quickly.

But I've exhausted my arguments and so I think it's time for
me to bow out of the discussion.

Martin

Re: [PATCH] Fix bitmap_bit_in_range_p (PR tree-optimization/82493).

[Jeff Law]

On 10/11/2017 12:13 AM, Martin Liška wrote:
> Hi.
> 
> This fixes some implementations mistakes in sbitmap.c 
> (bitmap_bit_in_range_p). There's reference
> to implementation one can take inspiration from:
> https://www.cs.umd.edu/class/sum2003/cmsc311/Notes/BitOp/bitRange.html
> 
> Problem with our implementation is that one can't do:
> (SBITMAP_ELT_TYPE)1 << SBITMAP_ELT_BITS (that would overflow)
> Thus I do conditionally ~(SBITMAP_ELT_TYPE)0 at some places in the code.
> 
> I also added quite some unit tests for the method. But another questions pop 
> up:
> 1) there are missing boundary asserts (or checking asserts) in sbitmap.c
> 2) we should probably include test-cases also for other functions
> 
> I can work on that (probably later) if desired?
> 
> And my patch breaks ssa-dse-26.c test-case, because now it properly returns 
> true for:
> 
> #0  bitmap_bit_in_range_p (bmap=0x21c4940, start=0, end=8) at 
> ../../gcc/sbitmap.c:326
> #1  0x00d618f5 in live_bytes_read (use_ref=..., ref=0x7fffd480, 
> live=0x21c4940) at ../../gcc/tree-ssa-dse.c:496
> #2  0x00d61c4d in dse_classify_store (ref=0x7fffd480, 
> stmt=0x13ea7d70, use_stmt=0x7fffd470, byte_tracking_enabled=true, 
> live_bytes=0x21c4940) at ../../gcc/tree-ssa-dse.c:594
> #3  0x00d6235b in dse_dom_walker::dse_optimize_stmt 
> (this=0x7fffd5c0, gsi=0x7fffd530) at ../../gcc/tree-ssa-dse.c:820
> #4  0x00d62461 in dse_dom_walker::before_dom_children 
> (this=0x7fffd5c0, bb=0x13d76270) at ../../gcc/tree-ssa-dse.c:852
> #5  0x013b1698 in dom_walker::walk (this=0x7fffd5c0, 
> bb=0x13d76270) at ../../gcc/domwalk.c:308
> #6  0x00d625ac in (anonymous namespace)::pass_dse::execute 
> (this=0x21d58c0, fun=0x13eac0b0) at ../../gcc/tree-ssa-dse.c:906
> #7  0x00b27441 in execute_one_pass (pass=pass@entry=0x21d58c0) at 
> ../../gcc/passes.c:2495
> #8  0x00b27d01 in execute_pass_list_1 (pass=0x21d58c0) at 
> ../../gcc/passes.c:2584
> #9  0x00b27d13 in execute_pass_list_1 (pass=0x21d5480) at 
> ../../gcc/passes.c:2585
> #10 0x00b27d55 in execute_pass_list (fn=, 
> pass=) at ../../gcc/passes.c:2595
> #11 0x00b26681 in do_per_function_toporder 
> (callback=callback@entry=0xb27d40 , 
> data=0x21d5300) at ../../gcc/passes.c:1737
> #12 0x00b283d7 in execute_ipa_pass_list (pass=0x21d52a0) at 
> ../../gcc/passes.c:2935
> #13 0x007d29d2 in ipa_passes () at ../../gcc/cgraphunit.c:2399
> #14 symbol_table::compile (this=this@entry=0x13d6e100) at 
> ../../gcc/cgraphunit.c:2534
> #15 0x007d5277 in symbol_table::compile (this=0x13d6e100) at 
> ../../gcc/cgraphunit.c:2695
> #16 symbol_table::finalize_compilation_unit (this=0x13d6e100) at 
> ../../gcc/cgraphunit.c:2692
> #17 0x00c118ac in compile_file () at ../../gcc/toplev.c:481
> #18 0x00c13eee in do_compile () at ../../gcc/toplev.c:2037
> #19 0x00c141da in toplev::main (this=0x7fffd85e, argc=21, 
> argv=0x7fffd958) at ../../gcc/toplev.c:2172
> #20 0x0061aeab in main (argc=21, argv=0x7fffd958) at 
> ../../gcc/main.c:39
> 
> (gdb) p debug(bmap)
> n_bits = 256, set = {8 9 10 11 12 13 14 15 }
> 
> Jeff can you please help me?
> Apart from that the patch can bootstrap on ppc64le-redhat-linux and survives 
> regression tests.
> 
> Ready to be installed?
> Martin
> 
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2017-10-10  Martin Liska  
> 
>   PR tree-optimization/82493
>   * sbitmap.c (bitmap_bit_in_range_p): Fix the implementation.
>   (test_range_functions): New function.
>   (sbitmap_c_tests): Likewise.
>   * selftest-run-tests.c (selftest::run_tests): Run new tests.
>   * selftest.h (sbitmap_c_tests): New function.
Go ahead and install.  I'll dig into the -26 testcase.

jeff

PING Re: [PATCH] C++: show location of problematic extern "C" specifications

[David Malcolm]

Ping

On Tue, 2017-09-26 at 15:27 -0400, David Malcolm wrote:
> There are a few places where the C++ FE will complain when attempting
> to do things within an extern "C" linkage specifier.
> 
> I've run into problems where it wasn't clear where the pertinent
> extern "C" was; for example, when failing to close an extern "C"
> linkage
> specifier in a header, leading to "template with C linkage" errors in
> a different source file.
> 
> As of r251026 there will be a message highlighting the unclosed '{',
> but
> this may be hard to spot at the very end of the errors.
> 
> This patch adds a note to the various diagnostics that complain
> about C linkage, showing the user where the extern "C" specification
> began.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
> 
> gcc/cp/ChangeLog:
>   * cp-tree.h (struct saved_scope): Add "location" field.
>   (maybe_show_extern_c_location): New decl.
>   * decl.c (grokfndecl): When complaining about literal operators
>   with C linkage, issue a note giving the location of the
>   extern "C".
>   * parser.c (cp_parser_linkage_specification): Store the
> location
>   of the "extern" token within the scope_chain.
>   (maybe_show_extern_c_location): New function.
>   (cp_parser_explicit_specialization): When complaining about
>   template specializations with C linkage, issue a note giving
> the
>   location of the extern "C".
>   (cp_parser_explicit_template_declaration): Likewise for
> templates.
> 
> gcc/testsuite/ChangeLog:
>   * g++.dg/cpp0x/udlit-extern-c.C: New test case.
>   * g++.dg/diagnostic/unclosed-extern-c.C: Add example of a
> template
>   erroneously covered by an unclosed extern "C".
>   * g++.dg/template/extern-c.C: New test case.
> ---
>  gcc/cp/cp-tree.h   |  3 ++
>  gcc/cp/decl.c  |  1 +
>  gcc/cp/parser.c| 19 ++-
>  gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C|  7 
>  .../g++.dg/diagnostic/unclosed-extern-c.C  | 11 +-
>  gcc/testsuite/g++.dg/template/extern-c.C   | 39
> ++
>  6 files changed, 78 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C
>  create mode 100644 gcc/testsuite/g++.dg/template/extern-c.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index e508598..762cc7b 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -1568,6 +1568,8 @@ struct GTY(()) saved_scope {
>hash_map *GTY((skip)) x_local_specializations;
>  
>struct saved_scope *prev;
> +
> +  location_t location;
>  };
>  
>  extern GTY(()) struct saved_scope *scope_chain;
> @@ -6352,6 +6354,7 @@ extern bool parsing_nsdmi (void);
>  extern bool parsing_default_capturing_generic_lambda_in_template
> (void);
>  extern void inject_this_parameter (tree, cp_cv_quals);
>  extern location_t defarg_location (tree);
> +extern void maybe_show_extern_c_location (void);
>  
>  /* in pt.c */
>  extern bool check_template_shadow(tree);
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 50fa1ba..d08ac9a 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -8724,6 +8724,7 @@ grokfndecl (tree ctype,
>if (DECL_LANGUAGE (decl) == lang_c)
>   {
> error ("literal operator with C linkage");
> +   maybe_show_extern_c_location ();
> return NULL_TREE;
>   }
>  
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index d831d66..b90f40d 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -13823,7 +13823,8 @@ cp_parser_linkage_specification (cp_parser*
> parser)
>tree linkage;
>  
>/* Look for the `extern' keyword.  */
> -  cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN);
> +  cp_token *extern_token
> += cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN);
>  
>/* Look for the string-literal.  */
>linkage = cp_parser_string_literal (parser, false, false);
> @@ -13843,6 +13844,7 @@ cp_parser_linkage_specification (cp_parser*
> parser)
>  
>/* We're now using the new linkage.  */
>push_lang_context (linkage);
> +  scope_chain->location = extern_token->location;
>  
>/* If the next token is a `{', then we're using the first
>   production.  */
> @@ -16589,6 +16591,19 @@ cp_parser_explicit_instantiation (cp_parser*
> parser)
>timevar_pop (TV_TEMPLATE_INST);
>  }
>  
> +/* Helper function for diagnostics that have complained about things
> +   being used with 'extern "C"' linkage.
> +
> +   Attempt to issue a note showing where the 'extern "C"' linkage
> began.  */
> +
> +void
> +maybe_show_extern_c_location (void)
> +{
> +  if (scope_chain->location != UNKNOWN_LOCATION)
> +inform (scope_chain->location, "% linkage started
> here");
> +}
> +
> +
>  /* Parse an explicit-specialization.
>  
> explicit-specialization:
> @@ -16623,6 +16638,7 @@ 

Re: [PATCH] Add a warning for invalid function casts

[Joseph Myers]

On Tue, 10 Oct 2017, Martin Sebor wrote:

> The ideal solution for 1) would be a function pointer that can
> never be used to call a function (i.e., the void* equivalent
> for functions).[X]

I don't think that's relevant.  The normal idiom for this in modern C 
code, if not just using void *, is void (*) (void), and since the warning 
is supposed to be avoiding excessive false positives and detecting the 
cases that are likely to be used for ABI-incompatible calls, the warning 
should allow void (*) (void) there.

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

PING Re: [PATCH 2/2] C/C++: add fix-it hints for various missing symbols (v3)

[David Malcolm]

Ping

On Thu, 2017-10-05 at 12:08 -0400, David Malcolm wrote:
> Here's a slight update to this patch, since v2 was made invalid by 
> r253411 ("C: underline parameters in mismatching function calls").
> 
> Both v2 and r253411 added code to c-parser.c/h to track the
> location_t
> of the last consumed token (although I somehow managed to name the
> new
> field in c_parser differently between the two versions...)
> 
> This version (v3) is the same as v2, but removes the copy of the
> above code, updating the usage sites to use the field name from
> r253411.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> in conjunction with patch 1 of the kit:
>   https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01745.html
> 
> OK for trunk?
> 
> Blurb from v2 follows:
> 
> The patch improves our C/C++ frontends' handling of missing
> symbols, by making c_parser_require and cp_parser_require use
> "better" locations for the diagnostic, and insert fix-it hints,
> under certain circumstances (see the comments in the patch for
> full details).
> 
> For example, for this code with a missing semicolon:
> 
>   $ cat test.c
>   int missing_semicolon (void)
>   {
> return 42
>   }
> 
>   trunk currently emits:
> 
>   test.c:4:1: error: expected ';' before '}' token
>}
>^
> 
> This patch adds a fix-it hint for the missing semicolon, and puts
> the error at the location of the missing semicolon, printing the
> followup token as a secondary location:
> 
>   test.c:3:12: error: expected ';' before '}' token
>  return 42
>   ^
>   ;
>}
>~
> 
> More examples can be seen in the test cases.
> 
> This is a revised version of the patch I posted here:
>   https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00135.html
> 
> Some of the changes in that patch landed in trunk in r251026
> (aka 3fe34694f0990d1d649711ede0326497f8a849dc
> "C/C++: show pertinent open token when missing a close token"),
> so this patch contains the remaining part, updated also for the
> previous patch that reunifies the cloned copies of cp_parser_error
> introduced in r251026.
> 
> It also:
> - fixes the typo seen by Jeff
> - eliminated some unnecessary changes to c-c++-common/missing-
> symbol.c
> - fixes some bugs
> 
> r250133, r250134, and r251026 already incorporated the suggestion
> from
> Richard Sandiford to consolidate note-printing when the matching
> location
> is near the primary location of the diagnostic.
> 
> This patch doesn't address Joseph's requests to tackle PR 7356 and
> PR 18248, but he said that it was OK to leave these for followups.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> in conjunction with patch 1 of the kit.
> 
> OK for trunk?
> 
> gcc/c-family/ChangeLog:
>   * c-common.c (enum missing_token_insertion_kind): New enum.
>   (get_missing_token_insertion_kind): New function.
>   (maybe_suggest_missing_token_insertion): New function.
>   * c-common.h (maybe_suggest_missing_token_insertion): New decl.
> 
> gcc/c/ChangeLog:
>   * c-parser.c (c_parser_require): Add "type_is_unique" param and
>   use it to guard calls to maybe_suggest_missing_token_insertion.
>   (c_parser_parms_list_declarator): Override default value of new
>   "type_is_unique" param to c_parser_require.
>   (c_parser_asm_statement): Likewise.
>   * c-parser.h (c_parser_require): Add "type_is_unique" param,
>   defaulting to true.
> 
> gcc/cp/ChangeLog:
>   * parser.c (get_required_cpp_ttype): New function.
>   (cp_parser_error_1): Call it, using the result to call
>   maybe_suggest_missing_token_insertion.
> 
> gcc/testsuite/ChangeLog:
>   * c-c++-common/cilk-plus/AN/parser_errors.c: Update expected
>   output to reflect changes to reported locations of missing
>   symbols.
>   * c-c++-common/cilk-plus/AN/parser_errors2.c: Likewise.
>   * c-c++-common/cilk-plus/AN/parser_errors3.c: Likewise.
>   * c-c++-common/cilk-plus/AN/pr61191.c: Likewise.
>   * c-c++-common/gomp/pr63326.c: Likewise.
>   * c-c++-common/missing-close-symbol.c: Likewise, also update
> for
>   new fix-it hints.
>   * c-c++-common/missing-symbol.c: Likewise, also add test
> coverage
>   for missing colon in ternary operator.
>   * g++.dg/cpp1y/digit-sep-neg.C: Likewise.
>   * g++.dg/cpp1y/pr65202.C: Likewise.
>   * g++.dg/missing-symbol-2.C: New test case.
>   * g++.dg/other/do1.C: Update expected output to reflect
>   changes to reported locations of missing symbols.
>   * g++.dg/parse/error11.C: Likewise.
>   * g++.dg/template/error11.C: Likewise.
>   * gcc.dg/missing-symbol-2.c: New test case.
>   * gcc.dg/missing-symbol-3.c: New test case.
>   * gcc.dg/noncompile/940112-1.c: Update expected output to
> reflect
>   changes to reported locations of missing symbols.
>   * gcc.dg/noncompile/971104-1.c: Likewise.
>   * obj-c++.dg/exceptions-6.mm: Likewise.
>   * 

PING: Re: [PATCH 1/2] C++: avoid partial duplicate implementation of cp_parser_error

[David Malcolm]

Ping

On Tue, 2017-09-26 at 09:56 -0400, David Malcolm wrote:
> In r251026 (aka 3fe34694f0990d1d649711ede0326497f8a849dc,
> "C/C++: show pertinent open token when missing a close token")
> I copied part of cp_parser_error into cp_parser_required_error,
> leading to duplication of code.
> 
> This patch eliminates this duplication by merging the two copies of
> the
> code into a new cp_parser_error_1 subroutine.
> 
> Doing so removes an indentation level, making the patch appear to
> have
> more churn than it really does.
> 
> The patch also undoes the change to g++.dg/parse/pragma2.C, as the
> old behavior is restored.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
> 
> gcc/cp/ChangeLog:
>   * parser.c (get_matching_symbol): Move to before...
>   (cp_parser_error): Split out into...
>   (cp_parser_error_1): ...this new function, merging in content
>   from...
>   (cp_parser_required_error): ...here.  Eliminate partial
> duplicate
>   of body of cp_parser_error in favor of a call to the new
>   cp_parser_error_1 helper function.
> 
> gcc/testsuite/ChangeLog:
>   * g++.dg/parse/pragma2.C: Update to reflect reinstatement of
> the
>   "#pragma is not allowed here" error.
> ---
>  gcc/cp/parser.c  | 169 -
> --
>  gcc/testsuite/g++.dg/parse/pragma2.C |   4 +-
>  2 files changed, 97 insertions(+), 76 deletions(-)
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 25b91df..56d9442 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -2767,53 +2767,116 @@ cp_lexer_peek_conflict_marker (cp_lexer
> *lexer, enum cpp_ttype tok1_kind,
>return true;
>  }
>  
> -/* If not parsing tentatively, issue a diagnostic of the form
> +/* Get a description of the matching symbol to TOKEN_DESC e.g. "("
> for
> +   RT_CLOSE_PAREN.  */
> +
> +static const char *
> +get_matching_symbol (required_token token_desc)
> +{
> +  switch (token_desc)
> +{
> +default:
> +  gcc_unreachable ();
> +  return "";
> +case RT_CLOSE_BRACE:
> +  return "{";
> +case RT_CLOSE_PAREN:
> +  return "(";
> +}
> +}
> +
> +/* Subroutine of cp_parser_error and cp_parser_required_error.
> +
> +   Issue a diagnostic of the form
>FILE:LINE: MESSAGE before TOKEN
> where TOKEN is the next token in the input stream.  MESSAGE
> (specified by the caller) is usually of the form "expected
> -   OTHER-TOKEN".  */
> +   OTHER-TOKEN".
> +
> +   This bypasses the check for tentative passing, and potentially
> +   adds material needed by cp_parser_required_error.
> +
> +   If MISSING_TOKEN_DESC is not RT_NONE, and MATCHING_LOCATION is
> not
> +   UNKNOWN_LOCATION, then we have an unmatched symbol at
> +   MATCHING_LOCATION; highlight this secondary location.  */
>  
>  static void
> -cp_parser_error (cp_parser* parser, const char* gmsgid)
> +cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
> +required_token missing_token_desc,
> +location_t matching_location)
>  {
> -  if (!cp_parser_simulate_error (parser))
> +  cp_token *token = cp_lexer_peek_token (parser->lexer);
> +  /* This diagnostic makes more sense if it is tagged to the line
> + of the token we just peeked at.  */
> +  cp_lexer_set_source_position_from_token (token);
> +
> +  if (token->type == CPP_PRAGMA)
>  {
> -  cp_token *token = cp_lexer_peek_token (parser->lexer);
> -  /* This diagnostic makes more sense if it is tagged to the
> line
> -  of the token we just peeked at.  */
> -  cp_lexer_set_source_position_from_token (token);
> +  error_at (token->location,
> + "%<#pragma%> is not allowed here");
> +  cp_parser_skip_to_pragma_eol (parser, token);
> +  return;
> +}
>  
> -  if (token->type == CPP_PRAGMA)
> +  /* If this is actually a conflict marker, report it as such.  */
> +  if (token->type == CPP_LSHIFT
> +  || token->type == CPP_RSHIFT
> +  || token->type == CPP_EQ_EQ)
> +{
> +  location_t loc;
> +  if (cp_lexer_peek_conflict_marker (parser->lexer, token->type, 
> ))
>   {
> -   error_at (token->location,
> - "%<#pragma%> is not allowed here");
> -   cp_parser_skip_to_pragma_eol (parser, token);
> +   error_at (loc, "version control conflict marker in file");
> return;
>   }
> +}
>  
> -  /* If this is actually a conflict marker, report it as
> such.  */
> -  if (token->type == CPP_LSHIFT
> -   || token->type == CPP_RSHIFT
> -   || token->type == CPP_EQ_EQ)
> - {
> -   location_t loc;
> -   if (cp_lexer_peek_conflict_marker (parser->lexer, token-
> >type, ))
> - {
> -   error_at (loc, "version control conflict marker in
> file");
> -   return;
> - }
> - }
> +  gcc_rich_location richloc (input_location);
> +
> +  bool added_matching_location = false;
> +
> +  if (missing_token_desc != 

Re: correct attribute ifunc C++ type safety (PR 82301)

[Jeff Law]

On 10/11/2017 05:14 AM, Nathan Sidwell wrote:
> On 10/04/2017 03:40 PM, Martin Sebor wrote:
>> On 09/28/2017 08:25 AM, Nathan Sidwell wrote:
> 
> 
>> Since the original tests (where the resolver returns void*) pass
>> across the board I assume it must work for all supported ABIs.
>> Or is there some subtlety between the before and after code that
>> I'm missing?
> 
> I had a vague notion of some (IBM?) target that might do something
> different.  Perhaps it's gone.  Your comment explains it fine.
> 
>>> oh, I think it's trying to spot the pointer to NON-static member
>>> function internal record type.  But brokenly. I think pmf record_types
>>> have DECL_ARTIFICIAL and BUILTIN_LOCATION, that might be useful.
>>
>> It turns out this code was superfluous with the C++ correction
>> and I was able to remove it with no impact on the tests.
> 
> Great.
> 
>>> Is this function called before we know whether we've enabled the
>>> appropriate warnings?  It would be better to bail out sooner if the
>>> warnings are disabled.
>>
>> I'm not sure I understand the question or suggestion here but
>> warnings in general are certainly enabled at this point.  The
>> function issues both errors and warnings so it can't very well
>> exit early without checking the compatibility.  Let me know if
>> I misunderstood your comment.
> 
> Oh, forgot it issued errors too, so my queston is moot.
> 
>> -signedness.
>> +signedness.  In C++ where incompatible pointer conversions ordinarily
>> cause
> Missing comma:  In C++, where incompatible ...
> 
>> +errors, the warning detects such conversions in GCC extensions that
>> aren't
>> +part of the standard language.  An example of a construct that might
>> perform
>> +such a conversion is the @code{alias} attribute.  @xref{Function
>> Attributes,,Declaring Attributes of Functions}.
> 
> Looks fine to me.  (pedantically, I don't think I can formally approve
> this, because it's not part of the C++ FE.)
That's good enough for me :-)  So I'll make it an official ACK.

jeff

[committed][PATCH] Fix typo in end of array address computation

[Jeff Law]

I was playing around a bit last night and found that
struct-layout-1_generate.c which creates the structure layout
compatibility tests has an out of bounds array index.

struct-layout-1_generate.c: In function ‘generate_fields’:
struct-layout-1_generate.c:1896:24: warning: array subscript is above
array bounds [-Warray-bounds]
 && e[n].type < _attrib_array_types[NAATYPES2])
^~

It's pretty obvious that this should have used NCAATYPES2.   Verified by
checking the warning is fixed and re-running the testsuite.



Installed on the trunk.

jeff

Re: [RFC] overflow safe scaling of 64bit values

[Jan Hubicka]

> > Any ideas for better version? If not I will go ahead with this variant and
> > increase profile probability base.
> 
> Why not use GCC wide int?

Because I would like to keep operations fast on in common case where overflows
do not happen. There was at least one case (the propagation of probabilities
to frequencies) where this was compilation time critical. This particular code
runs on sreals but similar type of propagation is done by rtl expansion (on
smaller scale)

But using wide int is what i do as fallback- see if multiplication fits in
64bit arithmetics, if 128bit arithmetics is available and go for wide int
otherwise.

Honza

Re: [PATCH] Improve FAIL message for dump-*-times functions.

[Segher Boessenkool]

Hi!

On Wed, Oct 11, 2017 at 10:14:29AM +0200, Martin Liška wrote:
> This patch helps to find why an expected number of scan patterns does not 
> match:
> 
> FAIL: gcc.dg/unroll-3.c scan-tree-dump-times cunrolli "loop with 3 iterations 
> completely unrolled" 222 (found 1 times)
> FAIL: c-c++-common/attr-simd-2.c  -Wc++-compat   scan-assembler-times 
> _ZGVbN4_simd_attr: 111 (found 1 times)

Cool, looks fine to me (but I can't approve it).  Some suggestions:

> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> index bab23e8e165..e90e61c29ae 100644
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -247,10 +247,11 @@ proc scan-assembler-times { args } {
>  set text [read $fd]
>  close $fd
>  
> -if { [llength [regexp -inline -all -- $pattern $text]] == [lindex $args 
> 1]} {
> +set pattern_count [llength [regexp -inline -all -- $pattern $text]]
> +if {$pattern_count == [lindex $args 1]} {
>   pass "$testcase scan-assembler-times $pp_pattern [lindex $args 1]"
>  } else {
> - fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1]"
> + fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1] 
> (found $pattern_count times)"
>  }
>  }

pattern_count is not such a great name (it's the result count, instead).

You could factor out the [lindex $args 1] to a variable.

You do both of these in scandump.exp already, so why not here :-)


Segher


> diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp
> index 2e6eebfaf33..4a64ac6e05d 100644
> --- a/gcc/testsuite/lib/scandump.exp
> +++ b/gcc/testsuite/lib/scandump.exp
> @@ -86,6 +86,7 @@ proc scan-dump-times { args } {
>  }
>  
>  set testcase [testname-for-summary]
> +set times [lindex $args 2]
>  set suf [dump-suffix [lindex $args 3]]
>  set printable_pattern [make_pattern_printable [lindex $args 1]]
>  set testname "$testcase scan-[lindex $args 0]-dump-times $suf 
> \"$printable_pattern\" [lindex $args 2]"
> @@ -101,10 +102,11 @@ proc scan-dump-times { args } {
>  set text [read $fd]
>  close $fd
>  
> -if { [llength [regexp -inline -all -- [lindex $args 1] $text]] == 
> [lindex $args 2]} {
> +set result_count [llength [regexp -inline -all -- [lindex $args 1] 
> $text]]
> +if {$result_count == $times} {
>  pass "$testname"
>  } else {
> -fail "$testname"
> +fail "$testname (found $result_count times)"
>  }
>  }

Re: [PATCH] Improve FAIL message for dump-*-times functions.

[Jeff Law]

On 10/11/2017 02:14 AM, Martin Liška wrote:
> Hi.
> 
> This patch helps to find why an expected number of scan patterns does not 
> match:
> 
> FAIL: gcc.dg/unroll-3.c scan-tree-dump-times cunrolli "loop with 3 iterations 
> completely unrolled" 222 (found 1 times)
> FAIL: c-c++-common/attr-simd-2.c  -Wc++-compat   scan-assembler-times 
> _ZGVbN4_simd_attr: 111 (found 1 times)
> 
> Ready to be installed after testing?
> Thanks,
> Martin
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-10-11  Martin Liska  
> 
>   * lib/scanasm.exp: Print how many times a regex pattern is
>   found.
>   * lib/scandump.exp: Likewise.
OK.  Definitely helpful.

jeff

Re: [PING] Re: correct attribute ifunc C++ type safety (PR 82301)

[Martin Sebor]

On 10/11/2017 10:32 AM, Nathan Sidwell wrote:

On 10/11/2017 12:21 PM, Martin Sebor wrote:

Ping?

Since I submitted the updated patch it has been suggested to me
that providing a new option to control the warning rather than
using an existing one would be preferable (see bug 82435 for
the background).  The attached update to the patch adds
-Wattribute-alias for this purpose and restores the original
disposition for -Wincompatible-pointer-types.


Makes sense to me. Did you mis my review earlier today?


Thanks.  For some reason I got both your replies at the same
time.  Since you're comfortable with the C++ aspects let me
see if Joseph is willing to approve the updated patch.

Martin

Re: [PATCH] DECL_ASSEMBLER_NAME and friends

[Nathan Sidwell]

On 10/11/2017 11:28 AM, Nathan Sidwell wrote:

On 10/11/2017 08:28 AM, Richard Biener wrote:

On Tue, Oct 10, 2017 at 3:03 PM, Nathan Sidwell  wrote:
I have a patch cleaning up a bit of the C++ FE, which will involve 
hashing
via DECL_ASSEMBLER_NAME.  however, all the items in that hash are 
known to

have it set, so its mapping to a function call is undesirable.



Ok.


I broke the patch into two.  This is the part introducing 
DECL_ASSEMBLER_NAME_RAW.


And this is the bit separating HAS_DECL_ASSEMBLER_NAME_P from 
DECL_ASSEMBLER_NAME_SET_P.


applying to trunk.

nathan
--
Nathan Sidwell
2017-10-11  Nathan Sidwell  

	* tree.h (DECL_ASSEMBLER_NAME_SET_P): Don't check
	HAS_DECL_ASSEMBLER_NAME_P.
	* gimple-expr.c (gimple_decl_printable_name: Check
	HAS_DECL_ASSEMBLER_NAME_P too.
	* ipa-utils.h (type_in_anonymous_namespace_p): Check
	DECL_ASSEMBLER_NAME_SET_P of TYPE_NAME.
	(odr_type_p): No need to assert TYPE_NAME is a TYPE_DECL.
	* passes.c (rest_of_decl_compilation): Check
	HAS_DECL_ASSEMBLER_NAME_P too.
	* recog.c (verify_changes): Likewise.
	* tree-pretty-print.c (dump_decl_name): Likewise.
	* tree-ssa-structalias.c (alias_get_name): Likewise.  Reimplement.

	c/
	* c-decl.c (grokdeclarator): Check HAS_DECL_ASSEMBLER_NAME_P too.

Index: c/c-decl.c
===
--- c/c-decl.c	(revision 253647)
+++ c/c-decl.c	(working copy)
@@ -7011,7 +7011,8 @@ grokdeclarator (const struct c_declarato
 
   /* This is the earliest point at which we might know the assembler
  name of a variable.  Thus, if it's known before this, die horribly.  */
-gcc_assert (!DECL_ASSEMBLER_NAME_SET_P (decl));
+gcc_assert (!HAS_DECL_ASSEMBLER_NAME_P (decl)
+		|| !DECL_ASSEMBLER_NAME_SET_P (decl));
 
 if (warn_cxx_compat
 	&& VAR_P (decl)
Index: gimple-expr.c
===
--- gimple-expr.c	(revision 253647)
+++ gimple-expr.c	(working copy)
@@ -337,9 +337,8 @@ gimple_decl_printable_name (tree decl, i
   if (!DECL_NAME (decl))
 return NULL;
 
-  if (DECL_ASSEMBLER_NAME_SET_P (decl))
+  if (HAS_DECL_ASSEMBLER_NAME_P (decl) && DECL_ASSEMBLER_NAME_SET_P (decl))
 {
-  const char *str, *mangled_str;
   int dmgl_opts = DMGL_NO_OPTS;
 
   if (verbosity >= 2)
@@ -352,9 +351,10 @@ gimple_decl_printable_name (tree decl, i
 	dmgl_opts |= DMGL_PARAMS;
 	}
 
-  mangled_str = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
-  str = cplus_demangle_v3 (mangled_str, dmgl_opts);
-  return (str) ? str : mangled_str;
+  const char *mangled_str
+	= IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME_RAW (decl));
+  const char *str = cplus_demangle_v3 (mangled_str, dmgl_opts);
+  return str ? str : mangled_str;
 }
 
   return IDENTIFIER_POINTER (DECL_NAME (decl));
Index: ipa-utils.h
===
--- ipa-utils.h	(revision 253647)
+++ ipa-utils.h	(working copy)
@@ -217,11 +217,11 @@ type_in_anonymous_namespace_p (const_tre
 {
   /* C++ FE uses magic  as assembler names of anonymous types.
  	 verify that this match with type_in_anonymous_namespace_p.  */
-  gcc_checking_assert (!in_lto_p || !DECL_ASSEMBLER_NAME_SET_P (t)
-			   || !strcmp
- ("",
-  IDENTIFIER_POINTER
- (DECL_ASSEMBLER_NAME (TYPE_NAME (t);
+  gcc_checking_assert (!in_lto_p
+			   || !DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t))
+			   || !strcmp ("",
+   IDENTIFIER_POINTER
+   (DECL_ASSEMBLER_NAME (TYPE_NAME (t);
   return true;
 }
   return false;
@@ -245,14 +245,13 @@ odr_type_p (const_tree t)
   if (type_in_anonymous_namespace_p (t))
 return true;
 
-  if (TYPE_NAME (t) && TREE_CODE (TYPE_NAME (t)) == TYPE_DECL
-  && DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t)))
+  if (TYPE_NAME (t) && DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t)))
 {
   /* C++ FE uses magic  as assembler names of anonymous types.
  	 verify that this match with type_in_anonymous_namespace_p.  */
   gcc_checking_assert (strcmp ("",
-  IDENTIFIER_POINTER
-	(DECL_ASSEMBLER_NAME (TYPE_NAME (t);
+   IDENTIFIER_POINTER
+   (DECL_ASSEMBLER_NAME (TYPE_NAME (t);
   return true;
 }
   return false;
Index: passes.c
===
--- passes.c	(revision 253647)
+++ passes.c	(working copy)
@@ -197,7 +197,9 @@ rest_of_decl_compilation (tree decl,
 
   /* Can't defer this, because it needs to happen before any
  later function definitions are processed.  */
-  if (DECL_ASSEMBLER_NAME_SET_P (decl) && DECL_REGISTER (decl))
+  if (HAS_DECL_ASSEMBLER_NAME_P (decl)
+  && DECL_ASSEMBLER_NAME_SET_P (decl)
+  && DECL_REGISTER (decl))
 make_decl_rtl (decl);
 
   /* Forward declarations for nested functions are not "external",
Index: recog.c
===
--- recog.c	

Re: [PING] Re: correct attribute ifunc C++ type safety (PR 82301)

[Nathan Sidwell]

On 10/11/2017 12:21 PM, Martin Sebor wrote:

Ping?

Since I submitted the updated patch it has been suggested to me
that providing a new option to control the warning rather than
using an existing one would be preferable (see bug 82435 for
the background).  The attached update to the patch adds
-Wattribute-alias for this purpose and restores the original
disposition for -Wincompatible-pointer-types.


Makes sense to me. Did you mis my review earlier today?

nathan

--
Nathan Sidwell

[PING] Re: correct attribute ifunc C++ type safety (PR 82301)

[Martin Sebor]

Ping?

Since I submitted the updated patch it has been suggested to me
that providing a new option to control the warning rather than
using an existing one would be preferable (see bug 82435 for
the background).  The attached update to the patch adds
-Wattribute-alias for this purpose and restores the original
disposition for -Wincompatible-pointer-types.

Thanks
Martin

On 10/04/2017 01:40 PM, Martin Sebor wrote:

On 09/28/2017 08:25 AM, Nathan Sidwell wrote:

On 09/24/2017 06:03 PM, Martin Sebor wrote:

r253041 enhanced type checking for alias and ifunc attributes to
detect declarations of incompatible aliases, or ifunc resolvers
that return pointers to functions of an incompatible type.  More
extensive testing exposed a bug in the implementation of the ifunc
attribute handling in C++ where the checker expected the ifunc
resolver to return a pointer to a member function when the
implementation actually expects it return a pointer to a non-
member function.

In a discussion of the test suite failures, Jakub also suggested
to break the enhanced warning out of -Wattributes and issue it
under a different option.

The attached patch corrects the C++ problem and moves the warning
under -Wincompatible-pointer-types.  Since this is a C-only option,
the patch also enables for it C++.  Since the option is enabled by
default, the patch further requires -Wextra to issue the warning
for ifunc resolvers returning void*.  However, the patched checker
diagnoses other incompatibilities without it.

Martin


I find the maybe_diag_incompatible_alias function confusing.


+/* Check declaration of the type of ALIAS for compatibility with its
TARGET
+   (which may be an ifunc resolver) and issue a diagnostic when they
are
+   not compatible according to language rules (plus a C++ extension for
+   non-static member functions).  */
+
+static void
+maybe_diag_incompatible_alias (tree alias, tree target)
+{
+  tree altype = TREE_TYPE (alias);
+  tree targtype = TREE_TYPE (target);
+
+  bool ifunc = lookup_attribute ("ifunc", DECL_ATTRIBUTES (alias));
+  if (ifunc)


I think it might be clearer if this was broken out into a diag_ifunc
function?  But see below ...


Thanks for the review.  I've updated the patch to incorporate
your suggestions.  My responses (mostly agreeing with your
comments or clarifying things, plus one question) are inline.




+{
+  /* Handle attribute ifunc first.  */
+
+  tree funcptr = altype;
+
+  /* Set FUNCPTR to the type of the alias target.  If the type
+ is a non-static member function of class C, construct a type
+ of an ordinary function taking C* as the first argument,
+ followed by the member function argument list, and use it
+ instead to check for compatibilties.  FUNCPTR is used only
+ in diagnostics.  */


This comment is self-contradictory.
  1 Set FUNCPTR
  2 Do some method-type shenanigans
  3 Use it to check for incompatibilites
  4 FUNCPTR is only used in diags

Which of #3 and #4 is true?


Both.  It's used to control diagnostics (as opposed to something
else).  But the comment is from an earlier version of the patch
where the function body was still a part of its caller, so it's
redundant now that all the code in maybe_diag_incompatible_alias
is only used to control diagnostics.


+
+  if (TREE_CODE (altype) == METHOD_TYPE)
+{


IMHO put the description of the METHOD_TYPE chicanery inside the block
doing it?  FWIW, although the change being made works on many (most?)
ABIs, it's not formally correct and I think fails on some where 'this'
is passed specially. You might want to note that?


Sure.  I've added a comment.

Since the original tests (where the resolver returns void*) pass
across the board I assume it must work for all supported ABIs.
Or is there some subtlety between the before and after code that
I'm missing?




+  tree rettype = TREE_TYPE (altype);
+  tree args = TYPE_ARG_TYPES (altype);
+  altype = build_function_type (rettype, args);
+  funcptr = altype;
+}
+



+  if ((!FUNC_OR_METHOD_TYPE_P (targtype)
+   || (prototype_p (altype)
+   && prototype_p (targtype)
+   && !types_compatible_p (altype, targtype
+{
+  funcptr = build_pointer_type (funcptr);
+
+  if (warning_at (DECL_SOURCE_LOCATION (target),
+  OPT_Wincompatible_pointer_types,
+  "% resolver for %qD should return %qT",
+  alias, funcptr))
+inform (DECL_SOURCE_LOCATION (alias),
+"resolver indirect function declared here");
+}


this block is almost the same as the non-ifunc block.  Surely they can
be the same code? (by generalizing one of the cases until it turns into
the other?)


The existing code does that but in this patch I made the warnings
and informational notes distinct.  It feels like a tossup between
parameterizing the code and making the flow more complex and harder
to follow and keeping the two 

[PATCH GCC]Refine comment and set type for partition merged from SCC

[Bin Cheng]

Hi,
When reading the code I found it's could be confusing without comment.
This patch adds comment explaining why we want merge PARALLEL type
partitions in a SCC, even though the result partition can no longer
be executed in parallel.  It also sets type of the result partition
to sequential.
Bootstrap and test on x86_64 and AArch64.  Is it OK?

Thanks,
bin
2017-10-10  Bin Cheng  

* tree-loop-distribution.c (break_alias_scc_partitions): Add comment
and set PTYPE_SEQUENTIAL for merged partition.diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
index 9ffac53..dc429cf 100644
--- a/gcc/tree-loop-distribution.c
+++ b/gcc/tree-loop-distribution.c
@@ -2062,7 +2062,7 @@ break_alias_scc_partitions (struct graph *rdg,
   auto_vec scc_types;
   struct partition *partition, *first;
 
-  /* If all paritions in a SCC has the same type, we can simply merge the
+  /* If all partitions in a SCC have the same type, we can simply merge the
 SCC.  This loop finds out such SCCS and record them in bitmap.  */
   bitmap_set_range (sccs_to_merge, 0, (unsigned) num_sccs);
   for (i = 0; i < num_sccs; ++i)
@@ -2075,6 +2075,10 @@ break_alias_scc_partitions (struct graph *rdg,
  if (pg->vertices[j].component != i)
continue;
 
+ /* Note we Merge partitions of parallel type on purpose, though
+the result partition is sequential.  The reason is vectorizer
+can do more accurate runtime alias check in this case.  Also
+it results in more conservative distribution.  */
  if (first->type != partition->type)
{
  bitmap_clear_bit (sccs_to_merge, i);
@@ -2096,7 +2100,7 @@ break_alias_scc_partitions (struct graph *rdg,
   if (bitmap_count_bits (sccs_to_merge) != (unsigned) num_sccs)
{
  /* Run SCC finding algorithm again, with alias dependence edges
-skipped.  This is to topologically sort paritions according to
+skipped.  This is to topologically sort partitions according to
 compilation time known dependence.  Note the topological order
 is stored in the form of pg's post order number.  */
  num_sccs_no_alias = graphds_scc (pg, NULL, pg_skip_alias_edge);
@@ -2139,6 +2143,8 @@ break_alias_scc_partitions (struct graph *rdg,
  data = (struct pg_vdata *)pg->vertices[k].data;
  gcc_assert (data->id == k);
  data->partition = NULL;
+ /* The result partition of merged SCC must be sequential.  */
+ first->type = PTYPE_SEQUENTIAL;
}
}
 }

Re: [PATCH] DECL_ASSEMBLER_NAME and friends

[Nathan Sidwell]

On 10/11/2017 08:28 AM, Richard Biener wrote:

On Tue, Oct 10, 2017 at 3:03 PM, Nathan Sidwell  wrote:

I have a patch cleaning up a bit of the C++ FE, which will involve hashing
via DECL_ASSEMBLER_NAME.  however, all the items in that hash are known to
have it set, so its mapping to a function call is undesirable.



Ok.


I broke the patch into two.  This is the part introducing 
DECL_ASSEMBLER_NAME_RAW.


nathan
--
Nathan Sidwell
2017-10-11  Nathan Sidwell  

	* tree.h (DECL_ASSEMBLER_NAME_RAW): New.
	(SET_DECL_ASSEMBLER_NAME): Use it.
	(DECL_ASSEMBLER_NAME_SET_P): Likewise.
	(COPY_DECL_ASSEMBLER_NAME): Likewise.
	* tree.c (decl_assembler_name): Use DECL_ASSEMBLER_NAME_RAW.

	lto/
	* lto.c (mentions_vars_p_decl_with_vis): Use
	DECL_ASSEMBLER_NAME_RAW.
	(lto_fixup_prevailing_decls): Likewise.

	cp
	* decl2.c (struct mangled_decl_hash): Use DECL_ASSEMBLER_NAME_RAW.
	(record_mangling): Likewise.

Index: gcc/cp/decl2.c
===
--- gcc/cp/decl2.c	(revision 253645)
+++ gcc/cp/decl2.c	(working copy)
@@ -103,7 +103,7 @@ static GTY(()) vec *no_link
 static GTY(()) vec *mangling_aliases;
 
 /* hash traits for declarations.  Hashes single decls via
-   DECL_ASSEMBLER_NAME.  */
+   DECL_ASSEMBLER_NAME_RAW.  */
 
 struct mangled_decl_hash : ggc_remove 
 {
@@ -112,11 +112,11 @@ struct mangled_decl_hash : ggc_remove ::create_ggc (499);
 
   gcc_checking_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
-  tree id = DECL_ASSEMBLER_NAME (decl);
+  tree id = DECL_ASSEMBLER_NAME_RAW (decl);
   tree *slot
 = mangled_decls->find_slot_with_hash (id, IDENTIFIER_HASH_VALUE (id),
 	  INSERT);
Index: gcc/lto/lto.c
===
--- gcc/lto/lto.c	(revision 253645)
+++ gcc/lto/lto.c	(working copy)
@@ -591,7 +591,7 @@ mentions_vars_p_decl_with_vis (tree t)
 return true;
 
   /* Accessor macro has side-effects, use field-name here. */
-  CHECK_NO_VAR (t->decl_with_vis.assembler_name);
+  CHECK_NO_VAR (DECL_ASSEMBLER_NAME_RAW (t));
   return false;
 }
 
@@ -2557,7 +2557,7 @@ lto_fixup_prevailing_decls (tree t)
 	}
   if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
 	{
-	  LTO_NO_PREVAIL (t->decl_with_vis.assembler_name);
+	  LTO_NO_PREVAIL (DECL_ASSEMBLER_NAME_RAW (t));
 	}
   if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
 	{
Index: gcc/tree.c
===
--- gcc/tree.c	(revision 253645)
+++ gcc/tree.c	(working copy)
@@ -671,7 +671,7 @@ decl_assembler_name (tree decl)
 {
   if (!DECL_ASSEMBLER_NAME_SET_P (decl))
 lang_hooks.set_decl_assembler_name (decl);
-  return DECL_WITH_VIS_CHECK (decl)->decl_with_vis.assembler_name;
+  return DECL_ASSEMBLER_NAME_RAW (decl);
 }
 
 /* When the target supports COMDAT groups, this indicates which group the
Index: gcc/tree.h
===
--- gcc/tree.h	(revision 253645)
+++ gcc/tree.h	(working copy)
@@ -2721,6 +2721,10 @@ extern void decl_value_expr_insert (tree
LTO compilation and C++.  */
 #define DECL_ASSEMBLER_NAME(NODE) decl_assembler_name (NODE)
 
+/* Raw accessor for DECL_ASSEMBLE_NAME.  */
+#define DECL_ASSEMBLER_NAME_RAW(NODE) \
+  (DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.assembler_name)
+
 /* Return true if NODE is a NODE that can contain a DECL_ASSEMBLER_NAME.
This is true of all DECL nodes except FIELD_DECL.  */
 #define HAS_DECL_ASSEMBLER_NAME_P(NODE) \
@@ -2731,11 +2735,11 @@ extern void decl_value_expr_insert (tree
yet.  */
 #define DECL_ASSEMBLER_NAME_SET_P(NODE) \
   (HAS_DECL_ASSEMBLER_NAME_P (NODE) \
-   && DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.assembler_name != NULL_TREE)
+   && DECL_ASSEMBLER_NAME_RAW (NODE) != NULL_TREE)
 
 /* Set the DECL_ASSEMBLER_NAME for NODE to NAME.  */
 #define SET_DECL_ASSEMBLER_NAME(NODE, NAME) \
-  (DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.assembler_name = (NAME))
+  (DECL_ASSEMBLER_NAME_RAW (NODE) = (NAME))
 
 /* Copy the DECL_ASSEMBLER_NAME from DECL1 to DECL2.  Note that if DECL1's
DECL_ASSEMBLER_NAME has not yet been set, using this macro will not cause
@@ -2747,10 +2751,7 @@ extern void decl_value_expr_insert (tree
which will try to set the DECL_ASSEMBLER_NAME for DECL1.  */
 
 #define COPY_DECL_ASSEMBLER_NAME(DECL1, DECL2)\
-  (DECL_ASSEMBLER_NAME_SET_P (DECL1)	\
-   ? (void) SET_DECL_ASSEMBLER_NAME (DECL2,\
- DECL_ASSEMBLER_NAME (DECL1))	\
-   : (void) 0)
+  SET_DECL_ASSEMBLER_NAME (DECL2, DECL_ASSEMBLER_NAME_RAW (DECL1))
 
 /* Records the section name in a section attribute.  Used to pass
the name from decl_attributes to make_function_rtl and make_decl_rtl.  */

Re: [PATCH][RFC] Instrument function exit with __builtin_unreachable in C++.

[Jason Merrill]

On Thu, Oct 5, 2017 at 12:53 PM, Martin Liška  wrote:
> On 10/05/2017 05:07 PM, Jason Merrill wrote:
>> On Thu, Oct 5, 2017 at 6:31 AM, Martin Liška  wrote:
>>> As discussed 2 days ago on IRC with Jakub and Jonathan, C++ standard says
>>> that having a non-return
>>> function with missing return statement is undefined behavior. We've got
>>> -fsanitize=return check for
>>> that and we can in such case instrument __builtin_unreachable(). This
>>> patch does that.
>>
>>
>> Great.
>>
>>> And there's still some fallout:
>>>
>>> FAIL: g++.dg/cpp0x/constexpr-diag3.C  -std=c++11  (test for errors, line
>>> 7)
>>> FAIL: g++.dg/cpp0x/constexpr-neg3.C  -std=c++11  (test for errors, line
>>> 12)
>>> FAIL: g++.dg/cpp1y/constexpr-return2.C  -std=c++14  (test for errors,
>>> line 7)
>>> FAIL: g++.dg/cpp1y/constexpr-return2.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/cpp1y/pr63996.C  -std=c++14  (test for errors, line 9)
>>> FAIL: g++.dg/cpp1y/pr63996.C  -std=c++14 (test for excess errors)
>>>
>>> 1) there are causing:
>>>
>>> ./xg++ -B.
>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp1y/pr63996.C
>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp1y/pr63996.C:9:23:
>>> in constexpr expansion of ‘foo(1)’
>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp1y/pr63996.C:4:1:
>>> error: ‘__builtin_unreachable()’ is not a constant expression
>>>   foo (int i)
>>>   ^~~
>>>
>>> Where we probably should not emit the built-in call. Am I right?
>>
>>
>> Or constexpr.c could give a more friendly diagnostic for
>> __builtin_unreachable.  It's correct to give a diagnostic here for
>> this testcase.
>
>
> Hi.
>
> That's good idea, any suggestion different from:
>
> ./xg++ -B.
> /home/marxin/Programming/gcc2/gcc/testsuite/g++.dg/cpp1y/pr63996.C
> /home/marxin/Programming/gcc2/gcc/testsuite/g++.dg/cpp1y/pr63996.C:9:23:
> in constexpr expansion of ‘foo(1)’
> : error: constexpr can't contain call of a non-return function
> ‘__builtin_unreachable’
>
> which is probably misleading as it points to a function call that is
> actually missing in source code.
> Should we distinguish between implicit and explicit __builtin_unreachable?

Probably without your change the constexpr code already diagnoses the
missing return as "constexpr call flows off the end of the function";
that same message seems appropriate.

> So turning on the warning by default for c++, we get about 500 failing 
> test-cases. Uf :/

Yes, we've been sloppy about this in the testsuite.  :(

Jason

[PATCH][GRAPHITE] Fix PR82451 (and PR82355 in a different way)

[Richard Biener]

For PR82355 I introduced a fake dimension to ISL to allow CHRECs
having an evolution in a loop that isn't fully part of the SESE
region we are processing.  That was easier than fending off those
CHRECs (without simply giving up on SESE regions with those).

But it didn't fully solve the issue as PR82451 shows where we run
into the issue that we eventually have to code-gen those
evolutions and thus in theory need a canonical IV of that containing loop.

So I decided (after Micha pressuring me a bit...) to revisit the
original issue and make SCEV analysis "properly" handle SE regions.
It turns out that it is mostly instantiate_scev lacking proper support
plus the necessary interfacing change (really just cosmetic in some sense)
from a instantiate_before basic-block to a instantiate_before edge.

data-ref interfaces have been similarly adjusted, here changing
the "loop nest" loop parameter to the entry edge for the SE region
and passing that down accordingly.

I've for now tried to keep other high-level loop-based interfaces the
same by simply using the loop preheader edge as entry where appropriate
(needing loop_preheader_edge cope with the loop root tree for simplicity).

In the process I ran into issues with us too overly aggressive
instantiating random trees and thus I cut those down.  That part
doesn't successfully test separately (when I remove the strange
ARRAY_REF instantiation), so it's part of this patch.  I've also
run into an SSA verification fail (the id-27.f90 testcase) which
shows we _do_ need to clear the SCEV cache after introducing
the versioned CFG (and added a comment before it).

On the previously failing testcases I've verified we produce
sensible instantiations for those pesky refs residing in "no" loop
in the SCOP and that we get away with the result in terms of
optimizing.

SPEC 2k6 testing shows

loop nest optimized: 311
loop nest not optimized, code generation error: 0
loop nest not optimized, optimized schedule is identical to original 
schedule: 173
loop nest not optimized, optimization timed out: 59
loop nest not optimized, ISL signalled an error: 9
loop nest: 552

for SPEC 2k6 and -floop-nest-optimize while adding -fgraphite-identity
still reveals some codegen errors:

loop nest optimized: 437
loop nest not optimized, code generation error: 25
loop nest not optimized, optimized schedule is identical to original 
schedule: 169
loop nest not optimized, optimization timed out: 60
loop nest not optimized, ISL signalled an error: 9
loop nest: 700

Bootstrap and regtest in progress on x86_64-unknown-linux-gnu
(with and without -fgraphite-identity -floop-nest-optimize).

Ok?

Thanks,
Richard.

2017-10-11  Richard Biener  

PR tree-optimization/82451
Revert
2017-10-02  Richard Biener  

PR tree-optimization/82355
* graphite-isl-ast-to-gimple.c (build_iv_mapping): Also build
a mapping for the enclosing loop but avoid generating one for
the loop tree root.
(copy_bb_and_scalar_dependences): Remove premature codegen
error on PHIs in blocks duplicated into multiple places.
* graphite-scop-detection.c
(scop_detection::stmt_has_simple_data_refs_p): For a loop not
in the region use it as loop and nest to analyze the DR in.
(try_generate_gimple_bb): Likewise.
* graphite-sese-to-poly.c (extract_affine_chrec): Adjust.
(add_loop_constraints): For blocks in a loop not in the region
create a dimension with a single iteration.
* sese.h (gbb_loop_at_index): Remove assert.

* cfgloop.c (loop_preheader_edge): For the loop tree root
return the single successor of the entry block.
* graphite-isl-ast-to-gimple.c (graphite_regenerate_ast_isl):
Reset the SCEV hashtable and niters.
* graphite-scop-detection.c
(scop_detection::graphite_can_represent_scev): Add SCOP parameter,
assert that we only have POLYNOMIAL_CHREC that vary in loops
contained in the region.
(scop_detection::graphite_can_represent_expr): Adjust.
(scop_detection::stmt_has_simple_data_refs_p): For loops
not in the region set loop to NULL.  The nest is now the
entry edge to the region.
(try_generate_gimple_bb): Likewise.
* sese.c (scalar_evolution_in_region): Adjust for
instantiate_scev change.
* tree-data-ref.h (graphite_find_data_references_in_stmt):
Make nest parameter the edge into the region.
(create_data_ref): Likewise.
* tree-data-ref.c (dr_analyze_indices): Make nest parameter an
entry edge into a region and adjust instantiate_scev calls.
(create_data_ref): Likewise.
(graphite_find_data_references_in_stmt): Likewise.
(find_data_references_in_stmt): Pass the loop preheader edge
from the nest argument.
* tree-scalar-evolution.h (instantiate_scev): Make instantiate_below
 

[og7] Enable 0-length array data mappings for implicit data clauses

[Cesar Philippidis]

I've pushed this patch to openacc-gcc-7-branch which teaches the
gimplifier how to create 0-length arrays mappings for certain pointer
and reference typed variables. This is necessary to satisfy the implicit
expectation in OpenACC where the user considers pointer variables used
like arrays as having array types.

In OpenACC, pointers variables officially classified as scalar values,
which requires the content of the scalar to be mapped verbatim onto the
accelerator (i.e., the host pointer doesn't get remapped to a device
address on the accelerator). However, as mentioned above, a lot of users
think that pointers used like arrays are arrays, and therefore expect
things such as

  int *p = ...

  #pragma acc enter data copyin (p[0:100])

  #pragma acc parallel loop
  for (i = ...)
  {
... p[i] ...
  }

to work as if 'p' were an array. Note that this patch does not do
anything special with dynamic arrays. E.g.

  int *p = (int *) malloc ...

  #pragma acc parallel loop
  for (i = ...)
  {
... p[i] ...
  }

In this case, by virtue of being a 0-length array mapping, p will be
transferred to the accelerator with the host value of [0]. I've seen
another compiler which goes through the trouble of mapping the dynamic
array to the accelerator automatically, but that's beyond the scope of
this patch.

Cesar
2017-10-11  Cesar Philippidis  

	gcc/
	* gimplify.c (oacc_default_clause): Create implicit 0-length
	array data clauses for pointers and reference types.

	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/fp-dyn-arrays.c: New test.


diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 2c10c6433a7..7a9cc241792 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6994,7 +6994,12 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, tree decl, unsigned flags)
 case ORT_ACC_PARALLEL:
   rkind = "parallel";
 
-  if (is_private)
+  if (TREE_CODE (type) == REFERENCE_TYPE
+	  && TREE_CODE (TREE_TYPE (type)) == POINTER_TYPE)
+	flags |= GOVD_MAP | GOVD_MAP_0LEN_ARRAY;
+  else if (!lang_GNU_Fortran () && TREE_CODE (type) == POINTER_TYPE)
+	flags |= GOVD_MAP | GOVD_MAP_0LEN_ARRAY;
+  else if (is_private)
 	flags |= GOVD_FIRSTPRIVATE;
   else if (on_device || declared)
 	flags |= GOVD_MAP;
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/fp-dyn-arrays.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/fp-dyn-arrays.c
new file mode 100644
index 000..c57261f2d12
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/fp-dyn-arrays.c
@@ -0,0 +1,42 @@
+/* Expect dynamic arrays to be mapped on the accelerator via
+   GOMP_MAP_0LEN_ARRAY.  */
+
+#include 
+#include 
+#include 
+
+int
+main ()
+{
+  const int n = 1000;
+  int *a, *b, *c, *d, i;
+
+  d = (int *) 12345;
+  a = (int *) malloc (sizeof (int) * n);
+  b = (int *) malloc (sizeof (int) * n);
+  c = (int *) malloc (sizeof (int) * n);
+
+  for (i = 0; i < n; i++)
+{
+  a[i] = -1;
+  b[i] = i+1;
+  c[i] = 2*(i+1);
+}
+
+#pragma acc enter data create(a[0:n]) copyin(b[:n], c[:n])
+#pragma acc parallel loop
+  for (i = 0; i < n; ++i)
+{
+  a[i] = b[i] + c[i] + *((int *));
+}
+#pragma acc exit data copyout(a[0:n]) delete(b[0:n], c[0:n])
+
+  for (i = 0; i < n; i++)
+assert (a[i] == 3*(i+1) + 12345);
+
+  free (a);
+  free (b);
+  free (c);
+
+  return 0;
+}

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 03:36:40PM +0200, Martin Liška wrote:
> > std::swap(addr1, addr2); ?  I don't see it used in any of libsanitizer
> > though, so not sure if the corresponding STL header is included.
> 
> They don't use it anywhere and I had some #include issues. That's why I did 
> it manually.

Ok.

> > There are many kinds of shadow memory markings.  My thought was that it
> > would start with a quick check, perhaps vectorized by hand (depending on if
> > the arch has unaligned loads maybe without or with a short loop for
> 
> Did that, but I have no experience how to make decision about prologue that 
> will
> align the pointer? Any examples?

First of all, why are you aligning anything?
> +  uptr aligned_addr1 = addr1 & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
> +  uptr aligned_addr2 = addr2 & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
You want to compare what the pointers point to, not what the aligned pointer
points to.

Actually, looking around, there already is __sanitizer::mem_is_zero
which does what we want.

Or even __asan_region_is_poisoned(addr1, addr2 - addr1).

Jakub

Re: [patch] configure option to override TARGET_LIBC_PROVIDES_SSP

[Gerald Pfeifer]

On Mon, 9 Oct 2017, Sandra Loosemore wrote:
>> The install.texi documentation for --disable-libssp only says "Specify
> that the run-time libraries for stack smashing protection should not be
>> built.".  I think it needs updating to mention these additional effects as
>> well.
> Oops.  :")  How about this version?

Makes sense to me, thanks.  (Though better give Joseph a day or two
to chime in as well, since he spotted this originally.)

Gerald

[og7] Enable fortran derived types in acc enter/exit data

[Cesar Philippidis]

This patch enables fortran derived type members to be used in acc
enter/exit data constructs. Now, both acc enter/exit data and acc update
support individual derived type members. Eventually, I'd like all of the
acc data clauses to support individual derived type members. But that
may not happen in the near term.

Cesar
2017-10-11  Cesar Philippidis  

	gcc/fortran/
	* openmp.c (match_acc): Add new argument derived_types. Propagate
	it to gfc_match_omp_clauses.
	(gfc_match_oacc_enter_data): Update call to match_acc.
	(gfc_match_oacc_exit_data): Likewise.

	gcc/testsuite/
	* gfortran.dg/goacc/derived-types.f90: Adjust test case.

	libgomp/
	* testsuite/libgomp.oacc-fortran/derived-type-2.f90: New test.


diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index a59b7d27e9c..5562f4e02f7 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2141,10 +2141,12 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, omp_mask mask,
 
 
 static match
-match_acc (gfc_exec_op op, const omp_mask mask, const omp_mask dtype_mask)
+match_acc (gfc_exec_op op, const omp_mask mask, const omp_mask dtype_mask,
+	   bool derived_types=false)
 {
   gfc_omp_clauses *c;
-  if (gfc_match_omp_clauses (, mask, dtype_mask, false, false, true)
+  if (gfc_match_omp_clauses (, mask, dtype_mask, false, false, true,
+			 derived_types)
   != MATCH_YES)
 return MATCH_ERROR;
   new_st.op = op;
@@ -2329,7 +2331,7 @@ match
 gfc_match_oacc_enter_data (void)
 {
   return match_acc (EXEC_OACC_ENTER_DATA, OACC_ENTER_DATA_CLAUSES,
-		OMP_MASK2_LAST);
+		OMP_MASK2_LAST, true);
 }
 
 
@@ -2337,7 +2339,7 @@ match
 gfc_match_oacc_exit_data (void)
 {
   return match_acc (EXEC_OACC_EXIT_DATA, OACC_EXIT_DATA_CLAUSES,
-		OMP_MASK2_LAST);
+		OMP_MASK2_LAST, true);
 }
 
 
diff --git a/gcc/testsuite/gfortran.dg/goacc/derived-types.f90 b/gcc/testsuite/gfortran.dg/goacc/derived-types.f90
index 44a38149560..11d055a79f2 100644
--- a/gcc/testsuite/gfortran.dg/goacc/derived-types.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/derived-types.f90
@@ -28,11 +28,14 @@ program derived_acc
   !$acc update self(var%a)
   
   !$acc enter data copyin(var)
-  !$acc enter data copyin(var%a) ! { dg-error "Syntax error in OpenMP" }
+  !$acc enter data copyin(var%a)
 
   !$acc exit data copyout(var)
-  !$acc exit data copyout(var%a) ! { dg-error "Syntax error in OpenMP" }
+  !$acc exit data copyout(var%a)
 
+  !$acc data copy(var%a) ! { dg-error "Syntax error in OpenMP" }
+  !$acc end data ! { dg-error "Unexpected ..ACC END DATA" }
+  
   !$acc data copy(var)
   !$acc end data
 
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/derived-type-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/derived-type-2.f90
new file mode 100644
index 000..3ed21953261
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-fortran/derived-type-2.f90
@@ -0,0 +1,67 @@
+! Test derived types in data clauses.
+
+! { dg-do run }
+
+module newtype
+  type dtype
+ integer :: a, b, c
+ integer, allocatable :: ary(:)
+  end type dtype
+end module newtype
+
+program main
+  use newtype
+  implicit none
+  integer, parameter :: n = 100
+  integer i
+  type (dtype), dimension(n) :: d1
+  type (dtype) :: d2
+  external process
+
+  allocate (d2%ary(n))
+
+  !$acc enter data create (d2%ary)
+
+  do i = 1, n
+ d2%ary(i) = 1
+  end do
+
+  !$acc update device (d2%ary)
+
+  call process (n, d2%ary)
+
+  !$acc exit data copyout (d2%ary)
+
+  do i = 1, n
+ if (d2%ary(i) /= i + 1) call abort
+  end do
+
+  !$acc data copy(d1(1:n))
+  !$acc parallel loop
+  do i = 1, n
+ d1(i)%a = i
+ d1(i)%b = i-1
+ d1(i)%c = i+1
+  end do
+  !$acc end data
+
+  do i = 1, n
+ if (d1(i)%a /= i) call abort
+ if (d1(i)%b /= i-1) call abort
+ if (d1(i)%c /= i+1) call abort
+  end do
+
+  deallocate (d2%ary)
+end program main
+
+subroutine process (a, b)
+  use newtype
+  implicit none
+  integer :: a, i
+  integer :: b(a)
+
+  !$acc parallel loop present (b(1:a))
+  do i = 1, a
+ b(i) = b(i) + i
+  end do
+end subroutine process

Re: [PATCH][RFC] Instrument function exit with __builtin_unreachable in C++.

[Martin Liška]

On 10/05/2017 06:53 PM, Martin Liška wrote:
>>  We probably want to provide a way to turn off this behavior.
>>
>> If we're going to enable this by default, we probably also want
>> -Wreturn-type on by default.
> 
> Agree.

Hi.

So turning on the warning by default for c++, we get about 500 failing 
test-cases. Uf :/

Martin
c-c++-common/asan/pr63638.c
c-c++-common/goacc/parallel-1.c
c-c++-common/gomp/sink-1.c
c-c++-common/missing-symbol.c
c-c++-common/pr36513-2.c
c-c++-common/pr36513.c
c-c++-common/pr49706-2.c
c-c++-common/pr65120.c
c-c++-common/tm/volatile-1.c
c-c++-common/tsan/race_on_mutex.c
c-c++-common/ubsan/pr71512-1.c
c-c++-common/vector-1.c
c-c++-common/vector-2.c
c-c++-common/Wimplicit-fallthrough-8.c
g++.dg/abi/abi-tag14.C
g++.dg/abi/abi-tag18a.C
g++.dg/abi/abi-tag18.C
g++.dg/abi/covariant2.C
g++.dg/abi/covariant3.C
g++.dg/abi/mangle7.C
g++.dg/asan/pr81340.C
g++.dg/concepts/fn8.C
g++.dg/concepts/pr65575.C
g++.dg/concepts/template-parm11.C
g++.dg/conversion/op6.C
g++.dg/cpp0x/access01.C
g++.dg/cpp0x/alignas3.C
g++.dg/cpp0x/auto2.C
g++.dg/cpp0x/constexpr-array17.C
g++.dg/cpp0x/constexpr-defarg2.C
g++.dg/cpp0x/constexpr-diag3.C
g++.dg/cpp0x/constexpr-memfn1.C
g++.dg/cpp0x/constexpr-neg3.C
g++.dg/cpp0x/dc1.C
g++.dg/cpp0x/dc3.C
g++.dg/cpp0x/decltype12.C
g++.dg/cpp0x/decltype17.C
g++.dg/cpp0x/decltype3.C
g++.dg/cpp0x/decltype41.C
g++.dg/cpp0x/defaulted28.C
g++.dg/cpp0x/enum_base3.C
g++.dg/cpp0x/gen-attrs-4.C
g++.dg/cpp0x/initlist96.C
g++.dg/cpp0x/lambda/lambda-58566.C
g++.dg/cpp0x/lambda/lambda-conv10.C
g++.dg/cpp0x/lambda/lambda-conv12.C
g++.dg/cpp0x/lambda/lambda-defarg3.C
g++.dg/cpp0x/lambda/lambda-ice3.C
g++.dg/cpp0x/lambda/lambda-ice5.C
g++.dg/cpp0x/lambda/lambda-nested2.C
g++.dg/cpp0x/lambda/lambda-template12.C
g++.dg/cpp0x/lambda/lambda-template2.C
g++.dg/cpp0x/lambda/lambda-this12.C
g++.dg/cpp0x/nolinkage1.C
g++.dg/cpp0x/nsdmi-template5.C
g++.dg/cpp0x/parse1.C
g++.dg/cpp0x/pr34054.C
g++.dg/cpp0x/pr47416.C
g++.dg/cpp0x/pr58781.C
g++.dg/cpp0x/pr70538.C
g++.dg/cpp0x/pr81325.C
g++.dg/cpp0x/range-for13.C
g++.dg/cpp0x/range-for14.C
g++.dg/cpp0x/rv2n.C
g++.dg/cpp0x/rv3n.C
g++.dg/cpp0x/static_assert10.C
g++.dg/cpp0x/static_assert11.C
g++.dg/cpp0x/static_assert12.C
g++.dg/cpp0x/static_assert13.C
g++.dg/cpp0x/trailing1.C
g++.dg/cpp0x/trailing5.C
g++.dg/cpp0x/variadic114.C
g++.dg/cpp0x/variadic57.C
g++.dg/cpp0x/variadic65.C
g++.dg/cpp0x/variadic66.C
g++.dg/cpp0x/variadic97.C
g++.dg/cpp0x/variadic98.C
g++.dg/cpp0x/Wunused-variable-1.C
g++.dg/cpp1y/auto-fn11.C
g++.dg/cpp1y/auto-fn29.C
g++.dg/cpp1y/auto-fn38.C
g++.dg/cpp1y/constexpr-return2.C
g++.dg/cpp1y/lambda-init7.C
g++.dg/cpp1y/pr63996.C
g++.dg/cpp1y/pr65202.C
g++.dg/cpp1y/pr66443-cxx14.C
g++.dg/cpp1y/pr79253.C
g++.dg/cpp1y/static_assert1.C
g++.dg/cpp1y/static_assert2.C
g++.dg/cpp1y/var-templ44.C
g++.dg/cpp1z/fold6.C
g++.dg/cpp1z/inline-var2.C
g++.dg/cpp1z/lambda-this1.C
g++.dg/cpp1z/static_assert-nomsg.C
g++.dg/debug/dwarf2/dwarf4-typedef.C
g++.dg/debug/dwarf2/icf.C
g++.dg/debug/dwarf2/pr61433.C
g++.dg/debug/dwarf-eh-personality-1.C
g++.dg/debug/nullptr01.C
g++.dg/debug/pr16792.C
g++.dg/debug/pr46241.C
g++.dg/debug/pr46338.C
g++.dg/debug/pr47106.C
g++.dg/debug/pr71057.C
g++.dg/debug/pr71432.C
g++.dg/debug/pr80461.C
g++.dg/dfp/44473-1.C
g++.dg/dfp/44473-2.C
g++.dg/eh/builtin1.C
g++.dg/eh/builtin2.C
g++.dg/eh/builtin3.C
g++.dg/eh/pr45569.C
g++.dg/eh/unwind2.C
g++.dg/expr/bitfield11.C
g++.dg/expr/static_cast7.C
g++.dg/ext/altivec-14.C
g++.dg/ext/asm13.C
g++.dg/ext/builtin-object-size3.C
g++.dg/ext/has_nothrow_assign_odr.C
g++.dg/ext/label7.C
g++.dg/ext/label8.C
g++.dg/ext/tmplattr7.C
g++.dg/ext/vector8.C
g++.dg/ext/visibility/anon1.C
g++.dg/ext/visibility/anon2.C
g++.dg/ext/visibility/namespace1.C
g++.dg/ext/vla16.C
g++.dg/goacc/reference.C
g++.dg/gomp/pr37189.C
g++.dg/gomp/pr39495-1.C
g++.dg/gomp/pr39495-2.C
g++.dg/gomp/pr41429.C
g++.dg/gomp/pr82054.C
g++.dg/inherit/covariant10.C
g++.dg/inherit/covariant11.C
g++.dg/inherit/protected1.C
g++.dg/init/inline1.C
g++.dg/init/new18.C
g++.dg/init/reference2.C
g++.dg/init/reference3.C
g++.dg/init/switch1.C
g++.dg/ipa/devirt-10.C
g++.dg/ipa/devirt-13.C
g++.dg/ipa/devirt-14.C
g++.dg/ipa/devirt-15.C
g++.dg/ipa/devirt-16.C
g++.dg/ipa/devirt-17.C
g++.dg/ipa/devirt-18.C
g++.dg/ipa/devirt-19.C
g++.dg/ipa/devirt-21.C
g++.dg/ipa/devirt-23.C
g++.dg/ipa/devirt-38.C
g++.dg/ipa/devirt-40.C
g++.dg/ipa/devirt-41.C
g++.dg/ipa/devirt-42.C
g++.dg/ipa/devirt-44.C
g++.dg/ipa/devirt-45.C
g++.dg/ipa/devirt-48.C
g++.dg/ipa/devirt-52.C
g++.dg/ipa/nothrow-1.C
g++.dg/ipa/pr43812.C
g++.dg/ipa/pr44372.C
g++.dg/ipa/pr45572-1.C
g++.dg/ipa/pr58371.C
g++.dg/ipa/pr59176.C
g++.dg/ipa/pr60640-1.C
g++.dg/ipa/pr61540.C
g++.dg/ipa/pr63470.C
g++.dg/ipa/pr63587-1.C
g++.dg/ipa/pr63587-2.C
g++.dg/ipa/pr63838.C
g++.dg/ipa/pr63894.C
g++.dg/ipa/pr64068.C
g++.dg/ipa/pr64896.C
g++.dg/ipa/pr65002.C
g++.dg/ipa/pr65008.C
g++.dg/ipa/pr65465.C
g++.dg/ipa/pr66896.C
g++.dg/ipa/pr68851.C
g++.dg/ipa/pr78211.C
g++.dg/ipa/pr79931.C
g++.dg/ipa/pure-const-1.C

[og7] Allow the accelerator to have more offloaded functions than the host

[Cesar Philippidis]

Consider the following example:

Let lib1.c contain function f1.
Let lib2.c contain function f2.

Both f1 and f2 contain offloaded functions. Create a static library with
both lib1.o and lib2.o.

Next create a program which using that static library, but only calls f1.

If you build this program without program-wide -flto, this will cause
the nvptx linker to embed the offloaded functions for both f1 and f2 in
the host's executable, whereas the host will only have the offloaded
function for f1. This is a problem because the libgomp expects both the
host and accelerator to have the same number of offloaded functions.

As a temporary workaround, this patch teaches libgomp to allow the
accelerator to possess more offloaded functions than the host.

I've applied this patch to openacc-gcc-7-branch. Is it also suitable for
trunk?

Cesar
2017-10-11  Cesar Philippidis  

	libgomp/
	* target.c (gomp_load_image_to_device): Allow the accelerator to
	possess more offloaded functions than the host.


diff --git a/libgomp/target.c b/libgomp/target.c
index a55c8f074d9..336581d2196 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1452,7 +1452,7 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
 = devicep->load_image_func (devicep->target_id, version,
 target_data, _table);
 
-  if (num_target_entries != num_funcs + num_vars)
+  if (num_target_entries < num_funcs + num_vars)
 {
   gomp_mutex_unlock (>lock);
   if (is_register_lock)

Re: [PATCH] DECL_ASSEMBLER_NAME and friends

[Jan Hubicka]

> On Tue, Oct 10, 2017 at 3:03 PM, Nathan Sidwell  wrote:
> > I have a patch cleaning up a bit of the C++ FE, which will involve hashing
> > via DECL_ASSEMBLER_NAME.  however, all the items in that hash are known to
> > have it set, so its mapping to a function call is undesirable.
> >
> > This patch adds DECL_ASSEMBLER_NAME_RAW, which gets at the field directly.
> > I can then use that for my hash table.  The cleanup to use that is fairly
> > trivial.
> >
> > However, I also looked more deeply at DECL_ASSEMBLER_NAME_SET_P. It does
> > more than the name suggests -- namely checking HAS_DECL_ASSEMBLER_NAME_P
> > too.  There are about 72 uses of DECL_ASSEMBLER_NAME_SET_P and investigation
> > showed only about 4 applying it to decls that are not
> > HAS_DECL_ASSEMBLER_NAME_P.  So, I remove the HAS_DECL_ASSEMBLER_NAME_P from
> > DECL_ASSEMBLER_NAME_SET_P and explicitly check it at the 4 locations that
> > need it.
> >
> > In doing this I noticed a couple of items:
> >
> > 1) ipa-utils.h (type_in_anonymous_namespace_p) is applying
> > HAS_DECL_ASSEMBLER_NAME_P to a type.  That's clearly wrong, It looks like a
> > thinko for TYPE_NAME (t).  Making that change seems fine.

Yep, it is a thinko. Thanks for spotting it!

Honza

Re: [PATCH][GRAPHITE] Fix PR82449

[Sebastian Pop]

On Oct 9, 2017 8:48 AM, "Richard Biener"  wrote:

On Mon, 9 Oct 2017, Richard Biener wrote:

> On Fri, 6 Oct 2017, Sebastian Pop wrote:
>
> > On Fri, Oct 6, 2017 at 8:33 AM, Richard Biener 
wrote:
> >
> > > On Fri, 6 Oct 2017, Sebastian Pop wrote:
> > >
> > > > On Fri, Oct 6, 2017 at 6:56 AM, Richard Biener 
> > > wrote:
> > > >
> > > > >
> > > > > The following fences off a few more SCEVs through
scev_analyzable_p
> > > given
> > > > > at the end we need those pass chrec_apply when getting a rename
through
> > > > > SCEV.
> > > > >
> > > > > The SCEV in question is
> > > > >
> > > > >   {(integer(kind=4)) {0, +, {1, +, 1}_1}_1, + 1}_2
> > > > >
> > > > > which fails to chrec_apply in the CHREC_LEFT part because that
part
> > > > > is not affine (and we're usually not replacing a IV with a
constant
> > > > > where chrec_apply might handle one or the other case).
> > > > >
> > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > > > >
> > > > > This fixes three out of the remaining 8 codegen errors in SPEC CPU
> > > 2006.
> > > > >
> > > > > Ok?
> > > > >
> > > > > Thanks,
> > > > > Richard.
> > > > >
> > > > > 2017-10-06  Richard Biener  
> > > > >
> > > > > PR tree-optimization/82449
> > > > > * sese.c (can_chrec_apply): New function.
> > > > > (scev_analyzable_p): Check we can call chrec_apply on the
SCEV.
> > > > >
> > > > > * gfortran.dg/graphite/pr82449.f: New testcase.
> > >
> > > >
> > > > > Index: gcc/sese.c
> > > > > 
===
> > > > > --- gcc/sese.c  (revision 253477)
> > > > > +++ gcc/sese.c  (working copy)
> > > > > @@ -421,6 +421,27 @@ invariant_in_sese_p_rec (tree t, const s
> > > > >return true;
> > > > >  }
> > > > >
> > > > > +/* Check whether we can call chrec_apply on CHREC with arbitrary
X and
> > > > > VAR.  */
> > >
> > > > +
> > > > > +static bool
> > > > > +can_chrec_apply (tree chrec)
> > >
> >
> > Could we use scev_is_linear_expression ?
> > It seems like can_chrec_apply has the same semantics.
>
> Looks like that works.
>
> >
> > > > > +{
> > > > > +  if (automatically_generated_chrec_p (chrec))
> > > > > +return false;
> > > > > +  switch (TREE_CODE (chrec))
> > > > > +{
> > > > > +case POLYNOMIAL_CHREC:
> > > > > +  if (evolution_function_is_affine_p (chrec))
> > > > > +   return (can_chrec_apply (CHREC_LEFT (chrec))
> > > > > +   && can_chrec_apply (CHREC_RIGHT (chrec)));
> > > > > +  return false;
> > > > > +CASE_CONVERT:
> > > > > +  return can_chrec_apply (TREE_OPERAND (chrec, 0));
> > > > > +default:;
> > > > > +  return tree_does_not_contain_chrecs (chrec);
> > > > > +}
> > > > > +}
> > > > > +
> > > > >  /* Return true when DEF can be analyzed in REGION by the scalar
> > > > > evolution analyzer.  */
> > > > >
> > > > > @@ -449,6 +470,7 @@ scev_analyzable_p (tree def, sese_l 
> > > > > || !defined_in_sese_p (scev, region))
> > > > >  && (tree_does_not_contain_chrecs (scev)
> > > > > || evolution_function_is_affine_p (scev))
> > > > >
> > > >
> > > > Why isn't evolution_function_is_affine_p returning false on {0, +,
{1, +,
> > > > 1}_1}_1?
> > > > This is quadratic.
> > >
> > > It returns false on that but the CHREC we ask it on is
> > >
> > > {(integer(kind=4)) {0, +, {1, +, 1}_1}_1, + 1}_2
> > >
> > > only the initial value is "quadratic".
> > >
> >
> > Right.
> > If I understand correctly, the scop is the body of loop_1,
> > and we do not need to represent the quadratic evolution
> > of the initial value.
>
> Giving the following full testing now.

And the following is what I have applied after bootstrap / testing
on x86_64-unknown-linux-gnu.

As you can see I needed some adjustments to not reject otherwise
valid SCEVs with address constants.

Richard.

2017-10-09  Richard Biener  

PR tree-optimization/82449
* sese.c (scev_analyzable_p): Check whether the SCEV is linear.
* tree-chrec.h (evolution_function_is_constant_p): Adjust to
allow constant addresses.
* tree-chrec.c (scev_is_linear_expression): Constant evolutions
are linear.

* gfortran.dg/graphite/pr82449.f: New testcase.


Looks good to me.

Thanks.


Index: gcc/sese.c
===
--- gcc/sese.c  (revision 253486)
+++ gcc/sese.c  (working copy)
@@ -444,14 +444,13 @@ scev_analyzable_p (tree def, sese_l 
   loop = loop_containing_stmt (SSA_NAME_DEF_STMT (def));
   scev = scalar_evolution_in_region (region, loop, def);

-  return !chrec_contains_undetermined (scev)
-&& (TREE_CODE (scev) != SSA_NAME
-   || !defined_in_sese_p (scev, region))
-&& (tree_does_not_contain_chrecs (scev)
-   || evolution_function_is_affine_p (scev))
-&& (! loop
-   || ! loop_in_sese_p (loop, region)
-   || ! 

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

[Martin Liška]

On 10/11/2017 09:37 AM, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 07:55:44AM +0200, Martin Liška wrote:
>>> Conceptually, these two instrumentations rely on address sanitization,
>>> not really sure if we should supporting them for kernel sanitization (but I
>>> bet it is just going to be too costly for kernel).
>>> So, we also need to make sure at least parts of SANITIZE_ADDRESS is enabled
>>> when these options are on.
>>> That can be done by erroring out if -fsanitize=pointer-compare is requested
>>> without -fsanitize=address, or by implicitly enabling -fsanitize=address for
>>> these, or by adding yet another SANITIZE_* bit which would cover
>>> sanitization of memory accesses for asan, that bit would be set by
>>> -fsanitize={address,kernel-address} in addition to the current 2 bits, but
>>> pointer-{subtract,compare} would set its own bit and SANITIZE_ADDRESS and
>>> SANITIZE_USER_ADDRESS only.  Without the new bit we'd emit red zones,
>>> function prologue/epilogue asan changes, registraction of global variables,
>>> but not actual instrumentation of memory accesses (and probably not
>>> instrumentation of C++ ctor ordering).
>>
>> Agree, would be much easier to just enable SANITIZE_ADDRESS with there 2 
>> options.
>> Question is how to make it also possible with -fsanitize=kernel-address:
>>
>> $ ./xgcc -B.   
>> /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c 
>> -fsanitize=pointer-compare,kernel-address
>> cc1: error: ‘-fsanitize=address’ is incompatible with 
>> ‘-fsanitize=kernel-address’
>>
>> Ideas?
> 

Hello.

Thanks for feedback.

> If we want to make it usable for both user and kernel address, then either
> we'll let pointer-compare/pointer-subtract implicitly enable user address,
> unless kernel-address has been enabled (that would mean set SANITIZE_ADDRESS
> bit in pointer-*, but not SANITIZE_USER_ADDRESS, and at the point where we
> diagnose option incompatibilities like -fsanitize=address,kernel-address
> check for the case (SANITIZE_ADDRESS bit set, none of SANITIZE_USER_ADDRESS
> nor SANITIZE_KERNEL_ADDRESS, and one of SANITIZE_POINTER_*) and set
> implicitly SANITIZE_USER_ADDRESS, or simply require that the user chooses,
> by erroring out if pointer-* is used without explicit address or
> kernel-address.  In any case, I think this should be also something
> discussed with the upstream sanitizer folks, so that LLVM (if it ever
> decides to actually implement it) behaves compatibly.

I've added support for automatic adding of -sanitize=address if none of them is 
added.
Problem is that LIBASAN_SPEC is still handled in driver. Thus I guess I'll need 
the hunks
I sent in first version of patch. Or do I miss something?


> 
>> --- a/libsanitizer/asan/asan_descriptions.cc
>> +++ b/libsanitizer/asan/asan_descriptions.cc
>> @@ -220,6 +220,15 @@ bool GetStackAddressInformation(uptr addr, uptr 
>> access_size,
>>return true;
>>  }
>>  
>> +bool GetStackVariableBeginning(uptr addr, uptr *shadow_addr)
>> +{
>> +  AsanThread *t = FindThreadByStackAddress(addr);
>> +  if (!t) return false;
>> +
>> +  *shadow_addr = t->GetStackFrameVariableBeginning (addr);
>> +  return true;
>> +}
>> +
>>  static void PrintAccessAndVarIntersection(const StackVarDescr , uptr 
>> addr,
>>uptr access_size, uptr 
>> prev_var_end,
>>uptr next_var_beg) {
>> diff --git a/libsanitizer/asan/asan_descriptions.h 
>> b/libsanitizer/asan/asan_descriptions.h
>> index 584b9ba6491..b7f23b1a71b 100644
>> --- a/libsanitizer/asan/asan_descriptions.h
>> +++ b/libsanitizer/asan/asan_descriptions.h
>> @@ -138,6 +138,7 @@ struct StackAddressDescription {
>>  
>>  bool GetStackAddressInformation(uptr addr, uptr access_size,
>>  StackAddressDescription *descr);
>> +bool GetStackVariableBeginning(uptr addr, uptr *shadow_addr);
>>  
>>  struct GlobalAddressDescription {
>>uptr addr;
>> diff --git a/libsanitizer/asan/asan_globals.cc 
>> b/libsanitizer/asan/asan_globals.cc
>> index f2292926e6a..ed707c0ca01 100644
>> --- a/libsanitizer/asan/asan_globals.cc
>> +++ b/libsanitizer/asan/asan_globals.cc
>> @@ -122,6 +122,31 @@ int GetGlobalsForAddress(uptr addr, Global *globals, 
>> u32 *reg_sites,
>>return res;
>>  }
>>  
>> +bool AreGlobalVariablesSame(uptr addr1, uptr addr2)
>> +{
>> +  if (addr1 > addr2)
>> +  {
>> +uptr tmp = addr1;
>> +addr1 = addr2;
>> +addr2 = tmp;
> 
> std::swap(addr1, addr2); ?  I don't see it used in any of libsanitizer
> though, so not sure if the corresponding STL header is included.

They don't use it anywhere and I had some #include issues. That's why I did it 
manually.

> 
>> +  }
>> +
>> +  BlockingMutexLock lock(_for_globals);
> 
> Why do you need a mutex for checking if there are no red zones in between?
> 
>> +  uptr aligned_addr1 = addr1 & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
>> +  uptr aligned_addr2 = addr2 & 

Re: [PATCH][mingw] Enable colorized diagnostics

[JonY]

On 10/09/2017 01:07 PM, Liu Hao wrote:
> On 2017/10/9 19:01, JonY wrote:
>> On 10/08/2017 11:39 AM, Liu Hao wrote:
>>
>> I'm not sure if it should be enabled by default due to the interleaving
>> problem, but seeing as the user has to go out to set GCC_COLORS to use
>> this feature, I suppose it is OK.
>>
>> I will commit soon if there are no more comments.
>>
>>
> 
> Thank you. By the way I noticed a mistake in the comments above
> `find_esc_terminator()`. This function returns zero on failure like its
> `find_esc_head()` counterpart, while the comments mistakenly referred
> -1. Please correct it before committing.
> 
> 

Committed to trunk r253645 with the appropriate change.



signature.asc
Description: OpenPGP digital signature

[PATCH] Fix infer_loop_bounds_from_pointer_arith

[Richard Biener]

The following fixes a common mistake in calling
analyze_scalar_evolution -- it's supposed to be called with the loop
the use is directly in.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2017-10-11  Richard Biener  

* tree-ssa-loop-niter.c (infer_loop_bounds_from_pointer_arith):
Properly call analyze_scalar_evolution with the loop of the stmt.

Index: gcc/tree-ssa-loop-niter.c
===
--- gcc/tree-ssa-loop-niter.c   (revision 253642)
+++ gcc/tree-ssa-loop-niter.c   (working copy)
@@ -3444,7 +3444,8 @@ infer_loop_bounds_from_pointer_arith (st
   if (TYPE_PRECISION (type) != TYPE_PRECISION (TREE_TYPE (var)))
 return;
 
-  scev = instantiate_parameters (loop, analyze_scalar_evolution (loop, def));
+  struct loop *uloop = loop_containing_stmt (stmt);
+  scev = instantiate_parameters (loop, analyze_scalar_evolution (uloop, def));
   if (chrec_contains_undetermined (scev))
 return;
 

Re: [PATCH PR82472]Update postorder number for merged partition.

[Richard Biener]

On Wed, Oct 11, 2017 at 12:06 PM, Bin Cheng  wrote:
> Hi,
> This patch fixes the reported ICE.  Root cause is postorder number is not 
> updated
> after merging partitions in SCC.  As a result, reduction partition may not be 
> scheduled
> as the last one because partitions are sorted in descending postorder.
> Bootstrap and test on x86_64 and AArch64.  Is it OK?

Ok.

Richard.

> Thanks,
> bin
> 2017-10-10  Bin Cheng  
>
> PR tree-optimization/82472
> * tree-loop-distribution.c (sort_partitions_by_post_order): Refine
> comment.
> (break_alias_scc_partitions): Update postorder number.
>
> gcc/testsuite
> 2017-10-10  Bin Cheng  
>
> PR tree-optimization/82472
> * gcc.dg/tree-ssa/pr82472.c: New test.

Re: [PATCH] Simplify SCEV entry

[Andreas Schwab]

On Okt 10 2017, Richard Biener <rguent...@suse.de> wrote:

>   * tree-cfgcleanup.c (cleanup_tree_cfg_noloop): Avoid compacting
>   blocks if SCEV is active.
>   * tree-scalar-evolution.c (analyze_scalar_evolution_1): Remove
>   dead code.
>   (analyze_scalar_evolution): Handle cached evolutions the obvious way.
>   (scev_initialize): Assert we are not yet initialized.

That breaks gfortran.dg/graphite/pr68279.f90:

during GIMPLE pass: graphite
/daten/aranym/gcc/gcc-20171011/gcc/testsuite/gfortran.dg/graphite/pr68279.f90:8:0:
 internal compiler error: in set_codegen_error, at 
graphite-isl-ast-to-gimple.c:216
0x57056a translate_isl_ast_to_gimple::set_codegen_error()
../../gcc/graphite-isl-ast-to-gimple.c:215
0x10eb522 translate_isl_ast_to_gimple::set_codegen_error()
../../gcc/graphite-isl-ast-to-gimple.c:215
0x10eb522 translate_isl_ast_to_gimple::get_rename_from_scev(tree_node*, 
gimple**, loop*, basic_block_def*, basic_block_def*, vec<tree_node*, va_heap, 
vl_ptr>)
../../gcc/graphite-isl-ast-to-gimple.c:1097
0x10ec77f 
translate_isl_ast_to_gimple::graphite_copy_stmts_from_block(basic_block_def*, 
basic_block_def*, vec<tree_node*, va_heap, vl_ptr>)
../../gcc/graphite-isl-ast-to-gimple.c:1247
0x10ed9c9 
translate_isl_ast_to_gimple::copy_bb_and_scalar_dependences(basic_block_def*, 
edge_def*, vec<tree_node*, va_heap, vl_ptr>)
../../gcc/graphite-isl-ast-to-gimple.c:1313
0x10ee385 
translate_isl_ast_to_gimple::translate_isl_ast_node_user(isl_ast_node*, 
edge_def*, std::map<isl_id*, tree_node*, std::less<isl_id*>, 
std::allocator<std::pair<isl_id* const, tree_node*> > >&)
../../gcc/graphite-isl-ast-to-gimple.c:803
0x10ee74e translate_isl_ast_to_gimple::translate_isl_ast_for_loop(loop*, 
isl_ast_node*, edge_def*, tree_node*, tree_node*, tree_node*, std::map<isl_id*, 
tree_node*, std::less<isl_id*>, std::allocator<std::pair<isl_id* const, 
tree_node*> > >&)
../../gcc/graphite-isl-ast-to-gimple.c:617
0x10ee9b3 translate_isl_ast_to_gimple::translate_isl_ast_node_for(loop*, 
isl_ast_node*, edge_def*, std::map<isl_id*, tree_node*, std::less<isl_id*>, 
std::allocator<std::pair<isl_id* const, tree_node*> > >&)
../../gcc/graphite-isl-ast-to-gimple.c:721
0x10eeaa5 translate_isl_ast_to_gimple::translate_isl_ast_node_block(loop*, 
isl_ast_node*, edge_def*, std::map<isl_id*, tree_node*, std::less<isl_id*>, 
std::allocator<std::pair<isl_id* const, tree_node*> > >&)
../../gcc/graphite-isl-ast-to-gimple.c:832
0x10eef9d graphite_regenerate_ast_isl(scop*)
../../gcc/graphite-isl-ast-to-gimple.c:1559
0x10e9817 graphite_transform_loops()
../../gcc/graphite.c:442
0x10ea7f0 graphite_transforms
../../gcc/graphite.c:486
0x10ea7f0 execute
../../gcc/graphite.c:563

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: X+Y < X iff Y<0 moved to match.pd

[Richard Biener]

On Tue, Oct 10, 2017 at 4:22 PM, Marc Glisse  wrote:
> On Mon, 9 Oct 2017, Richard Biener wrote:
>
>> On Sun, Oct 8, 2017 at 1:22 PM, Marc Glisse  wrote:
>>>
>>> Hello,
>>>
>>> this moves (and extends a bit) one more transformation from fold-const.c
>>> to
>>> match.pd. The single_use restriction is necessary for consistency with
>>> the
>>> existing X+CST1 CMP CST2 transformation (if we do only one of the 2
>>> transformations, gcc.dg/tree-ssa/vrp54.c regresses because DOM fails to
>>> simplify 2 conditions based on different variables). The relaxation of
>>> the
>>> condition to simplify (T) X == (T) Y is an easier way to avoid regressing
>>> gcc.dg/tree-ssa/foldaddr-1.c than adding plenty of convert? in the
>>> patterns,
>>> although that may still prove necessary at some point. I left a few odd
>>> float cases in fold-const.c for later.
>>
>>
>> +/* X + Y < Y is the same as X < 0 when there is no overflow.  */
>> +(for op  (lt le ge gt)
>> + rop (gt ge le lt)
>> + (simplify
>> +  (op (plus:c@2 @0 @1) @1)
>> +  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> +   && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
>> +   && (CONSTANT_CLASS_P (@0) || single_use (@2)))
>> +   (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
>> + (simplify
>> +  (op @1 (plus:c@2 @0 @1))
>> +  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> +   && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
>> +   && (CONSTANT_CLASS_P (@0) || single_use (@2)))
>> +   (rop @0 { build_zero_cst (TREE_TYPE (@0)); }
>>
>> any reason (op:c (plus... on the first of the two patterns wouldn't have
>> catched
>> the second?  :c on comparison swaps the comparison code.
>
>
> I had completely forgotten that you had added this cool feature.
>
>> +/* For equality, this is also true with wrapping overflow.  */
>> +(for op (eq ne)
>> + (simplify
>> +  (op:c (plus:c@2 @0 @1) @1)
>> +  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> +   && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
>> +  || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
>> +   && (CONSTANT_CLASS_P (@0) || single_use (@2)))
>> +   (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
>> + (simplify
>> +  (op:c (convert?@3 (pointer_plus@2 @0 @1)) (convert? @0))
>> +  (if (CONSTANT_CLASS_P (@1) || (single_use (@2) && single_use (@3)))
>> +   (op @1 { build_zero_cst (TREE_TYPE (@1)); }
>>
>> for consistency can you add the convert?s to the integer variant as well?
>
>
> The only relevant case I could think of is, for unsigned u,
> (unsigned)((int)u+1)==u. All the other cases seem to be handled some other
> way. I still wrote it to allow conversions all over the place, but that's
> uglier than the MINUS_EXPR transformation below where I didn't.
>
>> I'm not sure you'll see the convers like you write them (with matching
>> @0).
>
>
> int f(char*p){
>   char*q=p+5;
>   return (long)q==(long)p;
> }
>
>> Eventually on GENERIC when we still have pointer conversions around?
>
>
> For generic we might want @@0 or pointers sometimes fail to match. At least
> in the POINTER_DIFF_EXPR experiment I had to do that in some transformations
> for constexpr.
>
>> You are allowing arbitrary conversions here - is that desired?  Isn't
>> a tree_nop_conversion_p missing?
>
>
> I had the impression that casts from a pointer to whatever were always NOP,
> for instance when I try to cast char* to short, gcc produces (short)(long)p.
> But reading the comment in verify_gimple_assign_unary it is more
> complicated, so yes I should add a constraint.
>
> /* Allow conversions from pointer type to integral type only if
>there is no sign or zero extension involved.
>For targets were the precision of ptrofftype doesn't match that
>of pointers we need to allow arbitrary conversions to ptrofftype.
> */
> if ((POINTER_TYPE_P (lhs_type)
>  && INTEGRAL_TYPE_P (rhs1_type))
> || (POINTER_TYPE_P (rhs1_type)
> && INTEGRAL_TYPE_P (lhs_type)
> && (TYPE_PRECISION (rhs1_type) >= TYPE_PRECISION (lhs_type)
> || ptrofftype_p (sizetype
>   return false;
>
> The code "|| ptrofftype_p (sizetype)" doesn't seem to match the comment :-(

Ugh.  Should have been || ptrofftype_p (lhs_type) of course...

The whole thing is fragile in the context of address-spaces as well (who can
theoretically have a different pointer size and thus different
[u]intptr_t).  But
that's not news and similar to the issues we have with the POINTER_PLUS_EXPR
requirement ...

>
> Here is a new version of the patch, same changelog/testing.

Ok.

Thanks,
Richard.

> --
> Marc Glisse

Re: [PATCH] DECL_ASSEMBLER_NAME and friends

[Richard Biener]

On Tue, Oct 10, 2017 at 3:03 PM, Nathan Sidwell  wrote:
> I have a patch cleaning up a bit of the C++ FE, which will involve hashing
> via DECL_ASSEMBLER_NAME.  however, all the items in that hash are known to
> have it set, so its mapping to a function call is undesirable.
>
> This patch adds DECL_ASSEMBLER_NAME_RAW, which gets at the field directly.
> I can then use that for my hash table.  The cleanup to use that is fairly
> trivial.
>
> However, I also looked more deeply at DECL_ASSEMBLER_NAME_SET_P. It does
> more than the name suggests -- namely checking HAS_DECL_ASSEMBLER_NAME_P
> too.  There are about 72 uses of DECL_ASSEMBLER_NAME_SET_P and investigation
> showed only about 4 applying it to decls that are not
> HAS_DECL_ASSEMBLER_NAME_P.  So, I remove the HAS_DECL_ASSEMBLER_NAME_P from
> DECL_ASSEMBLER_NAME_SET_P and explicitly check it at the 4 locations that
> need it.
>
> In doing this I noticed a couple of items:
>
> 1) ipa-utils.h (type_in_anonymous_namespace_p) is applying
> HAS_DECL_ASSEMBLER_NAME_P to a type.  That's clearly wrong, It looks like a
> thinko for TYPE_NAME (t).  Making that change seems fine.
>
> 2) tree-ssa-structalias.c (alias_get_name) seemed overly complicated, so I
> reimplemented it.
>
> I checked the target-specific code, and there are only two uses of
> DECL_ASSEMBLER_NAME_SET_P.  Both look safe to me (the avr one seems
> superfluous, and DECL_ASSEMBLER_NAME would be fine there.
>
> booted on x86_64-linux
>
> ok?

Ok.

Thanks,
Richard.

> nathan
> --
> Nathan Sidwell

Re: [PATCH GCC][5/7]Extend loop distribution for two-level innermost loop nest

[Richard Biener]

On Wed, Oct 11, 2017 at 2:05 PM, Bin.Cheng  wrote:
> On Mon, Oct 9, 2017 at 2:48 PM, Richard Biener
>  wrote:
>> On Thu, Oct 5, 2017 at 3:17 PM, Bin Cheng  wrote:
>>> Hi,
>>> For now distribution pass only handles the innermost loop.  This patch 
>>> extends the pass
>>> to cover two-level innermost loop nest.  It also refactors code in 
>>> pass_loop_distribution::execute
>>> for better reading.  Note I restrict it to 2-level loop nest on purpose 
>>> because of high
>>> cost in data dependence computation.  Some compilation time optimizations 
>>> like reusing
>>> the data reference finding, data dependence computing, would require a 
>>> rewrite of this
>>> pass like the proposed loop interchange implementation.  But that's another 
>>> task.
>>>
>>> This patch introduces a temporary TODO for loop nest builtin partition 
>>> which is covered
>>> by next two patches.
>>>
>>> With this patch, kernel loop in bwaves now can be distributed, thus exposed 
>>> for further
>>> interchange.  This patch adds new test for matrix multiplication, as well 
>>> as adjusts
>>> test strings of existing tests.
>>> Bootstrap and test in patch set on x86_64 and AArch64, is it OK?
>>
>> @ -714,9 +719,11 @@ ssa_name_has_uses_outside_loop_p (tree def, loop_p loop)
>>
>>FOR_EACH_IMM_USE_FAST (use_p, imm_iter, def)
>>  {
>> -  gimple *use_stmt = USE_STMT (use_p);
>> -  if (!is_gimple_debug (use_stmt)
>> - && loop != loop_containing_stmt (use_stmt))
>> +  if (is_gimple_debug (USE_STMT (use_p)))
>> +   continue;
>> +
>> +  basic_block use_bb = gimple_bb (USE_STMT (use_p));
>> +  if (use_bb == NULL || !flow_bb_inside_loop_p (loop, use_bb))
>> return true;
>>
>> use_bb should never be NULL.
> Done.
>>
>> +  /* Don't support loop nest distribution under runtime alias check
>> +since it's not likely to enable many vectorization opportunities.  
>> */
>> +  if (loop->inner)
>> +   {
>> + merge_dep_scc_partitions (rdg, , false);
>> +   }
>>
>> extra {}
> Done.
>>
>> +  /* Support loop nest distribution enclosing current innermost loop.
>> +For the moment, we only support the innermost two-level loop nest.  
>> */
>> +  if (flag_tree_loop_distribution
>> + && outer->num > 0 && outer->inner == loop && loop->next == NULL
>>
>> The canonical check for is-this-non-root is loop_outer (outer) instead
>> of outer->num > 0.
> Done.
>>
>> + && single_exit (outer)
>>
>> not sure how exits are counted but if the inner loop exits also the
>> outer loop do
>> we correctly handle/reject this case?
> I tend to believe this can be handled if it's not rejected by
> niters/exit condition,
> but I am not very sure about this.
>>
>> -  if (nb_generated_loops + nb_generated_calls > 0)
>> -   {
>> - changed = true;
>> - dump_printf_loc (MSG_OPTIMIZED_LOCATIONS,
>> -  loc, "Loop %d distributed: split to %d loops "
>> -  "and %d library calls.\n",
>> -  num, nb_generated_loops, nb_generated_calls);
>> + if (nb_generated_loops + nb_generated_calls > 0)
>> +   {
>> + changed = true;
>> + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS,
>> +  loc, "Loop%s %d distributed: split to %d 
>> loops "
>> +  "and %d library calls.\n",
>> +  loop_nest_p ? " nest" : "", loop->num,
>> +  nb_generated_loops, nb_generated_call
>> ...
>>
>> can you adjust the printfs to say "loop nest distributed" in case we 
>> distributed
>> a nest?
> Done.
>>
>> Can you rewrite the iteration over the nest so it would theoretically support
>> arbitrary deep perfect nests?  Thus simply initialize loop_nest_p less
>> cleverly...
> Done.  I factored it out as a function "prepare_perfect_loop_nest".  I
> also tested
> the updated patch by enabling full loop nest distribution, there is no failure
> in bootstrap, regression test, spec benchmarks.  Of course, the final patch
> still only supports 2-level innermost loop nest.
>
> Is this OK?

Ok.

Thanks,
Richard.

> Thanks,
> bin
> 2017-10-04  Bin Cheng  
>
> * tree-loop-distribution.c: Adjust the general comment.
> (NUM_PARTITION_THRESHOLD): New macro.
> (ssa_name_has_uses_outside_loop_p): Support loop nest distribution.
> (classify_partition): Skip builtin pattern of loop nest's inner loop.
> (merge_dep_scc_partitions): New parameter ignore_alias_p and use it
> in call to build_partition_graph.
> (finalize_partitions): New parameter.  Make loop distribution more
> conservative by fusing more partitions.
> (distribute_loop): Don't do runtime alias check in case of loop nest
> distribution.
> (find_seed_stmts_for_distribution): New function.
> 

Re: [PATCH] Fix use-after-scope error.

[Martin Liška]

On 10/11/2017 09:15 AM, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 08:17:25AM +0200, Martin Liška wrote:
>> One can see use-after-scope error in boostrap-asan:
>>
>> gcc/ChangeLog:
>>
>> 2017-10-10  Martin Liska  
>>
>>  * print-rtl.c (print_insn): Move declaration of idbuf
>>  to same scope as name.
>> ---
>>  gcc/print-rtl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
> 
>> diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
>> index 79ec463df45..28d99862cad 100644
>> --- a/gcc/print-rtl.c
>> +++ b/gcc/print-rtl.c
>> @@ -1792,11 +1792,11 @@ print_insn (pretty_printer *pp, const rtx_insn *x, 
>> int verbose)
>>  case DEBUG_INSN:
>>{
>>  const char *name = "?";
>> +char idbuf[32];
>>  
>>  if (DECL_P (INSN_VAR_LOCATION_DECL (x)))
>>{
>>  tree id = DECL_NAME (INSN_VAR_LOCATION_DECL (x));
>> -char idbuf[32];
>>  if (id)
>>name = IDENTIFIER_POINTER (id);
>>  else if (TREE_CODE (INSN_VAR_LOCATION_DECL (x))
> 
> Ok.  This should IMHO go into release branches too.

Yes, will do that.

Martin

> 
>   Jakub
> 

Re: [PATCH] Fix a test-case for Darwin.

[Martin Liška]

On 10/11/2017 09:13 AM, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 08:15:16AM +0200, Martin Liška wrote:
>> Hello.
>>
>> This should address failing test-case on Darwin.
> 
> Guess this should fix it not just on Darwin, but also on any target
> where the assembler doesn't have a call instruction but something different
> (e.g. bl and many others).

Yes, I can see it on ppc64le.

> 
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
>> Martin
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-10-10  Martin Liska  
>>
>>  * c-c++-common/ubsan/ptr-overflow-sanitization-1.c: Scan
>>  optimized dump rather than assembly.
> 
> Ok, thanks.  With a nit:
> 
>> diff --git a/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c 
>> b/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
>> index 42c14523764..b966d52a61d 100644
>> --- a/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
>> +++ b/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
>> @@ -1,5 +1,5 @@
>>  /* { dg-require-effective-target lp64 } */
> 
> Do you really need the above line?  I thought now that the test
> is using __PTRDIFF_MAX__ you shouldn't really need that...

Done that and installed.

Thanks,
Martin

> 
>> -/* { dg-options "-O -fsanitize=pointer-overflow" } */
>> +/* { dg-options "-O -fsanitize=pointer-overflow -fdump-tree-optimized" } */
>>  /* { dg-skip-if "" { *-*-* } "-flto" } */
>>  
>>  #define SMAX   __PTRDIFF_MAX__
>> @@ -76,5 +76,4 @@ void negative_to_negative (char *ptr)
>>p2 += 5;
>>  }
>>  
>> -
>> -/* { dg-final { scan-assembler-times 
>> "call\\s+__ubsan_handle_pointer_overflow" 17 } } */
>> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 17 
>> "optimized" } } */
>>
> 
> 
>   Jakub
> 

Re: [PATCH] preprocessor stringizing raw strings

[Andreas Schwab]

On Okt 11 2017, Nathan Sidwell  wrote:

> Index: g++.dg/cpp/string-3.C
> ===
> --- g++.dg/cpp/string-3.C (revision 253622)
> +++ g++.dg/cpp/string-3.C (working copy)
> @@ -6,4 +6,4 @@
>  BEGIN STRINGIZE(R"(
>  )") END
>  
> -// { dg-final { scan-file string-3.ii "BEGIN \"R\\\"(\\n)\\\"\"\n END" } }
> +// { dg-final { scan-file string-3.i "BEGIN \"R\"\\(n\\)\"\"\n   
>  END" } }

You can avoid the inflation by using braces.

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."

[PATCH] Shrink POLYNOMIAL_CHREC

[Richard Biener]

This moves CHREC_VARIABLE to the tree base union as it just records
a loop number there's no need to allocate a tree node and tree operand
for it.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2017-10-11  Richard Biener  

* tree.def (POLYNOMIAL_CHREC): Remove CHREC_VARIABLE tree operand.
* tree-core.h (tree_base): Add chrec_var union member.
* tree.h (CHREC_VAR): Remove.
(CHREC_LEFT, CHREC_RIGHT, CHREC_VARIABLE): Adjust.
* tree-chrec.h (build_polynomial_chrec): Adjust.
* tree-chrec.c (reset_evolution_in_loop): Use build_polynomial_chrec.
* tree-pretty-print.c (dump_generic_node): Use CHREC_VARIABLE.

Index: gcc/tree-chrec.c
===
--- gcc/tree-chrec.c(revision 253632)
+++ gcc/tree-chrec.c(working copy)
@@ -872,8 +872,7 @@ reset_evolution_in_loop (unsigned loop_n
   new_evol);
   tree right = reset_evolution_in_loop (loop_num, CHREC_RIGHT (chrec),
new_evol);
-  return build3 (POLYNOMIAL_CHREC, TREE_TYPE (left),
-CHREC_VAR (chrec), left, right);
+  return build_polynomial_chrec (CHREC_VARIABLE (chrec), left, right);
 }
 
   while (TREE_CODE (chrec) == POLYNOMIAL_CHREC
Index: gcc/tree-chrec.h
===
--- gcc/tree-chrec.h(revision 253632)
+++ gcc/tree-chrec.h(working copy)
@@ -157,8 +157,9 @@ build_polynomial_chrec (unsigned loop_nu
   if (chrec_zerop (right))
 return left;
 
-  return build3 (POLYNOMIAL_CHREC, TREE_TYPE (left),
-build_int_cst (NULL_TREE, loop_num), left, right);
+  tree chrec = build2 (POLYNOMIAL_CHREC, TREE_TYPE (left), left, right);
+  CHREC_VARIABLE (chrec) = loop_num;
+  return chrec;
 }
 
 /* Determines whether the expression CHREC is a constant.  */
Index: gcc/tree-core.h
===
--- gcc/tree-core.h (revision 253632)
+++ gcc/tree-core.h (working copy)
@@ -981,6 +981,9 @@ struct GTY(()) tree_base {
 /* SSA version number.  This field is only used with SSA_NAME.  */
 unsigned int version;
 
+/* CHREC_VARIABLE.  This field is only used with POLYNOMIAL_CHREC.  */
+unsigned int chrec_var;
+
 /* Internal function code.  */
 enum internal_fn ifn;
 
Index: gcc/tree-pretty-print.c
===
--- gcc/tree-pretty-print.c (revision 253632)
+++ gcc/tree-pretty-print.c (working copy)
@@ -2819,8 +2819,7 @@ dump_generic_node (pretty_printer *pp, t
   dump_generic_node (pp, CHREC_LEFT (node), spc, flags, false);
   pp_string (pp, ", +, ");
   dump_generic_node (pp, CHREC_RIGHT (node), spc, flags, false);
-  pp_string (pp, "}_");
-  dump_generic_node (pp, CHREC_VAR (node), spc, flags, false);
+  pp_printf (pp, "}_%u", CHREC_VARIABLE (node));
   is_stmt = false;
   break;
 
Index: gcc/tree.def
===
--- gcc/tree.def(revision 253632)
+++ gcc/tree.def(working copy)
@@ -982,8 +982,8 @@ DEFTREECODE (SCEV_KNOWN, "scev_known", t
 DEFTREECODE (SCEV_NOT_KNOWN, "scev_not_known", tcc_expression, 0)
 
 /* Polynomial chains of recurrences.
-   Under the form: cr = {CHREC_LEFT (cr), +, CHREC_RIGHT (cr)}.  */
-DEFTREECODE (POLYNOMIAL_CHREC, "polynomial_chrec", tcc_expression, 3)
+   cr = {CHREC_LEFT (cr), +, CHREC_RIGHT (cr)}_CHREC_VARIABLE (cr).  */
+DEFTREECODE (POLYNOMIAL_CHREC, "polynomial_chrec", tcc_expression, 2)
 
 /* Used to chain children of container statements together.
Use the interface in tree-iterator.h to access this node.  */
Index: gcc/tree.h
===
--- gcc/tree.h  (revision 253632)
+++ gcc/tree.h  (working copy)
@@ -1241,10 +1241,9 @@ extern void protected_set_expr_location
 #define COND_EXPR_ELSE(NODE)   (TREE_OPERAND (COND_EXPR_CHECK (NODE), 2))
 
 /* Accessors for the chains of recurrences.  */
-#define CHREC_VAR(NODE)   TREE_OPERAND (POLYNOMIAL_CHREC_CHECK (NODE), 
0)
-#define CHREC_LEFT(NODE)  TREE_OPERAND (POLYNOMIAL_CHREC_CHECK (NODE), 
1)
-#define CHREC_RIGHT(NODE) TREE_OPERAND (POLYNOMIAL_CHREC_CHECK (NODE), 
2)
-#define CHREC_VARIABLE(NODE)  TREE_INT_CST_LOW (CHREC_VAR (NODE))
+#define CHREC_LEFT(NODE)  TREE_OPERAND (POLYNOMIAL_CHREC_CHECK (NODE), 
0)
+#define CHREC_RIGHT(NODE) TREE_OPERAND (POLYNOMIAL_CHREC_CHECK (NODE), 
1)
+#define CHREC_VARIABLE(NODE)  POLYNOMIAL_CHREC_CHECK 
(NODE)->base.u.chrec_var
 
 /* LABEL_EXPR accessor. This gives access to the label associated with
the given label expression.  */

Re: [PATCH GCC][5/7]Extend loop distribution for two-level innermost loop nest

[Bin.Cheng]

On Mon, Oct 9, 2017 at 2:48 PM, Richard Biener
 wrote:
> On Thu, Oct 5, 2017 at 3:17 PM, Bin Cheng  wrote:
>> Hi,
>> For now distribution pass only handles the innermost loop.  This patch 
>> extends the pass
>> to cover two-level innermost loop nest.  It also refactors code in 
>> pass_loop_distribution::execute
>> for better reading.  Note I restrict it to 2-level loop nest on purpose 
>> because of high
>> cost in data dependence computation.  Some compilation time optimizations 
>> like reusing
>> the data reference finding, data dependence computing, would require a 
>> rewrite of this
>> pass like the proposed loop interchange implementation.  But that's another 
>> task.
>>
>> This patch introduces a temporary TODO for loop nest builtin partition which 
>> is covered
>> by next two patches.
>>
>> With this patch, kernel loop in bwaves now can be distributed, thus exposed 
>> for further
>> interchange.  This patch adds new test for matrix multiplication, as well as 
>> adjusts
>> test strings of existing tests.
>> Bootstrap and test in patch set on x86_64 and AArch64, is it OK?
>
> @ -714,9 +719,11 @@ ssa_name_has_uses_outside_loop_p (tree def, loop_p loop)
>
>FOR_EACH_IMM_USE_FAST (use_p, imm_iter, def)
>  {
> -  gimple *use_stmt = USE_STMT (use_p);
> -  if (!is_gimple_debug (use_stmt)
> - && loop != loop_containing_stmt (use_stmt))
> +  if (is_gimple_debug (USE_STMT (use_p)))
> +   continue;
> +
> +  basic_block use_bb = gimple_bb (USE_STMT (use_p));
> +  if (use_bb == NULL || !flow_bb_inside_loop_p (loop, use_bb))
> return true;
>
> use_bb should never be NULL.
Done.
>
> +  /* Don't support loop nest distribution under runtime alias check
> +since it's not likely to enable many vectorization opportunities.  */
> +  if (loop->inner)
> +   {
> + merge_dep_scc_partitions (rdg, , false);
> +   }
>
> extra {}
Done.
>
> +  /* Support loop nest distribution enclosing current innermost loop.
> +For the moment, we only support the innermost two-level loop nest.  
> */
> +  if (flag_tree_loop_distribution
> + && outer->num > 0 && outer->inner == loop && loop->next == NULL
>
> The canonical check for is-this-non-root is loop_outer (outer) instead
> of outer->num > 0.
Done.
>
> + && single_exit (outer)
>
> not sure how exits are counted but if the inner loop exits also the
> outer loop do
> we correctly handle/reject this case?
I tend to believe this can be handled if it's not rejected by
niters/exit condition,
but I am not very sure about this.
>
> -  if (nb_generated_loops + nb_generated_calls > 0)
> -   {
> - changed = true;
> - dump_printf_loc (MSG_OPTIMIZED_LOCATIONS,
> -  loc, "Loop %d distributed: split to %d loops "
> -  "and %d library calls.\n",
> -  num, nb_generated_loops, nb_generated_calls);
> + if (nb_generated_loops + nb_generated_calls > 0)
> +   {
> + changed = true;
> + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS,
> +  loc, "Loop%s %d distributed: split to %d loops 
> "
> +  "and %d library calls.\n",
> +  loop_nest_p ? " nest" : "", loop->num,
> +  nb_generated_loops, nb_generated_call
> ...
>
> can you adjust the printfs to say "loop nest distributed" in case we 
> distributed
> a nest?
Done.
>
> Can you rewrite the iteration over the nest so it would theoretically support
> arbitrary deep perfect nests?  Thus simply initialize loop_nest_p less
> cleverly...
Done.  I factored it out as a function "prepare_perfect_loop_nest".  I
also tested
the updated patch by enabling full loop nest distribution, there is no failure
in bootstrap, regression test, spec benchmarks.  Of course, the final patch
still only supports 2-level innermost loop nest.

Is this OK?

Thanks,
bin
2017-10-04  Bin Cheng  

* tree-loop-distribution.c: Adjust the general comment.
(NUM_PARTITION_THRESHOLD): New macro.
(ssa_name_has_uses_outside_loop_p): Support loop nest distribution.
(classify_partition): Skip builtin pattern of loop nest's inner loop.
(merge_dep_scc_partitions): New parameter ignore_alias_p and use it
in call to build_partition_graph.
(finalize_partitions): New parameter.  Make loop distribution more
conservative by fusing more partitions.
(distribute_loop): Don't do runtime alias check in case of loop nest
distribution.
(find_seed_stmts_for_distribution): New function.
(prepare_perfect_loop_nest): New function.
(pass_loop_distribution::execute): Refactor code finding seed stmts
and loop nest into above functions.  Support loop nest distribution.
Adjust dump information accordingly.

gcc/testsuite/ChangeLog
2017-10-04  Bin Cheng  

Re: [PATCH] preprocessor stringizing raw strings

[Nathan Sidwell]

Christophe,

Didn't you forget to add -save-temps in the testcase?
I'm seeing it failing because:
g++.dg/cpp/string-3.C  -std=c++11 : output file does not exist
UNRESOLVED: g++.dg/cpp/string-3.C  -std=c++11  scan-file BEGIN
"R\\"(\\n)\\""\n END


Surprisingly we emit a .i file, but I'd also flubbed the regexp with 
insufficient escaping.


Fixed thusly.

nathan

--
Nathan Sidwell
2017-10-11  Nathan Sidwell  

	* g++.dg/cpp/string-3.C: Fix dg-final.

Index: g++.dg/cpp/string-3.C
===
--- g++.dg/cpp/string-3.C	(revision 253622)
+++ g++.dg/cpp/string-3.C	(working copy)
@@ -6,4 +6,4 @@
 BEGIN STRINGIZE(R"(
 )") END
 
-// { dg-final { scan-file string-3.ii "BEGIN \"R\\\"(\\n)\\\"\"\n END" } }
+// { dg-final { scan-file string-3.i "BEGIN \"R\"\\(n\\)\"\"\nEND" } }

[PATCH 2/2] S/390: Do not end groups after fallthru edge

[Robin Dapp]

This patch fixes cases where we start a new group although the previous one has
not ended.

Regression tested on s390x.

gcc/ChangeLog:

2017-10-11  Robin Dapp  

* config/s390/s390.c (s390_has_ok_fallthru): New function.
(s390_sched_score): Temporarily change s390_sched_state.
(s390_sched_variable_issue): Use s390_last_sched_state in case the
group is not finished yet.
(s390_sched_init): Init s390_last_sched_state.


---
 gcc/config/s390/s390.c | 44 +---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 2411c67..7f08ba2 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -14346,6 +14346,31 @@ s390_z10_prevent_earlyload_conflicts (rtx_insn 
**ready, int *nready_p)
   ready[0] = tmp;
 }
 
+static bool
+s390_has_ok_fallthru (rtx_insn *insn)
+{
+  edge e, fallthru_edge;
+  edge_iterator ei;
+  bool other_likely_edge = false;
+
+  fallthru_edge =
+find_fallthru_edge (BLOCK_FOR_INSN (insn)->preds);
+  FOR_EACH_EDGE (e, ei, BLOCK_FOR_INSN (insn)->preds)
+{
+  if (e != fallthru_edge
+ && e->probability >= profile_probability::unlikely ())
+   {
+ other_likely_edge = true;
+ break;
+   }
+}
+
+  if (fallthru_edge && !other_likely_edge)
+return true;
+
+  return false;
+}
+
 
 /* The s390_sched_state variable tracks the state of the current or
the last instruction group.
@@ -14355,6 +14380,7 @@ s390_z10_prevent_earlyload_conflicts (rtx_insn **ready, 
int *nready_p)
4 the last group was a cracked/expanded insn */
 
 static int s390_sched_state;
+static int s390_last_sched_state;
 
 #define S390_SCHED_STATE_NORMAL  3
 #define S390_SCHED_STATE_CRACKED 4
@@ -14430,7 +14456,14 @@ s390_sched_score (rtx_insn *insn)
   unsigned int mask = s390_get_sched_attrmask (insn);
   int score = 0;
 
-  switch (s390_sched_state)
+  int temp_sched_state = s390_sched_state;
+
+  if (s390_sched_state < s390_last_sched_state
+  && s390_last_sched_state < S390_SCHED_STATE_NORMAL
+  && s390_has_ok_fallthru (insn))
+temp_sched_state = s390_last_sched_state;
+
+  switch (temp_sched_state)
 {
 case 0:
   /* Try to put insns into the first slot which would otherwise
@@ -14656,11 +14689,15 @@ s390_sched_variable_issue (FILE *file, int verbose, 
rtx_insn *insn, int more)
  switch (s390_sched_state)
{
case 0:
+ if (s390_last_sched_state > 0 && s390_has_ok_fallthru (insn))
+   s390_sched_state = s390_last_sched_state;
+
+ if (s390_sched_state == 0)
+   starts_group = true;
+ /* fallthrough */
case 1:
case 2:
case S390_SCHED_STATE_NORMAL:
- if (s390_sched_state == 0)
-   starts_group = true;
  if (s390_sched_state == S390_SCHED_STATE_NORMAL)
{
  starts_group = true;
@@ -14768,6 +14805,7 @@ s390_sched_init (FILE *file ATTRIBUTE_UNUSED,
 {
   last_scheduled_insn = NULL;
   memset (last_scheduled_unit_distance, 0, MAX_SCHED_UNITS * sizeof (int));
+  s390_last_sched_state = s390_sched_state;
   s390_sched_state = 0;
 }
 
-- 
2.9.4

[PATCH 1/2] S/390: Handle long-running instructions

[Robin Dapp]

This patch introduces balancing of long-running instructions that may clog the
pipeline.


gcc/ChangeLog:

2017-10-11  Robin Dapp  

* config/s390/s390.c (NUM_SIDES): New constant.
(LONGRUNNING_THRESHOLD): New constant.
(LATENCY_FACTOR): New constant.
(s390_sched_score): Lower score for long-running instructions on same
side.
(s390_sched_variable_issue): Bookkeeping for long-running instructions.


---
 gcc/config/s390/s390.c | 64 +-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 36bc67d..2430933 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -355,6 +355,18 @@ static rtx_insn *last_scheduled_insn;
 #define MAX_SCHED_UNITS 3
 static int last_scheduled_unit_distance[MAX_SCHED_UNITS];
 
+#define NUM_SIDES 2
+static int current_side = 1;
+#define LONGRUNNING_THRESHOLD 5
+
+/* Estimate of number of cycles a long-running insn occupies an
+   execution unit.  */
+static unsigned fxu_longrunning[NUM_SIDES];
+static unsigned vfu_longrunning[NUM_SIDES];
+
+/* Factor to scale latencies by, determined by measurements.  */
+#define LATENCY_FACTOR 4
+
 /* The maximum score added for an instruction whose unit hasn't been
in use for MAX_SCHED_MIX_DISTANCE steps.  Increase this value to
give instruction mix scheduling more priority over instruction
@@ -14483,7 +14495,24 @@ s390_sched_score (rtx_insn *insn)
if (m & unit_mask)
  score += (last_scheduled_unit_distance[i] * MAX_SCHED_MIX_SCORE /
MAX_SCHED_MIX_DISTANCE);
+
+  unsigned latency = insn_default_latency (insn);
+
+  int other_side = 1 - current_side;
+
+  /* Try to delay long-running insns when side is busy.  */
+  if (latency > LONGRUNNING_THRESHOLD)
+   {
+ if (get_attr_z13_unit_fxu (insn) && fxu_longrunning[current_side]
+ && fxu_longrunning[other_side] <= fxu_longrunning[current_side])
+   score = MAX (0, score - 10);
+
+ if (get_attr_z13_unit_vfu (insn) && vfu_longrunning[current_side]
+ && vfu_longrunning[other_side] <= vfu_longrunning[current_side])
+   score = MAX (0, score - 10);
+   }
 }
+
   return score;
 }
 
@@ -14602,6 +14631,8 @@ s390_sched_variable_issue (FILE *file, int verbose, 
rtx_insn *insn, int more)
 {
   last_scheduled_insn = insn;
 
+  bool starts_group = false;
+
   if (s390_tune >= PROCESSOR_2827_ZEC12
   && reload_completed
   && recog_memoized (insn) >= 0)
@@ -14609,6 +14640,11 @@ s390_sched_variable_issue (FILE *file, int verbose, 
rtx_insn *insn, int more)
   unsigned int mask = s390_get_sched_attrmask (insn);
 
   if ((mask & S390_SCHED_ATTR_MASK_CRACKED) != 0
+ || (mask & S390_SCHED_ATTR_MASK_EXPANDED) != 0
+ || (mask & S390_SCHED_ATTR_MASK_GROUPALONE) != 0)
+   starts_group = true;
+
+  if ((mask & S390_SCHED_ATTR_MASK_CRACKED) != 0
  || (mask & S390_SCHED_ATTR_MASK_EXPANDED) != 0)
s390_sched_state = S390_SCHED_STATE_CRACKED;
   else if ((mask & S390_SCHED_ATTR_MASK_ENDGROUP) != 0
@@ -14623,8 +14659,13 @@ s390_sched_variable_issue (FILE *file, int verbose, 
rtx_insn *insn, int more)
case 1:
case 2:
case S390_SCHED_STATE_NORMAL:
+ if (s390_sched_state == 0)
+   starts_group = true;
  if (s390_sched_state == S390_SCHED_STATE_NORMAL)
-   s390_sched_state = 1;
+   {
+ starts_group = true;
+ s390_sched_state = 1;
+   }
  else
s390_sched_state++;
 
@@ -14650,6 +14691,27 @@ s390_sched_variable_issue (FILE *file, int verbose, 
rtx_insn *insn, int more)
  last_scheduled_unit_distance[i]++;
}
 
+  /* If this insn started a new group, the side flipped.  */
+  if (starts_group)
+   current_side = current_side ? 0 : 1;
+
+  for (int i = 0; i < 2; i++)
+   {
+ if (fxu_longrunning[i] >= 1)
+   fxu_longrunning[i] -= 1;
+ if (vfu_longrunning[i] >= 1)
+   vfu_longrunning[i] -= 1;
+   }
+
+  unsigned latency = insn_default_latency (insn);
+  if (latency > LONGRUNNING_THRESHOLD)
+   {
+ if (get_attr_z13_unit_fxu (insn))
+   fxu_longrunning[current_side] = latency * LATENCY_FACTOR;
+ else
+   vfu_longrunning[current_side] = latency * LATENCY_FACTOR;
+   }
+
   if (verbose > 5)
{
  unsigned int sched_mask;
-- 
2.9.4

Re: correct attribute ifunc C++ type safety (PR 82301)

[Nathan Sidwell]

On 10/04/2017 03:40 PM, Martin Sebor wrote:

On 09/28/2017 08:25 AM, Nathan Sidwell wrote:




Since the original tests (where the resolver returns void*) pass
across the board I assume it must work for all supported ABIs.
Or is there some subtlety between the before and after code that
I'm missing?


I had a vague notion of some (IBM?) target that might do something 
different.  Perhaps it's gone.  Your comment explains it fine.



oh, I think it's trying to spot the pointer to NON-static member
function internal record type.  But brokenly. I think pmf record_types
have DECL_ARTIFICIAL and BUILTIN_LOCATION, that might be useful.


It turns out this code was superfluous with the C++ correction
and I was able to remove it with no impact on the tests.


Great.


Is this function called before we know whether we've enabled the
appropriate warnings?  It would be better to bail out sooner if the
warnings are disabled.


I'm not sure I understand the question or suggestion here but
warnings in general are certainly enabled at this point.  The
function issues both errors and warnings so it can't very well
exit early without checking the compatibility.  Let me know if
I misunderstood your comment.


Oh, forgot it issued errors too, so my queston is moot.


-signedness.
+signedness.  In C++ where incompatible pointer conversions ordinarily cause

Missing comma:  In C++, where incompatible ...


+errors, the warning detects such conversions in GCC extensions that aren't
+part of the standard language.  An example of a construct that might perform
+such a conversion is the @code{alias} attribute.  @xref{Function 
Attributes,,Declaring Attributes of Functions}.


Looks fine to me.  (pedantically, I don't think I can formally approve 
this, because it's not part of the C++ FE.)


nathan
--
Nathan Sidwell

Re: [RFC] overflow safe scaling of 64bit values

[Richard Biener]

On Tue, 10 Oct 2017, Jan Hubicka wrote:

> Hi,
> in order to drop frequencies from basic blocks and counts from edges I need
> to make probabilities more precise (so we do not get all those roundoff errors
> from 1-base fixpoint arithmetics).  Increasing base is easy now, but it
> means that in temporaries one can get overflows easily.
> 
> I need to compute (a*b)/c in a overflow safe way when a*b does not necessarily
> need to fit into 64bit integer.  The following implement safe_scale_64bit for
> it but I am not quite sure if that is most reasonable implementation.
> (it uses builtins to detect overflows, inline 64bit computation if it is safe
> and 128bit computation if it is not).
> 
> Any ideas for better version? If not I will go ahead with this variant and
> increase profile probability base.
> 
> Honza
> 
>   * profile-count.h (slow_safe_scale_64bit): New function.
>   (safe_scale_64bit): New inline.
>   (profile_count::max_safe_multiplier): Remove; use safe_scale_64bit.
> Index: profile-count.h
> ===
> --- profile-count.h   (revision 253512)
> +++ profile-count.h   (working copy)
> @@ -43,6 +43,35 @@ enum profile_quality {
>  
>  #define RDIV(X,Y) (((X) + (Y) / 2) / (Y))
>  
> +bool slow_safe_scale_64bit (uint64_t a, uint64_t b, uint64_t c, uint64_t 
> *res);
> +
> +/* Compute RES=(a*b + c/2)/c capping and return false if overflow happened.  
> */
> +
> +inline bool
> +safe_scale_64bit (uint64_t a, uint64_t b, uint64_t c, uint64_t *res)
> +{
> +#if (GCC_VERSION >= 5000)
> +  uint64_t tmp;
> +  if (!__builtin_mul_overflow (a, b, )
> +  && !__builtin_add_overflow (tmp, c/2, ))
> +{
> +  *res = tmp / c;
> +  return true;
> +}
> +  if (c == 1)
> +{
> +  *res = (uint64_t) -1;
> +  return false;
> +}
> +#else
> +  if (a < ((uint64_t)1 << 31)
> +  && b < ((uint64_t)1 << 31)
> +  && c < ((uint64_t)1 << 31))

I think you can allow (uint64_t)1 << 32, the maximum value you'll get
is 0x then which won't overflow uint64_t.  c could be even
larger, up to (1 << 34) - 4 I think but I guess that doesn't matter.

"safe_scale_64bit" sounds a bit odd but I don't have a better one.

Richard.

> +return (a * b + (c / 2)) / c;
> +#endif
> +  return slow_safe_scale_64bit (a, b, c, res);
> +}
> +
>  /* Data type to hold probabilities.  It implements fixed point arithmetics
> with capping so probability is always in range [0,1] and scaling requiring
> values greater than 1 needs to be represented otherwise.
> @@ -87,7 +116,8 @@ class GTY((user)) profile_probability
>  
>static const int n_bits = 30;
>static const uint32_t max_probability = REG_BR_PROB_BASE;
> -  static const uint32_t uninitialized_probability = ((uint32_t) 1 << n_bits) 
> - 1;
> +  static const uint32_t uninitialized_probability
> +  = ((uint32_t) 1 << (n_bits - 1)) - 1;
>  
>uint32_t m_val : 30;
>enum profile_quality m_quality : 2;
> @@ -535,11 +565,6 @@ class GTY(()) profile_count
>  
>uint64_t m_val : n_bits;
>enum profile_quality m_quality : 2;
> -
> -  /* Assume numbers smaller than this to multiply.  This is set to make
> - testsuite pass, in future we may implement precise multiplication in 
> higer
> - rangers.  */
> -  static const uint64_t max_safe_multiplier = 131072;
>  public:
>  
>/* Used for counters which are expected to be never executed.  */
> @@ -790,12 +815,9 @@ public:
>   return *this;
>  
>profile_count ret;
> -  /* Take care for overflows!  */
> -  if (num.m_val < max_safe_multiplier || m_val < max_safe_multiplier)
> - ret.m_val = RDIV (m_val * num.m_val, den.m_val);
> -  else
> - ret.m_val = RDIV (m_val * RDIV (num.m_val * max_safe_multiplier,
> - den.m_val), max_safe_multiplier);
> +  uint64_t val;
> +  safe_scale_64bit (m_val, num.m_val, den.m_val, );
> +  ret.m_val = MIN (val, max_count);
>ret.m_quality = MIN (m_quality, profile_adjusted);
>return ret;
>  }
> Index: profile-count.c
> ===
> --- profile-count.c   (revision 253512)
> +++ profile-count.c   (working copy)
> @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.
>  #include "gimple.h"
>  #include "data-streamer.h"
>  #include "cgraph.h"
> +#include "wide-int.h"
>  
>  /* Dump THIS to F.  */
>  
> @@ -194,3 +195,21 @@ profile_probability::stream_out (struct
>streamer_write_uhwi_stream (ob, m_val);
>streamer_write_uhwi_stream (ob, m_quality);
>  }
> +
> +/* Compute RES=(a*b + c/2)/c capping and return false if overflow happened.  
> */
> +
> +bool
> +slow_safe_scale_64bit (uint64_t a, uint64_t b, uint64_t c, uint64_t *res)
> +{
> +  FIXED_WIDE_INT (128) tmp = a;
> +  bool overflow;
> +  tmp = wi::udiv_floor (wi::umul (tmp, b, ) + (c / 2), c);
> +  gcc_checking_assert (!overflow);
> +  if (wi::fits_uhwi_p 

[PATCH PR82472]Update postorder number for merged partition.

[Bin Cheng]

Hi,
This patch fixes the reported ICE.  Root cause is postorder number is not 
updated
after merging partitions in SCC.  As a result, reduction partition may not be 
scheduled
as the last one because partitions are sorted in descending postorder.
Bootstrap and test on x86_64 and AArch64.  Is it OK?

Thanks,
bin
2017-10-10  Bin Cheng  

PR tree-optimization/82472
* tree-loop-distribution.c (sort_partitions_by_post_order): Refine
comment.
(break_alias_scc_partitions): Update postorder number.

gcc/testsuite
2017-10-10  Bin Cheng  

PR tree-optimization/82472
* gcc.dg/tree-ssa/pr82472.c: New test.diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr82472.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr82472.c
new file mode 100644
index 000..445c95f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr82472.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-loop-distribution" } */
+
+long int xj;
+
+int
+cx (long int *ox, short int mk, char tf)
+{
+  int si, f9;
+  char *p4 = 
+  short int *rm = (tf != 0) ? (short int *) : 
+
+  for (f9 = 0; f9 < 2; ++f9)
+{
+  *rm = 0;
+  *p4 = *ox;
+  si = mk;
+  xj = 0;
+  while (p4 < (char *)rm)
+++p4;
+}
+
+  return si;
+}
diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
index 26b8b9a..9ffac53 100644
--- a/gcc/tree-loop-distribution.c
+++ b/gcc/tree-loop-distribution.c
@@ -1939,7 +1939,8 @@ build_partition_graph (struct graph *rdg,
   return pg;
 }
 
-/* Sort partitions in PG by post order and store them in PARTITIONS.  */
+/* Sort partitions in PG in descending post order and store them in
+   PARTITIONS.  */
 
 static void
 sort_partitions_by_post_order (struct graph *pg,
@@ -1948,7 +1949,7 @@ sort_partitions_by_post_order (struct graph *pg,
   int i;
   struct pg_vdata *data;
 
-  /* Now order the remaining nodes in postorder.  */
+  /* Now order the remaining nodes in descending postorder.  */
   qsort (pg->vertices, pg->n_vertices, sizeof (vertex), pgcmp);
   partitions->truncate (0);
   for (i = 0; i < pg->n_vertices; ++i)
@@ -2044,7 +2045,7 @@ break_alias_scc_partitions (struct graph *rdg,
vec *partitions,
vec *alias_ddrs)
 {
-  int i, j, num_sccs, num_sccs_no_alias;
+  int i, j, k, num_sccs, num_sccs_no_alias;
   /* Build partition dependence graph.  */
   graph *pg = build_partition_graph (rdg, partitions, false);
 
@@ -2117,18 +2118,26 @@ break_alias_scc_partitions (struct graph *rdg,
  for (j = 0; partitions->iterate (j, ); ++j)
if (cbdata.vertices_component[j] == i)
  break;
- for (++j; partitions->iterate (j, ); ++j)
+ for (k = j + 1; partitions->iterate (k, ); ++k)
{
  struct pg_vdata *data;
 
- if (cbdata.vertices_component[j] != i)
+ if (cbdata.vertices_component[k] != i)
continue;
 
+ /* Update postorder number so that merged reduction partition is
+sorted after other partitions.  */
+ if (!partition_reduction_p (first)
+ && partition_reduction_p (partition))
+   {
+ gcc_assert (pg->vertices[k].post < pg->vertices[j].post);
+ pg->vertices[j].post = pg->vertices[k].post;
+   }
  partition_merge_into (NULL, first, partition, FUSE_SAME_SCC);
- (*partitions)[j] = NULL;
+ (*partitions)[k] = NULL;
  partition_free (partition);
- data = (struct pg_vdata *)pg->vertices[j].data;
- gcc_assert (data->id == j);
+ data = (struct pg_vdata *)pg->vertices[k].data;
+ gcc_assert (data->id == k);
  data->partition = NULL;
}
}

[PATCH] Improve FAIL message for dump-*-times functions.

[Martin Liška]

Hi.

This patch helps to find why an expected number of scan patterns does not match:

FAIL: gcc.dg/unroll-3.c scan-tree-dump-times cunrolli "loop with 3 iterations 
completely unrolled" 222 (found 1 times)
FAIL: c-c++-common/attr-simd-2.c  -Wc++-compat   scan-assembler-times 
_ZGVbN4_simd_attr: 111 (found 1 times)

Ready to be installed after testing?
Thanks,
Martin

gcc/testsuite/ChangeLog:

2017-10-11  Martin Liska  

* lib/scanasm.exp: Print how many times a regex pattern is
found.
* lib/scandump.exp: Likewise.
---
 gcc/testsuite/lib/scanasm.exp  | 5 +++--
 gcc/testsuite/lib/scandump.exp | 6 --
 2 files changed, 7 insertions(+), 4 deletions(-)


diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
index bab23e8e165..e90e61c29ae 100644
--- a/gcc/testsuite/lib/scanasm.exp
+++ b/gcc/testsuite/lib/scanasm.exp
@@ -247,10 +247,11 @@ proc scan-assembler-times { args } {
 set text [read $fd]
 close $fd
 
-if { [llength [regexp -inline -all -- $pattern $text]] == [lindex $args 1]} {
+set pattern_count [llength [regexp -inline -all -- $pattern $text]]
+if {$pattern_count == [lindex $args 1]} {
 	pass "$testcase scan-assembler-times $pp_pattern [lindex $args 1]"
 } else {
-	fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1]"
+	fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1] (found $pattern_count times)"
 }
 }
 
diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp
index 2e6eebfaf33..4a64ac6e05d 100644
--- a/gcc/testsuite/lib/scandump.exp
+++ b/gcc/testsuite/lib/scandump.exp
@@ -86,6 +86,7 @@ proc scan-dump-times { args } {
 }
 
 set testcase [testname-for-summary]
+set times [lindex $args 2]
 set suf [dump-suffix [lindex $args 3]]
 set printable_pattern [make_pattern_printable [lindex $args 1]]
 set testname "$testcase scan-[lindex $args 0]-dump-times $suf \"$printable_pattern\" [lindex $args 2]"
@@ -101,10 +102,11 @@ proc scan-dump-times { args } {
 set text [read $fd]
 close $fd
 
-if { [llength [regexp -inline -all -- [lindex $args 1] $text]] == [lindex $args 2]} {
+set result_count [llength [regexp -inline -all -- [lindex $args 1] $text]]
+if {$result_count == $times} {
 pass "$testname"
 } else {
-fail "$testname"
+fail "$testname (found $result_count times)"
 }
 }
 

Re: [PATCH] Do not error for no_sanitize attributes (PR sanitizer/82490).

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 09:45:27AM +0200, Markus Trippelsdorf wrote:
> On 2017.10.11 at 09:39 +0200, Jakub Jelinek wrote:
> > On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote:
> > > Hi.
> > > 
> > > This changes error to a warning:
> > > warning: ‘foobar’ attribute directive ignored [-Wattributes]
> > > 
> > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> > 
> > What is the rationale for not warning?  LLVM compatibility?
> > I think in some cases it would be nice to find out that I wrote a typo...
> 
> The patch does warn. But yes, not erroring out improves LLVM
> compatibility. Chromium uses __attribute__(no_sanitize("function")) for
> example.

Ah, ok.  The patch is ok with the nit testcase fixed.

Jakub

Re: [PATCH] Do not error for no_sanitize attributes (PR sanitizer/82490).

[Markus Trippelsdorf]

On 2017.10.11 at 09:39 +0200, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote:
> > Hi.
> > 
> > This changes error to a warning:
> > warning: ‘foobar’ attribute directive ignored [-Wattributes]
> > 
> > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> What is the rationale for not warning?  LLVM compatibility?
> I think in some cases it would be nice to find out that I wrote a typo...

The patch does warn. But yes, not erroring out improves LLVM
compatibility. Chromium uses __attribute__(no_sanitize("function")) for
example.

-- 
Markus

Re: Zen tuning part 6: Break up CPU specific tuning bits to central place

[Uros Bizjak]

Hello!

> * i386/i386.c Move all CPU cost tables to x86-tune-costs.h
> (COSTS_N_BYTES): Move to x86-tune-costs.h
> (DUMMY_STRINGOP_ALGS):x86-tune-costs.h

Missing full stops.  Partial sentence in the last one.

> (TARGET_SCHED_DISPATCH): Move to ix86-tnue-sched-bd.c.
> (TARGET_SCHED_DISPATCH_DO): Move to ix86-tnue-sched-bd.c.
> (TARGET_SCHED_REORDER): Move to ix86-tnue-sched-bd.c.
> (DISPATCH_WINDOW_SIZE): Move to ix86-tnue-sched-bd.c.
> (MAX_DISPATCH_WINDOWS): Move to ix86-tnue-sched-bd.c.
> (MAX_INSN): Move to ix86-tnue-sched-bd.c.
> (MAX_IMM): Move to ix86-tnue-sched-bd.c.
> (MAX_IMM_SIZE): Move to ix86-tnue-sched-bd.c.
> (MAX_IMM_32): Move to ix86-tnue-sched-bd.c.
> (MAX_IMM_64): Move to ix86-tnue-sched-bd.c.
> (MAX_LOAD): Move to ix86-tnue-sched-bd.c.
> (MAX_STORE): Move to ix86-tnue-sched-bd.c.
> (BIG): Move to ix86-tnue-sched-bd.c.
> (enum dispatch_group): Move to ix86-tnue-sched-bd.c.
> (enum insn_path): Move to ix86-tnue-sched-bd.c.
> (get_mem_group): Move to ix86-tnue-sched-bd.c.
> (is_cmp): Move to ix86-tnue-sched-bd.c.
> (dispatch_violation): Move to ix86-tnue-sched-bd.c.
> (is_branch): Move to ix86-tnue-sched-bd.c.
> (is_prefetch): Move to ix86-tnue-sched-bd.c.
> (init_window): Move to ix86-tnue-sched-bd.c.
> (allocate_window): Move to ix86-tnue-sched-bd.c.
> (init_dispatch_sched): Move to ix86-tnue-sched-bd.c.
> (is_end_basic_block): Move to ix86-tnue-sched-bd.c.
> (process_end_window): Move to ix86-tnue-sched-bd.c.
> (allocate_next_window): Move to ix86-tnue-sched-bd.c.
> (find_constant): Move to ix86-tnue-sched-bd.c.
> (get_num_immediates): Move to ix86-tnue-sched-bd.c.
> (has_immediate): Move to ix86-tnue-sched-bd.c.
> (get_insn_path): Move to ix86-tnue-sched-bd.c.
> (get_insn_group): Move to ix86-tnue-sched-bd.c.
> (count_num_restricted): Move to ix86-tnue-sched-bd.c.
> (fits_dispatch_window): Move to ix86-tnue-sched-bd.c.
> (add_insn_window): Move to ix86-tnue-sched-bd.c.
> (add_to_dispatch_window): Move to ix86-tnue-sched-bd.c.
> (debug_dispatch_window_file): Move to ix86-tnue-sched-bd.c.
> (debug_dispatch_window): Move to ix86-tnue-sched-bd.c.
> (debug_insn_dispatch_info_file): Move to ix86-tnue-sched-bd.c.
> (debug_ready_dispatch): Move to ix86-tnue-sched-bd.c.
> (do_dispatch): Move to ix86-tnue-sched-bd.c.
> (has_dispatch): Move to ix86-tnue-sched-bd.c.

tnue -> tune in the above.

Uros.

Re: [PATCH] Do not error for no_sanitize attributes (PR sanitizer/82490).

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote:
> Hi.
> 
> This changes error to a warning:
> warning: ‘foobar’ attribute directive ignored [-Wattributes]
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

What is the rationale for not warning?  LLVM compatibility?
I think in some cases it would be nice to find out that I wrote a typo...

> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/ubsan/attrib-5.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=undefined" } */
> +
> +__attribute__((no_sanitize(("foobar"

Too many ()s around the no_sanitize argument.

Jakub

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 07:55:44AM +0200, Martin Liška wrote:
> > Conceptually, these two instrumentations rely on address sanitization,
> > not really sure if we should supporting them for kernel sanitization (but I
> > bet it is just going to be too costly for kernel).
> > So, we also need to make sure at least parts of SANITIZE_ADDRESS is enabled
> > when these options are on.
> > That can be done by erroring out if -fsanitize=pointer-compare is requested
> > without -fsanitize=address, or by implicitly enabling -fsanitize=address for
> > these, or by adding yet another SANITIZE_* bit which would cover
> > sanitization of memory accesses for asan, that bit would be set by
> > -fsanitize={address,kernel-address} in addition to the current 2 bits, but
> > pointer-{subtract,compare} would set its own bit and SANITIZE_ADDRESS and
> > SANITIZE_USER_ADDRESS only.  Without the new bit we'd emit red zones,
> > function prologue/epilogue asan changes, registraction of global variables,
> > but not actual instrumentation of memory accesses (and probably not
> > instrumentation of C++ ctor ordering).
> 
> Agree, would be much easier to just enable SANITIZE_ADDRESS with there 2 
> options.
> Question is how to make it also possible with -fsanitize=kernel-address:
> 
> $ ./xgcc -B.   
> /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c 
> -fsanitize=pointer-compare,kernel-address
> cc1: error: ‘-fsanitize=address’ is incompatible with 
> ‘-fsanitize=kernel-address’
> 
> Ideas?

If we want to make it usable for both user and kernel address, then either
we'll let pointer-compare/pointer-subtract implicitly enable user address,
unless kernel-address has been enabled (that would mean set SANITIZE_ADDRESS
bit in pointer-*, but not SANITIZE_USER_ADDRESS, and at the point where we
diagnose option incompatibilities like -fsanitize=address,kernel-address
check for the case (SANITIZE_ADDRESS bit set, none of SANITIZE_USER_ADDRESS
nor SANITIZE_KERNEL_ADDRESS, and one of SANITIZE_POINTER_*) and set
implicitly SANITIZE_USER_ADDRESS, or simply require that the user chooses,
by erroring out if pointer-* is used without explicit address or
kernel-address.  In any case, I think this should be also something
discussed with the upstream sanitizer folks, so that LLVM (if it ever
decides to actually implement it) behaves compatibly.

> --- a/libsanitizer/asan/asan_descriptions.cc
> +++ b/libsanitizer/asan/asan_descriptions.cc
> @@ -220,6 +220,15 @@ bool GetStackAddressInformation(uptr addr, uptr 
> access_size,
>return true;
>  }
>  
> +bool GetStackVariableBeginning(uptr addr, uptr *shadow_addr)
> +{
> +  AsanThread *t = FindThreadByStackAddress(addr);
> +  if (!t) return false;
> +
> +  *shadow_addr = t->GetStackFrameVariableBeginning (addr);
> +  return true;
> +}
> +
>  static void PrintAccessAndVarIntersection(const StackVarDescr , uptr 
> addr,
>uptr access_size, uptr 
> prev_var_end,
>uptr next_var_beg) {
> diff --git a/libsanitizer/asan/asan_descriptions.h 
> b/libsanitizer/asan/asan_descriptions.h
> index 584b9ba6491..b7f23b1a71b 100644
> --- a/libsanitizer/asan/asan_descriptions.h
> +++ b/libsanitizer/asan/asan_descriptions.h
> @@ -138,6 +138,7 @@ struct StackAddressDescription {
>  
>  bool GetStackAddressInformation(uptr addr, uptr access_size,
>  StackAddressDescription *descr);
> +bool GetStackVariableBeginning(uptr addr, uptr *shadow_addr);
>  
>  struct GlobalAddressDescription {
>uptr addr;
> diff --git a/libsanitizer/asan/asan_globals.cc 
> b/libsanitizer/asan/asan_globals.cc
> index f2292926e6a..ed707c0ca01 100644
> --- a/libsanitizer/asan/asan_globals.cc
> +++ b/libsanitizer/asan/asan_globals.cc
> @@ -122,6 +122,31 @@ int GetGlobalsForAddress(uptr addr, Global *globals, u32 
> *reg_sites,
>return res;
>  }
>  
> +bool AreGlobalVariablesSame(uptr addr1, uptr addr2)
> +{
> +  if (addr1 > addr2)
> +  {
> +uptr tmp = addr1;
> +addr1 = addr2;
> +addr2 = tmp;

std::swap(addr1, addr2); ?  I don't see it used in any of libsanitizer
though, so not sure if the corresponding STL header is included.

> +  }
> +
> +  BlockingMutexLock lock(_for_globals);

Why do you need a mutex for checking if there are no red zones in between?

> +  uptr aligned_addr1 = addr1 & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
> +  uptr aligned_addr2 = addr2 & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
> +
> +  u8 *shadow_ptr1 = (u8*)MemToShadow(aligned_addr1);
> +  u8 *shadow_ptr2 = (u8*)MemToShadow(aligned_addr2);
> +
> +  while (shadow_ptr1 <= shadow_ptr2
> +  && *shadow_ptr1 != kAsanGlobalRedzoneMagic) {
> +shadow_ptr1++;
> +  }

There are many kinds of shadow memory markings.  My thought was that it
would start with a quick check, perhaps vectorized by hand (depending on if
the arch has unaligned loads maybe without or with a short loop for
alignment) where say 

Re: [PATCH] Fix use-after-scope error.

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 08:17:25AM +0200, Martin Liška wrote:
> One can see use-after-scope error in boostrap-asan:
> 
> gcc/ChangeLog:
> 
> 2017-10-10  Martin Liska  
> 
>   * print-rtl.c (print_insn): Move declaration of idbuf
>   to same scope as name.
> ---
>  gcc/print-rtl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 

> diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
> index 79ec463df45..28d99862cad 100644
> --- a/gcc/print-rtl.c
> +++ b/gcc/print-rtl.c
> @@ -1792,11 +1792,11 @@ print_insn (pretty_printer *pp, const rtx_insn *x, 
> int verbose)
>  case DEBUG_INSN:
>{
>   const char *name = "?";
> + char idbuf[32];
>  
>   if (DECL_P (INSN_VAR_LOCATION_DECL (x)))
> {
>   tree id = DECL_NAME (INSN_VAR_LOCATION_DECL (x));
> - char idbuf[32];
>   if (id)
> name = IDENTIFIER_POINTER (id);
>   else if (TREE_CODE (INSN_VAR_LOCATION_DECL (x))

Ok.  This should IMHO go into release branches too.

Jakub

Re: [PATCH] Fix a test-case for Darwin.

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 08:15:16AM +0200, Martin Liška wrote:
> Hello.
> 
> This should address failing test-case on Darwin.

Guess this should fix it not just on Darwin, but also on any target
where the assembler doesn't have a call instruction but something different
(e.g. bl and many others).

> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-10-10  Martin Liska  
> 
>   * c-c++-common/ubsan/ptr-overflow-sanitization-1.c: Scan
>   optimized dump rather than assembly.

Ok, thanks.  With a nit:

> diff --git a/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c 
> b/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
> index 42c14523764..b966d52a61d 100644
> --- a/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
> +++ b/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
> @@ -1,5 +1,5 @@
>  /* { dg-require-effective-target lp64 } */

Do you really need the above line?  I thought now that the test
is using __PTRDIFF_MAX__ you shouldn't really need that...

> -/* { dg-options "-O -fsanitize=pointer-overflow" } */
> +/* { dg-options "-O -fsanitize=pointer-overflow -fdump-tree-optimized" } */
>  /* { dg-skip-if "" { *-*-* } "-flto" } */
>  
>  #define SMAX   __PTRDIFF_MAX__
> @@ -76,5 +76,4 @@ void negative_to_negative (char *ptr)
>p2 += 5;
>  }
>  
> -
> -/* { dg-final { scan-assembler-times 
> "call\\s+__ubsan_handle_pointer_overflow" 17 } } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 17 
> "optimized" } } */
> 


Jakub

Re: [PATCH] preprocessor stringizing raw strings

[Christophe Lyon]

Hi Nathan,


On 10 October 2017 at 20:54, Nathan Sidwell  wrote:
> This patch fixes PR 82506, where we fail to properly stringize a raw string
> literal, which can contain a raw LF character.
>
> When we're not just preprocessing, there isn't a problem.  The string
> literal gets correctly escaped into the assembly file.  This is just a
> problem with preprocessing.
>
> Applying to trunk.
>

Didn't you forget to add -save-temps in the testcase?
I'm seeing it failing because:
g++.dg/cpp/string-3.C  -std=c++11 : output file does not exist
UNRESOLVED: g++.dg/cpp/string-3.C  -std=c++11  scan-file BEGIN
"R\\"(\\n)\\""\n END

Christophe

> nathan
> --
> Nathan Sidwell

[PATCH] Do not error for no_sanitize attributes (PR sanitizer/82490).

[Martin Liška]

Hi.

This changes error to a warning:
warning: ‘foobar’ attribute directive ignored [-Wattributes]

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

Ready to be installed?
Martin

gcc/ChangeLog:

2017-10-10  Martin Liska  

PR sanitizer/82490
* opts.c (parse_no_sanitize_attribute): Do not use error_value
variable.
* opts.h (parse_no_sanitize_attribute): Remove last argument.

gcc/c-family/ChangeLog:

2017-10-10  Martin Liska  

PR sanitizer/82490
* c-attribs.c (handle_no_sanitize_attribute): Report directly
Wattributes warning.

gcc/testsuite/ChangeLog:

2017-10-10  Martin Liska  

PR sanitizer/82490
* c-c++-common/ubsan/attrib-5.c: New test.
---
 gcc/c-family/c-attribs.c|  9 +
 gcc/opts.c  |  8 
 gcc/opts.h  |  2 +-
 gcc/testsuite/c-c++-common/ubsan/attrib-5.c | 11 +++
 4 files changed, 17 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/ubsan/attrib-5.c


diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 4e6754fba20..bd8ca306c2d 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -613,15 +613,8 @@ handle_no_sanitize_attribute (tree *node, tree name, tree args, int,
   return NULL_TREE;
 }
 
-  char *error_value = NULL;
   char *string = ASTRDUP (TREE_STRING_POINTER (id));
-  unsigned int flags = parse_no_sanitize_attribute (string, _value);
-
-  if (error_value)
-{
-  error ("wrong argument: \"%s\"", error_value);
-  return NULL_TREE;
-}
+  unsigned int flags = parse_no_sanitize_attribute (string);
 
   add_no_sanitize_value (*node, flags);
 
diff --git a/gcc/opts.c b/gcc/opts.c
index 5aa5d066dbe..adf3d89851d 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1700,11 +1700,10 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,
 }
 
 /* Parse string values of no_sanitize attribute passed in VALUE.
-   Values are separated with comma.  Wrong argument is stored to
-   WRONG_ARGUMENT variable.  */
+   Values are separated with comma.  */
 
 unsigned int
-parse_no_sanitize_attribute (char *value, char **wrong_argument)
+parse_no_sanitize_attribute (char *value)
 {
   unsigned int flags = 0;
   unsigned int i;
@@ -1722,7 +1721,8 @@ parse_no_sanitize_attribute (char *value, char **wrong_argument)
 	  }
 
   if (sanitizer_opts[i].name == NULL)
-	*wrong_argument = q;
+	warning (OPT_Wattributes,
+		 "%<%s%> attribute directive ignored", q);
 
   q = strtok (NULL, ",");
 }
diff --git a/gcc/opts.h b/gcc/opts.h
index 2774e2c8b40..10938615725 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -390,7 +390,7 @@ extern void handle_common_deferred_options (void);
 unsigned int parse_sanitizer_options (const char *, location_t, int,
   unsigned int, int, bool);
 
-unsigned int parse_no_sanitize_attribute (char *value, char **wrong_argument);
+unsigned int parse_no_sanitize_attribute (char *value);
 extern bool common_handle_option (struct gcc_options *opts,
   struct gcc_options *opts_set,
   const struct cl_decoded_option *decoded,
diff --git a/gcc/testsuite/c-c++-common/ubsan/attrib-5.c b/gcc/testsuite/c-c++-common/ubsan/attrib-5.c
new file mode 100644
index 000..1dfe50dd0b4
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/attrib-5.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined" } */
+
+__attribute__((no_sanitize(("foobar"
+static void
+float_cast2 (void)
+{ /* { dg-warning "attribute directive ignored" } */
+  volatile double d = 300;
+  volatile signed char c;
+  c = d;
+}

[PATCH] Fix use-after-scope error.

[Martin Liška]

Hello.

One can see use-after-scope error in boostrap-asan:

Executing on host: /home/marxin/gcc/objdir2/gcc/xgcc 
-B/home/marxin/gcc/objdir2/gcc/  -fno-diagnostics-show-caret 
-fdiagnostics-color=never  -w  -O3 -g   -dumpbase dump1/dump-noaddr.c -DMASK=1 
-x c --param ggc-min-heapsize=1 -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -
fdump-noaddr -c   -o /home/marxin/gcc/objdir2/gcc/testsuite/gcc27/dump-noaddr.o 
/home/marxin/gcc/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.c(timeout 
= 300)
spawn /home/marxin/gcc/objdir2/gcc/xgcc -B/home/marxin/gcc/objdir2/gcc/ 
-fno-diagnostics-show-caret -fdiagnostics-color=never -w -O3 -g -dumpbase 
dump1/dump-noaddr.c -DMASK=1 -x c --param ggc-min-heapsize=1 -fdump-ipa-all 
-fdump-rtl-all -fdump-tree-all -fdump-noaddr -c -o
 /home/marxin/gcc/objdir2/gcc/testsuite/gcc27/dump-noaddr.o 
/home/marxin/gcc/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.c
=
==7==ERROR: AddressSanitizer: stack-use-after-scope on address 
0x7fff9890 at pc 0x009d9361 bp 0x7fff9280 sp 0x7fff8a30
READ of size 4 at 0x7fff9890 thread T0
#0 0x9d9360 in __interceptor_strlen 
../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:225
#1 0x3189f03 in pp_string(pretty_printer*, char const*) 
../../gcc/pretty-print.c:990
#2 0x318cb5a in pp_format(pretty_printer*, text_info*) 
../../gcc/pretty-print.c:599
#3 0x318ecfe in pp_printf(pretty_printer*, char const*, ...) 
../../gcc/pretty-print.c:937
#4 0x17ac387 in print_insn(pretty_printer*, rtx_insn const*, int) 
../../gcc/print-rtl.c:1816
#5 0x17ac837 in print_insn_with_notes ../../gcc/print-rtl.c:1897
#6 0x17b5c73 in dump_insn_slim(_IO_FILE*, rtx_insn const*) 
../../gcc/print-rtl.c:1934
#7 0x2e61d85 in combine_instructions ../../gcc/combine.c:1218
#8 0x2e61d85 in rest_of_handle_combine ../../gcc/combine.c:14784
#9 0x2e61d85 in execute ../../gcc/combine.c:14829
#10 0x173d971 in execute_one_pass(opt_pass*) ../../gcc/passes.c:2495
#11 0x173f126 in execute_pass_list_1 ../../gcc/passes.c:2584
#12 0x173f150 in execute_pass_list_1 ../../gcc/passes.c:2585
#13 0x173f1af in execute_pass_list(function*, opt_pass*) 
../../gcc/passes.c:2595
#14 0xeb7957 in cgraph_node::expand() ../../gcc/cgraphunit.c:2115
#15 0xeba71e in expand_all_functions ../../gcc/cgraphunit.c:2251
#16 0xeba71e in symbol_table::compile() ../../gcc/cgraphunit.c:2599
#17 0xec0e40 in symbol_table::compile() ../../gcc/cgraphunit.c:2695
#18 0xec0e40 in symbol_table::finalize_compilation_unit() 
../../gcc/cgraphunit.c:2692
#19 0x19e3a8b in compile_file ../../gcc/toplev.c:481
#20 0x9a2a3f in do_compile ../../gcc/toplev.c:2037
#21 0x9a2a3f in toplev::main(int, char**) ../../gcc/toplev.c:2172
#22 0x9acd24 in main ../../gcc/main.c:39
#23 0x76a396e4 in __libc_start_main (/lib64/libc.so.6+0x206e4)
#24 0x9adf28 in _start (/home/marxin/gcc/objdir2/gcc/cc1+0x9adf28)

Address 0x7fff9890 is located in stack of thread T0 at offset 96 in frame
#0 0x17abfdf in print_insn(pretty_printer*, rtx_insn const*, int) 
../../gcc/print-rtl.c:1777

  This frame has 2 object(s):
[32, 64) 'uid_prefix'
[96, 128) 'idbuf' <== Memory access at offset 96 is inside this variable
...

This is fix of that. It's quite clear.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2017-10-10  Martin Liska  

* print-rtl.c (print_insn): Move declaration of idbuf
to same scope as name.
---
 gcc/print-rtl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 79ec463df45..28d99862cad 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -1792,11 +1792,11 @@ print_insn (pretty_printer *pp, const rtx_insn *x, int verbose)
 case DEBUG_INSN:
   {
 	const char *name = "?";
+	char idbuf[32];
 
 	if (DECL_P (INSN_VAR_LOCATION_DECL (x)))
 	  {
 	tree id = DECL_NAME (INSN_VAR_LOCATION_DECL (x));
-	char idbuf[32];
 	if (id)
 	  name = IDENTIFIER_POINTER (id);
 	else if (TREE_CODE (INSN_VAR_LOCATION_DECL (x))

[PATCH] Fix a test-case for Darwin.

[Martin Liška]

Hello.

This should address failing test-case on Darwin.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/testsuite/ChangeLog:

2017-10-10  Martin Liska  

* c-c++-common/ubsan/ptr-overflow-sanitization-1.c: Scan
optimized dump rather than assembly.
---
 gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


diff --git a/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c b/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
index 42c14523764..b966d52a61d 100644
--- a/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
+++ b/gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
@@ -1,5 +1,5 @@
 /* { dg-require-effective-target lp64 } */
-/* { dg-options "-O -fsanitize=pointer-overflow" } */
+/* { dg-options "-O -fsanitize=pointer-overflow -fdump-tree-optimized" } */
 /* { dg-skip-if "" { *-*-* } "-flto" } */
 
 #define SMAX   __PTRDIFF_MAX__
@@ -76,5 +76,4 @@ void negative_to_negative (char *ptr)
   p2 += 5;
 }
 
-
-/* { dg-final { scan-assembler-times "call\\s+__ubsan_handle_pointer_overflow" 17 } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 17 "optimized" } } */

[PATCH] Fix bitmap_bit_in_range_p (PR tree-optimization/82493).

[Martin Liška]

Hi.

This fixes some implementations mistakes in sbitmap.c (bitmap_bit_in_range_p). 
There's reference
to implementation one can take inspiration from:
https://www.cs.umd.edu/class/sum2003/cmsc311/Notes/BitOp/bitRange.html

Problem with our implementation is that one can't do:
(SBITMAP_ELT_TYPE)1 << SBITMAP_ELT_BITS (that would overflow)
Thus I do conditionally ~(SBITMAP_ELT_TYPE)0 at some places in the code.

I also added quite some unit tests for the method. But another questions pop up:
1) there are missing boundary asserts (or checking asserts) in sbitmap.c
2) we should probably include test-cases also for other functions

I can work on that (probably later) if desired?

And my patch breaks ssa-dse-26.c test-case, because now it properly returns 
true for:

#0  bitmap_bit_in_range_p (bmap=0x21c4940, start=0, end=8) at 
../../gcc/sbitmap.c:326
#1  0x00d618f5 in live_bytes_read (use_ref=..., ref=0x7fffd480, 
live=0x21c4940) at ../../gcc/tree-ssa-dse.c:496
#2  0x00d61c4d in dse_classify_store (ref=0x7fffd480, 
stmt=0x13ea7d70, use_stmt=0x7fffd470, byte_tracking_enabled=true, 
live_bytes=0x21c4940) at ../../gcc/tree-ssa-dse.c:594
#3  0x00d6235b in dse_dom_walker::dse_optimize_stmt 
(this=0x7fffd5c0, gsi=0x7fffd530) at ../../gcc/tree-ssa-dse.c:820
#4  0x00d62461 in dse_dom_walker::before_dom_children 
(this=0x7fffd5c0, bb=0x13d76270) at ../../gcc/tree-ssa-dse.c:852
#5  0x013b1698 in dom_walker::walk (this=0x7fffd5c0, 
bb=0x13d76270) at ../../gcc/domwalk.c:308
#6  0x00d625ac in (anonymous namespace)::pass_dse::execute 
(this=0x21d58c0, fun=0x13eac0b0) at ../../gcc/tree-ssa-dse.c:906
#7  0x00b27441 in execute_one_pass (pass=pass@entry=0x21d58c0) at 
../../gcc/passes.c:2495
#8  0x00b27d01 in execute_pass_list_1 (pass=0x21d58c0) at 
../../gcc/passes.c:2584
#9  0x00b27d13 in execute_pass_list_1 (pass=0x21d5480) at 
../../gcc/passes.c:2585
#10 0x00b27d55 in execute_pass_list (fn=, 
pass=) at ../../gcc/passes.c:2595
#11 0x00b26681 in do_per_function_toporder 
(callback=callback@entry=0xb27d40 , 
data=0x21d5300) at ../../gcc/passes.c:1737
#12 0x00b283d7 in execute_ipa_pass_list (pass=0x21d52a0) at 
../../gcc/passes.c:2935
#13 0x007d29d2 in ipa_passes () at ../../gcc/cgraphunit.c:2399
#14 symbol_table::compile (this=this@entry=0x13d6e100) at 
../../gcc/cgraphunit.c:2534
#15 0x007d5277 in symbol_table::compile (this=0x13d6e100) at 
../../gcc/cgraphunit.c:2695
#16 symbol_table::finalize_compilation_unit (this=0x13d6e100) at 
../../gcc/cgraphunit.c:2692
#17 0x00c118ac in compile_file () at ../../gcc/toplev.c:481
#18 0x00c13eee in do_compile () at ../../gcc/toplev.c:2037
#19 0x00c141da in toplev::main (this=0x7fffd85e, argc=21, 
argv=0x7fffd958) at ../../gcc/toplev.c:2172
#20 0x0061aeab in main (argc=21, argv=0x7fffd958) at 
../../gcc/main.c:39

(gdb) p debug(bmap)
n_bits = 256, set = {8 9 10 11 12 13 14 15 }

Jeff can you please help me?
Apart from that the patch can bootstrap on ppc64le-redhat-linux and survives 
regression tests.

Ready to be installed?
Martin

Thanks,
Martin

gcc/ChangeLog:

2017-10-10  Martin Liska  

PR tree-optimization/82493
* sbitmap.c (bitmap_bit_in_range_p): Fix the implementation.
(test_range_functions): New function.
(sbitmap_c_tests): Likewise.
* selftest-run-tests.c (selftest::run_tests): Run new tests.
* selftest.h (sbitmap_c_tests): New function.
---
 gcc/sbitmap.c| 118 ---
 gcc/selftest-run-tests.c |   1 +
 gcc/selftest.h   |   1 +
 3 files changed, 103 insertions(+), 17 deletions(-)


diff --git a/gcc/sbitmap.c b/gcc/sbitmap.c
index 4bf13a11a1d..baef4d05f0d 100644
--- a/gcc/sbitmap.c
+++ b/gcc/sbitmap.c
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "sbitmap.h"
+#include "selftest.h"
 
 typedef SBITMAP_ELT_TYPE *sbitmap_ptr;
 typedef const SBITMAP_ELT_TYPE *const_sbitmap_ptr;
@@ -322,29 +323,22 @@ bitmap_set_range (sbitmap bmap, unsigned int start, unsigned int count)
 bool
 bitmap_bit_in_range_p (const_sbitmap bmap, unsigned int start, unsigned int end)
 {
+  gcc_checking_assert (start <= end);
   unsigned int start_word = start / SBITMAP_ELT_BITS;
   unsigned int start_bitno = start % SBITMAP_ELT_BITS;
 
-  /* Testing within a word, starting at the beginning of a word.  */
-  if (start_bitno == 0 && (end - start) < SBITMAP_ELT_BITS)
-{
-  SBITMAP_ELT_TYPE mask = ((SBITMAP_ELT_TYPE)1 << (end - start)) - 1;
-  return (bmap->elms[start_word] & mask) != 0;
-}
-
   unsigned int end_word = end / SBITMAP_ELT_BITS;
   unsigned int end_bitno = end % SBITMAP_ELT_BITS;
 
-  /* Testing starts somewhere in the middle of a word.