Re: [RFC] Fix for PR58201

[Jan Hubicka]

> >+ 2013-09-04  Jan Hubicka  
> >+
> >+PR middle-end/58201
> >+* cgraphunit.c (analyze_functions): Clear AUX fields
> >+after processing; initialize assembler name has.
> >+
> I checked and double checked and with this commit a C++ test regressed:
> 
> FAIL: g++.dg/template/cond2.C -std=gnu++98 (test for warnings, line 9)
> FAIL: g++.dg/template/cond2.C -std=gnu++11 (test for warnings, line 9)
> 
> In practice we emit a message off by one line, line 10 instead of
> the correct line 9. Is it possible that you are clearing too much,
> so to speak?

Amazing, this testcase was triggering an ICE and I had to disable initialization
of assembler name hash when error arrived.  That fixed the problem, but I did 
not
notice it is still having wrong line info.

Adding an alias into the unit will trigger wrong line info before my change, 
since
assembler name hash is populated too.

The error is output from DECL_ASSEMBLER_NAME hook that by itself is strange.

#0  0x01041490 in error(char const*, ...) ()
#1  0x00735039 in write_expression(tree_node*) () at 
../../gcc/cp/mangle.c:3008
#2  0x0073a70f in write_template_arg(tree_node*) () at 
../../gcc/cp/mangle.c:3179
#3  0x0073ae69 in write_template_args(tree_node*) () at 
../../gcc/cp/mangle.c:2551
#4  0x007332a4 in write_name(tree_node*, int) () at 
../../gcc/cp/mangle.c:821
#5  0x0073700c in write_type(tree_node*) () at 
../../gcc/cp/mangle.c:2522
#6  0x00737279 in write_type(tree_node*) () at 
../../gcc/cp/mangle.c:2017
#7  0x00738a18 in write_method_parms(tree_node*, int, tree_node*) () at 
../../gcc/cp/mangle.c:2509
#8  0x00738e2f in write_bare_function_type(tree_node*, int, tree_node*) 
() at ../../gcc/cp/mangle.c:2451
#9  0x00733b9e in write_mangled_name(tree_node*, bool) () at 
../../gcc/cp/mangle.c:689
#10 0x0073c5b6 in mangle_decl_string(tree_node*) () at 
../../gcc/cp/mangle.c:3446
#11 0x0073c7e9 in mangle_decl(tree_node*) () at 
../../gcc/cp/mangle.c:3468
#12 0x00d1df01 in decl_assembler_name(tree_node*) () at 
../../gcc/tree.c:546
#13 0x0083b1f5 in insert_to_assembler_name_hash(symtab_node_def*, bool) 
()
#14 0x0083b352 in symtab_initialize_asm_name_hash() [clone .part.3] ()

It seems to be latent bug in C++ FE to not set location correctly to error - no 
one
promise that the current locus will correspond to the declaration being mangled
during the lazy call of assembler name.  So I suppose error needs to be updated
to get correct locus?

Honza

> 
> After the recent breakages, up to date testresults are arriving only
> now on gcc-testresults confirming my finding, for example Andreas
> Schwab already posted a couple.
> 
> Thanks!
> Paolo.

Fix gimple thunks

[Jan Hubicka]

Hi,
this is a variant of patch I tested and comitted after discussion on 
DECL_ARGUMENTS change.
Basically ARGUMENTS are now part of a functio nbody and we need to stream them 
for thunks
in order to be able to expand them.  The patch also fixes misplaced pop_cfun in 
lto-streamer-in.c.

Bootstrapped/regtested x86_64-linux, comitted.

* cgraphunit.c (expand_thunk): Get body before touching arguments.
* lto-streamer-out.c: Stream thunks, too.
* lto-streamer-in.c (input_function): Pop cfun here
(lto_read_body): Instead of here.
Index: cgraphunit.c
===
--- cgraphunit.c(revision 202335)
+++ cgraphunit.c(working copy)
@@ -1433,7 +1433,11 @@ expand_thunk (struct cgraph_node *node)
   tree virtual_offset = NULL;
   tree alias = node->callees->callee->symbol.decl;
   tree thunk_fndecl = node->symbol.decl;
-  tree a = DECL_ARGUMENTS (thunk_fndecl);
+  tree a;
+
+  if (in_lto_p)
+cgraph_get_body (node);
+  a = DECL_ARGUMENTS (thunk_fndecl);
 
   current_function_decl = thunk_fndecl;
 
Index: lto-streamer-out.c
===
--- lto-streamer-out.c  (revision 202335)
+++ lto-streamer-out.c  (working copy)
@@ -1985,8 +1985,7 @@ lto_output (void)
   cgraph_node *node = dyn_cast  (snode);
   if (node
  && lto_symtab_encoder_encode_body_p (encoder, node)
- && !node->symbol.alias
- && !node->thunk.thunk_p)
+ && !node->symbol.alias)
{
 #ifdef ENABLE_CHECKING
  gcc_assert (!bitmap_bit_p (output, DECL_UID (node->symbol.decl)));
Index: lto-streamer-in.c
===
--- lto-streamer-in.c   (revision 202335)
+++ lto-streamer-in.c   (working copy)
@@ -998,6 +998,7 @@ input_function (tree fn_decl, struct dat
   free_dominance_info (CDI_DOMINATORS);
   free_dominance_info (CDI_POST_DOMINATORS);
   free (stmts);
+  pop_cfun ();
 }
 
 
@@ -1086,8 +1087,6 @@ lto_read_body (struct lto_file_decl_data
 
   /* Restore decl state */
   file_data->current_decl_state = file_data->global_decl_state;
-
-  pop_cfun ();
 }
 
   lto_data_in_delete (data_in);

[PATCH, bootstrap]: Initialize deref_align in ipa_modify_call_arguments to fix profiledbootstrap

[Uros Bizjak]

Hello!

It looks that it is too hard for the compiler to track deref_align
initialization through dependent deref_base boolean. The patch bellow
fixes "may be used uninitialized" warning that breaks
profiledbootstrap.

2013-09-07  Uros Bizjak  

* ipa-prop.c (ipa_modify_call_arguments): Initialize deref_align.

Tested on x86_64-pc-linux-gnu with LTO profiledbootstrap.

OK for mainline?

Uros.
Index: ipa-prop.c
===
--- ipa-prop.c  (revision 202352)
+++ ipa-prop.c  (working copy)
@@ -3526,7 +3526,7 @@ ipa_modify_call_arguments (struct cgraph_edge *cs,
{
  tree expr, base, off;
  location_t loc;
- unsigned int deref_align;
+ unsigned int deref_align = 0;
  bool deref_base = false;
 
  /* We create a new parameter out of the value of the old one, we can

Re: Fix missed propagation opportunity in DOM

[Andreas Schwab]

Jeff Law  writes:

> +2013-09-06  Jeff Law  
> +
> + * tree-ssa-dom.c (cprop_into_successor_phis): Also propagate
> + edge implied equivalences into successor phis.

This is causing bootstrap miscompare (in gcc/compare-elim.o) on ia64.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC] Fix for PR58201

[Jan Hubicka]

> > >+ 2013-09-04  Jan Hubicka  
> > >+
> > >+  PR middle-end/58201
> > >+  * cgraphunit.c (analyze_functions): Clear AUX fields
> > >+  after processing; initialize assembler name has.
> > >+
> > I checked and double checked and with this commit a C++ test regressed:
> > 
> > FAIL: g++.dg/template/cond2.C -std=gnu++98 (test for warnings, line 9)
> > FAIL: g++.dg/template/cond2.C -std=gnu++11 (test for warnings, line 9)
> > 
> > In practice we emit a message off by one line, line 10 instead of
> > the correct line 9. Is it possible that you are clearing too much,
> > so to speak?
> 
> Amazing, this testcase was triggering an ICE and I had to disable 
> initialization
> of assembler name hash when error arrived.  That fixed the problem, but I did 
> not
> notice it is still having wrong line info.
> 
> Adding an alias into the unit will trigger wrong line info before my change, 
> since
> assembler name hash is populated too.
> 
> The error is output from DECL_ASSEMBLER_NAME hook that by itself is strange.
> 
> #0  0x01041490 in error(char const*, ...) ()
> #1  0x00735039 in write_expression(tree_node*) () at 
> ../../gcc/cp/mangle.c:3008
> #2  0x0073a70f in write_template_arg(tree_node*) () at 
> ../../gcc/cp/mangle.c:3179
> #3  0x0073ae69 in write_template_args(tree_node*) () at 
> ../../gcc/cp/mangle.c:2551
> #4  0x007332a4 in write_name(tree_node*, int) () at 
> ../../gcc/cp/mangle.c:821
> #5  0x0073700c in write_type(tree_node*) () at 
> ../../gcc/cp/mangle.c:2522
> #6  0x00737279 in write_type(tree_node*) () at 
> ../../gcc/cp/mangle.c:2017
> #7  0x00738a18 in write_method_parms(tree_node*, int, tree_node*) () 
> at ../../gcc/cp/mangle.c:2509
> #8  0x00738e2f in write_bare_function_type(tree_node*, int, 
> tree_node*) () at ../../gcc/cp/mangle.c:2451
> #9  0x00733b9e in write_mangled_name(tree_node*, bool) () at 
> ../../gcc/cp/mangle.c:689
> #10 0x0073c5b6 in mangle_decl_string(tree_node*) () at 
> ../../gcc/cp/mangle.c:3446
> #11 0x0073c7e9 in mangle_decl(tree_node*) () at 
> ../../gcc/cp/mangle.c:3468
> #12 0x00d1df01 in decl_assembler_name(tree_node*) () at 
> ../../gcc/tree.c:546
> #13 0x0083b1f5 in insert_to_assembler_name_hash(symtab_node_def*, 
> bool) ()
> #14 0x0083b352 in symtab_initialize_asm_name_hash() [clone .part.3] ()

In the tree prior my change:
jh@gcc10:~/trunk/build4/gcc$ ./xgcc -B ./ -O2 
../../gcc/testsuite/g++.dg/template/cond2.C 
../../gcc/testsuite/g++.dg/template/cond2.C: In instantiation of 'int test(c<(X 
?  :  Y)>&) [with int X = 0; int Y = 2]':
../../gcc/testsuite/g++.dg/template/cond2.C:9:17:   required from here
../../gcc/testsuite/g++.dg/template/cond2.C:6:28: error: omitted middle operand 
to '?:' operand cannot be mangled
 template int test(c&); // { dg-error "omitted" }
^
jh@gcc10:~/trunk/build4/gcc$ ./xgcc -B ./ -O2 
../../gcc/testsuite/g++.dg/template/cond2.C  -flto
../../gcc/testsuite/g++.dg/template/cond2.C: In instantiation of 'int test(c<(X 
?  :  Y)>&) [with int X = 0; int Y = 2]':
../../gcc/testsuite/g++.dg/template/cond2.C:6:28:   required from here
../../gcc/testsuite/g++.dg/template/cond2.C:6:28: error: omitted middle operand 
to '?:' operand cannot be mangled
 template int test(c&); // { dg-error "omitted" }

So it is just an accident that the line info is output sanely (if line 9 is
sane, I don't exactly know)

What I can think of is to hide the stale source location when it no longer have
defined meaning:
Index: cgraphunit.c
===
--- cgraphunit.c(revision 202352)
+++ cgraphunit.c(working copy)
@@ -913,6 +913,7 @@ analyze_functions (void)
 
   bitmap_obstack_initialize (NULL);
   cgraph_state = CGRAPH_STATE_CONSTRUCTION;
+  input_location = UNKNOWN_LOCATION;
 
   /* Ugly, but the fixup can not happen at a time same body alias is created;
  C++ FE is confused about the COMDAT groups being right.  */

but of course this just leads to:
jh@gcc10:~/trunk/build7/gcc$ ./xgcc -B ./ -O2 
../../gcc/testsuite/g++.dg/template/cond2.C
../../gcc/testsuite/g++.dg/template/cond2.C: In instantiation of 'int test(c<(X 
?  :  Y)>&) [with int X = 0; int Y = 2]':
:0:0:   required from here
../../gcc/testsuite/g++.dg/template/cond2.C:6:28: error: omitted middle operand 
to '?:' operand cannot be mangled
 template int test(c&); // { dg-error "omitted" }

So i think it is up to C++ FE to correctly set and restore location info if it
wants to have error output correctly.  DECL_SOURCE_LOCATION of the decl being
mangled is 6 that leads to useless mesage:

jh@gcc10:~/trunk/build7/gcc$ ./xgcc -B ./ -O2 
../../gcc/testsuite/g++.dg/template/cond2.C
../../gcc/testsuite/g++.dg/template/cond2.C: In instantiation of 'int test(c<(X 
?  :  Y)>&) [with int X = 0; int Y = 2]':
../../gcc/testsuite/g++.dg/template/cond2.C:6:28:   required from here
../../gcc/testsui

Re: [RFC] Fix for PR58201

[Paolo Carlini]

Hi Honza,

and thanks for the analysis, now I understand the issue a little more.

On 09/07/2013 10:28 AM, Jan Hubicka wrote:
So it is just an accident that the line info is output sanely (if line 
9 is sane, I don't exactly know)
I would say that in general it's definitely sane, because that is the 
instantiation point. And 10 is wrong, too late, it points to the closing 
bracket.


However, I notice that clang doesn't even try to output a message having 
to do with line 9: if I understand correctly, if that operator cannot be 
mangled, that happens unconditionally, isn't something that may succeed. 
Thus I wonder if, instead of outputting garbage line numbers we could at 
least suppress in such cases the whole "In instantiation of..." part of 
the diagnostic, it would be better than garbage. Do we have a mechanism 
for that?


Now I also understand that this should be a very uncommon error message, 
but, I'm wondering, is it possible that other errors, for other issues, 
are also affected? That is, other diagnostic happening very late and 
sensitive to the recent rework?


Paolo.

Re: [RFC] Fix for PR58201

[Jan Hubicka]

> Hi Honza,
> 
> and thanks for the analysis, now I understand the issue a little more.
> 
> On 09/07/2013 10:28 AM, Jan Hubicka wrote:
> >So it is just an accident that the line info is output sanely (if
> >line 9 is sane, I don't exactly know)
> I would say that in general it's definitely sane, because that is
> the instantiation point. And 10 is wrong, too late, it points to the
> closing bracket.
> 
> However, I notice that clang doesn't even try to output a message
> having to do with line 9: if I understand correctly, if that
> operator cannot be mangled, that happens unconditionally, isn't
> something that may succeed. Thus I wonder if, instead of outputting
> garbage line numbers we could at least suppress in such cases the
> whole "In instantiation of..." part of the diagnostic, it would be
> better than garbage. Do we have a mechanism for that?

It seems to be what older GCCs are doing, too.
> 
> Now I also understand that this should be a very uncommon error
> message, but, I'm wondering, is it possible that other errors, for
> other issues, are also affected? That is, other diagnostic happening
> very late and sensitive to the recent rework?

Any use of source_location from DECL_ASSEMBLER_NAME hanghook is wrong, since
source_location may point anywhere when the lazy mangling is triggered.  It
surprised me that DECL_ASSEMBLER_NAME triggers error/warning messages.  It is
up to backend when on at what symbols it will call it, so it is definitely
source of inconsistency.  So from design point of view I would preffer C++
FE to output these errors somewhere else.  But practicaly we can perhaps just
modify the message to not expect thelocation?

I am just leaving for flight to Pisa, will be back online at the hotel.

Honza

RFA: Fix debug-insn sensitivity in RA

[Richard Sandiford]

While doing some work on wide-int, I hit a bootstrap comparison failure that
was caused by the RA output depending on debug insns.  There was a register R
that was used normally by all "real" insns, but which was used in a paradoxical
subreg by one of the debug insns.  There was also a natural equivalence
between R and a memory.  The problem was that the paradoxical subreg in
the debug insn was stopping that equivalence from being used.

The problem seems to be split across IRA and LRA.  In IRA we have:

  FOR_EACH_BB (bb)
FOR_BB_INSNS (bb, insn)
  {
if (! INSN_P (insn))
  continue;
for_each_rtx (&insn, set_paradoxical_subreg, (void *)pdx_subregs);
  }

which sets pdx_subregs[R] if R is used in a paradoxical subreg, followed by:

  /* Don't set reg (if pdx_subregs[regno] == true) equivalent to a mem. 
 */
  if (MEM_P (src) && pdx_subregs[regno])
{
  note_stores (set, no_equiv, NULL);
  continue;
}

I think this should only happen when a paradoxical subreg is seen
in a nondebug insn.  If we use the equivalence when a debug insn has
a paradoxical subreg, the subreg will be replaced with a MEM that might
overlap surrounding data, but I think it's a valid transformation when
no run-time access will take place.  The data in the defined part of
the MEM should still be correct.

Then in LRA we keep track of the biggest mode that is used to refer to each
register (including references via paradoxical subregs).  This is done when
recording register references in an instruction:

static struct lra_insn_reg *
new_insn_reg (int regno, enum op_type type, enum machine_mode mode,
  bool subreg_p, bool early_clobber, struct lra_insn_reg *next)
{
  struct lra_insn_reg *ir;

  ir = (struct lra_insn_reg *) pool_alloc (insn_reg_pool);
  ir->type = type;
  ir->biggest_mode = mode;
  if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (lra_reg_info[regno].biggest_mode))
lra_reg_info[regno].biggest_mode = mode;

This then affects allocation decisions in several places.  Here too I think
we need to guard for nondebug insns, although we still need to record
register references for debug insns.

This of course means that when substituting the allocation into the debug
insns, we have to cope with paradoxical subregs that are wider than the
largest recorded mode.  I don't think that's a new requirement though,
since we needed the same thing for reload.  AFAIK the existing code
should already handle it.

The patch below fixes the wide-int problem for me.  Bootstrapped &
regression-tested on trunk for x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
* ira.c (update_equiv_regs): Only call set_paradoxical_subreg
for non-debug insns.
* lra.c (new_insn_reg): Take the containing insn as a parameter.
Only modify lra_reg_info[].biggest_mode if it's non-debug insn.
(collect_non_operand_hard_regs, add_regs_to_insn_regno_info): Update
accordingly.

Index: gcc/ira.c
===
--- gcc/ira.c   2013-09-01 12:39:40.739835911 +0100
+++ gcc/ira.c   2013-09-07 09:42:34.993934881 +0100
@@ -2944,11 +2944,8 @@ update_equiv_regs (void)
  prevent access beyond allocated memory for paradoxical memory subreg.  */
   FOR_EACH_BB (bb)
 FOR_BB_INSNS (bb, insn)
-  {
-   if (! INSN_P (insn))
- continue;
-   for_each_rtx (&insn, set_paradoxical_subreg, (void *)pdx_subregs);
-  }
+  if (NONDEBUG_INSN_P (insn))
+   for_each_rtx (&insn, set_paradoxical_subreg, (void *) pdx_subregs);
 
   /* Scan the insns and find which registers have equivalences.  Do this
  in a separate scan of the insns because (due to -fcse-follow-jumps)
Index: gcc/lra.c
===
--- gcc/lra.c   2013-07-17 08:36:09.260013240 +0100
+++ gcc/lra.c   2013-09-07 09:42:35.004934955 +0100
@@ -480,13 +480,13 @@ init_insn_regs (void)
 = create_alloc_pool ("insn regs", sizeof (struct lra_insn_reg), 100);
 }
 
-/* Create LRA insn related info about referenced REGNO with TYPE
-   (in/out/inout), biggest reference mode MODE, flag that it is
+/* Create LRA insn related info about a reference to REGNO in INSN with
+   TYPE (in/out/inout), biggest reference mode MODE, flag that it is
reference through subreg (SUBREG_P), flag that is early clobbered
in the insn (EARLY_CLOBBER), and reference to the next insn reg
info (NEXT). */
 static struct lra_insn_reg *
-new_insn_reg (int regno, enum op_type type, enum machine_mode mode,
+new_insn_reg (rtx insn, int regno, enum op_type type, enum machine_mode mode,
  bool subreg_p, bool early_clobber, struct lra_insn_reg *next)
 {
   struct lra_insn_reg *ir;
@@ -494,7 +494,8 @@ new_insn_reg (int regno, enum op_type ty
   ir = (struct lra_insn_reg *) pool_alloc (insn_reg_pool);
   ir->type = type;
   ir->biggest_mode = mode;

Re: [RFC] Changes to the wide-int classes

[Richard Sandiford]

Richard Sandiford  writes:
>> I've looked at the resulting wide-int.h and like it a lot
>> compared to what is on the branch (less code duplication for one).
>>
>> I think we should go ahead with this change (keeping the double-int
>> changes out for now, I didn't yet look at that patch).  We can
>> iterate on the details on the branch.
>
> Thanks.  Kenny also asked me to commit it on IRC, so I was going to go
> ahead.  But I just tried boostrapping the patch again after Mike's
> recent merge with trunk, and it now fails during stage 2 with:

That turned out to be because of the __optimize__(0)s that I'd added
to work around the RA bug.  After checking with Kenny, I've now committed
both the RA patch and this patch to the branch.

Thanks,
Richard

Re: [RFC] Fix for PR58201

[Paolo Carlini]

Hi

>What I can think of is to hide the stale source location when it no
>longer have
>defined meaning:
>Index: cgraphunit.c
>===
>--- cgraphunit.c(revision 202352)
>+++ cgraphunit.c(working copy)
>@@ -913,6 +913,7 @@ analyze_functions (void)
>
>   bitmap_obstack_initialize (NULL);
>   cgraph_state = CGRAPH_STATE_CONSTRUCTION;
>+  input_location = UNKNOWN_LOCATION;
>
>/* Ugly, but the fixup can not happen at a time same body alias is
>created;
>  C++ FE is confused about the COMDAT groups being right.  */
>
>but of course this just leads to:
>jh@gcc10:~/trunk/build7/gcc$ ./xgcc -B ./ -O2
>../../gcc/testsuite/g++.dg/template/cond2.C
>../../gcc/testsuite/g++.dg/template/cond2.C: In instantiation of 'int
>test(c<(X ?  :  Y)>&) [with int X = 0; int Y = 2]':
>:0:0:   required from here
>../../gcc/testsuite/g++.dg/template/cond2.C:6:28: error: omitted middle
>operand to '?:' operand cannot be mangled
>template int test(c&); // { dg-error "omitted" }

Actually this kind of change makes a lot of sense to me (cmp clang too): since 
at that point we do *not* really know the location of the "required from" bit, 
just plainly admit it. Would it be possible in such cases to have a conditional 
in the diagnostic machinery and avoid the output of that bit completely when 
the location is UNKNOWN? I don't think there are existing cases where it's sane 
to say :0:0 as "required from"!?!

Short term at least, it would seem a good enough solution to me + a comment in 
testcase too.

Then indeed as you explained in your last message before leaving (enjoy Pisa! 
All in all I spent there ~10 years!) we should audit, avoid as much as possible 
such late diagnostic, etc.

What do you think?

Paolo

operator new returns nonzero

[Marc Glisse]

Hello,

this patch teaches the compiler that operator new, when it can throw, 
isn't allowed to return a null pointer. Note that it doesn't work for 
placement new (that one may have to be handled in the front-end or the 
inliner), and it will not work on user-defined operator new if they are 
inlined. I guess it would be possible to teach the inliner to insert an 
assert_expr or something when inlining such a function, but that's not for 
now. The code I removed from vrp_visit_stmt was duplicated in 
stmt_interesting_for_vrp and thus useless.


I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper 
testing when trunk bootstraps again.


2013-09-07  Marc Glisse  

PR c++/19476
gcc/
* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
Likewise.
(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
* g++.dg/tree-ssa/pr19476-1.C: New file.
* g++.dg/tree-ssa/pr19476-2.C: Likewise.

--
Marc GlisseIndex: fold-const.c
===
--- fold-const.c(revision 202351)
+++ fold-const.c(working copy)
@@ -16171,21 +16171,29 @@ tree_expr_nonzero_warnv_p (tree t, bool
 case MODIFY_EXPR:
 case BIND_EXPR:
   return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
strict_overflow_p);
 
 case SAVE_EXPR:
   return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
strict_overflow_p);
 
 case CALL_EXPR:
-  return alloca_call_p (t);
+  {
+   tree fn = CALL_EXPR_FN (t);
+   if (TREE_CODE (fn) != ADDR_EXPR) return false;
+   tree fndecl = TREE_OPERAND (fn, 0);
+   if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
+   if (DECL_IS_OPERATOR_NEW (fndecl) && !TREE_NOTHROW (fndecl))
+ return true;
+   return alloca_call_p (t);
+  }
 
 default:
   break;
 }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
Handle warnings about undefined signed overflow.  */
 
Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-1.C   (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C   (revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include 
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-2.C   (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C   (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include 
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___
Added: svn:eol-style
   + native
Added: svn:keywords
   + Author Date Id Revision URL

Index: tree-vrp.c
===
--- tree-vrp.c  (revision 202351)
+++ tree-vrp.c  (working copy)
@@ -1047,21 +1047,27 @@ gimple_assign_nonzero_warnv_p (gimple st
*STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
 {
 case GIMPLE_ASSIGN:
   return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
 case GIMPLE_CALL:
-  return gimple_alloca_call_p (stmt);
+  {
+   tree fndecl = gimple_call_fndecl (stmt);
+   if (!fndecl) return false;
+   if (DECL_IS_OPERATOR_NEW (fndecl) && !TREE_NOTHROW (fndecl))
+ return true;
+   return gimple_alloca_call_p (stmt);
+  }
 default:
   gcc_unreachable ();
 }
 }
 
 /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
obtained so far.  */
 
 static bool
 vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
@@ -6486,21 +6492,22 @@ stmt_interesting_for_vrp (gimple stmt)
   tree lhs = gimple_get

Re: [RFC] Fix for PR58201

[Jakub Jelinek]

On Sat, Sep 07, 2013 at 11:00:22AM +0200, Paolo Carlini wrote:
> and thanks for the analysis, now I understand the issue a little more.
> 
> On 09/07/2013 10:28 AM, Jan Hubicka wrote:
> >So it is just an accident that the line info is output sanely (if
> >line 9 is sane, I don't exactly know)
> I would say that in general it's definitely sane, because that is
> the instantiation point. And 10 is wrong, too late, it points to the
> closing bracket.
> 
> However, I notice that clang doesn't even try to output a message
> having to do with line 9: if I understand correctly, if that
> operator cannot be mangled, that happens unconditionally, isn't
> something that may succeed. Thus I wonder if, instead of outputting
> garbage line numbers we could at least suppress in such cases the
> whole "In instantiation of..." part of the diagnostic, it would be
> better than garbage. Do we have a mechanism for that?
> 
> Now I also understand that this should be a very uncommon error
> message, but, I'm wondering, is it possible that other errors, for
> other issues, are also affected? That is, other diagnostic happening
> very late and sensitive to the recent rework?

As I wrote in the PR, IMHO mangle_decl should
  location_t save_location = input_location;
  input_location = DECL_SOURCE_LOCATION (decl);
...
  input_location = save_location;
around the call, and perhaps also all the error calls inside of
mangle.c should be actually error_at (EXPR_LOC_OR_HERE (expr), ...),
so that if expr has locus, it will print it at that point, otherwise
will use locus of the decl.

Jakub

Re: RFA: Fix debug-insn sensitivity in RA

[Steven Bosscher]

On Sat, Sep 7, 2013 at 11:14 AM, Richard Sandiford wrote:
> The problem seems to be split across IRA and LRA.  In IRA we have:
>
>   FOR_EACH_BB (bb)
> FOR_BB_INSNS (bb, insn)
>   {
> if (! INSN_P (insn))
>   continue;
> for_each_rtx (&insn, set_paradoxical_subreg, (void *)pdx_subregs);
>   }
>
> which sets pdx_subregs[R] if R is used in a paradoxical subreg, followed by:
>
>   /* Don't set reg (if pdx_subregs[regno] == true) equivalent to a 
> mem.  */
>   if (MEM_P (src) && pdx_subregs[regno])
> {
>   note_stores (set, no_equiv, NULL);
>   continue;
> }
>
> I think this should only happen when a paradoxical subreg is seen
> in a nondebug insn.

Agreed. Otherwise these debug insns would have influence on the
generated code and that's not supposed to happen.


> The patch below fixes the wide-int problem for me.  Bootstrapped &
> regression-tested on trunk for x86_64-linux-gnu.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> * ira.c (update_equiv_regs): Only call set_paradoxical_subreg
> for non-debug insns.
> * lra.c (new_insn_reg): Take the containing insn as a parameter.
> Only modify lra_reg_info[].biggest_mode if it's non-debug insn.
> (collect_non_operand_hard_regs, add_regs_to_insn_regno_info): Update
> accordingly.

I don't think this falls under "RTL optimizers", but the patch looks OK to me.

Can you please add a test case?

Ciao!
Steven

Re: RFA: Fix debug-insn sensitivity in RA

[Richard Sandiford]

Steven Bosscher  writes:
> Can you please add a test case?

What kind of test would you suggest?  Do we have a harness for testing
that -O2 and -O2 -g .text output is identical?

I suppose the main testcase will be bootstrap if wide-int ever does get
accepted.

Thanks,
Richard

Re: RFA: Fix debug-insn sensitivity in RA

[Steven Bosscher]

On Sat, Sep 7, 2013 at 1:37 PM, Richard Sandiford wrote:
> Steven Bosscher  writes:
>> Can you please add a test case?
>
> What kind of test would you suggest?  Do we have a harness for testing
> that -O2 and -O2 -g .text output is identical?


Not .text, but the assembly output or the final RTL dumps. This is
what the guality test framework does.

Ciao!
Steven

Re: RFA: Fix debug-insn sensitivity in RA

[Jakub Jelinek]

On Sat, Sep 07, 2013 at 12:37:14PM +0100, Richard Sandiford wrote:
> Steven Bosscher  writes:
> > Can you please add a test case?
> 
> What kind of test would you suggest?  Do we have a harness for testing
> that -O2 and -O2 -g .text output is identical?

No, but we have -fcompare-debug, which is even stronger than that (compares
final RTL, so can catch even differences that aren't visible in the .s file,
but still are relevant).  And, the default way of bootstrapping also
compares .text etc. after stripping, between stage2 (built without debug)
and stage3 (with debug).

Jakub

Re: operator new returns nonzero

[Basile Starynkevitch]

On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote:
> Hello,
> 
> this patch teaches the compiler that operator new, when it can throw, 
> isn't allowed to return a null pointer. Note that it doesn't work for 
> placement new (that one may have to be handled in the front-end or the 
> inliner), and it will not work on user-defined operator new if they are 
> inlined. I guess it would be possible to teach the inliner to insert an 
> assert_expr or something when inlining such a function, but that's not for 
> now. The code I removed from vrp_visit_stmt was duplicated in 
> stmt_interesting_for_vrp and thus useless.


Interesting patch. However, I feel that we should give advanced users
the ability to say that their new operator is never returning null...
In particular, if I define my own new operator which never returns nil,
I find strange that it would be less optimized than the system's
operator new.

Perhaps we need an attribute `nonnullresult'  which whould means that.
(we already the nonnull attribute for function arguments)


Cheers.
-- 
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 6:44 AM, Basile Starynkevitch
 wrote:
> On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote:
>> Hello,
>>
>> this patch teaches the compiler that operator new, when it can throw,
>> isn't allowed to return a null pointer. Note that it doesn't work for
>> placement new (that one may have to be handled in the front-end or the
>> inliner), and it will not work on user-defined operator new if they are
>> inlined. I guess it would be possible to teach the inliner to insert an
>> assert_expr or something when inlining such a function, but that's not for
>> now. The code I removed from vrp_visit_stmt was duplicated in
>> stmt_interesting_for_vrp and thus useless.
>
>
> Interesting patch. However, I feel that we should give advanced users
> the ability to say that their new operator is never returning null...
> In particular, if I define my own new operator which never returns nil,
> I find strange that it would be less optimized than the system's
> operator new.
>
> Perhaps we need an attribute `nonnullresult'  which whould means that.
> (we already the nonnull attribute for function arguments)

'operator new' is a replaceable function, so when you define it, you have
to abide by the standard semantics.

-- Gaby

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Basile Starynkevitch wrote:


On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote:

Hello,

this patch teaches the compiler that operator new, when it can throw,
isn't allowed to return a null pointer. Note that it doesn't work for
placement new (that one may have to be handled in the front-end or the
inliner), and it will not work on user-defined operator new if they are
inlined. I guess it would be possible to teach the inliner to insert an
assert_expr or something when inlining such a function, but that's not for
now. The code I removed from vrp_visit_stmt was duplicated in
stmt_interesting_for_vrp and thus useless.



Interesting patch. However, I feel that we should give advanced users
the ability to say that their new operator is never returning null...


Easy, don't specify noexcept on it and that's what the patch already does, 
as long as the function is not inlined.



In particular, if I define my own new operator which never returns nil,
I find strange that it would be less optimized than the system's
operator new.

Perhaps we need an attribute `nonnullresult'  which whould means that.
(we already the nonnull attribute for function arguments)


There is already a PR about that, linked from this PR. But even if we 
create this attribute, we will still want to be able to guess that new 
should have it even if it isn't written.


--
Marc Glisse

Re: RFA: Fix debug-insn sensitivity in RA

[Richard Sandiford]

Jakub Jelinek  writes:
> On Sat, Sep 07, 2013 at 12:37:14PM +0100, Richard Sandiford wrote:
>> Steven Bosscher  writes:
>> > Can you please add a test case?
>> 
>> What kind of test would you suggest?  Do we have a harness for testing
>> that -O2 and -O2 -g .text output is identical?
>
> No, but we have -fcompare-debug, which is even stronger than that (compares
> final RTL, so can catch even differences that aren't visible in the .s file,
> but still are relevant).  And, the default way of bootstrapping also
> compares .text etc. after stripping, between stage2 (built without debug)
> and stage3 (with debug).

Right, the bootstrap thing is what forced me to write the patch in
the first place.  I probably wasn't clear, sorry, but the original
problem was a bootstrap comparison failure between stage 2 and stage 3
because the tree.o code depended on whether debug info was enabled.

I'll try to use -fcompare-debug though, thanks.

Richard

Re: RFA: Fix debug-insn sensitivity in RA

[Richard Sandiford]

Jakub Jelinek  writes:
> On Sat, Sep 07, 2013 at 12:37:14PM +0100, Richard Sandiford wrote:
>> Steven Bosscher  writes:
>> > Can you please add a test case?
>> 
>> What kind of test would you suggest?  Do we have a harness for testing
>> that -O2 and -O2 -g .text output is identical?
>
> No, but we have -fcompare-debug, which is even stronger than that (compares
> final RTL, so can catch even differences that aren't visible in the .s file,
> but still are relevant).  And, the default way of bootstrapping also
> compares .text etc. after stripping, between stage2 (built without debug)
> and stage3 (with debug).

Here's the patch again with an -fcompare-debug test.  I spent a while
trying to whittle it down, but beyond this it gets very sensitive.

OK to install?

Richard


gcc/
* ira.c (update_equiv_regs): Only call set_paradoxical_subreg
for non-debug insns.
* lra.c (new_insn_reg): Take the containing insn as a parameter.
Only modify lra_reg_info[].biggest_mode if it's non-debug insn.
(collect_non_operand_hard_regs, add_regs_to_insn_regno_info): Update
accordingly.

gcc/testsuite/
* g++.dg/debug/ra1.C: New test.

Index: gcc/ira.c
===
--- gcc/ira.c   2013-09-07 13:59:23.049113927 +0100
+++ gcc/ira.c   2013-09-07 13:59:26.362142367 +0100
@@ -2944,11 +2944,8 @@ update_equiv_regs (void)
  prevent access beyond allocated memory for paradoxical memory subreg.  */
   FOR_EACH_BB (bb)
 FOR_BB_INSNS (bb, insn)
-  {
-   if (! INSN_P (insn))
- continue;
-   for_each_rtx (&insn, set_paradoxical_subreg, (void *)pdx_subregs);
-  }
+  if (NONDEBUG_INSN_P (insn))
+   for_each_rtx (&insn, set_paradoxical_subreg, (void *) pdx_subregs);
 
   /* Scan the insns and find which registers have equivalences.  Do this
  in a separate scan of the insns because (due to -fcse-follow-jumps)
Index: gcc/lra.c
===
--- gcc/lra.c   2013-09-07 13:59:23.049113927 +0100
+++ gcc/lra.c   2013-09-07 13:59:26.363142375 +0100
@@ -480,13 +480,13 @@ init_insn_regs (void)
 = create_alloc_pool ("insn regs", sizeof (struct lra_insn_reg), 100);
 }
 
-/* Create LRA insn related info about referenced REGNO with TYPE
-   (in/out/inout), biggest reference mode MODE, flag that it is
+/* Create LRA insn related info about a reference to REGNO in INSN with
+   TYPE (in/out/inout), biggest reference mode MODE, flag that it is
reference through subreg (SUBREG_P), flag that is early clobbered
in the insn (EARLY_CLOBBER), and reference to the next insn reg
info (NEXT). */
 static struct lra_insn_reg *
-new_insn_reg (int regno, enum op_type type, enum machine_mode mode,
+new_insn_reg (rtx insn, int regno, enum op_type type, enum machine_mode mode,
  bool subreg_p, bool early_clobber, struct lra_insn_reg *next)
 {
   struct lra_insn_reg *ir;
@@ -494,7 +494,8 @@ new_insn_reg (int regno, enum op_type ty
   ir = (struct lra_insn_reg *) pool_alloc (insn_reg_pool);
   ir->type = type;
   ir->biggest_mode = mode;
-  if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (lra_reg_info[regno].biggest_mode))
+  if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (lra_reg_info[regno].biggest_mode)
+  && NONDEBUG_INSN_P (insn))
 lra_reg_info[regno].biggest_mode = mode;
   ir->subreg_p = subreg_p;
   ir->early_clobber = early_clobber;
@@ -976,7 +977,7 @@ collect_non_operand_hard_regs (rtx *x, l
 && ! (FIRST_STACK_REG <= regno
   && regno <= LAST_STACK_REG));
 #endif
-   list = new_insn_reg (regno, type, mode, subreg_p,
+   list = new_insn_reg (data->insn, regno, type, mode, subreg_p,
 early_clobber, list);
  }
  }
@@ -1575,7 +1576,7 @@ add_regs_to_insn_regno_info (lra_insn_re
   expand_reg_info ();
   if (bitmap_set_bit (&lra_reg_info[regno].insn_bitmap, uid))
{
- data->regs = new_insn_reg (regno, type, mode, subreg_p,
+ data->regs = new_insn_reg (data->insn, regno, type, mode, subreg_p,
 early_clobber, data->regs);
  return;
}
@@ -1587,8 +1588,9 @@ add_regs_to_insn_regno_info (lra_insn_re
if (curr->subreg_p != subreg_p || curr->biggest_mode != mode)
  /* The info can not be integrated into the found
 structure.  */
- data->regs = new_insn_reg (regno, type, mode, subreg_p,
-early_clobber, data->regs);
+ data->regs = new_insn_reg (data->insn, regno, type, mode,
+subreg_p, early_clobber,
+data->regs);
else
  {
if (curr->type != type)
Index: gcc/testsuite/g++.dg/debug/ra1.

Re: [RFC] Fix for PR58201

[Jason Merrill]

On 09/07/2013 06:13 AM, Paolo Carlini wrote:

Actually this kind of change makes a lot of sense to me (cmp clang too): since at that point we do 
*not* really know the location of the "required from" bit, just plainly admit it. Would 
it be possible in such cases to have a conditional in the diagnostic machinery and avoid the output 
of that bit completely when the location is UNKNOWN? I don't think there are existing cases where 
it's sane to say :0:0 as "required from"!?!


That does make sense.  We should disable "required from" for 
UNKNOWN_LOCATION.


Jason

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Marc Glisse wrote:

this patch teaches the compiler that operator new, when it can throw, isn't 
allowed to return a null pointer. Note that it doesn't work for placement new 
(that one may have to be handled in the front-end or the inliner),


Placement new is a completely different thing. For one, it is nothrow (so 
the test doesn't apply). But mostly, it is a condition on the argument 
more than the result. Using the nonnull attribute would make sense, but 
the compiler doesn't seem clever enough to use that information. The 
easiest seems to be in the library:


--- o/new   2013-09-07 11:11:17.388756320 +0200
+++ i/new   2013-09-07 18:00:32.204797291 +0200
@@ -144,9 +144,9 @@

 // Default placement versions of operator new.
 inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }
 inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }

 // Default placement versions of operator delete.
 inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }



Though I am not sure what the policy is for this kind of thing. And that's 
assuming it is indeed undefined to pass a null pointer to it, I don't have 
a good reference for that.




and it will not work on user-defined operator new if they are inlined.


But then if that operator new is inlined, it may already contain a line 
like if(p==0)throw(); which lets the compiler know all it needs.


I guess it 
would be possible to teach the inliner to insert an assert_expr or something 
when inlining such a function, but that's not for now. The code I removed 
from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and thus 
useless.


I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper 
testing when trunk bootstraps again.


2013-09-07  Marc Glisse  

PR c++/19476
gcc/
* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
Likewise.
(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
* g++.dg/tree-ssa/pr19476-1.C: New file.
* g++.dg/tree-ssa/pr19476-2.C: Likewise.


--
Marc Glisse

Re: [PATCH] Fix PR58300, issue with -fvtable-verify=preinit

[Jason Merrill]

OK.

Jason

Re: Ping: RFA: Consider int and same-size short as equivalent vector components

[Jason Merrill]

On 09/06/2013 08:58 PM, Joern Rennecke wrote:

vector_targets_convertible_p is used for pointer types.  The callers
do a hop, skip and dance to check that the qualifiers are satisfactory,
while OTOH vector_targets_convertible_p ignores the number of elements
in the vectors.  That's fine with vectors as we can consider, say,
a vector of 8 elements as two consecutive vectors of 4 elements, and
that works fine with pointers.

vector_types_convertible is used for pointer value types.
It could in principle call vector_targets_convertible_p as a subroutine,
but then the check for vector type would be duplicated with its callers,
and also the purpose of vector_targets_convertible_p would become
muddled.
Where vector_types_convertible returns true, a conversion might still be
needed to make the types match.

vector_types_compatible_elements_p was supposed to be a minimal change
from same_scalar_type_ignoring_signedness to fix the treatment of
opaque vectors in binary operators.  Where it returns true, and
the other checks of the caller succeed (being vector types in he first
place, and matching number of elements), we can just treat the types
as essentially the same.


Your earlier patch is OK if you add this information to the comments for 
the various functions and have them mention each other.


Jason

Re: [PATCH, C++, PR58282] Handle noexcept on transactions with -fno-exceptions

[Jason Merrill]

OK.

Jason

Re: [c++-concepts] Class template constraints

[Jason Merrill]

On 09/06/2013 12:03 PM, Andrew Sutton wrote:

+// Returns the template type of the class scope being entered. If we're
+// entering a constrained class scope. TMPL is the most general template
+// of the scope being entered, and TYPE is its type.


TMPL is not part of the interface of fixup_template_type, so it should 
be documented when it is declared rather than before the function.


OK with that tweak.


+  tree cur_constr = TREE_TYPE (parms);


In a separate patch, I'd like to use a different macro name for getting 
constraints from template parms.


Jason

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse  wrote:
> On Sat, 7 Sep 2013, Marc Glisse wrote:
>
>> this patch teaches the compiler that operator new, when it can throw,
>> isn't allowed to return a null pointer. Note that it doesn't work for
>> placement new (that one may have to be handled in the front-end or the
>> inliner),
>
>
> Placement new is a completely different thing. For one, it is nothrow (so
> the test doesn't apply). But mostly, it is a condition on the argument more
> than the result. Using the nonnull attribute would make sense, but the
> compiler doesn't seem clever enough to use that information. The easiest
> seems to be in the library:
>
> --- o/new   2013-09-07 11:11:17.388756320 +0200
> +++ i/new   2013-09-07 18:00:32.204797291 +0200
> @@ -144,9 +144,9 @@
>
>  // Default placement versions of operator new.
>  inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
> -{ return __p; }
> +{ if (!__p) __builtin_unreachable(); return __p; }
>  inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
> -{ return __p; }
> +{ if (!__p) __builtin_unreachable(); return __p; }
>
>  // Default placement versions of operator delete.
>  inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
>
>
>
> Though I am not sure what the policy is for this kind of thing. And that's
> assuming it is indeed undefined to pass a null pointer to it, I don't have a
> good reference for that.
>
>
>
>> and it will not work on user-defined operator new if they are inlined.
>
>
> But then if that operator new is inlined, it may already contain a line like
> if(p==0)throw(); which lets the compiler know all it needs.

I am not sure I like this version of the patch.

I think the appropriate patch should be in the compiler, not
here.  C++ has several places where certain parameters cannot
have non-null values. For example, the implicit parameter 'this'
in non-static member functions. This is another instance.

Furthermore, I do think that the compiler should have special nodes
for both standard placement new and the global operator new functions,
as I explained in previous messages.

-- Gaby

>
>
>> I guess it would be possible to teach the inliner to insert an assert_expr
>> or something when inlining such a function, but that's not for now. The code
>> I removed from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and
>> thus useless.
>>
>> I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper
>> testing when trunk bootstraps again.
>>
>> 2013-09-07  Marc Glisse  
>>
>> PR c++/19476
>> gcc/
>> * fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
>> * tree-vrp.c (gimple_stmt_nonzero_warnv_p,
>> stmt_interesting_for_vrp):
>> Likewise.
>> (vrp_visit_stmt): Remove duplicated code.
>>
>> gcc/testsuite/
>> * g++.dg/tree-ssa/pr19476-1.C: New file.
>> * g++.dg/tree-ssa/pr19476-2.C: Likewise.
>
>
> --
> Marc Glisse

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Gabriel Dos Reis wrote:


On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse  wrote:

On Sat, 7 Sep 2013, Marc Glisse wrote:


this patch teaches the compiler that operator new, when it can throw,
isn't allowed to return a null pointer. Note that it doesn't work for
placement new (that one may have to be handled in the front-end or the
inliner),



Placement new is a completely different thing. For one, it is nothrow (so
the test doesn't apply). But mostly, it is a condition on the argument more
than the result. Using the nonnull attribute would make sense, but the
compiler doesn't seem clever enough to use that information. The easiest
seems to be in the library:

--- o/new   2013-09-07 11:11:17.388756320 +0200
+++ i/new   2013-09-07 18:00:32.204797291 +0200
@@ -144,9 +144,9 @@

 // Default placement versions of operator new.
 inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }
 inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }

 // Default placement versions of operator delete.
 inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }



Though I am not sure what the policy is for this kind of thing. And that's
assuming it is indeed undefined to pass a null pointer to it, I don't have a
good reference for that.




and it will not work on user-defined operator new if they are inlined.



But then if that operator new is inlined, it may already contain a line like
if(p==0)throw(); which lets the compiler know all it needs.


I am not sure I like this version of the patch.


The 2 patches are really independent, one (in the compiler) for regular 
operator new, and one (in the library) for the placement version.


I mostly like this second patch because it is so easy ;-)
But I will be happy if someone posts a better solution.


I think the appropriate patch should be in the compiler, not
here.  C++ has several places where certain parameters cannot
have non-null values. For example, the implicit parameter 'this'
in non-static member functions. This is another instance.


Indeed, I didn't check how gcc handles "this" being non-0.


Furthermore, I do think that the compiler should have special nodes
for both standard placement new and the global operator new functions,


That's one way to do it. Since this is the first time I look at those, I 
don't really see the advantage compared to the current status, but I 
trust you. What would you do with this special-node placement new? Keep it 
as is until after vrp so we can use the !=0 property and then expand it to 
its first argument? Or expand it early to the equivalent of the library 
code I wrote above?



as I explained in previous messages.


I couldn't find them, sorry if they contained information that makes this 
post irrelevant.


--
Marc Glisse

Cygwin x86_64 support for GCC 4.8.x

[Vasili Galka]

Hi,

Cygwin x86_64 support was implemented on trunk r197171. Given it is a
minor configuration files change I recommend merging it to GCC 4.8
branch.

Best regards,
Vasili

Re: Cygwin x86_64 support for GCC 4.8.x

[Kai Tietz]

2013/9/7 Vasili Galka :
> Hi,
>
> Cygwin x86_64 support was implemented on trunk r197171. Given it is a
> minor configuration files change I recommend merging it to GCC 4.8
> branch.
>
> Best regards,
> Vasili

No, this won't be back-merged.  It isn't a "minor configuration file change".
The gcc 4.9 series will be the first official gcc supporting cygwin
64-bit target.

Regards,
Kai

Re: Cygwin x86_64 support for GCC 4.8.x

[Vasili Galka]

Hi Kai,

I see. My apologies, I'm not familiar with the GCC merging policy.

In any case, where does GCC document the list of supported
target/host/build configurations?

I was trying to build GCC 4.8.1 with following configuration: HOST &
BUILD: Cygwin 64bit, TARGET=arm-none-eabi

I ended up with successful build/install that generated runtime errors
which I had to investigate (while same on Cygwin 32 was fine).
So it shall be either documented that such configuration is not
supported or (better) it shall generate a warning/error at configure stage.

Best,
Vasili

On Sat, Sep 7, 2013 at 9:37 PM, Kai Tietz  wrote:
> 2013/9/7 Vasili Galka :
>> Hi,
>>
>> Cygwin x86_64 support was implemented on trunk r197171. Given it is a
>> minor configuration files change I recommend merging it to GCC 4.8
>> branch.
>>
>> Best regards,
>> Vasili
>
> No, this won't be back-merged.  It isn't a "minor configuration file change".
> The gcc 4.9 series will be the first official gcc supporting cygwin
> 64-bit target.
>
> Regards,
> Kai

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 3:33 AM, Marc Glisse  wrote:
> this patch teaches the compiler that operator new, when it can throw, isn't 
> allowed to return a null pointer.

You sure:

@item -fcheck-new
@opindex fcheck-new
Check that the pointer returned by @code{operator new} is non-null
before attempting to modify the storage allocated.  This check is
normally unnecessary because the C++ standard specifies that
@code{operator new} only returns @code{0} if it is declared
@samp{throw()}, in which case the compiler always checks the
return value even without this option.  In all other cases, when
@code{operator new} has a non-empty exception specification, memory
exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
@samp{new (nothrow)}.

?

Re: Cygwin x86_64 support for GCC 4.8.x

[Kai Tietz]

Hi Vasili,

2013/9/7 Vasili Galka :
> Hi Kai,
>
> I see. My apologies, I'm not familiar with the GCC merging policy.
>
> In any case, where does GCC document the list of supported
> target/host/build configurations?
>
> I was trying to build GCC 4.8.1 with following configuration: HOST &
> BUILD: Cygwin 64bit, TARGET=arm-none-eabi
>
> I ended up with successful build/install that generated runtime errors
> which I had to investigate (while same on Cygwin 32 was fine).
> So it shall be either documented that such configuration is not
> supported or (better) it shall generate a warning/error at configure stage.
>
> Best,
> Vasili

Why so?  A target is supported when it gets mentioned in
release-notes.   The gcc 4.8.x doesn't support that target.  There is
the 4.8.x version cygwin uses for building cygwin 64-bit.  The used
gcc version is locally patched.  You can get those patches (actual
back-merge of changes already applied to trunk plus some additions
AFAIK) by the cygwin-folk directly.

Regards,
Kai

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 12:59 PM, Marc Glisse  wrote:

>> Furthermore, I do think that the compiler should have special nodes
>> for both standard placement new and the global operator new functions,
>
>
> That's one way to do it. Since this is the first time I look at those, I
> don't really see the advantage compared to the current status, but I trust
> you. What would you do with this special-node placement new?

placement new really is about "calling" a contractor, and marking the
beginning of the lifetime of a new object, hence aiding lifetime-based
alias analysis.

> Keep it as is
> until after vrp so we can use the !=0 property and then expand it to its
> first argument? Or expand it early to the equivalent of the library code I
> wrote above?
>
>
>> as I explained in previous messages.
>
>
> I couldn't find them, sorry if they contained information that makes this
> post irrelevant.

  http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01276.html
  http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01291.html

-- Gaby

[wide-int] Fixup filename/class spellings

[Mike Stump]

This fixes up the spellings of filenames and classes...

Index: wide-int.cc
===
--- wide-int.cc (revision 202354)
+++ wide-int.cc (working copy)
@@ -103,7 +103,7 @@ canonize (HOST_WIDE_INT *val, unsigned i
 }
 
 /*
- * Conversion routines in and out of wide-int.
+ * Conversion routines in and out of wide_int.
  */
 
 /* Copy XLEN elements from XVAL to VAL.  If NEED_CANON, canonize the
@@ -254,7 +254,7 @@ wi::from_mpz (const_tree type, mpz_t x, 
make up for the fact that double int's could not represent the
min and max values of all types.  This code should be removed
because the min and max values can always be represented in
-   wide-ints and int-csts.  */
+   wide_ints and int-csts.  */
 wide_int
 wi::max_value (unsigned int precision, signop sgn)
 {
Index: wide-int.h
===
--- wide-int.h  (revision 202354)
+++ wide-int.h  (working copy)
@@ -20,13 +20,13 @@ along with GCC; see the file COPYING3.  
 #ifndef WIDE_INT_H
 #define WIDE_INT_H
 
-/* Wide-int.[cc|h] implements a class that efficiently performs
-   mathematical operations on finite precision integers.  Wide-ints
+/* wide-int.[cc|h] implements a class that efficiently performs
+   mathematical operations on finite precision integers.  wide_ints
are designed to be transient - they are not for long term storage
-   of values.  There is tight integration between wide-ints and the
+   of values.  There is tight integration between wide_ints and the
other longer storage GCC representations (rtl and tree).
 
-   The actual precision of a wide-int depends on the flavor.  There
+   The actual precision of a wide_int depends on the flavor.  There
are three predefined flavors:
 
  1) wide_int (the default).  This flavor does the math in the
@@ -143,7 +143,7 @@ along with GCC; see the file COPYING3.  
as long as they can be reconstructed from the top bit that is being
represented.
 
-   There are constructors to create the various forms of wide-int from
+   There are constructors to create the various forms of wide_int from
trees, rtl and constants.  For trees and constants, you can simply say:
 
  tree t = ...;
@@ -172,10 +172,10 @@ along with GCC; see the file COPYING3.  
  without having to special case the front ends.
 
* When a constant that has an integer type is converted to a
- wide-int it comes in with precision 0.  For these constants the
+ wide_int it comes in with precision 0.  For these constants the
  top bit does accurately reflect the sign of that constant; this
  is an exception to the normal rule that the signedness is not
- represented.  When used in a binary operation, the wide-int
+ represented.  When used in a binary operation, the wide_int
  implementation properly extends these constants so that they
  properly match the other operand of the computation.  This allows
  you write:

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Mike Stump wrote:


On Sep 7, 2013, at 3:33 AM, Marc Glisse  wrote:

this patch teaches the compiler that operator new, when it can throw, isn't 
allowed to return a null pointer.


You sure:

@item -fcheck-new
@opindex fcheck-new
Check that the pointer returned by @code{operator new} is non-null
before attempting to modify the storage allocated.  This check is
normally unnecessary because the C++ standard specifies that
@code{operator new} only returns @code{0} if it is declared
@samp{throw()}, in which case the compiler always checks the
return value even without this option.  In all other cases, when
@code{operator new} has a non-empty exception specification, memory
exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
@samp{new (nothrow)}.

?


Thanks, I didn't know that option. But it doesn't do the same. -fcheck-new 
is about the front-end inserting a test !=0 between malloc and the 
constructor. My patch is about teaching the middle-end that the value is 
not zero (so even user-coded comparisons with 0 can be simplified).


Now flag_check_new should probably disable this optimization... The 3 
-fcheck-new testcases in the testsuite probably only pass because they 
don't have -O2.


--
Marc Glisse

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse  wrote:
> On Sat, 7 Sep 2013, Mike Stump wrote:
>
>> On Sep 7, 2013, at 3:33 AM, Marc Glisse  wrote:
>>>
>>> this patch teaches the compiler that operator new, when it can throw,
>>> isn't allowed to return a null pointer.
>>
>>
>> You sure:
>>
>> @item -fcheck-new
>> @opindex fcheck-new
>> Check that the pointer returned by @code{operator new} is non-null
>> before attempting to modify the storage allocated.  This check is
>> normally unnecessary because the C++ standard specifies that
>> @code{operator new} only returns @code{0} if it is declared
>> @samp{throw()}, in which case the compiler always checks the
>> return value even without this option.  In all other cases, when
>> @code{operator new} has a non-empty exception specification, memory
>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>> @samp{new (nothrow)}.
>>
>> ?
>
>
> Thanks, I didn't know that option. But it doesn't do the same.

Indeed.

-- Gaby

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis  
wrote:

> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse  wrote:
>> On Sat, 7 Sep 2013, Mike Stump wrote:
>> 
>>> On Sep 7, 2013, at 3:33 AM, Marc Glisse  wrote:
 
 this patch teaches the compiler that operator new, when it can throw,
 isn't allowed to return a null pointer.
>>> 
>>> 
>>> You sure:
>>> 
>>> @item -fcheck-new
>>> @opindex fcheck-new
>>> Check that the pointer returned by @code{operator new} is non-null
>>> before attempting to modify the storage allocated.  This check is
>>> normally unnecessary because the C++ standard specifies that
>>> @code{operator new} only returns @code{0} if it is declared
>>> @samp{throw()}, in which case the compiler always checks the
>>> return value even without this option.  In all other cases, when
>>> @code{operator new} has a non-empty exception specification, memory
>>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>>> @samp{new (nothrow)}.
>>> 
>>> ?
>> 
>> 
>> Thanks, I didn't know that option. But it doesn't do the same.
> 
> Indeed.

Can this throw:

void *operator new (long unsigned int s) {
  return 0;
}

?  Is this allowed to return 0?

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 12:27 PM, Marc Glisse  wrote:
> Now flag_check_new should probably disable this optimization…

Yes, this why my point.

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Mike Stump wrote:



On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis  
wrote:


On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse  wrote:

On Sat, 7 Sep 2013, Mike Stump wrote:


On Sep 7, 2013, at 3:33 AM, Marc Glisse  wrote:


this patch teaches the compiler that operator new, when it can throw,
isn't allowed to return a null pointer.



You sure:

@item -fcheck-new
@opindex fcheck-new
Check that the pointer returned by @code{operator new} is non-null
before attempting to modify the storage allocated.  This check is
normally unnecessary because the C++ standard specifies that
@code{operator new} only returns @code{0} if it is declared
@samp{throw()}, in which case the compiler always checks the
return value even without this option.  In all other cases, when
@code{operator new} has a non-empty exception specification, memory
exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
@samp{new (nothrow)}.

?



Thanks, I didn't know that option. But it doesn't do the same.


Indeed.


Can this throw:

void *operator new (long unsigned int s) {
 return 0;
}

?  Is this allowed to return 0?


I think using this function is illegal. It isn't marked noexcept, so it 
isn't allowed to return 0.


--
Marc Glisse

Re: [RFC] Fix for PR58201

[Paolo Carlini]

Hi all, Jakub,

On 09/07/2013 01:16 PM, Jakub Jelinek wrote:

As I wrote in the PR, IMHO mangle_decl should
   location_t save_location = input_location;
   input_location = DECL_SOURCE_LOCATION (decl);
...
   input_location = save_location;
around the call,
I had a look and I'm afraid this is already happening and isn't enough: 
in mangle_decl_string, the call of write_mangled_name is wrapped in 
exactly what you are suggesting. Or you mean something else?


Otherwise, I'm afraid we have to resort to what Jason too appears to 
find acceptable, thus what Honza proposed about UNKNOWN_LOCATION + the 
tweak to print_instantiation_partial_context_line (essentially just 
return immediately if loc == UNKNOWN_LOCATION, I think)


Paolo.

Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...

[Paolo Carlini]

-- Caroline,

something seems wrong with the patch, I can't build anymore. Something 
in libvtv/testsuite:


make[4]: Entering directory 
`/home/paolo/Gcc/svn-dirs/trunk-build/x86_64-unknown-linux-gnu/libvtv/testsuite'

Makefile:369: *** missing separator.  Stop.

Paolo.

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Mike Stump wrote:


On Sep 7, 2013, at 12:27 PM, Marc Glisse  wrote:

Now flag_check_new should probably disable this optimization…


Yes, this why my point.


Ok, here it is (again, no proper testing until bootstrap is fixed)

2013-09-07  Marc Glisse  

PR c++/19476
gcc/
* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
Likewise.
(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
* g++.dg/tree-ssa/pr19476-1.C: New file.
* g++.dg/tree-ssa/pr19476-2.C: Likewise.
* g++.dg/tree-ssa/pr19476-3.C: Likewise.

--
Marc GlisseIndex: testsuite/g++.dg/tree-ssa/pr19476-1.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-1.C   (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C   (revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include 
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-2.C   (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C   (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include 
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-3.C   (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-3.C   (revision 0)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
+
+#include 
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
___
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: fold-const.c
===
--- fold-const.c(revision 202351)
+++ fold-const.c(working copy)
@@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
 case MODIFY_EXPR:
 case BIND_EXPR:
   return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
strict_overflow_p);
 
 case SAVE_EXPR:
   return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
strict_overflow_p);
 
 case CALL_EXPR:
-  return alloca_call_p (t);
+  {
+   tree fn = CALL_EXPR_FN (t);
+   if (TREE_CODE (fn) != ADDR_EXPR) return false;
+   tree fndecl = TREE_OPERAND (fn, 0);
+   if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
+   if (!flag_check_new
+   && DECL_IS_OPERATOR_NEW (fndecl)
+   && !TREE_NOTHROW (fndecl))
+ return true;
+   return alloca_call_p (t);
+  }
 
 default:
   break;
 }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
Handle warnings about undefined signed overflow.  */
 
Index: tree-vrp.c
===
--- tree-vrp.c  (revision 202351)
+++ tree-vrp.c  (working copy)
@@ -1047,21 +1047,29 @@ gimple_assign_nonzero_warnv_p (gimple st
*STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
 {
 case GIMPLE_ASSIGN:
   return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
 case GIMPLE_CALL:
-  return gimple_alloca_call_p (stmt);
+  {
+   tree fndecl = gimple_call_fndecl (stmt);
+   if (!fndecl) return false;
+   if (!flag_check_new
+   && DECL_IS_OPERATO

Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...

[Paolo Carlini]

Hi again,

On 09/07/2013 10:41 PM, Paolo Carlini wrote:

-- Caroline,

something seems wrong with the patch, I can't build anymore. Something 
in libvtv/testsuite:


make[4]: Entering directory 
`/home/paolo/Gcc/svn-dirs/trunk-build/x86_64-unknown-linux-gnu/libvtv/testsuite'

Makefile:369: *** missing separator.  Stop.
basing on your committed patch, I think you simply forgot to commit the 
Makefile.* changes in libvtv/testsuite. Thus I'm going to test and 
commit as obvious the below if an x86_64-linux bootstrap succeeds again 
for me.


Thanks,
Paolo.

/
Index: testsuite/Makefile.am
===
--- testsuite/Makefile.am   (revision 202345)
+++ testsuite/Makefile.am   (working copy)
@@ -39,12 +39,8 @@ check-script: ${testing_script} stamp-subdir
-@(chmod +x ${testing_script}; \
${testing_script} ${libvtv_srcdir} ${libvtv_builddir})
 
-if ENABLE_VTABLE_VERIFY
 check-am:
$(MAKE) $(AM_MAKEFLAGS) check-script
-else
-check-am:
-endif
 
 .PHONY: check-script
 

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Marc Glisse wrote:


On Sat, 7 Sep 2013, Mike Stump wrote:


Can this throw:

void *operator new (long unsigned int s) {
 return 0;
}

?  Is this allowed to return 0?


I think using this function is illegal. It isn't marked noexcept, so it isn't 
allowed to return 0.


And if I compile your code with gcc, I get nice warnings (though I get 
them twice and the column number is not so good):


m.cc: In function 'void* operator new(long unsigned int)':
m.cc:2:12: warning: 'operator new' must not return NULL unless it is 
declared 'throw()' (or -fcheck-new is in effect) [enabled by default]

 return 0;
^
m.cc: At global scope:
m.cc:1:7: warning: unused parameter 's' [-Wunused-parameter]
 void *operator new (long unsigned int s) {
   ^


--
Marc Glisse

Re: [RFC] Fix for PR58201

[Jan Hubicka]

> Hi all, Jakub,
> 
> On 09/07/2013 01:16 PM, Jakub Jelinek wrote:
> >As I wrote in the PR, IMHO mangle_decl should
> >   location_t save_location = input_location;
> >   input_location = DECL_SOURCE_LOCATION (decl);
> >...
> >   input_location = save_location;
> >around the call,
> I had a look and I'm afraid this is already happening and isn't
> enough: in mangle_decl_string, the call of write_mangled_name is
> wrapped in exactly what you are suggesting. Or you mean something
> else?
> 
> Otherwise, I'm afraid we have to resort to what Jason too appears to
> find acceptable, thus what Honza proposed about UNKNOWN_LOCATION +
> the tweak to print_instantiation_partial_context_line (essentially
> just return immediately if loc == UNKNOWN_LOCATION, I think)

Yes, I think those two changes makes perfect case.  Middle end probably should
be more consistent about not setting input_locations beyhond of this point (it
does not really make much sense).  It is not how things works unforutnately -
RTL expansion still lives in statement at a time and uses input location to 
propagate things down and there are many other random setters of this variable.

I will try to audit things incrementally.  With the change to cgraphunit I made
this week all mangling ought to happen early (this is also not 100% true: in
some very special cases where we late access virtual tables through BINFOs, I
have independent fix for that).

Honza
> 
> Paolo.

Re: [RFC] Fix for PR58201

[Jan Hubicka]

> > Hi all, Jakub,
> > 
> > On 09/07/2013 01:16 PM, Jakub Jelinek wrote:
> > >As I wrote in the PR, IMHO mangle_decl should
> > >   location_t save_location = input_location;
> > >   input_location = DECL_SOURCE_LOCATION (decl);
> > >...
> > >   input_location = save_location;
> > >around the call,
> > I had a look and I'm afraid this is already happening and isn't
> > enough: in mangle_decl_string, the call of write_mangled_name is
> > wrapped in exactly what you are suggesting. Or you mean something
> > else?
> > 
> > Otherwise, I'm afraid we have to resort to what Jason too appears to
> > find acceptable, thus what Honza proposed about UNKNOWN_LOCATION +
> > the tweak to print_instantiation_partial_context_line (essentially
> > just return immediately if loc == UNKNOWN_LOCATION, I think)
> 
> Yes, I think those two changes makes perfect case.  Middle end probably should
> be more consistent about not setting input_locations beyhond of this point (it
> does not really make much sense).  It is not how things works unforutnately -
> RTL expansion still lives in statement at a time and uses input location to 
> propagate things down and there are many other random setters of this 
> variable.
> 
> I will try to audit things incrementally.  With the change to cgraphunit I 
> made
> this week all mangling ought to happen early (this is also not 100% true: in
> some very special cases where we late access virtual tables through BINFOs, I
> have independent fix for that).

Also I think with David's changes to get rid of global contextes, we probably 
should
have universum (w/o any location), function (with start/end locations as we 
have in cfun
now) and expression (with its own location) contextes now.

It would map to our global state, cfun+current_function_decl and
input_location+rtl_profile_for_bb in our current state of things.

Diagnostic than should know if it binds to specific function or specific 
statement
and can pick the porper location.

Honza
> 
> Honza
> > 
> > Paolo.

Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 3:41 PM, Paolo Carlini  wrote:
> -- Caroline,
>
> something seems wrong with the patch, I can't build anymore. Something in
> libvtv/testsuite:
>
> make[4]: Entering directory
> `/home/paolo/Gcc/svn-dirs/trunk-build/x86_64-unknown-linux-gnu/libvtv/testsuite'
> Makefile:369: *** missing separator.  Stop.
>
> Paolo.

Indeed.  I saw this last night.  With --disable-libvtv, I was stopped by:

/home/gdr/build/gcc/./prev-gcc/xg++ -B/home/gdr/build/gcc/./prev-gcc/
-B/home/gdr/gnu/x86_64-unknown-linux-gnu/bin/ -nostdinc++
-B/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs
-B/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs
-I/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu
-I/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/include
-I/home/gdr/src/gcc.svn/libstdc++-v3/libsupc++
-L/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs
-L/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs
-c  -DIN_GCC_FRONTEND -g -O2 -DIN_GCC   -fno-exceptions -fno-rtti
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
-Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long
-Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common
-DHAVE_CONFIG_H -I. -Icp -I/home/gdr/src/gcc.svn/gcc
-I/home/gdr/src/gcc.svn/gcc/cp -I/home/gdr/src/gcc.svn/gcc/../include
-I/home/gdr/src/gcc.svn/gcc/../libcpp/include
-I/home/gdr/src/gcc.svn/gcc/../libdecnumber
-I/home/gdr/src/gcc.svn/gcc/../libdecnumber/bid -I../libdecnumber
-I/home/gdr/src/gcc.svn/gcc/../libbacktrace
/home/gdr/src/gcc.svn/gcc/cp/typeck.c -o cp/typeck.o
/home/gdr/src/gcc.svn/gcc/cp/pt.c: In function 'tree_node*
maybe_get_template_decl_from_type_decl(tree)':
/home/gdr/src/gcc.svn/gcc/cp/pt.c:7064:1: internal compiler error: in
propagate_threaded_block_debug_into, at tree-ssa-threadedge.c:623
 maybe_get_template_decl_from_type_decl (tree decl)
 ^
0x528a20 ???
../sysdeps/x86_64/start.S:123
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
make[3]: *** [cp/pt.o] Error 1

-- Gaby

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 2:46 PM, Mike Stump  wrote:
>
> On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis  
> wrote:
>
>> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse  wrote:
>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>>
 On Sep 7, 2013, at 3:33 AM, Marc Glisse  wrote:
>
> this patch teaches the compiler that operator new, when it can throw,
> isn't allowed to return a null pointer.


 You sure:

 @item -fcheck-new
 @opindex fcheck-new
 Check that the pointer returned by @code{operator new} is non-null
 before attempting to modify the storage allocated.  This check is
 normally unnecessary because the C++ standard specifies that
 @code{operator new} only returns @code{0} if it is declared
 @samp{throw()}, in which case the compiler always checks the
 return value even without this option.  In all other cases, when
 @code{operator new} has a non-empty exception specification, memory
 exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
 @samp{new (nothrow)}.

 ?
>>>
>>>
>>> Thanks, I didn't know that option. But it doesn't do the same.
>>
>> Indeed.
>
> Can this throw:
>
> void *operator new (long unsigned int s) {
>   return 0;
> }
>
> ?  Is this allowed to return 0?

If that is supposed to be a definition for the global function 'operator new',
then it fails the requirement of the C++ standards since 1998.

-- Gaby

Re: [PATCH GCC]Catch more MEM_REFs sharing common addressing part in gimple strength reduction

[Bill Schmidt]

On Mon, 2013-09-02 at 11:15 +0200, Richard Biener wrote:
> On Mon, Sep 2, 2013 at 8:56 AM, bin.cheng  wrote:
> > Hi,
> >
> > The gimple-ssa-strength-reduction pass handles CAND_REFs in order to find
> > different MEM_REFs sharing common part in addressing expression.  If such
> > MEM_REFs are found, the pass rewrites MEM_REFs, and produces more efficient
> > addressing expression during the RTL passes.
> > The pass analyzes addressing expression in each MEM_REF to see if it can be
> > formalized as follows:
> >  base:MEM_REF (T1, C1)
> >  offset:  MULT_EXPR (PLUS_EXPR (T2, C2), C3)
> >  bitpos:  C4 * BITS_PER_UNIT
> > Then restructures it into below form:
> >  MEM_REF (POINTER_PLUS_EXPR (T1, MULT_EXPR (T2, C3)),
> >   C1 + (C2 * C3) + C4)
> > At last, rewrite the MEM_REFs if there are two or more sharing common
> > (non-constant) part.
> > The problem is it doesn't back trace T2.  If T2 is recorded as a CAND_ADD in
> > form of "T2' + C5", the MEM_REF should be restructure into:
> >  MEM_REF (POINTER_PLUS_EXPR (T1, MULT_EXPR (T2', C3)),
> >   C1 + (C2 * C3) + C4 + (C5 * C3))
> >
> > The patch also includes a test case to illustrate the problem.
> >
> > Bootstrapped and tested on x86/x86_64/arm-a15, is it ok?
> 
> This looks ok to me if Bill is ok with it.

Sorry, I've been on vacation and haven't been checking in until now.
I'll have a look at this tomorrow -- sounds good on the surface!

Thanks,
Bill

> 
> Thanks,
> Richard.
> 
> > Thanks.
> > bin
> >
> > 2013-09-02  Bin Cheng  
> >
> > * gimple-ssa-strength-reduction.c (backtrace_base_for_ref): New.
> > (restructure_reference): Call backtrace_base_for_ref.
> >
> > gcc/testsuite/ChangeLog
> > 2013-09-02  Bin Cheng  
> >
> > * gcc.dg/tree-ssa/slsr-39.c: New test.
>