Re: [PATCH 03/25] Improve TARGET_MANGLE_DECL_ASSEMBLER_NAME.

[Julian Brown]

On Wed, 12 Sep 2018 13:34:06 -0400
Julian Brown  wrote:

> On Wed, 12 Sep 2018 17:31:58 +0100
> Andrew Stubbs  wrote:
> 
> > On 12/09/18 16:16, Richard Biener wrote:  
> > > I think the symptom GCN sees needs to be better understood - like
> > > wheter it is generally OK to mangle things arbitrarily.
> > 
> > The name mangling is a horrible workaround for a bug in the HSA
> > runtime code (which we do not own, cannot fix, and would want to
> > support old versions of anyway).  Basically it refuses to load any
> > binary that has the same symbol as another, already loaded, binary,
> > regardless of the symbol's linkage.  Worse, it also rejects any
> > binary that has duplicate symbols within it, despite the fact that
> > it already linked just fine.
> > 
> > Adding the extra lookups is enough to build GCN binaries, with
> > mangled names, whereas the existing name mangling support was either
> > more specialized or bit rotten (I don't know which).
> > 
> > It may well be that there's a better way to solve the problem, or
> > at least to do the lookups.
> > 
> > It may also be that there are some unintended consequences, such as 
> > false name matches, but I don't know of any at present.
> > 
> > Julian, can you comment, please?  
> 
> I did the local-symbol name mangling in two places:
> 
> - The ASM_FORMAT_PRIVATE_NAME macro (good for local statics)
> - The TARGET_MANGLE_DECL_ASSEMBLER_NAME hook (for file-scope
>   local/statics)
> 
> Possibly, this was an abuse of these hooks, but it's arguably wrong
> that that e.g. handle_alias_pairs has the "assembler name" leak
> through into the user's source code -- if it's expected that the hook
> could make arbitrary transformations to the string. (The latter hook
> is only used by PE code for x86 at present, by the look of it, and
> the default handles only special-purpose mangling indicated by
> placing a '*' at the front of the symbol.)

One possibility might be to allow
symbol_table::decl_assembler_name_hash and
symbol_table::assembler_names_equal_p to be overridden by a target
hook, and define them for GCN to ignore the symbol "localisation"
magic. At the moment they will just ignore "*" at the start of a
symbol, and a (fixed) user label prefix, no matter what
TARGET_MANGLE_DECL_ASSEMBLER_NAME does.

Another way would be to do some appropriate mangling for local symbols
in the assembler, rather than the compiler (though we're using the LLVM
assembler, and so far have got away with not making any invasive
changes to that).

> If we had a symtab_node::get_for_name () using a suitable hash table,
> I think it'd probably be right to use that. Can that be done
> (easily), or is there some equivalent way? Introducing a new hash
> table everywhere for a bug workaround for a relatively obscure
> feature on a single target seems unfortunate.

An "obvious" solution of calling targetm.mangle_decl_assembler_name
before looking up in symtab_node::get_for_asmname, something like:

static void
handle_alias_pairs (void)
{
  alias_pair *p;
  unsigned i;

  for (i = 0; alias_pairs && alias_pairs->iterate (i, );)
{
  tree asmname = targetm.mangle_decl_assembler_name (p->decl, p->target);
  symtab_node *target_node = symtab_node::get_for_asmname (asmname);
  [...]

seems like it could possibly work for handle_alias_pairs, but not so
much for c-pragma.c:maybe_apply_pending_pragma_weaks, where there is no
decl available to pass as the first argument to the target hook.

Julian

[committed] hppa: Fix canonicalize of method and void pointers in comparison operations

[John David Anglin]

The attached change fixes the canonicalization of method and void 
pointers in comparisons against
another method or function pointer on 32-bit hppa targets.  As far as I 
know, 32-bit hppa is the only
architecture that requires function pointer canonicalization due to lazy 
binding.


Tested on hppa2.0w-hp-hpux11.11 and hppa-unknown-linux-gnu, GCC trunk 
and 8.  Committed to

trunk and gcc-8 branch.

Dave

--
John David Anglin  dave.ang...@bell.net

2018-09-14  John David Anglin  

PR middle-end/87188
* dojump.c (do_compare_and_jump): Canonicalize function pointers
when one operand is a function pointer.  Use POINTER_TYPE_P and
FUNC_OR_METHOD_TYPE_P.
* expr.c (do_store_flag): Use POINTER_TYPE_P and FUNC_OR_METHOD_TYPE_P.
* fold-const.c (build_range_check): Likewise.
* match.pd (simple_comparison): Likewise.

Index: dojump.c
===
--- dojump.c(revision 264245)
+++ dojump.c(working copy)
@@ -1214,15 +1214,15 @@
   code = unsignedp ? unsigned_code : signed_code;
 
   /* If function pointers need to be "canonicalized" before they can
- be reliably compared, then canonicalize them.
- Only do this if *both* sides of the comparison are function pointers.
- If one side isn't, we want a noncanonicalized comparison.  See PR
- middle-end/17564.  */
+ be reliably compared, then canonicalize them.  Canonicalize the
+ expression when one of the operands is a function pointer.  This
+ handles the case where the other operand is a void pointer.  See
+ PR middle-end/17564.  */
   if (targetm.have_canonicalize_funcptr_for_compare ()
-  && POINTER_TYPE_P (TREE_TYPE (treeop0))
-  && POINTER_TYPE_P (TREE_TYPE (treeop1))
-  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (treeop0)))
-  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (treeop1
+  && ((POINTER_TYPE_P (TREE_TYPE (treeop0))
+  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (treeop0
+ || (POINTER_TYPE_P (TREE_TYPE (treeop1))
+ && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (treeop1))
 {
   rtx new_op0 = gen_reg_rtx (mode);
   rtx new_op1 = gen_reg_rtx (mode);
Index: expr.c
===
--- expr.c  (revision 264245)
+++ expr.c  (working copy)
@@ -11532,12 +11532,10 @@
   /* We won't bother with store-flag operations involving function pointers
  when function pointers must be canonicalized before comparisons.  */
   if (targetm.have_canonicalize_funcptr_for_compare ()
-  && ((TREE_CODE (TREE_TYPE (arg0)) == POINTER_TYPE
-  && (TREE_CODE (TREE_TYPE (TREE_TYPE (arg0)))
-  == FUNCTION_TYPE))
- || (TREE_CODE (TREE_TYPE (arg1)) == POINTER_TYPE
- && (TREE_CODE (TREE_TYPE (TREE_TYPE (arg1)))
- == FUNCTION_TYPE
+  && ((POINTER_TYPE_P (TREE_TYPE (arg0))
+  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (arg0
+ || (POINTER_TYPE_P (TREE_TYPE (arg1))
+ && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (arg1))
 return 0;
 
   STRIP_NOPS (arg0);
Index: fold-const.c
===
--- fold-const.c(revision 264245)
+++ fold-const.c(working copy)
@@ -4922,8 +4922,8 @@
   /* Disable this optimization for function pointer expressions
  on targets that require function pointer canonicalization.  */
   if (targetm.have_canonicalize_funcptr_for_compare ()
-  && TREE_CODE (etype) == POINTER_TYPE
-  && TREE_CODE (TREE_TYPE (etype)) == FUNCTION_TYPE)
+  && POINTER_TYPE_P (etype)
+  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (etype)))
 return NULL_TREE;
 
   if (! in_p)
Index: match.pd
===
--- match.pd(revision 264245)
+++ match.pd(working copy)
@@ -3462,8 +3462,8 @@
/* Disable this optimization if we're casting a function pointer
  type on targets that require function pointer canonicalization.  */
&& !(targetm.have_canonicalize_funcptr_for_compare ()
-   && TREE_CODE (TREE_TYPE (@00)) == POINTER_TYPE
-   && TREE_CODE (TREE_TYPE (TREE_TYPE (@00))) == FUNCTION_TYPE)
+   && POINTER_TYPE_P (TREE_TYPE (@00))
+   && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (@00
&& single_use (@0))
(if (TYPE_PRECISION (TREE_TYPE (@00)) == TYPE_PRECISION (TREE_TYPE (@0))
&& (TREE_CODE (@10) == INTEGER_CST

Re: [PATCH] look harder for MEM_REF operand equality to avoid -Wstringop-truncation (PR 84561)

[Martin Sebor]

On 09/14/2018 03:35 PM, Jeff Law wrote:

On 9/12/18 11:46 AM, Martin Sebor wrote:

On 08/31/2018 04:07 AM, Richard Biener wrote:

On Thu, Aug 30, 2018 at 7:39 PM Martin Sebor  wrote:


On 08/30/2018 11:22 AM, Richard Biener wrote:

On August 30, 2018 6:54:21 PM GMT+02:00, Martin Sebor
 wrote:

On 08/30/2018 02:35 AM, Richard Biener wrote:

On Thu, Aug 30, 2018 at 2:12 AM Martin Sebor 

wrote:


The attached patch adds code to work harder to determine whether
the destination of an assignment involving MEM_REF is the same
as the destination of a prior strncpy call.  The included test
case demonstrates when this situation comes up.  During ccp,
dstbase and lhsbase returned by get_addr_base_and_unit_offset()
end up looking like this:


"During CCP" means exactly when?  The CCP lattice tracks copies
so CCP should already know that _1 == _8.  I suppose during
substitute_and_fold then?  But that replaces uses before folding
the stmt.


Yes, when ccp_finalize() performs the final substitution during
substitute_and_fold().


But then you shouldn't need the loop but at most look at the pointer
SSA Def to get at the non-invariant ADDR_EXPR.


I don't follow.   Are you suggesting to compare
SSA_NAME_DEF_STMT (dstbase) to SSA_NAME_DEF_STMT (lhsbase) for
equality?  They're not equal.


No.


The first loop iterates once and retrieves

   1.  _8 = _3(D)->a;

The second loop iterates three times and retrieves:

   1.  _1 = _9
   2.  _9 = _8
   3.  _8 = _3(D)->a;

How do I get from _1 to _3(D)->a without iterating?  Or are
you saying to still iterate but compare the SSA_NAME_DEF_STMT?


I say you should retrieve _8 = _3(D)->a immediately since the
copies should be
propagated out at this stage.


The warning is issued as the strncpy call is being folded (during
the dom walk in substitute_and_fold_engine::substitute_and_fold)
but before the subsequent statements have been folded (during
the subsequent loop to eliminate statements).  So at the point
of the strncpy folding the three assignments above are still
there.

I can't think of a good way to solve this problem that's not
overly intrusive.  Unless you have some suggestions for how
to deal with it, is the patch okay as is?

In what pass do you see the the naked copies?  In general those should
have been propagated away.


As I said above, this happens during the dom walk in the ccp
pass:

  substitute_and_fold_dom_walker walker (CDI_DOMINATORS, this);
  walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));

The warning is issued during the walker.walk() call as
strncpy is being folded into memcpy.  The prior assignments are
only propagated later, when the next statement after the strncpy
call is reached.  It happens in
substitute_and_fold_dom_walker::before_dom_children(). So during
the strncpy folding we see the next statement as:

  MEM[(struct S *)_1].a[n_7] = 0;

After the strncpy call is transformed to memcpy, the assignment
above is transformed to

  MEM[(struct S *)_8].a[3] = 0;



If they're only discovered as copies within the pass where you're trying
to issue the diagnostic, then you'd want to see if the pass has any
internal structures that tell you about equivalences.


I don't know if this is possible.  I don't see any APIs in
tree-ssa-propagate.h that would let me query the internal data
somehow to find out during folding (when the warning is issued).

Martin

Re: [PATCH v2] combine: perform jump threading at the end

[Segher Boessenkool]

On Wed, Sep 05, 2018 at 02:00:59PM +0200, Ilya Leoshkevich wrote:
> +/* { dg-final { scan-assembler 
> "\n\tlt\t.*\n\tjne\t(\\.L\\d+)\n(.*\n)*\tcs\t.*\n\tber\t%r14\n\\1:\n\tjg\tbar\n"
>  } } */

About this RE.  "." matches anything, also newlines, so (.*\n)* is the
same as just .* (ignoring captures).  But you probably do not want this,
so you should put (?n) in your RE (traditionally, at the start of it).

You also might want to quote the string with {} instead of "".  Read
https://www.tcl.tk/man/tcl8.4/TclCmd/Tcl.htm for why (it is short; when
you have read it, you will know *all* Tcl syntax.  Well maybe read re_syntax
as well).


Segher

Re: [committed] Fix minor bug in recently added sanity test in tree-ssa-dse.c

[Jeff Law]

On 8/28/18 3:37 AM, Richard Biener wrote:
> On Mon, Aug 27, 2018 at 10:31 PM Jeff Law  wrote:
>>
>>
>> We recently changes tree-ssa-dse.c to not trim stores outside the bounds
>> of the referenced object.  This is generally a good thing.
>>
>> However, there are cases where the object doesn't have a usable size.
>> We see this during kernel builds, at least on the microblaze target.
>>
>> We've got...
>>
>> _1 = p_47(D)->stack;
>> childregs_48 = [(void *)_1 + 8040B];
>> [ ... ]
>> memset (childregs_48, 0, 152);
>> [ ... ]
>> MEM[(struct pt_regs *)_1 + 8040B].pt_mode = 1;
>>
>>
>> We want to trim the memset call as the assignment to pt_mode is the last
>> word written by the memset call, thus making the store of that word via
>> memset dead.
>>
>> Our ref->base is:
>>
>> (gdb) p debug_tree ($66)
>>  > type > align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
>> 0x7fffe9e210a8
>> pointer_to_this >
>>
>> arg:0 > type > void>
>> public unsigned SI
>> size 
>> unit-size 
>> align:32 warn_if_not_align:0 symtab:0 alias-set 8
>> canonical-type 0x7fffe9e21150 context > 0x7fffe8f72438 /tmp/process.i>
>> pointer_to_this 
>> reference_to_this >
>> visited
>> def_stmt _1 = p_47(D)->stack;
>> version:1
>> ptr-info 0x7fffe8f65fa8>
>> arg:1 
>> constant 8040>>
>>
>>
>> Note the void type.  Those do not have a suitable TYPE_SIZE_UNIT, thus
>> causing an ICE when we try to reference it within compute_trims:
>> /* But don't trim away out of bounds accesses, as this defeats
>>  proper warnings.  */
>>   if (*trim_tail
>>   && compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (ref->base)),
>>last_orig) <= 0)
>>
>>
>> The fix is obvious enough.  Don't do the check when the underlying type
>> has no usable TYPE_SIZE_UNIT.
>>
>> I pondered moving the tests into the code that determines whether or not
>> we do byte tracking DSE, but decided the current location was fine.
>>
>> Bootstrapped and regression tested on x86.  Also verified the original
>> testfile will build with a microblaze cross compiler.
>>
>> Installing on the trunk momentarily.
> 
> Hmm, you seem to get ref->base from an address but you know types
> on addresses do not have any meaning, so ...  how does
It doesn't affect correctness.  Essentially when we have information
which indicates there might be an out of bounds write, we leave the
statement as-is.

So if the type specified a smaller size than reality, then we
potentially might suppress an optimization.

If the type specified a large size than reality, then nothing changes.



> 
>   /* But don't trim away out of bounds accesses, as this defeats
>  proper warnings.
> 
>  We could have a type with no TYPE_SIZE_UNIT or we could have a VLA
>  where TYPE_SIZE_UNIT is not a constant.  */
>   if (*trim_tail
>   && TYPE_SIZE_UNIT (TREE_TYPE (ref->base))
>   && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (ref->base))) == INTEGER_CST
>   && compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (ref->base)),
>last_orig) <= 0)
> *trim_tail = 0;
> 
> make any sense in the above case where ref->base is even based on a
> pointer?  I'd say the
> above should be at least
> 
> if (*trim_tail
> && DECL_P (ref->base)
> && 
> 
> ?  Not sure if we ever have decls with incomplete types so eventually
> the new check
> isn't needed.
We could have something casted to a void type.  Can't remember where it
happened, but it certainly came up.

jeff

Re: [PATCH] Add -Wabsolute-value

[Jeff Law]

On 9/4/18 3:08 AM, Martin Jambor wrote:
> Hi,
> 
> On Fri, Aug 31 2018, Joseph Myers wrote:
>> On Fri, 31 Aug 2018, Martin Jambor wrote:
>>
>>> diff --git a/gcc/common.opt b/gcc/common.opt
>>> index ebc3ef43ce2..2950760fb2a 100644
>>> --- a/gcc/common.opt
>>> +++ b/gcc/common.opt
>>> @@ -815,6 +815,10 @@ Wvector-operation-performance
>>>  Common Var(warn_vector_operation_performance) Warning
>>>  Warn when a vector operation is compiled outside the SIMD.
>>>  
>>> +Wabsolute-value
>>> +Common Var(warn_absolute_value) Warning EnabledBy(Wextra)
>>> +Warn on suspicious calls of standard functions computing absolute values.
>>
>> If it's C-specific I'd expect it to be in c.opt (in the appropriate 
>> alphabetical position) and have "C ObjC" instead of Common.
> 
> I see, fixed.
> 
>>
>>> +@item -Wabsolute-value
>>> +@opindex Wabsolute-value
>>> +@opindex Wno-absolute-value
>>> +Warn when a wrong absolute value function seems to be used or when it
>>> +does not have any effect because its argument is an unsigned type.
>>> +This warning is specific to C language and can be suppressed with an
>>> +explicit type cast.  This warning is also enabled by @option{-Wextra}.
>>
>> And then the @item would have @r{(C and Objective-C only)}, similar to 
>> other such options (rather than saying "specific to C language" in the 
>> text).
>>
> 
> Likewise.
> 
> I have bootstrapped and tested the updated (and re-based) patch on
> x86-64-linux.  OK for trunk now?
> 
> Thank you very much,
> 
> Martin
> 
> 
> 2018-09-03  Martin Jambor  
> 
>   gcc/
>   * doc/invoke.texi (Warning Options): Likewise.
> 
>   gcc/c-family/
>   * c.opt (Wabsolute-value): New.
> 
>   gcc/c/
>   * c-parser.c: (warn_for_abs): New function.
>   (c_parser_postfix_expression_after_primary): Call it.
> 
>   testsuite/
>   * gcc.dg/warn-abs-1.c: New test.
>   * gcc.dg/dfp/warn-abs-2.c: Likewise.
OK.
jeff

Re: [PATCH v2] combine: perform jump threading at the end

[Segher Boessenkool]

On Mon, Sep 10, 2018 at 12:58:19PM +0200, Ilya Leoshkevich wrote:
> > Am 06.09.2018 um 20:11 schrieb Jeff Law :
> > This is (of course) much easier to do when the target block has no insns
> > other than the conditional branch.  So perhaps only do this when the
> > target block has just the conditional?
> Sounds reasonable, I implemented this in the new patch.  The only thing
> is that combine leaves (note NOTE_INSN_DELETED) around, so I needed to
> also account for those.  I used side_effects_p for that.

Combine cannot delete insns because various things in combine refer to
insns by uid.  It could of course delete insns after it is done, hrm,
I'll have a look at that.  Either way, you'll also have lots of other
notes still hanging around (esp. with debug info).


Segher

Re: [PATCH] i386: move alignment defaults to processor_costs.

[Jeff Law]

On 9/12/18 6:44 AM, Martin Liška wrote:
> Hi.
> 
> Honza asked me to do the follow-up. It moves definition of alignments
> related to a CPU into *_cost scructures. Advantage of it is that there's
> no redundant definition for CPUs that have equal cost.
> 
> I verified that it produces same output for all valid -march options:
> gcc --help=common -Q -O2 -march=*
> 
> I need to reg the patch. Will it be fine after I'll test it?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-09-12  Martin Liska  
> 
>   * common/config/i386/i386-common.c (ix86_get_valid_option_values):
>   Use processor_names table.
>   * config/i386/i386.c (ix86_default_align): Use
>   processor_cost_table for alignment values.
>   (ix86_option_override_internal): Use processor_names.
>   (ix86_function_specific_print): Likewise.
>   * config/i386/i386.h (struct processor_costs):
>   Add alignment values.
>   (struct ptt): Remove and replace with const char *.
>   * config/i386/x86-tune-costs.h (struct processor_costs):
>   Declare default alignments for all costs.
Ok after suitable testing.

jeff

Re: [PATCH v3] combine: perform jump threading at the end

[Segher Boessenkool]

Hi!

On Mon, Sep 10, 2018 at 12:59:19PM +0200, Ilya Leoshkevich wrote:
> Consider the following RTL:
> 
> (code_label 11 10 26 4 2 (nil) [1 uses])
> (note 26 11 12 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
> (insn 12 26 15 4 (set (reg:SI 65)
> (if_then_else:SI (eq (reg:CCZ 33 %cc)
> (const_int 0 [0]))
> (const_int 1 [0x1])
> (const_int 0 [0]))) "pr80080-4.c":9 1674 {*movsicc})
> (insn 15 12 16 4 (parallel [
> (set (reg:CCZ 33 %cc)
> (compare:CCZ (reg:SI 65)
> (const_int 0 [0])))
> (clobber (scratch:SI))
> ]) "pr80080-4.c":9 1216 {*tstsi_cconly_extimm})
> (jump_insn 16 15 17 4 (set (pc)
> (if_then_else (ne (reg:CCZ 33 %cc)
> (const_int 0 [0]))
> (label_ref:DI 23)
> (pc))) "pr80080-4.c":9 1897 {*cjump_64})
> 
> Combine simplifies this into:
> 
> (code_label 11 10 26 4 2 (nil) [1 uses])
> (note 26 11 12 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
> (note 12 26 15 4 NOTE_INSN_DELETED)
> (note 15 12 16 4 NOTE_INSN_DELETED)
> (jump_insn 16 15 17 4 (set (pc)
> (if_then_else (eq (reg:CCZ 33 %cc)
> (const_int 0 [0]))
> (label_ref:DI 23)
> (pc))) "pr80080-4.c":9 1897 {*cjump_64})
> 
> opening up the possibility to perform jump threading.  Since this
> happens infrequently, perform jump threading only when there is a
> changed basic block, whose sole side effect is a trailing jump.

So this happens because now there is *only* a conditional jump in this BB?

Could you please show generated code before and after this patch?
I mean generated assembler code.  What -S gives you.

> +/* Return true iff the only side effect of BB is its trailing jump_insn.  */
> +
> +static bool
> +is_single_jump_bb (basic_block bb)
> +{
> +  rtx_insn *end = BB_END (bb);
> +  rtx_insn *insn;
> +
> +  if (!JUMP_P (end))
> +return false;
> +
> +  for (insn = BB_HEAD (bb); insn != end; insn = NEXT_INSN (insn))
> +if (INSN_P (insn) && side_effects_p (PATTERN (insn)))
> +  return false;
> +  return true;
> +}

Hrm, so it is more than that.

Why does the existing jump threading not work for you; should it happen
at another time?


Segher

Re: [PATCH] look harder for MEM_REF operand equality to avoid -Wstringop-truncation (PR 84561)

[Jeff Law]

On 9/12/18 11:46 AM, Martin Sebor wrote:
> On 08/31/2018 04:07 AM, Richard Biener wrote:
>> On Thu, Aug 30, 2018 at 7:39 PM Martin Sebor  wrote:
>>>
>>> On 08/30/2018 11:22 AM, Richard Biener wrote:
 On August 30, 2018 6:54:21 PM GMT+02:00, Martin Sebor
  wrote:
> On 08/30/2018 02:35 AM, Richard Biener wrote:
>> On Thu, Aug 30, 2018 at 2:12 AM Martin Sebor 
> wrote:
>>>
>>> The attached patch adds code to work harder to determine whether
>>> the destination of an assignment involving MEM_REF is the same
>>> as the destination of a prior strncpy call.  The included test
>>> case demonstrates when this situation comes up.  During ccp,
>>> dstbase and lhsbase returned by get_addr_base_and_unit_offset()
>>> end up looking like this:
>>
>> "During CCP" means exactly when?  The CCP lattice tracks copies
>> so CCP should already know that _1 == _8.  I suppose during
>> substitute_and_fold then?  But that replaces uses before folding
>> the stmt.
>
> Yes, when ccp_finalize() performs the final substitution during
> substitute_and_fold().

 But then you shouldn't need the loop but at most look at the pointer
 SSA Def to get at the non-invariant ADDR_EXPR.
>>>
>>> I don't follow.   Are you suggesting to compare
>>> SSA_NAME_DEF_STMT (dstbase) to SSA_NAME_DEF_STMT (lhsbase) for
>>> equality?  They're not equal.
>>
>> No.
>>
>>> The first loop iterates once and retrieves
>>>
>>>    1.  _8 = _3(D)->a;
>>>
>>> The second loop iterates three times and retrieves:
>>>
>>>    1.  _1 = _9
>>>    2.  _9 = _8
>>>    3.  _8 = _3(D)->a;
>>>
>>> How do I get from _1 to _3(D)->a without iterating?  Or are
>>> you saying to still iterate but compare the SSA_NAME_DEF_STMT?
>>
>> I say you should retrieve _8 = _3(D)->a immediately since the
>> copies should be
>> propagated out at this stage.
> 
> The warning is issued as the strncpy call is being folded (during
> the dom walk in substitute_and_fold_engine::substitute_and_fold)
> but before the subsequent statements have been folded (during
> the subsequent loop to eliminate statements).  So at the point
> of the strncpy folding the three assignments above are still
> there.
> 
> I can't think of a good way to solve this problem that's not
> overly intrusive.  Unless you have some suggestions for how
> to deal with it, is the patch okay as is?
In what pass do you see the the naked copies?  In general those should
have been propagated away.

If they're only discovered as copies within the pass where you're trying
to issue the diagnostic, then you'd want to see if the pass has any
internal structures that tell you about equivalences.

Jeff

Re: VRP: convert pointers of known quantity better

[Jeff Law]

On 9/14/18 4:31 AM, Aldy Hernandez wrote:
> Apparently, my work on VRP will never finish. There's an infinity of
> things that can be tweaked ;-).
> 
> First, we shouldn't drop to null/non-null when we know what the actual
> pointer value is.  For example, [1, 3] which we get when we store magic
> numbers in a pointer (p == (char *)1).
> 
> BTW, for this bit, I would much rather change range_int_cst_p to allow
> VR_ANTI_RANGE, instead of inlining as I've done.  Is there a reason
> range_int_cst_p doesn't handle anti ranges?
> 
> Also, [, ] is known to be non-null.  Don't drop to varying.
> 
> OK?
> 
> curr.patch
> 
> commit 7f42e101d5d26ea866b739692858289a8dff4396
> Author: Aldy Hernandez 
> Date:   Fri Sep 14 00:11:34 2018 +0200
> 
> * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of
> known quantity.
> 
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 622ccbc2df7..22e5ee3c729 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -1842,9 +1842,14 @@ extract_range_from_unary_expr (value_range *vr,
>tree inner_type = op0_type;
>tree outer_type = type;
>  
> -  /* If the expression evaluates to a pointer, we are only interested in
> -  determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]).  */
> -  if (POINTER_TYPE_P (type))
> +  /* If the expression evaluates to a pointer of unknown quantity,
> +  we are only interested in determining if it evaluates to NULL
> +  [0, 0] or non-NULL (~[0, 0]).  */
> +  if (POINTER_TYPE_P (type)
> +   && !((vr0.type == VR_RANGE
> + || vr0.type == VR_ANTI_RANGE)
> +&& TREE_CODE (vr0.min) == INTEGER_CST
> +&& TREE_CODE (vr0.max) == INTEGER_CST))
>   {
> if (!range_includes_zero_p ())
>   set_value_range_to_nonnull (vr, type);
I think this part is fine.  Though I question how much time we want to
spend dealing some clowns that do things like store integers into
pointer objects :-)

> @@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr,
> return;
>   }
>  
> +  /* If we have a non-constant range that we know is non-zero (for
> +  example [, ] or [, +MAX]), make it known, so we
> +  don't drop to VR_VARYING later.  */
> +  if (POINTER_TYPE_P (op0_type)
> +   && vr0.type == VR_RANGE
> +   && (TREE_CODE (vr0.min) != INTEGER_CST
> +   || TREE_CODE (vr0.max) != INTEGER_CST)
> +   && !range_includes_zero_p ())
> + set_value_range_to_nonnull (, op0_type);
> +
I don't think this is correct in the presence of weak symbols.  I think
you can query maybe_nonzero_address on the min/max and if both return >
0, then you know the result is non-null.

Jeff

Re: [PATCH][Middle-end][Version 2]Add a new option to control inlining only on static functions

[Qing Zhao]

> On Sep 14, 2018, at 3:42 PM, Andrew Pinski  wrote:
> 
> On Fri, Sep 14, 2018 at 1:34 PM Qing Zhao  > wrote:
>> 
>> Hi,
>> 
>> this is the 2nd version of the patch to add a new first-class option
>> 
>> -finline-only-static
>> 
>> to guide inlining only on static functions.
>> 
>> -finline-only-static
>>  By default, GCC inlines functions without considering whether they are 
>> static
>>  or not. This flag guides inliner to only inline static functions.
>> 
>>  Off by default.
>> 
>> The major purpose of this option is to provide a way for the user to disable 
>> inlining of global functions.
>> this is mainly to help online patching users to control the memory 
>> consumption and ease the debugging.
>> 
>> please take a look and let me know any comments and suggestions:
> 
> How does this interact with -finline-functions?  Or rather isn't this
> just the opposite of -finline-functions?

This option is OFF by default, only when explicitly specified by option 
-finline-only-static, it controls the inliner (including early inliner and ipa 
inliner) to ONLY
inline the static functions.  i.e. if the -finline-only-static is specified, 
GCC only inlines static functions, all global functions will be disabled by 
inliner.

hope this is clear.

Qing


> 
> Thanks,
> Andrew
> 
>> 
>> thanks a lot.
>> 
>> Qing
>> 
>> gcc/ChangeLog
>> 
>> +2018-09-13  Qing Zhao  mailto:qing.z...@oracle.com>>
>> +
>> +   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
>> +   * common.opt (-finline-only-static): New option.
>> +   * doc/invoke.texi: Document -finline-only-static.
>> +   * ipa-inline.c (can_inline_edge_p): Control inlining based on
>> +   function's visibility.
>> 
>> gcc/testsuite/ChangeLog
>> 
>> +2018-09-13  Qing Zhao  mailto:qing.z...@oracle.com>>
>> +
>> +   * gcc.dg/inline_only_static.c: New test.
>> +
>> 
>> 
>> 
>>> On Sep 11, 2018, at 10:12 AM, Qing Zhao  wrote:
>>> 
>>> Hi,
>>> 
>>> This is a simple patch to add a new first-class option
>>> 
>>> -finline-visibility={all|extern|static}
>>> 
>>> to finer control inlining based on function’s visibility.
>>> 
>>> '-finline-visibility=[all|extern|static]'
>>>By default, GCC inlines functions without considering their
>>>visibility.  This flag allows finer control of inlining based on
>>>their visibility.
>>> 
>>>The value 'extern' tells the compiler to only inline functions with
>>>external visibility.  The value 'static' tells the compiler to only
>>>inline functions with static visibility.  The value 'all' tells the
>>>compilers to inline functions without considering their visibility.
>>> 
>>>The default value of '-finline-visibility' is 'all'.
>>> 
>>> The major purpose of this option is to provide a way for the user
>>> to finer choose the inline candidates based on function’s visibility.
>>> For example, some online patching users might want to limit the inlining
>>> to only static functions to avoid patching the callers of global functions
>>> in order to control the memory consumption caused by online patching.
>>> 
>>> let me know any comments and suggestions.
>>> 
>>> thanks.
>>> 
>>> Qing

Re: [PATCH][Middle-end][Version 2]Add a new option to control inlining only on static functions

[Andrew Pinski]

On Fri, Sep 14, 2018 at 1:42 PM Andrew Pinski  wrote:
>
> On Fri, Sep 14, 2018 at 1:34 PM Qing Zhao  wrote:
> >
> > Hi,
> >
> > this is the 2nd version of the patch to add a new first-class option
> >
> > -finline-only-static
> >
> > to guide inlining only on static functions.
> >
> > -finline-only-static
> >   By default, GCC inlines functions without considering whether they are 
> > static
> >   or not. This flag guides inliner to only inline static functions.
> >
> >   Off by default.
> >
> > The major purpose of this option is to provide a way for the user to 
> > disable inlining of global functions.
> > this is mainly to help online patching users to control the memory 
> > consumption and ease the debugging.
> >
> > please take a look and let me know any comments and suggestions:
>
> How does this interact with -finline-functions?  Or rather isn't this
> just the opposite of -finline-functions?

Seems to me, that this new option should just disable
-finline-small-functions and -finline-functions.
That is it is an alias with -fno-inline-small-functions -fno-inline-functions.

My question comes up if someone uses -finline-functions
-finline-only-static; or -finline-only-static -finline-functions.
That it is a rather interesting combition of options but the
interaction of the two options are not documented and seems backwards.

Thanks,
Andrew Pinski

>
> Thanks,
> Andrew
>
> >
> > thanks a lot.
> >
> > Qing
> >
> > gcc/ChangeLog
> >
> > +2018-09-13  Qing Zhao  
> > +
> > +   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
> > +   * common.opt (-finline-only-static): New option.
> > +   * doc/invoke.texi: Document -finline-only-static.
> > +   * ipa-inline.c (can_inline_edge_p): Control inlining based on
> > +   function's visibility.
> >
> > gcc/testsuite/ChangeLog
> >
> > +2018-09-13  Qing Zhao  
> > +
> > +   * gcc.dg/inline_only_static.c: New test.
> > +
> >
> >
> >
> > > On Sep 11, 2018, at 10:12 AM, Qing Zhao  wrote:
> > >
> > > Hi,
> > >
> > > This is a simple patch to add a new first-class option
> > >
> > > -finline-visibility={all|extern|static}
> > >
> > > to finer control inlining based on function’s visibility.
> > >
> > > '-finline-visibility=[all|extern|static]'
> > > By default, GCC inlines functions without considering their
> > > visibility.  This flag allows finer control of inlining based on
> > > their visibility.
> > >
> > > The value 'extern' tells the compiler to only inline functions with
> > > external visibility.  The value 'static' tells the compiler to only
> > > inline functions with static visibility.  The value 'all' tells the
> > > compilers to inline functions without considering their visibility.
> > >
> > > The default value of '-finline-visibility' is 'all'.
> > >
> > > The major purpose of this option is to provide a way for the user
> > > to finer choose the inline candidates based on function’s visibility.
> > > For example, some online patching users might want to limit the inlining
> > > to only static functions to avoid patching the callers of global functions
> > > in order to control the memory consumption caused by online patching.
> > >
> > > let me know any comments and suggestions.
> > >
> > > thanks.
> > >
> > > Qing
> >

Re: C++ PATCH to implement P1064R0, Virtual Function Calls in Constant Expressions

[Marek Polacek]

On Fri, Sep 14, 2018 at 04:30:46PM -0400, Jason Merrill wrote:
> On Fri, Sep 14, 2018 at 1:19 PM, Marek Polacek  wrote:
> > This patch implements another bit of C++20, virtual calls in constant
> > expression:
> > 
> > The basic idea is that since in a constant expression we know the dynamic
> > type (to detect invalid code etc.), the restriction that prohibits virtual
> > calls is unnecessary.
> >
> > Handling virtual function calls turned out to be fairly easy (as 
> > anticipated);
> > I simply let the constexpr machinery figure out the dynamic type and then
> > OBJ_TYPE_REF_TOKEN gives us the index into the virtual function table.  That
> > way we get the function decl we're interested in, and 
> > cxx_eval_call_expression
> > takes it from there.
> >
> > But handling pointer-to-virtual-member-functions doesn't work like that.
> > get_member_function_from_ptrfunc creates a COND_EXPR which looks like
> > if (pf.__pfn & 1) // is it a virtual function?
> >   // yes, find the pointer in the vtable
> > else
> >   // no, just return the pointer
> > so ideally we want to evaluate the then-branch.  Eventually it'll evaluate 
> > it
> > to something like _ZTV2X2[2], but the vtable isn't constexpr so we'd end up
> > with "not a constant expression" error.
> 
> Then let's mark the vtable as constexpr, there's no reason for it not to be.

Ok, unfortunately it wasn't as easy as merely marking it 
DECL_DECLARED_CONSTEXPR_P
in initialize_artificial_var because then I saw "used in its own initializer"
error.  Which I don't know why, but now that I know you agree with this 
direction
I can dig deeper.
 
> > Since the vtable initializer is
> > a compile-time constant, I thought we could make it work by a hack as the 
> > one
> > in cxx_eval_array_reference.  We'll then let cxx_eval_call_expression do its
> > job and everything is hunky-dory.
> >
> > Except when it isn't: I noticed that the presence of _vptr doesn't make the
> > class non-empty, and when cxx_eval_constant_expression saw a decl with an 
> > empty
> > class type, it just evaluated it to { }.  But such a class still had gotten 
> > an
> > initializer that looks like {.D.2082 = {._vptr.X2 = &_ZTV2X2 + 16}}.  So
> > replacing it with { } will lose the proper initializer whereupon we fail.
> > The check I've added to cxx_eval_constant_expression won't win any beauty
> > contests but unfortunately EMPTY_CONSTRUCTOR_P doesn't work there.
> 
> Perhaps we should check !TYPE_POLYMORPHIC_P as well as
> is_really_empty_class.  Perhaps there should be a predicate for that,
> say, is_really_nearly_empty_class...

Ack.  Thanks,

Marek

Re: [PATCH][Middle-end][Version 2]Add a new option to control inlining only on static functions

[Andrew Pinski]

On Fri, Sep 14, 2018 at 1:34 PM Qing Zhao  wrote:
>
> Hi,
>
> this is the 2nd version of the patch to add a new first-class option
>
> -finline-only-static
>
> to guide inlining only on static functions.
>
> -finline-only-static
>   By default, GCC inlines functions without considering whether they are 
> static
>   or not. This flag guides inliner to only inline static functions.
>
>   Off by default.
>
> The major purpose of this option is to provide a way for the user to disable 
> inlining of global functions.
> this is mainly to help online patching users to control the memory 
> consumption and ease the debugging.
>
> please take a look and let me know any comments and suggestions:

How does this interact with -finline-functions?  Or rather isn't this
just the opposite of -finline-functions?

Thanks,
Andrew

>
> thanks a lot.
>
> Qing
>
> gcc/ChangeLog
>
> +2018-09-13  Qing Zhao  
> +
> +   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
> +   * common.opt (-finline-only-static): New option.
> +   * doc/invoke.texi: Document -finline-only-static.
> +   * ipa-inline.c (can_inline_edge_p): Control inlining based on
> +   function's visibility.
>
> gcc/testsuite/ChangeLog
>
> +2018-09-13  Qing Zhao  
> +
> +   * gcc.dg/inline_only_static.c: New test.
> +
>
>
>
> > On Sep 11, 2018, at 10:12 AM, Qing Zhao  wrote:
> >
> > Hi,
> >
> > This is a simple patch to add a new first-class option
> >
> > -finline-visibility={all|extern|static}
> >
> > to finer control inlining based on function’s visibility.
> >
> > '-finline-visibility=[all|extern|static]'
> > By default, GCC inlines functions without considering their
> > visibility.  This flag allows finer control of inlining based on
> > their visibility.
> >
> > The value 'extern' tells the compiler to only inline functions with
> > external visibility.  The value 'static' tells the compiler to only
> > inline functions with static visibility.  The value 'all' tells the
> > compilers to inline functions without considering their visibility.
> >
> > The default value of '-finline-visibility' is 'all'.
> >
> > The major purpose of this option is to provide a way for the user
> > to finer choose the inline candidates based on function’s visibility.
> > For example, some online patching users might want to limit the inlining
> > to only static functions to avoid patching the callers of global functions
> > in order to control the memory consumption caused by online patching.
> >
> > let me know any comments and suggestions.
> >
> > thanks.
> >
> > Qing
>

[PATCH][Middle-end][Version 2]Add a new option to control inlining only on static functions

[Qing Zhao]

Hi,

this is the 2nd version of the patch to add a new first-class option

-finline-only-static

to guide inlining only on static functions.

-finline-only-static
  By default, GCC inlines functions without considering whether they are static 
  or not. This flag guides inliner to only inline static functions. 

  Off by default.

The major purpose of this option is to provide a way for the user to disable 
inlining of global functions.
this is mainly to help online patching users to control the memory consumption 
and ease the debugging.

please take a look and let me know any comments and suggestions:

thanks a lot.

Qing

gcc/ChangeLog

+2018-09-13  Qing Zhao  
+
+   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
+   * common.opt (-finline-only-static): New option.
+   * doc/invoke.texi: Document -finline-only-static.
+   * ipa-inline.c (can_inline_edge_p): Control inlining based on
+   function's visibility. 

gcc/testsuite/ChangeLog

+2018-09-13  Qing Zhao  
+
+   * gcc.dg/inline_only_static.c: New test.
+



0001-Add-a-new-option-to-limit-inlining-on-static-functio.patch
Description: Binary data


> On Sep 11, 2018, at 10:12 AM, Qing Zhao  wrote:
> 
> Hi, 
> 
> This is a simple patch to add a new first-class option 
> 
> -finline-visibility={all|extern|static}
> 
> to finer control inlining based on function’s visibility. 
> 
> '-finline-visibility=[all|extern|static]'
> By default, GCC inlines functions without considering their
> visibility.  This flag allows finer control of inlining based on
> their visibility.
> 
> The value 'extern' tells the compiler to only inline functions with
> external visibility.  The value 'static' tells the compiler to only
> inline functions with static visibility.  The value 'all' tells the
> compilers to inline functions without considering their visibility.
> 
> The default value of '-finline-visibility' is 'all'.
> 
> The major purpose of this option is to provide a way for the user 
> to finer choose the inline candidates based on function’s visibility.
> For example, some online patching users might want to limit the inlining
> to only static functions to avoid patching the callers of global functions
> in order to control the memory consumption caused by online patching. 
> 
> let me know any comments and suggestions.
> 
> thanks.
> 
> Qing

Re: VRP: make worst case scenario for ABS_EXPR is still the set of positives

[Jeff Law]

On 9/14/18 4:25 AM, Aldy Hernandez wrote:
> First, there's no sense handling !VR_RANGE and symbolics now that we've
> normalized the inputs.
> 
> Second, even if we wrap around while calculating ABS, we at the least
> know the result is positive ;-).  BTW, we've already handled ABS(-MIN)
> and -frapv by the time we get here, so we know we won't get nonsensical
> input.
> 
> OK?
> 
> curr.patch
> 
> commit 6e874be13a0bd53811e095328c1d175723f11f84
> Author: Aldy Hernandez 
> Date:   Fri Sep 14 00:03:04 2018 +0200
> 
> * tree-vrp.c (extract_range_from_unary_expr): Do not special case
> symbolics or VR_VARYING ranges for ABS_EXPR.
> * wide-int-range.cc (wide_int_range_abs): Return positive numbers
> when range will wrap.
OK.
jeff

Re: [PATCH] Resolve the TODO in fold_builtin_strlen

[Jeff Law]

On 9/14/18 1:51 PM, Bernd Edlinger wrote:
> Hi,
> 
> this resolves the TODO I left in fold_builtin_strlen by adding some test cases
> that fail without the c_strlen call with the TODO comment and pass when the
> additional c_strlen call is done.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-fix-todo.diff
> 
> 2018-09-14  Bernd Edlinger  
> 
>   * builtins.c (fold_builtin_strlen): Remove TODO comment.
> 
> testsuite:
> 2018-09-14  Bernd Edlinger  
> 
>   * gcc.dg/warn-strlen-no-nul.c: Add some missing test cases.
OK.
jeff

Re: C++ PATCH to implement P1064R0, Virtual Function Calls in Constant Expressions

[Jason Merrill]

On Fri, Sep 14, 2018 at 1:19 PM, Marek Polacek  wrote:
> This patch implements another bit of C++20, virtual calls in constant
> expression:
> 
> The basic idea is that since in a constant expression we know the dynamic
> type (to detect invalid code etc.), the restriction that prohibits virtual
> calls is unnecessary.
>
> Handling virtual function calls turned out to be fairly easy (as anticipated);
> I simply let the constexpr machinery figure out the dynamic type and then
> OBJ_TYPE_REF_TOKEN gives us the index into the virtual function table.  That
> way we get the function decl we're interested in, and cxx_eval_call_expression
> takes it from there.
>
> But handling pointer-to-virtual-member-functions doesn't work like that.
> get_member_function_from_ptrfunc creates a COND_EXPR which looks like
> if (pf.__pfn & 1) // is it a virtual function?
>   // yes, find the pointer in the vtable
> else
>   // no, just return the pointer
> so ideally we want to evaluate the then-branch.  Eventually it'll evaluate it
> to something like _ZTV2X2[2], but the vtable isn't constexpr so we'd end up
> with "not a constant expression" error.

Then let's mark the vtable as constexpr, there's no reason for it not to be.

> Since the vtable initializer is
> a compile-time constant, I thought we could make it work by a hack as the one
> in cxx_eval_array_reference.  We'll then let cxx_eval_call_expression do its
> job and everything is hunky-dory.
>
> Except when it isn't: I noticed that the presence of _vptr doesn't make the
> class non-empty, and when cxx_eval_constant_expression saw a decl with an 
> empty
> class type, it just evaluated it to { }.  But such a class still had gotten an
> initializer that looks like {.D.2082 = {._vptr.X2 = &_ZTV2X2 + 16}}.  So
> replacing it with { } will lose the proper initializer whereupon we fail.
> The check I've added to cxx_eval_constant_expression won't win any beauty
> contests but unfortunately EMPTY_CONSTRUCTOR_P doesn't work there.

Perhaps we should check !TYPE_POLYMORPHIC_P as well as
is_really_empty_class.  Perhaps there should be a predicate for that,
say, is_really_nearly_empty_class...

Jason

[PATCH] Resolve the TODO in fold_builtin_strlen

[Bernd Edlinger]

Hi,

this resolves the TODO I left in fold_builtin_strlen by adding some test cases
that fail without the c_strlen call with the TODO comment and pass when the
additional c_strlen call is done.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
2018-09-14  Bernd Edlinger  

	* builtins.c (fold_builtin_strlen): Remove TODO comment.

testsuite:
2018-09-14  Bernd Edlinger  

	* gcc.dg/warn-strlen-no-nul.c: Add some missing test cases.


diff -Npur gcc/builtins.c gcc/builtins.c
--- gcc/builtins.c	2018-08-30 08:21:13.0 +0200
+++ gcc/builtins.c	2018-08-30 21:46:11.155211333 +0200
Index: gcc/builtins.c
===
--- gcc/builtins.c	(revision 264318)
+++ gcc/builtins.c	(working copy)
@@ -8417,7 +8417,7 @@ fold_builtin_strlen (location_t loc, tree type, tr
 	return fold_convert_loc (loc, type, len);
 
   if (!nonstr)
-	c_strlen (arg, 1, ); /* TODO: add test coverage here.  */
+	c_strlen (arg, 1, );
 
   if (nonstr)
 	{
Index: gcc/testsuite/gcc.dg/warn-strlen-no-nul.c
===
--- gcc/testsuite/gcc.dg/warn-strlen-no-nul.c	(revision 264318)
+++ gcc/testsuite/gcc.dg/warn-strlen-no-nul.c	(working copy)
@@ -9,6 +9,7 @@ const char a[5] = "12345";   /* { dg-message "decl
 
 int v0 = 0;
 int v1 = 1;
+volatile int v2;
 
 void sink (int, ...);
 
@@ -117,10 +118,8 @@ T (v0 ? b[i0] : [i3][i0] + i1);/* { dg-warni
 T (v0 ? b[i0] : [i3][i0] + i1);/* { dg-warning "nul" }  */
 T (v0 ? b[i1] : [i3][i1] + v0);/* { dg-warning "nul" }  */
 
-/* It's possible to detect the missing nul in the following two
-   expressions but GCC doesn't do it yet.  */
-T (v0 ? [3][1] + v0 : b[2]);/* { dg-warning "nul" "bug" }  */
-T (v0 ? [3][v0] : [3][v1]);   /* { dg-warning "nul" "bug" }  */
+T (v0 ? [3][1] + v0 : b[2]);/* { dg-warning "nul" }  */
+T (v0 ? [3][v0] : [3][v1]);   /* { dg-warning "nul" }  */
 
 
 struct A { char a[5], b[5]; };
@@ -299,3 +298,7 @@ T (v0 ? [3].a[1].a[1] :  ba[0].a[0].a);  /*
 
 T (v0 ? ba[0].a[0].a : ba[0].a[1].b);
 T (v0 ? ba[0].a[1].b : ba[0].a[0].a);
+
+T (v2 ? b[1] : [3][1] + v2);/* { dg-warning "nul" }  */
+T (v2 ? [3][1] + v2 : b[2]);/* { dg-warning "nul" }  */
+T (v2 ? [3][v2] : [2][v2]);   /* { dg-warning "nul" }  */

libgo patch committed: Correct gccgo buildid file on ARM

[Ian Lance Taylor]

This libgo patch changes the go tool to use %progbits rather than
@progbits on ARM.  This is necessary because gas on ARM uses @ as a
comment character.  This is a copy of https://golang.org/cl/135297 in
the master sources.  This fixes GCC PR 87260.  Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu.  Committed to mainline and GCC 8
branch.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 264325)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-3fd61802286c81e5fb672f682d9e661181184d1f
+92a14213215fd93df7240fa9d376a1213b1d5a74
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/cmd/go/internal/work/buildid.go
===
--- libgo/go/cmd/go/internal/work/buildid.go(revision 264245)
+++ libgo/go/cmd/go/internal/work/buildid.go(working copy)
@@ -337,8 +337,12 @@ func (b *Builder) gccgoBuildIDELFFile(a
}
fmt.Fprintf(, "\n")
if cfg.Goos != "solaris" {
-   fmt.Fprintf(, "\t"+`.section 
.note.GNU-stack,"",@progbits`+"\n")
-   fmt.Fprintf(, "\t"+`.section 
.note.GNU-split-stack,"",@progbits`+"\n")
+   secType := "@progbits"
+   if cfg.Goarch == "arm" {
+   secType = "%progbits"
+   }
+   fmt.Fprintf(, "\t"+`.section .note.GNU-stack,"",%s`+"\n", 
secType)
+   fmt.Fprintf(, "\t"+`.section 
.note.GNU-split-stack,"",%s`+"\n", secType)
}
 
if cfg.BuildN || cfg.BuildX {

Re: C++ PATCH to implement P1064R0, Virtual Function Calls in Constant Expressions (v2)

[Marek Polacek]

On Fri, Sep 14, 2018 at 07:36:47PM +0200, Jakub Jelinek wrote:
> On Fri, Sep 14, 2018 at 01:19:50PM -0400, Marek Polacek wrote:
> > +   /* We expect something in the form of  get x. */
> > +   if (TREE_CODE (obj) != ADDR_EXPR)
> > + return t;
> 
> Shouldn't it then be a gcc_assert instead, or code like:
>   if (TREE_CODE (obj) != ADDR_EXPR)
> {
>   if (!ctx->quiet)
> error (...);
>   *non_constant_p = true;
> }
> to make it clear that we haven't handled it and don't consider it a constant
> expression?

Not an assert, but setting *non_constant_p is probably sensible.  Thus:

v2: Set *non_constant_p if OBJ_TYPE_REF is not in expected format.

Bootstrapped/regtested on x86_64-linux.

2018-09-14  Marek Polacek  

P1064R0 - Allowing Virtual Function Calls in Constant Expressions
* call.c (build_over_call): Add FIXME.
* constexpr.c (cxx_eval_array_reference): Handle referencing the vtable.
(cxx_eval_constant_expression): Don't ignore _vptr's initializer.
(potential_constant_expression_1): Handle OBJ_TYPE_REF.
* decl.c (grokdeclarator): Change error to pedwarn.  Only warn when
pedantic and not C++2a.

* g++.dg/cpp0x/constexpr-virtual5.C: Adjust dg-error.
* g++.dg/cpp2a/constexpr-virtual1.C: New test.
* g++.dg/cpp2a/constexpr-virtual2.C: New test.
* g++.dg/cpp2a/constexpr-virtual3.C: New test.
* g++.dg/cpp2a/constexpr-virtual4.C: New test.
* g++.dg/cpp2a/constexpr-virtual5.C: New test.
* g++.dg/cpp2a/constexpr-virtual6.C: New test.
* g++.dg/cpp2a/constexpr-virtual7.C: New test.
* g++.dg/cpp2a/constexpr-virtual8.C: New test.
* g++.dg/cpp2a/constexpr-virtual9.C: New test.
* g++.dg/diagnostic/virtual-constexpr.C: Skip for C++2a.  Use
-pedantic-errors.  Adjust dg-error.

diff --git gcc/cp/call.c gcc/cp/call.c
index 69503ca7920..6c70874af40 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -8401,7 +8401,8 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
 
   if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0
   /* Don't mess with virtual lookup in instantiate_non_dependent_expr;
-virtual functions can't be constexpr.  */
+virtual functions can't be constexpr.  FIXME Actually, no longer
+true in C++2a.  */
   && !in_template_function ())
 {
   tree t;
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 88c73787961..eb6b8fa1842 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -2414,16 +2414,27 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, 
tree t,
  bool *non_constant_p, bool *overflow_p)
 {
   tree oldary = TREE_OPERAND (t, 0);
+  tree oldidx = TREE_OPERAND (t, 1);
+
+  /* The virtual table isn't constexpr, but has static storage duration and its
+ initializer is a compile-time constant, so we handle referencing an 
element
+ in the table specially.  */
+  if (TREE_TYPE (t) == vtable_entry_type)
+{
+  VERIFY_CONSTANT (oldidx);
+  tree ctor = DECL_INITIAL (oldary);
+  return CONSTRUCTOR_ELT (ctor, tree_to_uhwi (oldidx))->value;
+}
+
   tree ary = cxx_eval_constant_expression (ctx, oldary,
   lval,
   non_constant_p, overflow_p);
-  tree index, oldidx;
+  tree index;
   HOST_WIDE_INT i = 0;
   tree elem_type = NULL_TREE;
   unsigned len = 0, elem_nchars = 1;
   if (*non_constant_p)
 return t;
-  oldidx = TREE_OPERAND (t, 1);
   index = cxx_eval_constant_expression (ctx, oldidx,
false,
non_constant_p, overflow_p);
@@ -4209,7 +4220,14 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
 CONST_DECL for aggregate constants.  */
   if (lval)
return t;
+  /* is_really_empty_class doesn't take into account _vptr, so initializing
+otherwise empty class with { } would overwrite the initializer that
+initialize_vtable created for us.  */
   if (COMPLETE_TYPE_P (TREE_TYPE (t))
+ && !(DECL_INITIAL (t)
+  && TREE_CODE (DECL_INITIAL (t)) == CONSTRUCTOR
+  /* But if DECL_INITIAL was { }, do mark it as constant.  */
+  && CONSTRUCTOR_NELTS (DECL_INITIAL (t)) > 0)
  && is_really_empty_class (TREE_TYPE (t)))
{
  /* If the class is empty, we aren't actually loading anything.  */
@@ -4778,7 +4796,6 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
 case MODOP_EXPR:
   /* GCC internal stuff.  */
 case VA_ARG_EXPR:
-case OBJ_TYPE_REF:
 case NON_DEPENDENT_EXPR:
 case BASELINK:
 case OFFSET_REF:
@@ -4788,6 +4805,34 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
   *non_constant_p = true;
   break;
 
+case OBJ_TYPE_REF:
+  {
+   

Re: [PATCHv2] Handle not explicitly zero terminated strings in merge sections

[Bernd Edlinger]

On 09/14/18 21:01, Jakub Jelinek wrote:
> On Fri, Sep 14, 2018 at 06:39:38PM +, Bernd Edlinger wrote:
>> Hi,
>>
>> this is an upate of the string-merge section, it is based on the 
>> V2-STRING_CST
>> semantic patch series, which was finally installed yesterday.
>> It merges single-byte string constants with or without terminating NUL.
>> The patch has the same Ada and C test cases that were already in the V1 
>> patch.
>>
>> Thus there are no pre-requisite patches necessary at this time.
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
> 
> I believe SHF_MERGE | SHF_STRINGS sections should only ever contain
> zero-terminated strings.
> See e.g. SCO ELF documentation:
> "The size of each element is specified in the section header's sh_entsize 
> field. If the SHF_STRINGS flag is also set, the data elements consist of 
> null-terminated character strings."
> http://www.sco.com/developers/gabi/2003-12-17/ch4.sheader.html
> https://docs.oracle.com/cd/E23824_01/html/819-0690/ggdlu.html
> 
> So, please never put non-zero terminated strings into MS sections.
> 

Yes, that is of course clear.

The idea is, if the string itself is not zero-terminated
another zero is added, that is only done in at the assembly level.

Strings with embedded zero bytes are not put in the merge section.


Bernd.

Re: [PATCHv2] Handle not explicitly zero terminated strings in merge sections

[Jakub Jelinek]

On Fri, Sep 14, 2018 at 06:39:38PM +, Bernd Edlinger wrote:
> Hi,
> 
> this is an upate of the string-merge section, it is based on the V2-STRING_CST
> semantic patch series, which was finally installed yesterday.
> It merges single-byte string constants with or without terminating NUL.
> The patch has the same Ada and C test cases that were already in the V1 patch.
> 
> Thus there are no pre-requisite patches necessary at this time.
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?

I believe SHF_MERGE | SHF_STRINGS sections should only ever contain
zero-terminated strings.
See e.g. SCO ELF documentation:
"The size of each element is specified in the section header's sh_entsize 
field. If the SHF_STRINGS flag is also set, the data elements consist of 
null-terminated character strings."
http://www.sco.com/developers/gabi/2003-12-17/ch4.sheader.html
https://docs.oracle.com/cd/E23824_01/html/819-0690/ggdlu.html

So, please never put non-zero terminated strings into MS sections.

Jakub

[PATCHv2] Handle not explicitly zero terminated strings in merge sections

[Bernd Edlinger]

Hi,

this is an upate of the string-merge section, it is based on the V2-STRING_CST
semantic patch series, which was finally installed yesterday.
It merges single-byte string constants with or without terminating NUL.
The patch has the same Ada and C test cases that were already in the V1 patch.

Thus there are no pre-requisite patches necessary at this time.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
gcc:
2018-08-27  Bernd Edlinger  

	* varasm.c (output_constant): Add new parameter merge_strings.
	Make strings properly zero terminated in merge string sections.
	(mergeable_string_section): Don't fail if the last char is non-zero.
	(assemble_variable_contents): Handle merge string sections.
	(assemble_variable): Likewise.
	(assemble_constant_contents): Likewise.
	(output_constant_def_contents): Likewise.

testsuite:
2018-08-27  Bernd Edlinger  

	* gnat.dg/string_merge1.adb: New test.
	* gnat.dg/string_merge2.adb: New test.
	* gcc.dg/merge-all-constants-1.c: Adjust test.
	* gcc.dg/merge-all-constants-2.c: New test.

diff -Npur gcc/varasm.c gcc/varasm.c
--- gcc/varasm.c	2018-08-26 15:02:43.157905415 +0200
+++ gcc/varasm.c	2018-08-26 17:57:26.488494866 +0200
@@ -111,7 +111,8 @@ static int compare_constant (const tree,
 static void output_constant_def_contents (rtx);
 static void output_addressed_constants (tree);
 static unsigned HOST_WIDE_INT output_constant (tree, unsigned HOST_WIDE_INT,
-	   unsigned int, bool);
+	   unsigned int, bool,
+	   bool = false);
 static void globalize_decl (tree);
 static bool decl_readonly_section_1 (enum section_category);
 #ifdef BSS_SECTION_ASM_OP
@@ -804,8 +805,8 @@ mergeable_string_section (tree decl ATTR
   && TREE_CODE (decl) == STRING_CST
   && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE
   && align <= 256
-  && (len = int_size_in_bytes (TREE_TYPE (decl))) > 0
-  && TREE_STRING_LENGTH (decl) >= len)
+  && (len = int_size_in_bytes (TREE_TYPE (decl))) >= 0
+  && TREE_STRING_LENGTH (decl) == len)
 {
   scalar_int_mode mode;
   unsigned int modesize;
@@ -835,7 +836,7 @@ mergeable_string_section (tree decl ATTR
 	  if (j == unit)
 		break;
 	}
-	  if (i == len - unit)
+	  if (i == len - unit || (unit == 1 && i == len))
 	{
 	  sprintf (name, "%s.str%d.%d", prefix,
 		   modesize / 8, (int) (align / 8));
@@ -2117,7 +2118,7 @@ assemble_noswitch_variable (tree decl, c
 
 static void
 assemble_variable_contents (tree decl, const char *name,
-			bool dont_output_data)
+			bool dont_output_data, bool merge_strings = false)
 {
   /* Do any machine/system dependent processing of the object.  */
 #ifdef ASM_DECLARE_OBJECT_NAME
@@ -2140,7 +2141,7 @@ assemble_variable_contents (tree decl, c
 	output_constant (DECL_INITIAL (decl),
 			 tree_to_uhwi (DECL_SIZE_UNIT (decl)),
 			 get_variable_align (decl),
-			 false);
+			 false, merge_strings);
   else
 	/* Leave space for it.  */
 	assemble_zeros (tree_to_uhwi (DECL_SIZE_UNIT (decl)));
@@ -2316,7 +2317,9 @@ assemble_variable (tree decl, int top_le
 	switch_to_section (sect);
   if (align > BITS_PER_UNIT)
 	ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align / BITS_PER_UNIT));
-  assemble_variable_contents (decl, name, dont_output_data);
+  assemble_variable_contents (decl, name, dont_output_data,
+  sect->common.flags & SECTION_MERGE
+  && sect->common.flags & SECTION_STRINGS);
   if (asan_protected)
 	{
 	  unsigned HOST_WIDE_INT int size
@@ -3471,7 +3474,8 @@ maybe_output_constant_def_contents (stru
constant's alignment in bits.  */
 
 static void
-assemble_constant_contents (tree exp, const char *label, unsigned int align)
+assemble_constant_contents (tree exp, const char *label, unsigned int align,
+			bool merge_strings = false)
 {
   HOST_WIDE_INT size;
 
@@ -3481,7 +3485,7 @@ assemble_constant_contents (tree exp, co
   targetm.asm_out.declare_constant_name (asm_out_file, label, exp, size);
 
   /* Output the value of EXP.  */
-  output_constant (exp, size, align, false);
+  output_constant (exp, size, align, false, merge_strings);
 
   targetm.asm_out.decl_end ();
 }
@@ -3522,10 +3526,13 @@ output_constant_def_contents (rtx symbol
 		   || (VAR_P (decl) && DECL_IN_CONSTANT_POOL (decl))
 		   ? DECL_ALIGN (decl)
 		   : symtab_node::get (decl)->definition_alignment ());
-  switch_to_section (get_constant_section (exp, align));
+  section *sect = get_constant_section (exp, align);
+  switch_to_section (sect);
   if (align > BITS_PER_UNIT)
 	ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align / BITS_PER_UNIT));
-  assemble_constant_contents (exp, XSTR (symbol, 0), align);
+  assemble_constant_contents (exp, XSTR (symbol, 0), align,
+  sect->common.flags & SECTION_MERGE
+  && sect->common.flags & SECTION_STRINGS);
   if (asan_protected)
 	{
 	  HOST_WIDE_INT size = get_constant_size (exp);
@@ -4838,7 +4845,7 @@ output_constructor 

Re: [PATCH 5/6] detect unterminated const arrays in stpcpy calls (PR 86552)

[Jeff Law]

On 8/13/18 3:28 PM, Martin Sebor wrote:
> The attached changes implement the detection of past-the-end reads
> by stpcpy due to unterminated arguments.
> 
> 
> gcc-86552-5.diff
> 
> PR tree-optimization/86552 - missing warning for reading past the end of 
> non-string arrays
> 
> gcc/ChangeLog:
> 
>   * builtins.c (unterminated_array): Handle ARRAY_REF.
>   (expand_builtin_stpcpy_1): Detect unterminated char arrays.
>   * builtins.h (unterminated_array): Declare extern.
>   * gimple-fold.c (gimple_fold_builtin_stpcpy): Detect unterminated
> arrays.
>   (gimple_fold_builtin_sprintf): Propagate NO_WARNING to transformed
>   calls.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.dg/warn-stpcpy-no-nul.c: New test.
So with this patch I just added initialization for a NONSTR passed down
to c_strlen.  Otherwise it just worked on top of all the recent changes.

I'll install it on the trunk momentarily.

I'll probably stop here today to let the testers run through another
cycle.  What's left of this kit is #4 (sprintf) and #6 (strnlen).

Jeff

Re: detect unterminated const arrays in strcpy calls (PR 86552)

[Jeff Law]

This is primarily Martin's work.  My changes:

First, this picks up the last bit of stuff from patch #1 of the 6 series
kit to handle unterminated strings -- I was mistaken last night when I
said the prior patch wrapped up the bits from patch #1 that we needed.

I made NONSTR simply passed through get_range_strlen, eliminating one
small block of changes.  That allows it to work better with the way
c_strlen and friends accumulate values in *NONSTR and prevents
unexpected failures in the new test.

In gimple_fold_builtin_strcpy we test NONSTR before checking the return
value of get_maxval_strlen  which allows us to detect missing NUL
terminations even when get_maxval_strlen returns NULL.

And the xfails on the new tests were removed for tests which pass now.

Bootstrapped and regression tested on the trunk.  Installing on the
trunk momentarily.

Jeff
commit cd05ac910c0336ae2c066ad90c962d58bfde68dc
Author: Jeff Law 
Date:   Wed Sep 12 18:41:43 2018 -0400

* builtins.c (unterminated_array): New.
(expand_builtin_strcpy): Adjust.
(expand_builtin_strcpy_args): Detect unterminated arrays.
* gimple-fold.c (get_maxval_strlen): Add argument.  Detect
unterminated arrays.
* gimple-fold.h (get_maxval_strlen): Add argument.
(gimple_fold_builtin_strcpy): Detec unterminated arrays.

* gimple-fold.c (get_range_strlen): Add argument.
(get_maxval_strlen): Adjust.
* gimple-fold.h (get_range_strlen): Add argument.

* gcc.dg/warn-strcpy-no-nul.c: New test.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 71dc508d219..84578a331d7 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,18 @@
+2018-09-14  Martin Sebor  
+   Jeff Law  
+
+   * builtins.c (unterminated_array): New.
+   (expand_builtin_strcpy): Adjust.
+   (expand_builtin_strcpy_args): Detect unterminated arrays.
+   * gimple-fold.c (get_maxval_strlen): Add argument.  Detect
+   unterminated arrays.
+   * gimple-fold.h (get_maxval_strlen): Add argument.
+   (gimple_fold_builtin_strcpy): Detec unterminated arrays.
+
+   * gimple-fold.c (get_range_strlen): Add argument.
+   (get_maxval_strlen): Adjust.
+   * gimple-fold.h (get_range_strlen): Add argument.
+
 2018-09-14  Wei Xiao  
 
* config/i386/movdirintrin.h: Fix copyright year.
diff --git a/gcc/builtins.c b/gcc/builtins.c
index a345704a8f8..be813db46b8 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -132,7 +132,7 @@ static rtx expand_builtin_mempcpy (tree, rtx);
 static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, int);
 static rtx expand_builtin_strcat (tree, rtx);
 static rtx expand_builtin_strcpy (tree, rtx);
-static rtx expand_builtin_strcpy_args (tree, tree, rtx);
+static rtx expand_builtin_strcpy_args (tree, tree, tree, rtx);
 static rtx expand_builtin_stpcpy (tree, rtx, machine_mode);
 static rtx expand_builtin_stpncpy (tree, rtx);
 static rtx expand_builtin_strncat (tree, rtx);
@@ -563,6 +563,34 @@ warn_string_no_nul (location_t loc, const char *fn, tree 
arg, tree decl)
 }
 }
 
+/* If EXP refers to an unterminated constant character array return
+   the declaration of the object of which the array is a member or
+   element.  Otherwise return null.  */
+
+static tree
+unterminated_array (tree exp)
+{
+  if (TREE_CODE (exp) == SSA_NAME)
+{
+  gimple *stmt = SSA_NAME_DEF_STMT (exp);
+  if (!is_gimple_assign (stmt))
+   return NULL_TREE;
+
+  tree rhs1 = gimple_assign_rhs1 (stmt);
+  tree_code code = gimple_assign_rhs_code (stmt);
+  if (code != POINTER_PLUS_EXPR)
+   return NULL_TREE;
+
+  exp = rhs1;
+}
+
+  tree nonstr = NULL;
+  if (c_strlen (exp, 1, , 1) == NULL && nonstr)
+return nonstr;
+
+  return NULL_TREE;
+}
+
 /* Compute the length of a null-terminated character string or wide
character string handling character sizes of 1, 2, and 4 bytes.
TREE_STRING_LENGTH is not the right way because it evaluates to
@@ -3879,7 +3907,7 @@ expand_builtin_strcpy (tree exp, rtx target)
src, destsize);
 }
 
-  if (rtx ret = expand_builtin_strcpy_args (dest, src, target))
+  if (rtx ret = expand_builtin_strcpy_args (exp, dest, src, target))
 {
   /* Check to see if the argument was declared attribute nonstring
 and if so, issue a warning since at this point it's not known
@@ -3899,8 +3927,17 @@ expand_builtin_strcpy (tree exp, rtx target)
expand_builtin_strcpy.  */
 
 static rtx
-expand_builtin_strcpy_args (tree dest, tree src, rtx target)
+expand_builtin_strcpy_args (tree exp, tree dest, tree src, rtx target)
 {
+  /* Detect strcpy calls with unterminated arrays..  */
+  if (tree nonstr = unterminated_array (src))
+{
+  /* NONSTR refers to the non-nul terminated constant array.  */
+  if (!TREE_NO_WARNING (exp))
+   warn_string_no_nul (EXPR_LOCATION (exp), "strcpy", src, nonstr);
+ 

Re: [PATCH][4/4][v2] RPO-style value-numbering for FRE/PRE

[Gerald Pfeifer]

On Thu, 13 Sep 2018, Richard Biener wrote:
> If it reproduces with clang as a host compiler maybe you can
> use ASAN with it?

That's an ancient version of clang (3.4), so not too useful, but...

On Thu, 13 Sep 2018, Richard Biener wrote:
>   PR bootstrap/87134
>   * tree-ssa-sccvn.c (vn_nary_op_insert_into): Fix assert.
>   (vn_nary_op_insert_pieces_predicated): Do not write useless
>   valid_dominated_by_p entry outside of the allocated storage.

...seems to have done the job. 

Thank you!

Gerald

Re: C++ PATCH to implement P1064R0, Virtual Function Calls in Constant Expressions

[Jakub Jelinek]

On Fri, Sep 14, 2018 at 01:19:50PM -0400, Marek Polacek wrote:
> + /* We expect something in the form of  get x. */
> + if (TREE_CODE (obj) != ADDR_EXPR)
> +   return t;

Shouldn't it then be a gcc_assert instead, or code like:
if (TREE_CODE (obj) != ADDR_EXPR)
  {
if (!ctx->quiet)
  error (...);
*non_constant_p = true;
  }
to make it clear that we haven't handled it and don't consider it a constant
expression?

Jakub

[PATCH 4/5] Add pp_humanized_limit and pp_humanized_range

[David Malcolm]

gcc/ChangeLog:
* pretty-print.c (get_power_of_two): New function.
(struct relative_to_power_of_2): New struct.
(pp_humanized_limit): New function.
(pp_humanized_range): New function.
(selftest::assert_pp_humanized_limit): New function.
(ASSERT_PP_HUMANIZED_LIMIT): New macro.
(selftest::assert_pp_humanized_range): New function.
(ASSERT_PP_HUMANIZED_RANGE): New macro.
(selftest::test_get_power_of_two): New function.
(selftest::test_humanized_printing): New function.
(selftest::pretty_print_c_tests): Call them.
* pretty-print.h (pp_humanized_limit): New function.
(pp_humanized_range): New function.
---
 gcc/pretty-print.c | 181 +
 gcc/pretty-print.h |   4 ++
 2 files changed, 185 insertions(+)

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 7dd900b..7a2cd30 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1799,6 +1799,107 @@ pp_end_quote (pretty_printer *pp, bool show_color)
   pp_string (pp, close_quote);
 }
 
+/* Get the largest power of two that is less that or equal to VAL.
+
+   FIXME: does this exist in stdlib (e.g. log2l in C99)?  */
+
+static unsigned
+get_power_of_two (unsigned HOST_WIDE_INT val)
+{
+  int power_of_two = 0;
+  while (val >= 2)
+{
+  val >>= 1;
+  power_of_two++;
+}
+  return power_of_two;
+}
+
+/* A way to express a number in terms of an offset from a power of two.  */
+
+struct relative_to_power_of_2
+{
+  relative_to_power_of_2 (unsigned HOST_WIDE_INT val)
+  {
+m_shift = get_power_of_two (val);
+m_offset = val - (1ul << m_shift);
+
+if (m_shift > 1)
+  if (m_offset > (1l << (m_shift - 1)))
+   {
+ m_shift++;
+ m_offset = val - (1ul << m_shift);
+   }
+  }
+
+  void print (pretty_printer *pp) const
+  {
+if (m_offset > 0)
+  pp_printf (pp, "(1<<%u)+%wd", m_shift, m_offset);
+else if (m_offset < 0)
+  pp_printf (pp, "(1<<%u)-%wd", m_shift, -m_offset);
+else
+  pp_printf (pp, "1<<%u", m_shift);
+  }
+
+  unsigned m_shift;
+  HOST_WIDE_INT m_offset;
+};
+
+/* Print VAL to PP, in a "humanized" way.
+   Small numbers are printed in decimal form (e.g. "42").
+   Large numbers close to a power of two are printed relative to a
+   power of two.  For example, 4294967295 is printed as "(1<<32)-1".  */
+
+void
+pp_humanized_limit (pretty_printer *pp, unsigned HOST_WIDE_INT val)
+{
+  /* For large numbers near a power of two, print them symbolically.
+ "65536" is about the most I can recognize by eye (dmalcolm),
+ 1<<17 is where I can't tell at a glance if it is indeed 1<<17.  */
+  const unsigned threshold = 1 << 16;
+  if (val > threshold)
+{
+  relative_to_power_of_2 rel2 (val);
+
+  /* How near to the large power of two should they be to be
+printed in terms of it?  */
+  const unsigned threshold_2 = 100;
+  if (abs (rel2.m_offset) <= threshold_2)
+   {
+ rel2.print (pp);
+ return;
+   }
+}
+
+  /* Otherwise, just print the value as a decimal.  */
+  pp_printf (pp, "%wu", val);
+}
+
+/* Print the range MIN to MAX to PP, in a "humanized" way.
+   For example if MIN == MAX, just one number is printed.
+
+   Return true if both values are 1, false otherwise.
+   FIXME: how do plural forms work for *ranges*?  */
+
+bool
+pp_humanized_range (pretty_printer *pp, unsigned HOST_WIDE_INT min,
+   unsigned HOST_WIDE_INT max)
+{
+  if (min == max)
+{
+  pp_humanized_limit (pp, min);
+  return min == 1;
+}
+  else
+{
+  pp_humanized_limit (pp, min);
+  pp_string (pp, "...");
+  pp_humanized_limit (pp, max);
+  return false;
+}
+}
+
 
 /* The string starting at P has LEN (at least 1) bytes left; if they
start with a valid UTF-8 sequence, return the length of that
@@ -2211,6 +2312,84 @@ test_pp_format ()
"problem with %qs at line %i", "bar", 10);
 }
 
+/* Implementation detail of ASSERT_PP_HUMANIZED_LIMIT.  */
+
+static void
+assert_pp_humanized_limit (const location , unsigned HOST_WIDE_INT val,
+  const char *expected)
+{
+  pretty_printer pp;
+  pp_humanized_limit (, val);
+  ASSERT_STREQ_AT (loc, expected, pp_formatted_text ());
+}
+
+/* Verify that pp_humanized_limit (pp, VAL) is EXPECTED.  */
+
+#define ASSERT_PP_HUMANIZED_LIMIT(VAL, EXPECTED)   \
+  SELFTEST_BEGIN_STMT  \
+assert_pp_humanized_limit ((SELFTEST_LOCATION), (VAL), (EXPECTED)); \
+  SELFTEST_END_STMT
+
+/* Implementation detail of ASSERT_PP_HUMANIZED_RANGE.  */
+
+static void
+assert_pp_humanized_range (const location , unsigned HOST_WIDE_INT min,
+  unsigned HOST_WIDE_INT max, const char *expected)
+{
+  pretty_printer pp;
+  pp_humanized_range (, min, max);
+  ASSERT_STREQ_AT (loc, expected, pp_formatted_text ());
+}

[PATCH 2/5] Add range_idx param to range_label::get_text

[David Malcolm]

This patch updates the pure virtual function range_label::get_text
(and its implementations) so that the index of the range is passed
in, allowing for one label instance to be shared by multiple ranges.

gcc/c-family/ChangeLog:
* c-format.c (range_label_for_format_type_mismatch::get_text):
Update for new param.

gcc/c/ChangeLog:
* c-objc-common.c (range_label_for_type_mismatch::get_text):
Update for new param.
* c-typeck.c (maybe_range_label_for_tree_type_mismatch::get_text):
Likewise.

gcc/cp/ChangeLog:
* error.c (range_label_for_type_mismatch::get_text): Update for
new param.

gcc/ChangeLog:
* diagnostic-show-locus.c (class layout_range): Add field
"m_original_idx".
(layout_range::layout_range): Add "original_idx" param and use it
to initialize new field.
(make_range): Use 0 for original_idx.
(layout::layout): Pass in index to calls to
maybe_add_location_range.
(layout::maybe_add_location_range): Add param "original_idx" and
pass it on to layout_range.
(layout::print_any_labels): Pass on range->m_original_idx to
get_text call.
(gcc_rich_location::add_location_if_nearby): Use 0 for
original_idx.
* gcc-rich-location.h (text_range_label::get_text): Update for new
param.
(range_label_for_type_mismatch::get_text): Likewise.

libcpp/ChangeLog:
* include/line-map.h (range_label::get_text): Add param
"range_idx".
---
 gcc/c-family/c-format.c |  4 ++--
 gcc/c/c-objc-common.c   |  2 +-
 gcc/c/c-typeck.c|  4 ++--
 gcc/cp/error.c  |  2 +-
 gcc/diagnostic-show-locus.c | 19 ++-
 gcc/gcc-rich-location.h |  4 ++--
 libcpp/include/line-map.h   |  6 --
 7 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index a7edfca..a1133c7 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -3597,9 +3597,9 @@ class range_label_for_format_type_mismatch
   {
   }
 
-  label_text get_text () const FINAL OVERRIDE
+  label_text get_text (unsigned range_idx) const FINAL OVERRIDE
   {
-label_text text = range_label_for_type_mismatch::get_text ();
+label_text text = range_label_for_type_mismatch::get_text (range_idx);
 if (text.m_buffer == NULL)
   return text;
 
diff --git a/gcc/c/c-objc-common.c b/gcc/c/c-objc-common.c
index 12e777a..fee5268 100644
--- a/gcc/c/c-objc-common.c
+++ b/gcc/c/c-objc-common.c
@@ -218,7 +218,7 @@ c_tree_printer (pretty_printer *pp, text_info *text, const 
char *spec,
range_label_for_type_mismatch.  */
 
 label_text
-range_label_for_type_mismatch::get_text () const
+range_label_for_type_mismatch::get_text (unsigned /*range_idx*/) const
 {
   if (m_labelled_type == NULL_TREE)
 return label_text (NULL, false);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 5f8df12..5388ddb 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -11049,7 +11049,7 @@ class maybe_range_label_for_tree_type_mismatch : public 
range_label
   {
   }
 
-  label_text get_text () const FINAL OVERRIDE
+  label_text get_text (unsigned range_idx) const FINAL OVERRIDE
   {
 if (m_expr == NULL_TREE
|| !EXPR_P (m_expr))
@@ -11061,7 +11061,7 @@ class maybe_range_label_for_tree_type_mismatch : public 
range_label
   other_type = TREE_TYPE (m_other_expr);
 
range_label_for_type_mismatch inner (expr_type, other_type);
-   return inner.get_text ();
+   return inner.get_text (range_idx);
   }
 
  private:
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 5bab3f3..601f6d2 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -4289,7 +4289,7 @@ qualified_name_lookup_error (tree scope, tree name,
Compare with print_template_differences above.  */
 
 label_text
-range_label_for_type_mismatch::get_text () const
+range_label_for_type_mismatch::get_text (unsigned /*range_idx*/) const
 {
   if (m_labelled_type == NULL_TREE)
 return label_text (NULL, false);
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 6ce8a0f..7dfb0a0 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -128,6 +128,7 @@ class layout_range
const expanded_location *finish_exploc,
enum range_display_kind range_display_kind,
const expanded_location *caret_exploc,
+   unsigned original_idx,
const range_label *label);
 
   bool contains_point (linenum_type row, int column) const;
@@ -137,6 +138,7 @@ class layout_range
   layout_point m_finish;
   enum range_display_kind m_range_display_kind;
   layout_point m_caret;
+  unsigned m_original_idx;
   const range_label *m_label;
 };
 
@@ -236,6 +238,7 @@ class layout
  diagnostic_t diagnostic_kind);
 
   bool maybe_add_location_range (const location_range *loc_range,
+unsigned original_idx,

[PATCH 3/5] gimple-ssa-sprintf.c: move struct call_info out of the dom_walker subclass

[David Malcolm]

gcc/ChangeLog:
* gimple-ssa-sprintf.c (struct call_info): New forward decl.
(class sprintf_dom_walker): Remove forward decl.
(struct sprintf_dom_walker::call_info): Rename to...
(struct call_info): ...this.
(should_warn_p): Update for move of call_info out of
sprintf_dom_walker.
(maybe_warn): Likewise.
(format_directive): Likewise.
(parse_directive): Likewise.
(is_call_safe): Likewise.
(try_substitute_return_value): Likewise.
(try_simplify_call): Likewise.
---
 gcc/gimple-ssa-sprintf.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 65c7f92..ab430fe 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -116,6 +116,7 @@ const pass_data pass_data_sprintf_length = {
 static int warn_level;
 
 struct format_result;
+struct call_info;
 
 class sprintf_dom_walker : public dom_walker
 {
@@ -127,7 +128,6 @@ class sprintf_dom_walker : public dom_walker
   void after_dom_children (basic_block) FINAL OVERRIDE;
   bool handle_gimple_call (gimple_stmt_iterator *);
 
-  struct call_info;
   bool compute_format_length (call_info &, format_result *);
   class evrp_range_analyzer evrp_range_analyzer;
 };
@@ -890,7 +890,7 @@ bytes_remaining (unsigned HOST_WIDE_INT navail, const 
format_result )
 
 /* Description of a call to a formatted function.  */
 
-struct sprintf_dom_walker::call_info
+struct call_info
 {
   /* Function call statement.  */
   gimple *callstmt;
@@ -2333,7 +2333,7 @@ format_plain (const directive , tree, vr_values *)
should be diagnosed given the AVAILable space in the destination.  */
 
 static bool
-should_warn_p (const sprintf_dom_walker::call_info ,
+should_warn_p (const call_info ,
   const result_range , const result_range )
 {
   if (result.max <= avail.min)
@@ -2404,7 +2404,7 @@ should_warn_p (const sprintf_dom_walker::call_info ,
 
 static bool
 maybe_warn (substring_loc , location_t argloc,
-   const sprintf_dom_walker::call_info ,
+   const call_info ,
const result_range _range, const result_range ,
const directive )
 {
@@ -2684,7 +2684,7 @@ maybe_warn (substring_loc , location_t argloc,
in *RES.  Return true if the directive has been handled.  */
 
 static bool
-format_directive (const sprintf_dom_walker::call_info ,
+format_directive (const call_info ,
  format_result *res, const directive ,
  class vr_values *vr_values)
 {
@@ -2963,7 +2963,7 @@ format_directive (const sprintf_dom_walker::call_info 
,
the directive.  */
 
 static size_t
-parse_directive (sprintf_dom_walker::call_info ,
+parse_directive (call_info ,
 directive , format_result *res,
 const char *str, unsigned *argno,
 vr_values *vr_values)
@@ -3491,7 +3491,7 @@ get_destination_size (tree dest)
of its return values.  */
 
 static bool
-is_call_safe (const sprintf_dom_walker::call_info ,
+is_call_safe (const call_info ,
  const format_result , bool under4k,
  unsigned HOST_WIDE_INT retval[2])
 {
@@ -3550,7 +3550,7 @@ is_call_safe (const sprintf_dom_walker::call_info ,
 
 static bool
 try_substitute_return_value (gimple_stmt_iterator *gsi,
-const sprintf_dom_walker::call_info ,
+const call_info ,
 const format_result )
 {
   tree lhs = gimple_get_lhs (info.callstmt);
@@ -3668,7 +3668,7 @@ try_substitute_return_value (gimple_stmt_iterator *gsi,
 
 static bool
 try_simplify_call (gimple_stmt_iterator *gsi,
-  const sprintf_dom_walker::call_info ,
+  const call_info ,
   const format_result )
 {
   unsigned HOST_WIDE_INT dummy[2];
-- 
1.8.5.3

[PATCH 5/5] gimple-ssa-sprintf.c: rewrite overflow/truncation diagnostic (PR middle-end/77696)

[David Malcolm]

gcc/ChangeLog:
PR middle-end/77696
* gimple-ssa-sprintf.c: Include "gcc-rich-location.h".
(struct directive_state): New struct.
(directive_state::get_text): New function.
(format_result::add_directive): New member function.
(format_result::should_warn): New field.
(format_result::m_directives): New field.
(fmtwarn_n): Delete.
(maybe_warn): Delete.
(format_directive): Add param "is_nul_terminator".  Rename locals
"start" and "length" to "start_idx" and "end_idx" respectively.
Call res->add_directive.  Eliminate call to maybe_warn in favor of
calling should_warn_p and updating res->should_warn.
Eliminate notes.
(class rich_format_location): New class.
(rich_format_location::flyweight_range_label::get_text): New
function.
(rich_format_location::get_label_for_buffer): New function.
(rich_format_location::get_label_for_directive): New function.
(sprintf_dom_walker::compute_format_length): Add param "dst_ptr".
Retain param "callloc".  Initialize res->should_warn.  Add local
"is_nul_terminator" and pass to format_directive.  Rather than
returning at the end of the format string, check for res->should_warn
and emit warnings using rich_format_location.
(sprintf_dom_walker::handle_gimple_call): Pass in dst_ptr to
compute_format_length.

gcc/testsuite/ChangeLog:
PR middle-end/77696
* gcc.dg/sprintf-diagnostics-1.c: New test.
* gcc.dg/sprintf-diagnostics-2.c: New test.
---
 gcc/gimple-ssa-sprintf.c | 646 ---
 gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c | 252 +++
 gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c |  57 +++
 3 files changed, 597 insertions(+), 358 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c
 create mode 100644 gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index ab430fe..fb6268c 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -83,6 +83,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 #include "vr-values.h"
 #include "gimple-ssa-evrp-analyze.h"
+#include "gcc-rich-location.h"
 
 /* The likely worst case value of MB_LEN_MAX for the target, large enough
for UTF-8.  Ideally, this would be obtained by a target hook if it were
@@ -128,7 +129,7 @@ class sprintf_dom_walker : public dom_walker
   void after_dom_children (basic_block) FINAL OVERRIDE;
   bool handle_gimple_call (gimple_stmt_iterator *);
 
-  bool compute_format_length (call_info &, format_result *);
+  bool compute_format_length (call_info &, format_result *, tree);
   class evrp_range_analyzer evrp_range_analyzer;
 };
 
@@ -193,10 +194,67 @@ struct result_range
   unsigned HOST_WIDE_INT unlikely;
 };
 
+/* Information about a particular directive (or the NUL terminator)
+   within a format string, for use when emitting warnings.  */
+
+struct directive_state
+{
+  directive_state (size_t fmt_start_idx, size_t fmt_end_idx,
+  bool is_nul_terminator,
+  unsigned HOST_WIDE_INT min_size,
+  unsigned HOST_WIDE_INT max_size)
+  : m_fmt_start_idx (fmt_start_idx), m_fmt_end_idx (fmt_end_idx),
+m_is_nul_terminator (is_nul_terminator),
+m_min_size (min_size), m_max_size (max_size)
+  {}
+
+  label_text get_text () const;
+
+  size_t m_fmt_start_idx;
+  size_t m_fmt_end_idx;
+  bool m_is_nul_terminator;
+  unsigned HOST_WIDE_INT m_min_size;
+  unsigned HOST_WIDE_INT m_max_size;
+};
+
+/* Generate a label for the directive within the format string, describing
+   the size consumed by the output, and highlighting the NUL terminator.  */
+
+label_text
+directive_state::get_text () const
+{
+  pretty_printer pp_range;
+  bool singular = pp_humanized_range (_range, m_min_size, m_max_size);
+  pretty_printer pp;
+  // FIXME: i18n for singular/plural
+  if (singular)
+pp_printf (, G_("%s byte"),
+  pp_formatted_text (_range));
+  else
+pp_printf (, G_("%s bytes"),
+  pp_formatted_text (_range));
+  if (m_is_nul_terminator)
+pp_string (, G_(" (for NUL terminator)"));
+  return label_text (xstrdup (pp_formatted_text ()), true);
+}
+
 /* The result of a call to a formatted function.  */
 
 struct format_result
 {
+  /* Record a directive from FMT_START_IDX through FMT_END_IDX,
+ and whether it's the nul terminator, consuming between
+ MIN_SIZE and MAX_SIZE bytes when output.  */
+  void add_directive (size_t fmt_start_idx, size_t fmt_end_idx,
+ bool is_nul_terminator,
+ unsigned HOST_WIDE_INT min_size,
+ unsigned HOST_WIDE_INT max_size)
+  {
+m_directives.safe_push (directive_state (fmt_start_idx, fmt_end_idx,
+is_nul_terminator,
+ 

[PATCH 0/5] RFC: gimple-ssa-sprintf.c: a new approach (PR middle-end/77696)

[David Malcolm]

Martin: on the way back from Cauldron I had a go at implementing new
output formats for the warnings from gimple-ssa-sprintf.c, to try to
better indicate to the end user what the problem is.  My plan was to
implement some of the ASCII art ideas we've been discussing.  However,
to try to understand what the pass is doing, I tried visualizing the
existing data structures, and I ended up liking the output of that so much,
that I think that it is what we ought to show the end-user.

Consider the examples from PR middle-end/77696:

char d[4];

typedef __SIZE_TYPE__ size_t;

extern int sprintf (char*, const char*, ...);
extern int snprintf (char*, size_t, const char*, ...);


void f (int i)
{
  // bounded, definite truncation in a directve
  snprintf (d, sizeof d, "%i", 1235);

  // bounded, definite truncation copying format string
  snprintf (d, sizeof d, "%iAB", 123);

  // unbounded, definite overflow in a directve
  sprintf (d, "%i", 1235);

  // unbounded, definite overflow copying format string
  sprintf (d, "%iAB", 123);

  // bounded, possible truncation a directve
  snprintf (d, sizeof d, "%i", i);

  // bounded, possible overflow copying format string
  snprintf (d, sizeof d, "%iAB", i);

  // unbounded, possible overflow in a directve
  sprintf (d, "%i", i);

  // unbounded, possible overflow copying format string
  sprintf (d, "%iAB", 123);

  // unbounded, possible overflow copying format string
  const char *s = i ? "123" : "1234";
  sprintf (d, "%sAB", s);
}

extern char buf_10[80];
extern char tmpdir[80];
extern char fname[8];

void g (int num)
{
  sprintf (buf_10, "/%s/%s-%i.tmp", tmpdir, fname, num);
}

trunk currently emits:

zz.c: In function ‘f’:
zzz.c:12:29: warning: ‘snprintf’ output truncated before the last format 
character [-Wformat-truncation=]
12 |   snprintf (d, sizeof d, "%i", 1235);
   | ^
zzz.c:12:3: note: ‘snprintf’ output 5 bytes into a destination of size 4
12 |   snprintf (d, sizeof d, "%i", 1235);
   |   ^~
zzz.c:15:30: warning: ‘AB’ directive output truncated writing 2 bytes into a 
region of size 1 [-Wformat-truncation=]
15 |   snprintf (d, sizeof d, "%iAB", 123);
   | ~^
zzz.c:15:3: note: ‘snprintf’ output 6 bytes into a destination of size 4
15 |   snprintf (d, sizeof d, "%iAB", 123);
   |   ^~~
zzz.c:18:18: warning: ‘sprintf’ writing a terminating nul past the end of the 
destination [-Wformat-overflow=]
18 |   sprintf (d, "%i", 1235);
   |  ^
zzz.c:18:3: note: ‘sprintf’ output 5 bytes into a destination of size 4
18 |   sprintf (d, "%i", 1235);
   |   ^~~
zzz.c:21:19: warning: ‘AB’ directive writing 2 bytes into a region of size 1 
[-Wformat-overflow=]
21 |   sprintf (d, "%iAB", 123);
   |  ~^
zzz.c:21:3: note: ‘sprintf’ output 6 bytes into a destination of size 4
21 |   sprintf (d, "%iAB", 123);
   |   ^~~~
zzz.c:33:19: warning: ‘AB’ directive writing 2 bytes into a region of size 1 
[-Wformat-overflow=]
33 |   sprintf (d, "%iAB", 123);
   |  ~^
zzz.c:33:3: note: ‘sprintf’ output 6 bytes into a destination of size 4
33 |   sprintf (d, "%iAB", 123);
   |   ^~~~
zzz.c:37:19: warning: ‘AB’ directive writing 2 bytes into a region of size 
between 0 and 1 [-Wformat-overflow=]
37 |   sprintf (d, "%sAB", s);
   |  ~^
zzz.c:37:3: note: ‘sprintf’ output between 6 and 7 bytes into a destination of 
size 4
37 |   sprintf (d, "%sAB", s);
   |   ^~
zzz.c: In function ‘g’:
zzz.c:46:24: warning: ‘/’ directive writing 1 byte into a region of size 
between 0 and 79 [-Wformat-overflow=]
46 |   sprintf (buf_10, "/%s/%s-%i.tmp", tmpdir, fname, num);
   |^
zzz.c:46:3: note: ‘sprintf’ output between 9 and 105 bytes into a destination 
of size 80
46 |   sprintf (buf_10, "/%s/%s-%i.tmp", tmpdir, fname, num);
   |   ^


With this patch kit, gcc emits:

zzz.c: In function ‘f’:
zzz.c:12:13: warning: ‘snprintf’ will truncate the output (5 bytes) to size 4 
[-Wformat-truncation=]
12 |   snprintf (d, sizeof d, "%i", 1235);
   | ^ ^~^
   | | | |
   | | | 1 byte (for NUL terminator)
   | | 4 bytes
   | capacity: 4 bytes
zzz.c:1:6: note: destination declared here
1 | char d[4];
  |  ^
zzz.c:15:13: warning: ‘snprintf’ will truncate the output (6 bytes) to size 4 
[-Wformat-truncation=]
15 |   snprintf (d, sizeof d, "%iAB", 123);
   | ^ ^~^~^
   | | | | |
   | | | | 1 byte (for NUL terminator)
   | | | 2 bytes
   | | 3 bytes
   | capacity: 4 bytes
zzz.c:1:6: note: destination declared here
1 | char d[4];
  |  ^
zzz.c:18:12: 

[PATCH 1/5] substring-locations: add class format_string_diagnostic_t

[David Malcolm]

With the addition of ranges in r263564, format_warning_at_substring_n
has 10 arguments.

Reduce the number of args by bundling the shared ones into a
class format_string_diagnostic_t.

gcc/c-family/ChangeLog:
* c-format.c (format_warning_at_char): Update for introduction of
format_string_diagnostic_t.
(format_type_warning): Likewise.

gcc/ChangeLog:
* gimple-ssa-sprintf.c (fmtwarn): Update for introduction of
format_string_diagnostic_t.
(fmtwarn_n): Likewise.
* substring-locations.c
(format_string_diagnostic_t::format_string_diagnostic_t) New ctor.
(format_warning_n_va): Convert to...
(format_string_diagnostic_t::emit_warning_n_va): ...this.
(format_warning_va): Convert to...
(format_string_diagnostic_t::emit_warning_va): ...this.
(format_warning_at_substring): Convert to...
(format_string_diagnostic_t::emit_warning): ...this.
(format_warning_at_substring_n): Convert to...
(format_string_diagnostic_t::emit_warning_n): ...this.
* substring-locations.h (class format_string_diagnostic_t): New
class.
(format_warning_va): Convert to
format_string_diagnostic_t::emit_warning_va.
(format_warning_n_va): Convert to
format_string_diagnostic_t::emit_warning_n_va.
(format_warning_at_substring): Convert to
format_string_diagnostic_t::emit_warning.
(format_warning_at_substring_n): Convert to
format_string_diagnostic_t::emit_warning_n.
---
 gcc/c-family/c-format.c   |  28 ++--
 gcc/gimple-ssa-sprintf.c  |  16 ---
 gcc/substring-locations.c | 113 +++---
 gcc/substring-locations.h |  74 +++---
 4 files changed, 115 insertions(+), 116 deletions(-)

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 98c49cf..a7edfca 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -100,8 +100,9 @@ format_warning_at_char (location_t fmt_string_loc, tree 
format_string_cst,
 
   substring_loc fmt_loc (fmt_string_loc, string_type, char_idx, char_idx,
 char_idx);
-  bool warned = format_warning_va (fmt_loc, NULL, UNKNOWN_LOCATION, NULL,
-  NULL, opt, gmsgid, );
+  format_string_diagnostic_t diag (fmt_loc, NULL, UNKNOWN_LOCATION, NULL,
+  NULL);
+  bool warned = diag.emit_warning_va (opt, gmsgid, );
   va_end (ap);
 
   return warned;
@@ -3694,13 +3695,13 @@ format_type_warning (const substring_loc _fmt_loc,
   char *corrected_substring
 = get_corrected_substring (fmt_loc, type, arg_type, fki,
   offset_to_type_start, conversion_char);
-
+  format_string_diagnostic_t diag (fmt_loc, _label, param_loc, 
_label,
+  corrected_substring);
   if (wanted_type_name)
 {
   if (arg_type)
-   format_warning_at_substring
- (fmt_loc, _label, param_loc, _label,
-  corrected_substring, OPT_Wformat_,
+   diag.emit_warning
+ (OPT_Wformat_,
   "%s %<%s%.*s%> expects argument of type %<%s%s%>, "
   "but argument %d has type %qT",
   gettext (kind_descriptions[kind]),
@@ -3708,9 +3709,8 @@ format_type_warning (const substring_loc _fmt_loc,
   format_length, format_start,
   wanted_type_name, p, arg_num, arg_type);
   else
-   format_warning_at_substring
- (fmt_loc, _label, param_loc, _label,
-  corrected_substring, OPT_Wformat_,
+   diag.emit_warning
+ (OPT_Wformat_,
   "%s %<%s%.*s%> expects a matching %<%s%s%> argument",
   gettext (kind_descriptions[kind]),
   (kind == CF_KIND_FORMAT ? "%" : ""),
@@ -3719,9 +3719,8 @@ format_type_warning (const substring_loc _fmt_loc,
   else
 {
   if (arg_type)
-   format_warning_at_substring
- (fmt_loc, _label, param_loc, _label,
-  corrected_substring, OPT_Wformat_,
+   diag.emit_warning
+ (OPT_Wformat_,
   "%s %<%s%.*s%> expects argument of type %<%T%s%>, "
   "but argument %d has type %qT",
   gettext (kind_descriptions[kind]),
@@ -3729,9 +3728,8 @@ format_type_warning (const substring_loc _fmt_loc,
   format_length, format_start,
   wanted_type, p, arg_num, arg_type);
   else
-   format_warning_at_substring
- (fmt_loc, _label, param_loc, _label,
-  corrected_substring, OPT_Wformat_,
+   diag.emit_warning
+ (OPT_Wformat_,
   "%s %<%s%.*s%> expects a matching %<%T%s%> argument",
   gettext (kind_descriptions[kind]),
   (kind == CF_KIND_FORMAT ? "%" : ""),
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 433e558..65c7f92 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -455,7 +455,8 @@ get_format_string (tree format, location_t *ploc)
 }
 
 

Re: [PATCH] genmatch: isolate reporting into a function

[Jeff Law]

On 9/3/18 1:56 AM, Martin Liška wrote:
> Hi.
> 
> This is small improvement for {gimple,generic}-match.c files.
> The code path that reports application of a pattern is now guarded
> with __builtin_expect. And reporting function lives in gimple.c.
> 
> For gimple-match.o, difference is:
> 
> bloaty /tmp/after.o -- /tmp/before.o
>  VM SIZE   FILE SIZE
>  ++ GROWING ++
>   [ = ]   0 .rela.debug_loc +58.5Ki  +0.5%
>   +0.7% +7.70Ki .text   +7.70Ki  +0.7%
>   [ = ]   0 .debug_info +3.53Ki  +0.6%
>   [ = ]   0 .rela.debug_ranges  +2.02Ki  +0.0%
>   [ = ]   0 .debug_loc  +1.86Ki  +0.7%
>   +0.7%+448 .eh_frame  +448  +0.7%
>   [ = ]   0 .rela.eh_frame +192  +0.7%
>   [ = ]   0 .rela.debug_line+48  +0.4%
>   [ = ]   0 .debug_str  +26  +0.0%
>   +6.9%  +9 .rodata.str1.1   +9  +6.9%
> 
>  -- SHRINKING   --
>  -97.5% -24.8Ki .rodata.str1.8  -24.8Ki -97.5%
>   [ = ]   0 .symtab -14.7Ki -26.1%
>   [ = ]   0 .strtab -3.57Ki  -2.2%
>   [ = ]   0 .rela.debug_info-2.81Ki  -0.0%
>   [ = ]   0 .debug_line -2.14Ki  -0.6%
>   [ = ]   0 .rela.text -816  -0.1%
>   [ = ]   0 .rela.text.unlikely-288  -0.1%
>   -0.1%-131 .text.unlikely -131  -0.1%
>   [ = ]   0 [Unmapped]  -23 -14.0%
>   [ = ]   0 .debug_abbrev-2  -0.1%
> 
>   -1.2% -16.8Ki TOTAL   +25.1Ki  +0.1%
> 
> I'm testing the patch.
> Thoughts?
> 
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-08-31  Martin Liska  
> 
>   * genmatch.c (output_line_directive): Add new argument
>   fnargs.
>   (dt_simplify::gen_1): Generate call to report_match_pattern
>   function and wrap that into __builtin_expect.
>   * gimple.c (report_match_pattern): New function.
>   * gimple.h (report_match_pattern): Likewise.
Seems reasonable, though I'm not generally a fan of __builtin_expect :-)

jeff

C++ PATCH to implement P1064R0, Virtual Function Calls in Constant Expressions

[Marek Polacek]

This patch implements another bit of C++20, virtual calls in constant
expression:

The basic idea is that since in a constant expression we know the dynamic
type (to detect invalid code etc.), the restriction that prohibits virtual
calls is unnecessary.

Handling virtual function calls turned out to be fairly easy (as anticipated);
I simply let the constexpr machinery figure out the dynamic type and then
OBJ_TYPE_REF_TOKEN gives us the index into the virtual function table.  That
way we get the function decl we're interested in, and cxx_eval_call_expression
takes it from there.

But handling pointer-to-virtual-member-functions doesn't work like that.
get_member_function_from_ptrfunc creates a COND_EXPR which looks like
if (pf.__pfn & 1) // is it a virtual function?
  // yes, find the pointer in the vtable
else
  // no, just return the pointer
so ideally we want to evaluate the then-branch.  Eventually it'll evaluate it
to something like _ZTV2X2[2], but the vtable isn't constexpr so we'd end up
with "not a constant expression" error.  Since the vtable initializer is
a compile-time constant, I thought we could make it work by a hack as the one
in cxx_eval_array_reference.  We'll then let cxx_eval_call_expression do its
job and everything is hunky-dory.

Except when it isn't: I noticed that the presence of _vptr doesn't make the
class non-empty, and when cxx_eval_constant_expression saw a decl with an empty
class type, it just evaluated it to { }.  But such a class still had gotten an
initializer that looks like {.D.2082 = {._vptr.X2 = &_ZTV2X2 + 16}}.  So
replacing it with { } will lose the proper initializer whereupon we fail.
The check I've added to cxx_eval_constant_expression won't win any beauty
contests but unfortunately EMPTY_CONSTRUCTOR_P doesn't work there.

I've made no attempt to handle virtual bases.  The proposal doesn't mention
them at all, and I thought this would be enough for starters.

Bootstrapped/regtested on x86_64-linux.

2018-09-14  Marek Polacek  

P1064R0 - Allowing Virtual Function Calls in Constant Expressions
* call.c (build_over_call): Add FIXME.
* constexpr.c (cxx_eval_array_reference): Handle referencing the vtable.
(cxx_eval_constant_expression): Don't ignore _vptr's initializer.
(potential_constant_expression_1): Handle OBJ_TYPE_REF.
* decl.c (grokdeclarator): Change error to pedwarn.  Only warn when
pedantic and not C++2a.

* g++.dg/cpp0x/constexpr-virtual5.C: Adjust dg-error.
* g++.dg/cpp2a/constexpr-virtual1.C: New test.
* g++.dg/cpp2a/constexpr-virtual2.C: New test.
* g++.dg/cpp2a/constexpr-virtual3.C: New test.
* g++.dg/cpp2a/constexpr-virtual4.C: New test.
* g++.dg/cpp2a/constexpr-virtual5.C: New test.
* g++.dg/cpp2a/constexpr-virtual6.C: New test.
* g++.dg/cpp2a/constexpr-virtual7.C: New test.
* g++.dg/cpp2a/constexpr-virtual8.C: New test.
* g++.dg/cpp2a/constexpr-virtual9.C: New test.
* g++.dg/diagnostic/virtual-constexpr.C: Skip for C++2a.  Use
-pedantic-errors.  Adjust dg-error.

diff --git gcc/cp/call.c gcc/cp/call.c
index 69503ca7920..6c70874af40 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -8401,7 +8401,8 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
 
   if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0
   /* Don't mess with virtual lookup in instantiate_non_dependent_expr;
-virtual functions can't be constexpr.  */
+virtual functions can't be constexpr.  FIXME Actually, no longer
+true in C++2a.  */
   && !in_template_function ())
 {
   tree t;
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 88c73787961..8ebd86b611c 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -2414,16 +2414,27 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, 
tree t,
  bool *non_constant_p, bool *overflow_p)
 {
   tree oldary = TREE_OPERAND (t, 0);
+  tree oldidx = TREE_OPERAND (t, 1);
+
+  /* The virtual table isn't constexpr, but has static storage duration and its
+ initializer is a compile-time constant, so we handle referencing an 
element
+ in the table specially.  */
+  if (TREE_TYPE (t) == vtable_entry_type)
+{
+  VERIFY_CONSTANT (oldidx);
+  tree ctor = DECL_INITIAL (oldary);
+  return CONSTRUCTOR_ELT (ctor, tree_to_uhwi (oldidx))->value;
+}
+
   tree ary = cxx_eval_constant_expression (ctx, oldary,
   lval,
   non_constant_p, overflow_p);
-  tree index, oldidx;
+  tree index;
   HOST_WIDE_INT i = 0;
   tree elem_type = NULL_TREE;
   unsigned len = 0, elem_nchars = 1;
   if (*non_constant_p)
 return t;
-  oldidx = TREE_OPERAND (t, 1);
   index = cxx_eval_constant_expression (ctx, oldidx,
   

Re: PR85787: Extend malloc_candidate_p to handle multiple phis.

[Jeff Law]

On 8/28/18 5:26 AM, Prathamesh Kulkarni wrote:
> H
> The attached patch extends malloc_candidate_p to handle multiple phis.
> There's a lot of noise in the patch because I moved most of
> malloc_candidate_p into
> new function malloc_candidate_p_1. The only real change is following hunk:
> 
> +   gimple *arg_def = SSA_NAME_DEF_STMT (arg);
> +   if (is_a (arg_def))
> + {
> +   if (!malloc_candidate_p_1 (fun, arg, phi, ipa))
> +   DUMP_AND_RETURN ("nested phi fail")
> +   continue;
> + }
> +
> 
> Which checks recursively that the phi argument is used only within
> comparisons against 0
> and the phi.
> 
> Bootstrapped+tested on x86_64-unknown-linux-gnu.
> OK to commit ?
> 
> Thanks,
> Prathamesh
> 
> 
> pr85787-1.txt
> 
> 2018-08-28  Prathamesh Kulkarni  
> 
>   PR tree-optimization/85787
>   * ipa-pure-const.c (malloc_candidate_p_1): Move most of 
> malloc_candidate_p
>   into this function and add support for detecting multiple phis.
>   (DUMP_AND_RETURN): Move from malloc_candidate_p into top-level macro.
> 
OK.
jeff

Re: [PATCH] Fix overeager spelling corrections (PR c/82967)

[Jeff Law]

On 8/24/18 9:24 AM, David Malcolm wrote:
> This patch tunes class best_match's cutoff for rejecting meaningless
> spelling suggestions.
> 
> Previously, we allowed an edit distance of up to half of the length of the
> longer of the goal string and closest candidate strings, rounded down.
> 
> With this patch, we now allow only up to a third - with some tuning of
> rounding (and for very short strings), to ensure that:
> (a) everything that worked before still works (with the removal of a
> couple of cases that shouldn't), and that
> (b) the new threshold is always at least as conservative as the old
> threshold and thus shouldn't offer new nonsensical suggestions (with
> the possible exception of cases where transposition has helped; see
> r261521 aka Damerau-Levenshtein; PR other/69968).
> 
> In particular, all of the bogus suggestions from PR c/82967 are now
> no longer offered.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> adds 4 PASS results to gcc.sum.
> 
> OK for trunk?
> 
> OK for backporting to gcc 8? (with the caveat that gcc 8's
> edit distance doesn't do transpositions)
> 
> gcc/ChangeLog:
>   PR c/82967
>   * spellcheck.c (get_edit_distance_cutoff): New function.
>   (selftest::test_edit_distance_unit_test_oneway): Rename to...
>   (selftest::test_get_edit_distance_one_way): ...this.
>   (selftest::test_get_edit_distance_unit): Rename to...
>   (selftest::test_get_edit_distance_both_ways): ...this.
>   (selftest::test_edit_distances): Move tests to this new function,
>   and test some more pairs of strings.  Update for above renaming.
>   (selftest::get_old_cutoff): New function.
>   (selftest::test_get_edit_distance_cutoff): New function.
>   (selftest::assert_suggested_for): New function.
>   (ASSERT_SUGGESTED_FOR): New macro.
>   (selftest::assert_not_suggested_for): New function.
>   (ASSERT_NOT_SUGGESTED_FOR): New macro.
>   (selftest::test_suggestions): New function.
>   (selftest::spellcheck_c_tests): Move test_get_edit_distance_unit
>   tests to selftest::test_edit_distances and call it.  Add calls to
>   selftest::test_get_edit_distance_cutoff and
>   selftest::test_suggestions.
>   * spellcheck.h (get_edit_distance_cutoff): New function declaration.
>   (best_match::consider): Replace hard-coded cutoff calculation with
>   a call to...
>   (best_match::get_cutoff): New declaration.
>   (best_match::get_best_meaningful_candidate): Likewise.
> 
> gcc/testsuite/ChangeLog:
>   PR c/82967
>   * c-c++-common/attributes-1.c: Remove bogus suggestion from
>   dg-prune-output.
>   * gcc.dg/diagnostic-token-ranges.c (undeclared_identifier): Remove
>   bogus suggestion.
>   * gcc.dg/spellcheck-identifiers-4.c: New test.
ISTM that while not technically listed as a maintainer for this stuff,
you're by far the most well versed in the implementation.
Rubber-stamped with an OK.

jeff

Re: [PATCH, testsuite] Don't assume that HOSTCXX can respond to TEST_ALWAYS_FLAGS.

[Jeff Law]

On 8/29/18 4:30 AM, Iain Sandoe wrote:
> Hi,
> 
> I have noticed, for some configuration cases, that the ms-sysv tests fail 
> with “can’t build generator”.
> 
> The setting of HOSTCXX in gcc/testsuite/gcc/site.exp is dependent on the top 
> level configuration;
> for example, configuring with —target,host,build = x86_64-linux-gnu or 
> CXX=/usr/bin/g++ sets the
> site.exp HOSTCXX to the discovered bootstrap compiler
> 
> Whereas, configuring with a plain …/configure  .. sets HOSTCXX to 
> prev-gcc/xg++.
> 
> In the latter case, the “host” compiler can respond to TEST_ALWAYS_FLAGS, in 
> the former cases
> it cannot necessarily (unless one is bootstrapping with gcc-9).
> 
> The patch below removes  TEST_ALWAYS_FLAGS from the generator build - and the 
> tests pass for
> me on x86_64-linux and x86_64-darwin with various configuration lines.
> 
> If that doesn’t cover enough cases, and it’s really necessary that the 
> generator is built with
> TEST_ALWAYS_FLAGS, then support for them in HOSTCXX needs to be checked 
> (there is already a
> check for the host compiler support for c++11).
> 
> OK for trunk?
> thanks
> Iain
> 
> gcc/testsuite/
> 
>   * gcc.target/x86_64/abi/ms-sysv/ms-sysv.exp (runtest_ms_sysv): Don’t
>   assume that the HOSTCXX supports TEST_ALWAYS_FLAGS.
OK.
jeff

Re: [PATCH, Darwin, config] Arrange for configure to detect "otool".

[Jeff Law]

On 8/30/18 9:45 AM, Iain Sandoe wrote:
> Currently, we fail to detect some Darwin assembler capabilities because the
> configure tests rely on a BINUTILS objdump, which isn’t generally available
> on Darwin.
> 
> Darwin uses a tool called "otool" to provide information about objects, and we
> should be able to extend or adapt config tests to use this.
> 
> libtool already ‘knows’ about otool, we just need to teach the GCC configury.
> 
> OK for trunk?
> Iain
> 
>   * configure.ac (NCN_STRICT_CHECK_TOOLS): Check otool.
>   (ACX_CHECK_INSTALLED_TARGET_TOOL): Likewise
>   (GCC_TARGET_TOOL): Likewise.
>   * Makefile.tpl (HOST_EXPORTS): Add OTOOL, OTOOL_FOR_TARGET.
>   (BASE_TARGET_EXPORTS): OTOOL, export OTOOL_FOR_TARGET.
>   OTOOL, OTOOL_FOR_TARGET: New substitutions. 
>   (EXTRA_HOST_FLAGS, EXTRA_TARGET_FLAGS): Add OTOOL.
>   * configure: Regenerate. 
>   * Makefile.in: Likewise.
> 
> gcc/
> 
>   * configure.ac (gcc_cv_otool): Set.
>   * configure: Regenerate.
Ranier suggested keeping things sorted alphabetically by tool name.
With that nit fixed this is fine.
jeff

Re: Fix a minor copyright year typo

[Jeff Law]

On 9/4/18 3:39 AM, wei xiao wrote:
> Hi,
> 
> According to commit log, this file is created in the year 2018, hence
> the copyright year should be corrected to 2018.
> Is below patch ok to fix this problem?
> 
> Thanks
> Wei Xiao
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 62e32c6..416dbd1 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-09-04  Wei Xiao  
> +
> +   * gcc/config/i386/movdirintrin.h: Update copyright year.
Thanks.  Installed.
jeff

Go patch committed: Don't use address of temporary for deferred delete

[Ian Lance Taylor]

This patch changes the Go frontend to nott use the address of a
temporary for a deferred delete.  This corrects the handling of a
deferred delete in a loop, to not use a temporary whose value will, at
deferred execution time, wind up being the last value in the loop.
The test for this is TestDeferDeleteSlow in the 1.11 runtime package.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 264295)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-218c9159635e06e39ae43d0efe1ac1e694fead2e
+3fd61802286c81e5fb672f682d9e661181184d1f
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 264259)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -7409,7 +7409,32 @@ Builtin_call_expression::do_lower(Gogo*,
  loc);
Expression* e3 = Expression::make_temporary_reference(key_temp,
  loc);
-   e3 = Expression::make_unary(OPERATOR_AND, e3, loc);
+
+   // If the call to delete is deferred, and is in a loop,
+   // then the loop will only have a single instance of the
+   // temporary variable.  Passing the address of the
+   // temporary variable here means that the deferred call
+   // will see the last value in the loop, not the current
+   // value.  So for this unusual case copy the value into
+   // the heap.
+   if (!this->is_deferred())
+ e3 = Expression::make_unary(OPERATOR_AND, e3, loc);
+   else
+ {
+   Expression* a = Expression::make_allocation(mt->key_type(),
+   loc);
+   Temporary_statement* atemp =
+ Statement::make_temporary(NULL, a, loc);
+   inserter->insert(atemp);
+
+   a = Expression::make_temporary_reference(atemp, loc);
+   a = Expression::make_dereference(a, NIL_CHECK_NOT_NEEDED, loc);
+   Statement* s = Statement::make_assignment(a, e3, loc);
+   inserter->insert(s);
+
+   e3 = Expression::make_temporary_reference(atemp, loc);
+ }
+
return Runtime::make_call(Runtime::MAPDELETE, this->location(),
  3, e1, e2, e3);
  }
@@ -9024,6 +9049,10 @@ Builtin_call_expression::do_copy()
 
   if (this->varargs_are_lowered())
 bce->set_varargs_are_lowered();
+  if (this->is_deferred())
+bce->set_is_deferred();
+  if (this->is_concurrent())
+bce->set_is_concurrent();
   return bce;
 }
 
@@ -9606,8 +9635,16 @@ Call_expression::do_lower(Gogo* gogo, Na
 
   // Recognize a call to a builtin function.
   if (fntype->is_builtin())
-return new Builtin_call_expression(gogo, this->fn_, this->args_,
-  this->is_varargs_, loc);
+{
+  Builtin_call_expression* bce =
+   new Builtin_call_expression(gogo, this->fn_, this->args_,
+   this->is_varargs_, loc);
+  if (this->is_deferred_)
+   bce->set_is_deferred();
+  if (this->is_concurrent_)
+   bce->set_is_concurrent();
+  return bce;
+}
 
   // If this call returns multiple results, create a temporary
   // variable to hold them.
@@ -10275,6 +10312,10 @@ Call_expression::do_copy()
 
   if (this->varargs_are_lowered_)
 call->set_varargs_are_lowered();
+  if (this->is_deferred_)
+call->set_is_deferred();
+  if (this->is_concurrent_)
+call->set_is_concurrent();
   return call;
 }
 
Index: gcc/go/gofrontend/parse.cc
===
--- gcc/go/gofrontend/parse.cc  (revision 264283)
+++ gcc/go/gofrontend/parse.cc  (working copy)
@@ -4304,9 +4304,15 @@ Parse::go_or_defer_stat()
   this->gogo_->start_block(stat_location);
   Statement* stat;
   if (is_go)
-stat = Statement::make_go_statement(call_expr, stat_location);
+{
+  stat = Statement::make_go_statement(call_expr, stat_location);
+  call_expr->set_is_concurrent();
+}
   else
-stat = Statement::make_defer_statement(call_expr, stat_location);
+{
+  stat = Statement::make_defer_statement(call_expr, stat_location);
+  call_expr->set_is_deferred();
+}
   this->gogo_->add_statement(stat);
   this->gogo_->add_block(this->gogo_->finish_block(stat_location),
 stat_location);

Re: Fold more boolean expressions

[Marc Glisse]

On Fri, 14 Sep 2018, MCC CS wrote:


+/* (~x & y) | ~(x | y) -> ~x */
+(simplify
+ (bit_ior:c (bit_and:c (bit_not @0) @1) (bit_not (bit_ior:c @0 @1)))
+ (bit_not @0))


As I said last time, you should not generate a new (bit_not @0) in the 
output when there is already one in the input. Maybe


(simplify
 (bit_ior:c (bit_and:c (bit_not@2 @0) @1) (bit_not (bit_ior:c @0 @1)))
 @2)

--
Marc Glisse

Re: [PATCH 23/25] Testsuite: GCN is always PIE.

[Jeff Law]

On 9/5/18 5:52 AM, a...@codesourcery.com wrote:
> 
> The GCN/HSA loader ignores the load address and uses a random location, so we
> build all GCN binaries as PIE, by default.
> 
> This patch makes the necessary testsuite adjustments to make this work
> correctly.
> 
> 2018-09-05  Andrew Stubbs  
> 
>   gcc/testsuite/
>   * gcc.dg/graphite/scop-19.c: Check pie_enabled.
>   * gcc.dg/pic-1.c: Disable on amdgcn.
>   * gcc.dg/pic-2.c: Disable on amdgcn.
>   * gcc.dg/pic-3.c: Disable on amdgcn.
>   * gcc.dg/pic-4.c: Disable on amdgcn.
>   * gcc.dg/pie-3.c: Disable on amdgcn.
>   * gcc.dg/pie-4.c: Disable on amdgcn.
>   * gcc.dg/uninit-19.c: Check pie_enabled.
>   * lib/target-supports.exp (check_effective_target_pie): Add amdgcn.
OK.  Commit at your leisure.

jeff

Re: [PATCH 24/25] Ignore LLVM's blank lines.

[Jeff Law]

On 9/5/18 5:52 AM, a...@codesourcery.com wrote:
> 
> The GCN toolchain must use the LLVM assembler and linker because there's no
> binutils port.  The LLVM tools do not have the same diagnostic style as
> binutils, so the "blank line(s) in output" tests are inappropriate (and very
> noisy).
> 
> The LLVM tools also have different command line options, so it's not possible
> to autodetect object formats in the same way.
> 
> This patch addresses both issues.
> 
> 2018-09-05  Andrew Stubbs  
> 
>   gcc/testsuite/
>   * lib/file-format.exp (gcc_target_object_format): Handle AMD GCN.
>   * lib/gcc-dg.exp (gcc-dg-prune): Ignore blank lines from the LLVM
>   linker.
>   * lib/target-supports.exp (check_effective_target_llvm_binutils): New.
This is fine.  It's a NOP for other targets, so feel free to commit when
it's convenient for you.

jeff

Re: [PATCH] rs6000: Add another Z to go with Y (PR87224)

[Segher Boessenkool]

On Fri, Sep 14, 2018 at 03:20:28PM +, Segher Boessenkool wrote:
> This is another case where we ICE because Y does not allow reg+reg, we
> need Z for that.

Backported to 8, too.


Segher


> 2018-09-14  Segher Boessenkool  
> 
>   PR target/87224
>   * config/rs6000/rs6000.md (*mov_hardfloat64): Add Z to the Y
>   alternatives.

[PATCH][AArch64][committed] Fix gcc.target/aarch64/combine_bfi_1.c

[Kyrill Tkachov]

Hi all,

This test started failing because some of the functions in the combine dump 
that it scans uses a different pattern to match the same instruction: 
insv_regsi rather than aarch64_bfi.
The code generation is still the same.
The patch changes the scan to look for the actual instruction we want in the 
assembly.

This fixes the test.
Committing to trunk as obvious.

Thanks,
Kyrill

2018-09-14  Kyrylo Tkachov  

* gcc.target/aarch64/combine_bfi_1.c: Scan for bfi instruction
rather than pattern name in combine dump.
diff --git a/gcc/testsuite/gcc.target/aarch64/combine_bfi_1.c b/gcc/testsuite/gcc.target/aarch64/combine_bfi_1.c
index 9cc3bdb3ddfc286bfb6c079aaba27fb5edb1806c..cee42ccafcce0e0524f30fa6857abd902ca5779a 100644
--- a/gcc/testsuite/gcc.target/aarch64/combine_bfi_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/combine_bfi_1.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-rtl-combine" } */
+/* { dg-options "-O2" } */
 
 int
 f1 (int x, int y)
@@ -31,4 +31,4 @@ f5 (long long x, long long y)
   return (x & ~0xull) | (y & 0x);
 }
 
-/* { dg-final { scan-rtl-dump-times "\\*aarch64_bfi" 5 "combine" } } */
+/* { dg-final { scan-assembler-times {\tbfi\t} 5 } } */

Re: [PATCH 01/25] Handle vectors that don't fit in an integer.

[Richard Sandiford]

 writes:
> GCN vector sizes range between 64 and 512 bytes, none of which have
> correspondingly sized integer modes.  This breaks a number of assumptions
> throughout the compiler, but I don't really want to create modes just for this
> purpose.
>
> Instead, this patch fixes up the cases that I've found, so far, such that the
> compiler tries something else, or fails to optimize, rather than just ICE.
>
> 2018-09-05  Andrew Stubbs  
> Kwok Cheung Yeung  
>   Jan Hubicka  
>   Martin Jambor  
>
>   gcc/
>   * combine.c (gen_lowpart_or_truncate): Return clobber if there is
>   not a integer mode if the same size as x.
>   (gen_lowpart_for_combine): Fail if there is no integer mode of the
>   same size.
>   * expr.c (expand_expr_real_1): Force first operand to be in memory
>   if it is a vector register and the result is in BLKmode.
>   * tree-vect-stmts.c (vectorizable_store): Don't ICE when
>   int_mode_for_size fails.
>   (vectorizable_load): Likewise.
> ---
>  gcc/combine.c | 13 -
>  gcc/expr.c|  8 
>  gcc/tree-vect-stmts.c |  8 
>  3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/combine.c b/gcc/combine.c
> index a2649b6..cbf9dae 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -8621,7 +8621,13 @@ gen_lowpart_or_truncate (machine_mode mode, rtx x)
>  {
>/* Bit-cast X into an integer mode.  */
>if (!SCALAR_INT_MODE_P (GET_MODE (x)))
> - x = gen_lowpart (int_mode_for_mode (GET_MODE (x)).require (), x);
> + {
> +   enum machine_mode imode =
> + int_mode_for_mode (GET_MODE (x)).require ();
> +   if (imode == BLKmode)
> + return gen_rtx_CLOBBER (mode, const0_rtx);
> +   x = gen_lowpart (imode, x);

require () will ICE if there isn't an integer mode and always returns
a scalar_int_mode, so this looks like a no-op.  I think you want
something like:

scalar_int_mode imode;
if (!int_mode_for_mode (GET_MODE (x)).exists ())
  ...

> @@ -11698,6 +11704,11 @@ gen_lowpart_for_combine (machine_mode omode, rtx x)
>if (omode == imode)
>  return x;
>  
> +  /* This can happen when there is no integer mode corresponding
> + to a size of vector mode.  */
> +  if (omode == BLKmode)
> +goto fail;
> +
>/* We can only support MODE being wider than a word if X is a
>   constant integer or has a mode the same size.  */
>if (maybe_gt (GET_MODE_SIZE (omode), UNITS_PER_WORD)

This seems like it's working around a bug in ther caller.

> diff --git a/gcc/expr.c b/gcc/expr.c
> index cd5cf12..776254a 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -10569,6 +10569,14 @@ expand_expr_real_1 (tree exp, rtx target, 
> machine_mode tmode,
> || maybe_gt (bitpos + bitsize,
>  GET_MODE_BITSIZE (mode2)));
>  
> + /* If the result is in BLKmode and the underlying object is a
> +vector in a register, and the size of the vector is larger than
> +the largest integer mode, then we must force OP0 to be in memory
> +as this is assumed in later code.  */
> + if (REG_P (op0) && VECTOR_MODE_P (mode2) && mode == BLKmode
> + && maybe_gt (bitsize, MAX_FIXED_MODE_SIZE))
> +   must_force_mem = 1;
> +
>   /* Handle CONCAT first.  */
>   if (GET_CODE (op0) == CONCAT && !must_force_mem)
> {

Are you sure this is still needed after:

2018-06-04  Richard Sandiford  

* expr.c (expand_expr_real_1): Force the operand into memory if
its TYPE_MODE is BLKmode and if there is no integer mode for
the number of bits being extracted.

If so, what case is it handling differently?

> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 8d94fca..607a2bd 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -6702,12 +6702,12 @@ vectorizable_store (stmt_vec_info stmt_info, 
> gimple_stmt_iterator *gsi,
>supported.  */
> unsigned lsize
>   = group_size * GET_MODE_BITSIZE (elmode);
> -   elmode = int_mode_for_size (lsize, 0).require ();
> unsigned int lnunits = const_nunits / group_size;
> /* If we can't construct such a vector fall back to
>element extracts from the original vector type and
>element size stores.  */
> -   if (mode_for_vector (elmode, lnunits).exists ()
> +   if (int_mode_for_size (lsize, 0).exists ()
> +   && mode_for_vector (elmode, lnunits).exists ()
> && VECTOR_MODE_P (vmode)
> && targetm.vector_mode_supported_p (vmode)
> && (convert_optab_handler (vec_extract_optab,
> @@ -7839,11 +7839,11 @@ vectorizable_load (stmt_vec_info stmt_info, 
> gimple_stmt_iterator *gsi,
>to a larger load.  */
> 

Re: Fold more boolean expressions

[MCC CS]

Thanks for the review! I've cleaned up the :s and :c flags.
You can see the changes here: https://www.diffchecker.com/sjNOq5TQ

Anyone else have time for another review? Would be really
appreciated.

2018-09-14 MCC CS 

gcc/
PR tree-optimization/87261
* match.pd: Add boolean optimizations,
fix whitespace.

2018-09-14 MCC CS 

gcc/testsuite/
PR tree-optimization/87261
* gcc.dg/pr87261.c: New test.

Index: gcc/match.pd
===
--- gcc/match.pd (revision 264170)
+++ gcc/match.pd (working copy)
@@ -92,7 +92,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
IFN_FMA IFN_FMS IFN_FNMA IFN_FNMS)
(define_operator_list COND_TERNARY
IFN_COND_FMA IFN_COND_FMS IFN_COND_FNMA IFN_COND_FNMS)
-
+
/* As opposed to convert?, this still creates a single pattern, so
it is not a suitable replacement for convert? in all cases. */
(match (nop_convert @0)
@@ -106,7 +106,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
&& tree_nop_conversion_p (TREE_TYPE (type), TREE_TYPE (TREE_TYPE (@0))
/* This one has to be last, or it shadows the others. */
(match (nop_convert @0)
- @0)
+ @0)

/* Transform likes of (char) ABS_EXPR <(int) x> into (char) ABSU_EXPR 
ABSU_EXPR returns unsigned absolute value of the operand and the operand
@@ -285,7 +285,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
And not for _Fract types where we can't build 1. */
(if (!integer_zerop (@0) && !ALL_FRACT_MODE_P (TYPE_MODE (type)))
{ build_one_cst (type); }))
- /* X / abs (X) is X < 0 ? -1 : 1. */
+ /* X / abs (X) is X < 0 ? -1 : 1. */
(simplify
(div:C @0 (abs @0))
(if (INTEGRAL_TYPE_P (type)
@@ -921,6 +921,31 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(bitop:c @0 (bit_not (bitop:cs @0 @1)))
(bitop @0 (bit_not @1

+/* (~x & y) | ~(x | y) -> ~x */
+(simplify
+ (bit_ior:c (bit_and:c (bit_not @0) @1) (bit_not (bit_ior:c @0 @1)))
+ (bit_not @0))
+
+/* (x | y) ^ (x | ~y) -> ~x */
+(simplify
+ (bit_xor:c (bit_ior:c @0 @1) (bit_ior:c @0 (bit_not @1)))
+ (bit_not @0))
+
+/* (x & y) | ~(x | y) -> ~(x ^ y) */
+(simplify
+ (bit_ior:c (bit_and @0 @1) (bit_not:s (bit_ior:s @0 @1)))
+ (bit_not (bit_xor @0 @1)))
+
+/* (~x | y) ^ (x ^ y) -> x | ~y */
+(simplify
+ (bit_xor:c (bit_ior:cs (bit_not @0) @1) (bit_xor:c @0 @1))
+ (bit_ior @0 (bit_not @1)))
+
+/* (x ^ y) | ~(x | y) -> ~(x & y) */
+(simplify
+ (bit_ior:c (bit_xor @0 @1) (bit_not:s (bit_ior @0 @1)))
+ (bit_not (bit_and @0 @1)))
+
/* (x | y) & ~x -> y & ~x */
/* (x & y) | ~x -> y | ~x */
(for bitop (bit_and bit_ior)
@@ -1131,7 +1156,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (tree_nop_conversion_p (type, TREE_TYPE (@0))
&& tree_nop_conversion_p (type, TREE_TYPE (@1)))
(mult (convert @0) (convert (negate @1)
-
+
/* -(A + B) -> (-B) - A. */
(simplify
(negate (plus:c @0 negate_expr_p@1))
@@ -3091,7 +3116,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (tree_int_cst_sgn (@1) < 0)
(scmp @0 @2)
(cmp @0 @2))
-
+
/* Simplify comparison of something with itself. For IEEE
floating-point, we can only do some of these simplifications. */
(for cmp (eq ge le)
@@ -3162,11 +3187,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
}
tree newtype
= (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (type1)
- ? TREE_TYPE (@0) : type1);
+ ? TREE_TYPE (@0) : type1);
}
(if (TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (newtype))
(cmp (convert:newtype @0) (convert:newtype @1))
-
+
(simplify
(cmp @0 REAL_CST@1)
/* IEEE doesn't distinguish +0 and -0 in comparisons. */
@@ -3414,7 +3439,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(FTYPE) N == CST -> 0
(FTYPE) N != CST -> 1. */
(if (cmp == EQ_EXPR || cmp == NE_EXPR)
- { constant_boolean_node (cmp == NE_EXPR, type); })
+ { constant_boolean_node (cmp == NE_EXPR, type); })
/* Otherwise replace with sensible integer constant. */
(with
{
@@ -3656,7 +3681,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(simplify
(cmp (bit_and@2 @0 integer_pow2p@1) @1)
(icmp @2 { build_zero_cst (TREE_TYPE (@0)); })))
-
+
/* If we have (A & C) != 0 ? D : 0 where C and D are powers of 2,
convert this into a shift followed by ANDing with D. */
(simplify
@@ -3876,7 +3901,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (cmp == LE_EXPR)
(ge (convert:st @0) { build_zero_cst (st); })
(lt (convert:st @0) { build_zero_cst (st); }))
-
+
(for cmp (unordered ordered unlt unle ungt unge uneq ltgt)
/* If the second operand is NaN, the result is constant. */
(simplify
@@ -4530,7 +4555,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (wi::to_wide (@1) == -1)
(rdiv { build_real (type, dconst1); } @0

-/* Narrowing of arithmetic and logical operations.
+/* Narrowing of arithmetic and logical operations.

These are conceptually similar to the transformations performed for
the C/C++ front-ends by shorten_binary_op and shorten_compare. Long
@@ -4602,7 +4627,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(convert (bit_and (op (convert:utype @0) (convert:utype @1))
(convert:utype @4

-/* Transform (@0 < @1 and @0 < @2) to use min,
+/* 

[PATCH] rs6000: Add another Z to go with Y (PR87224)

[Segher Boessenkool]

This is another case where we ICE because Y does not allow reg+reg, we
need Z for that.

Committing.


Segher


2018-09-14  Segher Boessenkool  

PR target/87224
* config/rs6000/rs6000.md (*mov_hardfloat64): Add Z to the Y
alternatives.

---
 gcc/config/rs6000/rs6000.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index e40dc42..b0889df 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -7455,13 +7455,13 @@ (define_insn "*mov_hardfloat64"
   [(set (match_operand:FMOVE64 0 "nonimmediate_operand"
"=m,   d,  d,  ,   wY,
  ,Z,  ,  ,  !r,
- Y,   r,  !r, *c*l,   !r,
+ YZ,  r,  !r, *c*l,   !r,
 *h,   r,  wg, r,  ")
 
(match_operand:FMOVE64 1 "input_operand"
 "d,   m,  d,  wY, ,
  Z,   ,   ,  ,  ,
- r,   Y,  r,  r,  h,
+ r,   YZ, r,  r,  h,
  0,   wg, r,  ,   r"))]
 
   "TARGET_POWERPC64 && TARGET_HARD_FLOAT
-- 
1.8.3.1

Re: [PR debug/87295] ICE in dwarf2out

[Richard Biener]

On Thu, Sep 13, 2018 at 5:22 PM Nathan Sidwell  wrote:
>
> Richard, Jan,  as early debug experts perhaps you can clue me in?
>
> 87295 is an ICE in lookup_external_ref, we strcmp a NULL
> die->die_id.die_symbol.
>
> The die is being cloned in clone_as_declaration during a
> copy_ancestor_tree walk ultimately coming from
> copy_decls_for_unworthy_types.
>
> The incoming die is DW_TAG_structure_type with comdat_type_p set.  We
> create a new_die_raw, which has NULL die_id.die_symbol and then never
> set that field (or another piece of the die_id union).
>
> How is this supposed to work?

I think we're forgetting to "undo" the -fdebug-types-section part of the early
LTO debug output before trying to do it again for the fat part and there we
get confused.

Test coverage is non-existant for -fdebug-types-section (mostly), so this
has gone unnoticed.  In dwarf2out_finish you'll find a if
(flag_generate_lto || ...)
section done early that undoes quite some stuff but appearantly that's not
enough ...

Richard.

>
> nathan
> --
> Nathan Sidwell

Re: [PATCH][tree-ssa-mathopts] PR tree-optimization/87259: Try execute_cse_reciprocals_1 when optimize_recip_sqrt fails

[Richard Biener]

On Wed, Sep 12, 2018 at 6:25 PM Kyrill Tkachov
 wrote:
>
>
> On 12/09/18 15:51, Richard Biener wrote:
> > On Wed, Sep 12, 2018 at 2:56 PM Kyrill Tkachov
> >  wrote:
> >> Hi all,
> >>
> >> This PR is an SSA validation error where the definition ends up not 
> >> dominating a use.
> >> This appeared after my recip_sqrt optimisation. Though the optimisation 
> >> doesn't get triggered
> >> as there is no sqrt involved it prevents execute_cse_reciprocals_1 from 
> >> running as it's currently
> >> an either/or situation. I think execute_cse_reciprocals_1 doesn't like it 
> >> when some divisions get
> >> execute_cse_reciprocals_1 called on them and some don't.
> >>
> >> This patch changes optimize_recip_sqrt to return a boolean indicating 
> >> success and call execute_cse_reciprocals_1
> >> if no transform was made so that execute_cse_reciprocals_1 has a chance to 
> >> optimise it as well.
> >>
> >> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> Ok for trunk?
> > OK.  But your analysis suggests to do it the other way around - call
> > execute_cse_reciprocals_1 and if that
> > leaves a division at *gsi process it with optimize_recip_sqrt?
> >
>
> Ah, that's a shorter approach, this version implements that.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> Is this preferable?

Yes.

OK for trunk.
Richard.

> Thanks,
> Kyrill
>
> 2018-09-12  Kyrylo Tkachov  
>
>  PR tree-optimization/87259
>  PR lto/87283
>  (pass_cse_reciprocals::execute): Run optimize_recip_sqrt after
>  execute_cse_reciprocals_1 has tried transforming.
>
> 2018-09-12  Kyrylo Tkachov  
>
>  PR tree-optimization/87259
>  * gcc.dg/pr87259.c: New test.
>

Re: [PATCH] Avoid another non zero terminated string constant (revert)

[Bernd Edlinger]

On 07/30/18 08:52, Richard Biener wrote:
> On Sun, 29 Jul 2018, Bernd Edlinger wrote:
> 
>> Hi!
>>
>> This fixes another not NUL terminated string literal that is created
>> in tree-ssa-forwprop.c at simplify_builtin_call.
>>
>> src_buf is set up to contain a NUL at src_buf[src_len], thus use src_len + 1
>> as length parameter to build_string_literal.  All other uses of
>> build_string_literal do it right, as far as I can see.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
> 
> OK.
> 
> Richard.
> 

Hi,

when I installed that patch it appeared to me that naturally all string
constants ought to be zero-terminated.

But since we have now the STRING_CST semantic changes installed,
which do equally support both zero and non-zero terminated string constants,
I want to revert this again:


r263068 | edlinger | 2018-07-30 15:26:25 +0200 (Mo, 30. Jul 2018) | 5 Zeilen
Geänderte Pfade:
M /trunk/gcc/ChangeLog
M /trunk/gcc/tree-ssa-forwprop.c

2018-07-30  Bernd Edlinger  

 * tree-ssa-forwprop.c (simplify_builtin_call): Don't create a not NUL
 terminated string literal.

So unless there are reasons to keep this zero-terminated string constant,
I am going to revert this patch today evening, after another
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.


Thanks
Bernd.
Index: gcc/ChangeLog
===
--- gcc/ChangeLog	(revision 263068)
+++ gcc/ChangeLog	(revision 263067)
@@ -1,8 +1,3 @@
-2018-07-30  Bernd Edlinger  
-
-	* tree-ssa-forwprop.c (simplify_builtin_call): Don't create a not NUL
-	terminated string literal.
-
 2018-07-30  Segher Boessenkool  
 
 	PR rtl-optimization/85160
Index: gcc/tree-ssa-forwprop.c
===
--- gcc/tree-ssa-forwprop.c	(revision 263068)
+++ gcc/tree-ssa-forwprop.c	(revision 263067)
@@ -1391,7 +1391,7 @@
 src_buf, ptr1_align, false))
 	break;
 
-	  new_str_cst = build_string_literal (src_len + 1, src_buf);
+	  new_str_cst = build_string_literal (src_len, src_buf);
 	  if (callee1)
 	{
 	  /* If STMT1 is a mem{,p}cpy call, adjust it and remove

Re: [PATCH] fixincludes: vxworks: regs.h: Guard include of vxTypesOld.h by !_ASMLANGUAGE

[Olivier Hainque]

> On 13 Sep 2018, at 00:25, Rasmus Villemoes  wrote:

>> What happens on your end if you just remove the hack ?

> Unfortunately, the libstdc++ build breaks:
> 
> In file included from
> /usr/powerpc-wrs-vxworks/wind_base/target/h/regs.h:66:0,
> from
> /bld/vestas/auto/work.20180912.214646/gcc-build/gcc/include-fixed/setjmp.h:57,
> from
> /bld/vestas/auto/work.20180912.214646/gcc-build/powerpc-wrs-vxworks/libstdc++-v3/include/csetjmp:42,
> from
> /bld/vestas/auto/work.20180912.214646/gcc-src/libstdc++-v3/include/precompiled/stdc++.h:42:
> /usr/powerpc-wrs-vxworks/wind_base/target/h/arch/ppc/regsPpc.h:33:5:
> error: 'UINT32' does not name a type
> UINT32 cr;   /* condition register */
> ^~

Ah, I see. Thanks for the experiment.

> I'm happy to add an include of vxCpu in the _ASMLANGUAGE case, along
> with a big comment. But, it's also a small enough patch that we can
> carry it internally, if you prefer that we don't touch this hack upstream.

I'm fine with a change here. It could only possibly impact inclusions
of regs.h from assembly, and should normally improve that, so the risk
of breaking something is very low.

We can always reassess if need be. We (AdaCore) have managed to work
without fixincludes so far but will probably need to start relying on
it in the near future. This will shake it some more.

I wonder how we haven't hit the stop above, as it indicates an
inclusion of regs.h without a prior inclusion of vxTypes from the
VxWorks setjmp.h, included unconditionally from precompiled/stdc++.h
via csetjmp. Maybe different set of headers for different versions
of the OS.

A comment would help and doesn't need to be big.

The general idea, I think, is that different pieces of regs.h are
useful in assembly or other contexts, the two contexts rely on slightly
different sets of prerequisites and the more complete set (vxTypesOld.h)
is incompatible with an inclusion in assembly for some configurations.

Olivier

Re: VRP: allow unsigned truncating conversions that will fit

[Michael Matz]

Hi,

On Fri, 14 Sep 2018, Aldy Hernandez wrote:

> Is there a subtle reason why we're avoiding unsigned truncating conversions of
> the form:
> 
>   [X, +INF]
> 
> If the X fits in the new type, why can't we just build [X, +INF] in the new
> type?

But (uin8_t)((unsigned)[255,+INF]) aka ([255,+INF] & 255) isn't [255,+INF] 
but rather [0,+INF].  What am I missing?  Even considering that the caller 
must canonicalize, I don't see how that would transform [255,+INF] (which 
is a normal range) into [0,+INF].


Ciao,
Michael.

Re: VRP: allow unsigned truncating conversions that will fit

[Aldy Hernandez]

On 09/14/2018 06:42 AM, Aldy Hernandez wrote:

I'm still thinking about this one...

Is there a subtle reason why we're avoiding unsigned truncating 
conversions of the form:


 [X, +INF]

If the X fits in the new type, why can't we just build [X, +INF] in the 
new type?  See attached patch.


For that matter, the new type doesn't even have to be unsigned, as long 
as the old type is unsigned.


[25, +INF] can happily convert from long long unsigned to signed char.



If there isn't, OK for trunk?

VRP: allow unsigned truncating conversions that will fit

[Aldy Hernandez]

I'm still thinking about this one...

Is there a subtle reason why we're avoiding unsigned truncating 
conversions of the form:


[X, +INF]

If the X fits in the new type, why can't we just build [X, +INF] in the 
new type?  See attached patch.


If there isn't, OK for trunk?
commit b2e8c9baeeca3e321957d205b2c4c8b83b9bc754
Author: Aldy Hernandez 
Date:   Fri Sep 14 12:02:42 2018 +0200

* wide-int-range.cc (wide_int_range_convert): Allow unsigned
truncating conversions that will fit in the new type.

diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index a85fe9f9ad7..4e323a63206 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -768,6 +768,24 @@ wide_int_range_convert (wide_int , wide_int ,
   return (!wi::eq_p (min, wi::min_value (outer_prec, outer_sign))
 	  || !wi::eq_p (max, wi::max_value (outer_prec, outer_sign)));
 }
+  /* Unsigned truncating conversions whose lower bound fits, but whose
+ upper bound is +INF can be handled straightforwardly.  This will
+ handle things like:
+
+ [1,+INF] (i.e. ~[0,0]) from long unsigned to unsigned short
+ [1000, +INF] from long unsigned to unsigned short.  */
+  if (inner_sign == UNSIGNED
+  && inner_sign == outer_sign
+  && inner_prec > outer_prec
+  && wi::le_p (vr0_min,
+		   wi::mask (outer_prec, false, inner_prec),
+		   UNSIGNED)
+  && vr0_max == wi::max_value (inner_prec, UNSIGNED))
+{
+  min = wide_int::from (vr0_min, outer_prec, inner_sign);
+  max = wi::mask (outer_prec, false, outer_prec);
+  return true;
+}
   return false;
 }
 

VRP: convert pointers of known quantity better

[Aldy Hernandez]

Apparently, my work on VRP will never finish. There's an infinity of 
things that can be tweaked ;-).


First, we shouldn't drop to null/non-null when we know what the actual 
pointer value is.  For example, [1, 3] which we get when we store magic 
numbers in a pointer (p == (char *)1).


BTW, for this bit, I would much rather change range_int_cst_p to allow 
VR_ANTI_RANGE, instead of inlining as I've done.  Is there a reason 
range_int_cst_p doesn't handle anti ranges?


Also, [, ] is known to be non-null.  Don't drop to varying.

OK?
commit 7f42e101d5d26ea866b739692858289a8dff4396
Author: Aldy Hernandez 
Date:   Fri Sep 14 00:11:34 2018 +0200

* tree-vrp.c (extract_range_from_unary_expr): Handle pointers of
known quantity.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 622ccbc2df7..22e5ee3c729 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1842,9 +1842,14 @@ extract_range_from_unary_expr (value_range *vr,
   tree inner_type = op0_type;
   tree outer_type = type;
 
-  /* If the expression evaluates to a pointer, we are only interested in
-	 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]).  */
-  if (POINTER_TYPE_P (type))
+  /* If the expression evaluates to a pointer of unknown quantity,
+	 we are only interested in determining if it evaluates to NULL
+	 [0, 0] or non-NULL (~[0, 0]).  */
+  if (POINTER_TYPE_P (type)
+	  && !((vr0.type == VR_RANGE
+		|| vr0.type == VR_ANTI_RANGE)
+	   && TREE_CODE (vr0.min) == INTEGER_CST
+	   && TREE_CODE (vr0.max) == INTEGER_CST))
 	{
 	  if (!range_includes_zero_p ())
 	set_value_range_to_nonnull (vr, type);
@@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr,
 	  return;
 	}
 
+  /* If we have a non-constant range that we know is non-zero (for
+	 example [, ] or [, +MAX]), make it known, so we
+	 don't drop to VR_VARYING later.  */
+  if (POINTER_TYPE_P (op0_type)
+	  && vr0.type == VR_RANGE
+	  && (TREE_CODE (vr0.min) != INTEGER_CST
+	  || TREE_CODE (vr0.max) != INTEGER_CST)
+	  && !range_includes_zero_p ())
+	set_value_range_to_nonnull (, op0_type);
+
   /* We normalize everything to a VR_RANGE, but for constant
 	 anti-ranges we must handle them by leaving the final result
 	 as an anti range.  This allows us to convert things like

VRP: make worst case scenario for ABS_EXPR is still the set of positives

[Aldy Hernandez]

First, there's no sense handling !VR_RANGE and symbolics now that we've 
normalized the inputs.


Second, even if we wrap around while calculating ABS, we at the least 
know the result is positive ;-).  BTW, we've already handled ABS(-MIN) 
and -frapv by the time we get here, so we know we won't get nonsensical 
input.


OK?
commit 6e874be13a0bd53811e095328c1d175723f11f84
Author: Aldy Hernandez 
Date:   Fri Sep 14 00:03:04 2018 +0200

* tree-vrp.c (extract_range_from_unary_expr): Do not special case
symbolics or VR_VARYING ranges for ABS_EXPR.
* wide-int-range.cc (wide_int_range_abs): Return positive numbers
when range will wrap.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 1adb919a6df..622ccbc2df7 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1894,11 +1894,6 @@ extract_range_from_unary_expr (value_range *vr,
 }
   else if (code == ABS_EXPR)
 {
-  if (vr0.type != VR_RANGE || symbolic_range_p ())
-	{
-	  set_value_range_to_varying (vr);
-	  return;
-	}
   wide_int wmin, wmax;
   wide_int vr0_min, vr0_max;
   extract_range_into_wide_ints (, sign, prec, vr0_min, vr0_max);
diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index 8a3dfd25684..a85fe9f9ad7 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -728,10 +728,13 @@ wide_int_range_abs (wide_int , wide_int ,
 }
 
   /* If the new range has its limits swapped around (MIN > MAX), then
- the operation caused one of them to wrap around, mark the new
- range VARYING.  */
+ the operation caused one of them to wrap around.  The only thing
+ we know is that the result is positive.  */
   if (wi::gt_p (min, max, sign))
-  return false;
+{
+  min = wi::zero (prec);
+  max = max_value;
+}
   return true;
 }
 

[PATCH] S/390: Improve s390-passes.def formatting

[Ilya Leoshkevich]

The result looks nicer in the generated pass-instances.def.

gcc/ChangeLog:

2018-09-13  Ilya Leoshkevich  

* config/s390/s390-passes.def (INSERT_PASS_BEFORE): Improve
formatting.

Obvious fix, committed as r264306.
---
 gcc/config/s390/s390-passes.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/s390/s390-passes.def b/gcc/config/s390/s390-passes.def
index 035c6e8bc0a..6d88d019ae7 100644
--- a/gcc/config/s390/s390-passes.def
+++ b/gcc/config/s390/s390-passes.def
@@ -17,4 +17,4 @@ You should have received a copy of the GNU General Public 
License
 along with GCC; see the file COPYING3.  If not see
 .  */
 
-INSERT_PASS_BEFORE (pass_thread_prologue_and_epilogue, 1, 
pass_s390_early_mach);
+ INSERT_PASS_BEFORE (pass_thread_prologue_and_epilogue, 1, 
pass_s390_early_mach);
-- 
2.19.0

Re: [GCC][PATCH][Aarch64] Added pattern to match zero extended bfxil

[Sam Tebbs]

On 08/28/2018 11:54 PM, James Greenhalgh wrote:




OK once the other one is approved, with the obvious rebase over the renamed
function.

James


Here is the rebased patch. Still OK for me to commit to trunk now that 
the other patch has been committed?


Sam




gcc/
2018-07-31  Sam Tebbs  

      PR target/85628
      * config/aarch64/aarch64.md (*aarch64_bfxilsi_uxtw): Define.

gcc/testsuite
2018-07-31  Sam Tebbs  

      PR target/85628
      * gcc.target/aarch64/combine_bfxil.c
(combine_zero_extended_int, foo6):
      New functions.


diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 88f66104db31320389f05cdd5d161db9992a77b8..2c0dbab92aaf2aa97096d1f1438354a03b5f2149 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5361,6 +5361,33 @@
   [(set_attr "type" "bfm")]
 )
 
+; Zero-extended version of above (aarch64_bfxil)
+(define_insn "*aarch64_bfxilsi_uxtw"
+  [(set (match_operand:DI 0 "register_operand" "=r,r")
+	(zero_extend:DI (ior:SI (and:SI (match_operand:SI 1 "register_operand"
+	"r,0")
+		(match_operand:SI 3 "const_int_operand" "n, Ulc"))
+	(and:SI (match_operand:SI 2 "register_operand" "0,r")
+		(match_operand:SI 4 "const_int_operand" "Ulc, n")]
+  "(INTVAL (operands[3]) == ~INTVAL (operands[4]))
+  && (aarch64_high_bits_all_ones_p (INTVAL (operands[3]))
+|| aarch64_high_bits_all_ones_p (INTVAL (operands[4])))"
+  {
+switch (which_alternative)
+{
+  case 0:
+	operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[3])));
+	return "bfxil\\t%0, %1, 0, %3";
+  case 1:
+	operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[4])));
+	return "bfxil\\t%0, %2, 0, %3";
+  default:
+	gcc_unreachable ();
+}
+  }
+  [(set_attr "type" "bfm")]
+)
+
 ;; There are no canonicalisation rules for the position of the lshiftrt, ashift
 ;; operations within an IOR/AND RTX, therefore we have two patterns matching
 ;; each valid permutation.
diff --git a/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c b/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
index adb0582ed9d8207f7b52c8912d03345369747448..98f6159fbda0978dd863f16d5d88dbff8c4b343f 100644
--- a/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
+++ b/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
@@ -4,6 +4,13 @@
 extern void abort (void);
 
 unsigned long long
+combine_zero_extended_int (unsigned int a, unsigned int b)
+{
+  /* { dg-final { scan-assembler-not "uxtw\\t" } } */
+  return (a & 0xll) | (b & 0xll);
+}
+
+unsigned long long
 combine_balanced (unsigned long long a, unsigned long long b)
 {
   return (a & 0xll) | (b & 0xll);
@@ -71,6 +78,13 @@ foo5 (unsigned int a, unsigned int b, unsigned int *c, unsigned int *d)
   *d = combine_unbalanced_int (b, a);
 }
 
+void
+foo6 (unsigned int a, unsigned int b, unsigned long long *c, unsigned long long *d)
+{
+  *c = combine_zero_extended_int(a, b);
+  *d = combine_zero_extended_int(b, a);
+}
+
 int
 main (void)
 {
@@ -92,7 +106,12 @@ main (void)
   foo5 (a2, b2, , );
   if (c2 != 0x01234598) abort ();
   if (d2 != 0xfedcba67) abort ();
+
+  unsigned long long c3, d3;
+  foo6 (a2, b2, , );
+  if (c3 != 0x0123ba98) abort ();
+  if (d3 != 0xfedc4567) abort ();
   return 0;
 }
 
-/* { dg-final { scan-assembler-times "bfxil\\t" 10 } } */
+/* { dg-final { scan-assembler-times "bfxil\\t" 13 } } */

Re: [PATCH][libgfortran] Fix uninitialized variable use in fallback_access

[Paul Richard Thomas]

Hi Kyrill,

This indeed is an obvious change. OK for trunk and 7- and 8-branches.

Thanks for the patch

Paul

On 14 September 2018 at 09:06, Kyrill  Tkachov
 wrote:
> Hi all,
>
> I've been tracking down a bug in a Fortran program on a newlib target and it
> boils down to fallback_access doing something bad.
> The unconditional calls to close cause havoc when open doesn't get called
> due to the short-circuiting in the if-statement above
> because the fd is uninitialised. In my environment GCC ends up calling close
> on file descriptor 0, thus trying to close stdin.
>
> This patch tightens up the calling so that close is called only when the
> corresponding open call succeeded.
> With this my runtime failure disappears.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Though that doesn't exercise this call I hope it's an obviously correct
> change.
>
> Ok for trunk and the branches?
>
> Thanks,
> Kyrill
>
> 2018-09-14  Kyrylo Tkachov  
>
> * io/unix.c (fallback_access): Avoid calling close on
> uninitialized file descriptor.



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

[PATCH][libgfortran] Fix uninitialized variable use in fallback_access

[Kyrill Tkachov]

Hi all,

I've been tracking down a bug in a Fortran program on a newlib target and it 
boils down to fallback_access doing something bad.
The unconditional calls to close cause havoc when open doesn't get called due 
to the short-circuiting in the if-statement above
because the fd is uninitialised. In my environment GCC ends up calling close on 
file descriptor 0, thus trying to close stdin.

This patch tightens up the calling so that close is called only when the 
corresponding open call succeeded.
With this my runtime failure disappears.

Bootstrapped and tested on aarch64-none-linux-gnu.
Though that doesn't exercise this call I hope it's an obviously correct change.

Ok for trunk and the branches?

Thanks,
Kyrill

2018-09-14  Kyrylo Tkachov  

* io/unix.c (fallback_access): Avoid calling close on
uninitialized file descriptor.
diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index 61e9f7997b25819514af546b50aa1d00b1d116f9..8081d6f96b2f6cd2489e876c8921cfb3510287ca 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -149,13 +149,21 @@ fallback_access (const char *path, int mode)
 {
   int fd;
 
-  if ((mode & R_OK) && (fd = open (path, O_RDONLY)) < 0)
-return -1;
-  close (fd);
+  if (mode & R_OK)
+{
+  if ((fd = open (path, O_RDONLY)) < 0)
+	return -1;
+  else
+	close (fd);
+}
 
-  if ((mode & W_OK) && (fd = open (path, O_WRONLY)) < 0)
-return -1;
-  close (fd);
+  if (mode & W_OK)
+{
+  if ((fd = open (path, O_WRONLY)) < 0)
+	return -1;
+  else
+	close (fd);
+}
 
   if (mode == F_OK)
 {

Re: VRP: normalize VR_VARYING in PLUS/MINUS_EXPR handling

[Richard Biener]

On Tue, Sep 11, 2018 at 12:34 PM Aldy Hernandez  wrote:
>
> For addition and subtraction, we're currently punting on things like
> [0,0] - VR_VARYING.  However, if we normalize VR_VARYING to the entire
> domain, we can see that the above is actually [-MIN+1, +MAX].
>
> We would've normally caught this sort of things with my rewrite of the
> binary operators, but PLUS/MINUS are special in that they're pretty much
> the only operator that keeps symbolics through it's calculations.  This
> is why we can't just blindly treat symbolics as [-MIN, +MAX] as I've
> done elsewhere.  PLUS/MINUS are handled specially (ugly) :-/.
>
> OK for trunk?

OK.

>

Re: Prune TYPE_NEXT_VARIANT lists in free_lang_data

[Richard Biener]

On Sun, 9 Sep 2018, Jan Hubicka wrote:

> > On September 8, 2018 10:49:02 AM GMT+01:00, Jan Hubicka  
> > wrote:
> > >Hi
> > >while working on path to replace type variant by first compatible one I
> > >run
> > >into issue that the first vairant was not seen by free_lang_data. I
> > >think this
> > >is a bug becuase nothing prevents middle-end from looking up a variant
> > >there
> > >and using it when it needs to do so.
> > >
> > >I tried to walk the variant lists and free lang data on them but that
> > >crahses
> > >because not all types in variant list passes verify_type (C++ puts
> > >there
> > >incomplete type variants whose canonical type is complete and thus they
> > >are
> > >considered bogus). So pruning those lists seems to be better variant.
> > >
> > >Bootstrapped/regtested x86_64-linux, OK?
> > 
> > But if they are still reachable by GC they are now bogously unlinked. We 
> > already do not stream unused variants, so what does this achieve? Maybe we 
> > should make the variant list GCable somehow? 
> 
> Main goal is to prevent middle end to walk the TYPE_NEXT_VARIANT list (for 
> example by get_qualified_variant)
> and pick up a variant that was not seen by free_lang_data and put it into the 
> IL.
> I know that if we end up referring to some type variant in a way not seen
> by free lang data then the type won't be reachable from the list, but why that
> would be a problem?

Duplicate types?  Also the type verifier could assert that a type variant
is in the variant list of its main variant - it looks bogus to break that
invariant at least.

Maybe the real fix is to prune the list in lang specific free-lang-data
so free_lang_data _can_ walk the variant list?  That is, can we track
down those C++ issues and fix them in some way?

Richard.

> Honza
> > 
> > >Honza
> > >
> > >   * tree.c (free_lang_data_in_cgraph): Prune TYPE_NEXT_VARIANT lists.
> > >Index: tree.c
> > >===
> > >--- tree.c (revision 263989)
> > >+++ tree.c (working copy)
> > >@@ -5845,7 +5845,12 @@ free_lang_data_in_cgraph (void)
> > > 
> > >   /* Traverse every type found freeing its language data.  */
> > >   FOR_EACH_VEC_ELT (fld.types, i, t)
> > >-free_lang_data_in_type (t);
> > >+{
> > >+  free_lang_data_in_type (t);
> > >+  while (TYPE_NEXT_VARIANT (t)
> > >+   && !fld.pset.contains (TYPE_NEXT_VARIANT (t)))
> > >+  TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (TYPE_NEXT_VARIANT (t));
> > >+}
> > >   if (flag_checking)
> > > {
> > >   FOR_EACH_VEC_ELT (fld.types, i, t)
> > 
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: RFA: PATCH to fix crash with --enable-gather-detailed-mem-stats

[Richard Biener]

On Fri, Sep 7, 2018 at 12:38 PM Jason Merrill  wrote:
>
> Turning on --enable-gather-detailed-mem-stats was producing for me a
> compiler that crashed before main() because initializing the
> hash_table sem_item::m_type_hash_cache was trying to add to
> hash_table_usage before the latter had been initialized.  This patch
> fixes this by changing hash_table_usage to be a function, so that it
> is initialized on demand.
>
> Tested x86_64-pc-linux-gnu.  OK for trunk?

Ah, nice idea - IIRC in the past this forced us to use pointers for
all global hashes.  I wonder if we inline the function, esp. at places
like

   for (unsigned i = HASH_TABLE_ORIGIN; i <= HASH_SET_ORIGIN; i++)
 {
   mem_alloc_origin origin = (mem_alloc_origin) i;
-  hash_table_usage.dump (origin);
+  hash_table_usage ().dump (origin);
 }
?

OK.

Richard.

Re: PING: Re: lightweight class to wide int ranges in VRP and friends

[Richard Biener]

On Mon, Sep 3, 2018 at 8:58 AM Aldy Hernandez  wrote:
>
>

Can you re-base and re-post please?

Sorry for being late here.

Richard.

>
>  Forwarded Message 
> Subject: Re: lightweight class to wide int ranges in VRP and friends
> Date: Fri, 24 Aug 2018 13:40:29 -0400
> From: Aldy Hernandez 
> To: Richard Biener 
> CC: GCC Patches 
>
> On 08/24/2018 06:32 AM, Richard Biener wrote:
> > On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez  wrote:
> >>
> >> I kept seeing the same patterns over and over in all this re-factoring:
> >>
> >> 1. extract value_range constants into pairs of wide ints
> >>
> >> 2. mimic symbolics as [-MIN, +MAX] (most of the time :))
> >>
> >> 3. perform some wide int operation on the wide int range
> >>
> >> 4. convert back to a value range
> >>
> >> So I've decided to clean this up even more.  Instead of passing a pair
> >> of wide ints plus sign, precision, and god knows what to every
> >> wide_int_range_* function, I've implemented a lighweight class
> >> (wi_range) for a pair of wide ints.  It is not meant for long term
> >> storage, but even so its footprint is minimal.
> >>
> >> The only extra bits are another 64-bit word per pair for keeping
> >> attributes such as precision, sign, overflow_wraps, and a few other
> >> things we'll need shortly.  In reality we only need one set of
> >> attributes for both sets of pairs, so we waste one 64-bit word.  I don't
> >> care.  They're short lived, and the resulting code looks an awful lot
> >> nicer.  For example, the shift code gets rewritten from this:
> >>
> >> if (range_int_cst_p ()
> >>&& !vrp_shift_undefined_p (vr1, prec))
> >>  {
> >>if (code == RSHIFT_EXPR)
> >>  {
> >>if (vr0.type != VR_RANGE || symbolic_range_p ())
> >>  {
> >>vr0.type = type = VR_RANGE;
> >>vr0.min = vrp_val_min (expr_type);
> >>vr0.max = vrp_val_max (expr_type);
> >>  }
> >>extract_range_from_multiplicative_op (vr, code, , );
> >>return;
> >>  }
> >>else if (code == LSHIFT_EXPR
> >> && range_int_cst_p ())
> >>  {
> >>wide_int res_lb, res_ub;
> >>if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
> >>   wi::to_wide (vr0.min),
> >>   wi::to_wide (vr0.max),
> >>   wi::to_wide (vr1.min),
> >>   wi::to_wide (vr1.max),
> >>   TYPE_OVERFLOW_UNDEFINED 
> >> (expr_type),
> >>   TYPE_OVERFLOW_WRAPS (expr_type)))
> >>  {
> >>min = wide_int_to_tree (expr_type, res_lb);
> >>max = wide_int_to_tree (expr_type, res_ub);
> >>set_and_canonicalize_value_range (vr, VR_RANGE,
> >>  min, max, NULL);
> >>return;
> >>  }
> >>  }
> >>  }
> >> set_value_range_to_varying (vr);
> >>
> >> into this:
> >>
> >> wi_range w0 (, expr_type);
> >> wi_range w1 (, expr_type);
> >> if (!range_int_cst_p ()
> >>|| wi_range_shift_undefined_p (w1)
> >>|| (code == LSHIFT_EXPR
> >>&& !range_int_cst_p ()))
> >>  {
> >>vr->set_varying ();
> >>return;
> >>  }
> >> bool success;
> >> if (code == RSHIFT_EXPR)
> >>  success = wi_range_multiplicative_op (res, code, w0, w1);
> >> else
> >>  success = wi_range_lshift (res, w0, w1);
> >>
> >> if (success)
> >>  vr->set_and_canonicalize (res, expr_type);
> >> else
> >>  vr->set_varying ();
> >> return;
> >
> > not sure whether I like the munging of lshift and right shift (what exactly 
> > gets
> > done is less clear in your version ...).  Having a light-weigth class for
> > the wi_range_ parameters is nice though.
>
> No worries.  I'm not emotionally attached to munging them :).
>
> >
> >> I also munged together the maybe/mustbe nonzero_bits into one structure.
> >>
> >> Finally, I've pontificated that wide_int_range_blah is too much typing.
> >> Everyone's RSI will thank me for rewriting the methods as wi_range_blah.
> >>
> >> I've tried to keep the functionality changes to a minimum.  However,
> >> there are some slight changes where I treat symbolics and VR_VARYING as
> >> [MIN, MAX] and let the constant code do its thing.  It is considerably
> >> easier to read.
> >>
> >> I am finally happy with the overall direction of this.  If this and the
> >> last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
> >> ABS_EXPR and I'm basically done with what