Re: [PATCH] rs6000: Parameterize some const values for density test

[Kewen.Lin via Gcc-patches]

on 2021/9/18 上午6:26, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Sep 15, 2021 at 04:52:49PM +0800, Kewen.Lin wrote:
>> This patch follows the discussion here[1], where Segher suggested
>> parameterizing those exact magic constants for density heuristics,
>> to make it easier to tweak if need.
>>
>> Since these heuristics are quite internal, I make these parameters
>> as undocumented and be mainly used by developers.
> 
> Okido.
> 
>> +  if (data->nloads > (unsigned int) rs6000_density_load_num_threshold
>> +  && load_pct > (unsigned int) rs6000_density_load_pct_threshold)
> 
> Those variables are unsigned int already.  Don't cast please.
> 

Unfortunately this is required by bootstrapping.  The UInteger for the
param definition is really confusing, in the underlying implementation
it's still "signed".  If you grep "(unsigned) param", you can see a few
examples.  I guess the "UInteger" is mainly for the param value range
checking.

>> +-param=rs6000-density-pct-threshold=
>> +Target Undocumented Joined UInteger Var(rs6000_density_pct_threshold) 
>> Init(85) IntegerRange(0, 99) Param
> 
> So make this and all other percentages (0, 100) please.
> 

I thought 99 is enough for the RHS in ">". just realized it's more clear
with 100.  Will fix!

>> +When costing for loop vectorization, we probably need to penalize the loop 
>> body cost if the existing cost model may not adequately reflect delays from 
>> unavailable vector resources.  We collect the cost for vectorized statements 
>> and non-vectorized statements separately, check the proportion of vec_cost 
>> to total cost of vec_cost and non vec_cost, and penalize only if the 
>> proportion exceeds the threshold specified by this parameter.  The default 
>> value is 85.
> 
> It would be good if we can use line breaks in the source code for things
> like this, but I don't think we can.  This message is mainly used for
> "--help=param", and it is good there to have as short messages as you
> can.  But given the nature of params you need quite a few words often,
> and you do not want to say so little that things are no clear, either.
> 
> So, dunno :-)

I did some testings, the line breaks writing can still survive in the
"--help=param" show, the lines are concatenated with " ".  Although
there seems no this kind of writing practices, I am guessing you want
me to do line breaks for their descriptions?  If so, I will make them
short as the above "Target Undocumented..." line.  Or do you want it
to align source code ColumnLimit 80 (for these cases, it would look
shorter)?

> 
> Oksy for trunk with these fixes and what Bill mentioned in the other
> thread.  Thanks!
> 

OK, thanks again!

BR,
Kewen

Re: [PATCH] Simplify paradoxical subreg extensions of TRUNCATE

[Jeff Law via Gcc-patches]

On 9/20/2021 6:23 PM, Segher Boessenkool wrote:

Hi!

On Sun, Sep 19, 2021 at 09:14:55AM -0600, Jeff Law wrote:

On 9/6/2021 8:24 AM, Segher Boessenkool wrote:

On Mon, Sep 06, 2021 at 12:32:13PM +0100, Roger Sayle wrote:

I think the current documentation is sufficient.  During compilation,
GCC's
combine pass will often substitute a register with an expression defining
it's value, and then attempt to simplify it.  As you point out, this often
produces
(temporary intermediate) expressions with poorly defined semantics.

Not "poorly defined".  The documentation says (in rtl.texi)

@findex subreg
@item (subreg:@var{m1} @var{reg:m2} @var{bytenum})

@code{subreg} expressions are used to refer to a register in a machine
mode other than its natural one, or to refer to one register of
a multi-part @code{reg} that actually refers to several registers.

It goes on to say there also are subregs of mem currently; it
exhaustively lists all things you can take a subreg of, what the
different semantics of those are, how the semantics are further
"modified" (read: completely different) if some RTL flags are set, etc.

The instruction combiner very often creates invalid RTL (only
structurally valid, like, no missing operands).  Invalid RTL is supposed
to never match (because backends will not have patterns that match
these).  combine even creates invalid RTL on purpose (a clobber of
const0_rtx) to ensure its result is deemed invalid, when it wants to
abort a combination attempt for some reason.


The
purpose of my patch is to avoid (constructing) SUBREGs that have TRUNCATE
as an argument (that some folks consider undefined) and instead simplify
them to either a single TRUNCATE or a SUBREG of a REG, both of which are
well defined.  I'm making/helping the implementation match the
documentation.

But this should never make it to simplify-rtx at all.  You can only
validly do such transformations in combine, because you need to know
what insns you started with etc.

simplify-rtx is a part of combine, sure, but it is used from other
contexts as well nowadays.  If you can safely do this from combine,
you can do it in combine.

[ ... ]
So what I think is missing here is some concrete path forward.  If I'm
understanding your objection Segher, you'd like to see Roger look at
where these (subreg (truncate)) expressions came from and address them
earlier than simplify_rtx?

There is no such thing as "earlier than simplify-rtx", that is the
point.  simplify-rtx is not a pass: it is like a library that is used
from all over the RTL routines.
I'm referring to earlier in the call chain, not an earlier pass. Sorry I 
wasn't clear about that.


If we were catching the scenario which led to the creation of (subreg 
(truncate)) in combine and instead of creating (subreg (truncate)) we 
instead created the simplified, correct form would that ease your concerns?




Jeff

Re: [PATCH] rs6000: Parameterize some const values for density test

[Kewen.Lin via Gcc-patches]

Hi Bill,

Thanks for the review!

on 2021/9/18 上午12:27, Bill Schmidt wrote:
> Hi Kewen,
> 
> On 9/15/21 3:52 AM, Kewen.Lin wrote:
>> Hi,
>>
>> This patch follows the discussion here[1], where Segher suggested
>> parameterizing those exact magic constants for density heuristics,
>> to make it easier to tweak if need.
>>
>> Since these heuristics are quite internal, I make these parameters
>> as undocumented and be mainly used by developers.
>>
>> The change here should be "No Functional Change".  But I verified
>> it with SPEC2017 at option sets O2-vect and Ofast-unroll on Power8,
>> the result is neutral as expected.
>>
>> Bootstrapped and regress-tested on powerpc64le-linux-gnu Power9.
>>
>> Is it ok for trunk?
>>
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579121.html
>>
>> BR,
>> Kewen
>> -
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000.opt (rs6000-density-pct-threshold,
>> rs6000-density-size-threshold, rs6000-density-penalty,
>> rs6000-density-load-pct-threshold,
>> rs6000-density-load-num-threshold): New parameter.
>> * config/rs6000/rs6000.c (rs6000_density_test): Adjust with
>> corresponding parameters.
>>
>> ---
>>   gcc/config/rs6000/rs6000.c   | 22 +++---
>>   gcc/config/rs6000/rs6000.opt | 21 +
>>   2 files changed, 28 insertions(+), 15 deletions(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index 9bc826e3a50..4ab23b0ab33 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -5284,9 +5284,6 @@ struct rs6000_cost_data
>>   static void
>>   rs6000_density_test (rs6000_cost_data *data)
>>   {
>> -  const int DENSITY_PCT_THRESHOLD = 85;
>> -  const int DENSITY_SIZE_THRESHOLD = 70;
>> -  const int DENSITY_PENALTY = 10;
>>     struct loop *loop = data->loop_info;
>>     basic_block *bbs = get_loop_body (loop);
>>     int nbbs = loop->num_nodes;
>> @@ -5322,26 +5319,21 @@ rs6000_density_test (rs6000_cost_data *data)
>>     free (bbs);
>>     density_pct = (vec_cost * 100) / (vec_cost + not_vec_cost);
>>
>> -  if (density_pct > DENSITY_PCT_THRESHOLD
>> -  && vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
>> +  if (density_pct > rs6000_density_pct_threshold
>> +  && vec_cost + not_vec_cost > rs6000_density_size_threshold)
>>   {
>> -  data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
>> +  data->cost[vect_body] = vec_cost * (100 + rs6000_density_penalty) / 
>> 100;
>>     if (dump_enabled_p ())
>>   dump_printf_loc (MSG_NOTE, vect_location,
>>    "density %d%%, cost %d exceeds threshold, penalizing "
>> - "loop body cost by %d%%\n", density_pct,
>> - vec_cost + not_vec_cost, DENSITY_PENALTY);
>> + "loop body cost by %u%%\n", density_pct,
>> + vec_cost + not_vec_cost, rs6000_density_penalty);
>>   }
>>
>>     /* Check whether we need to penalize the body cost to account
>>    for excess strided or elementwise loads.  */
>>     if (data->extra_ctor_cost > 0)
>>   {
>> -  /* Threshold for load stmts percentage in all vectorized stmts.  */
>> -  const int DENSITY_LOAD_PCT_THRESHOLD = 45;
>> -  /* Threshold for total number of load stmts.  */
>> -  const int DENSITY_LOAD_NUM_THRESHOLD = 20;
>> -
>>     gcc_assert (data->nloads <= data->nstmts);
>>     unsigned int load_pct = (data->nloads * 100) / data->nstmts;
>>
>> @@ -5355,8 +5347,8 @@ rs6000_density_test (rs6000_cost_data *data)
>>     the loads.
>>    One typical case is the innermost loop of the hotspot of SPEC2017
>>    503.bwaves_r without loop interchange.  */
>> -  if (data->nloads > DENSITY_LOAD_NUM_THRESHOLD
>> -  && load_pct > DENSITY_LOAD_PCT_THRESHOLD)
>> +  if (data->nloads > (unsigned int) rs6000_density_load_num_threshold
>> +  && load_pct > (unsigned int) rs6000_density_load_pct_threshold)
>>   {
>>     data->cost[vect_body] += data->extra_ctor_cost;
>>     if (dump_enabled_p ())
>> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
>> index 0538db387dc..563983f3269 100644
>> --- a/gcc/config/rs6000/rs6000.opt
>> +++ b/gcc/config/rs6000/rs6000.opt
>> @@ -639,3 +639,24 @@ Enable instructions that guard against return-oriented 
>> programming attacks.
>>   mprivileged
>>   Target Var(rs6000_privileged) Init(0)
>>   Generate code that will run in privileged state.
>> +
>> +-param=rs6000-density-pct-threshold=
>> +Target Undocumented Joined UInteger Var(rs6000_density_pct_threshold) 
>> Init(85) IntegerRange(0, 99) Param
>> +When costing for loop vectorization, we probably need to penalize the loop 
>> body cost if the existing cost model may not adequately reflect delays from 
>> unavailable vector resources.  We collect the cost for vectorized statements 
>> and non-vectorized statements separately, check the proportion of vec_cost 
>> to total cost of vec_cost and non vec_cost, and 

Re: [PATCH] rs6000: Modify the way for extra penalized cost

[Kewen.Lin via Gcc-patches]

Hi Segher,

Thanks for the review!

on 2021/9/18 上午6:01, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Sep 16, 2021 at 09:14:15AM +0800, Kewen.Lin wrote:
>> The way with nunits * stmt_cost can get one much exaggerated
>> penalized cost, such as: for V16QI on P8, it's 16 * 20 = 320,
>> that's why we need one bound.  To make it scale, this patch
>> doesn't use nunits * stmt_cost any more, but it still keeps
>> nunits since there are actually nunits scalar loads there.  So
>> it uses one cost adjusted from stmt_cost, since the current
>> stmt_cost sort of considers nunits, we can stablize the cost
>> for big nunits and retain the cost for small nunits.  After
>> some tries, this patch gets the adjusted cost as:
>>
>> stmt_cost / (log2(nunits) * log2(nunits))
> 
> So for  V16QI it gives *16/(4*4) so *1
> V8HI  it gives *8/(3*3)  so *8/9
> V4SI  it gives *4/(2*2)  so *1
> V2DI  it gives *2/(1*1)  so *2
> and for V1TI  it gives *1/(0*0) which is UB (no, does not crash for us,
> just gives wildly wrong answers; the div returns 0 on recent systems).
> 

I don't expected we will have V1TI for strided/elementwise load,
if it's one unit vector, it's the whole vector itself.
Besides, the below assertion should exclude it already.

>> For V16QI, the adjusted cost would be 1 and total penalized
>> cost is 16, it isn't exaggerated.  For V2DI, the adjusted
>> cost would be 2 and total penalized cost is 4, which is the
>> same as before.  btw, I tried to use one single log2(nunits),
>> but the penalized cost is still big enough and can't fix the
>> degraded bmk blender_r.
> 
> Does it make sense to treat V2DI (and V2DF) as twice more expensive than
> other vectors, which are all pretty much equal cost (except those that
> end up with cost 0)?  If so, there are simpler ways to do that.
> 

Yeah, from the SPEC2017 evaluation, it's good with this.  The costing
framework of vectorization doesn't consider the dependent insn chain
and available #unit etc. like local scheduling (it can't either), so
we have to use some heuristics to handle some special cases.  For more
units vector construction, the used instructions are more.  It has more
chances to schedule them better (even run in parallelly when enough
available units at the time), so we don't need to penalize more for them.
For V2DI, the load result is fed into construction directly, the current
stmt_cost is to consider merging and only 2, penalizing it with one is
not enough from the bwaves experiment.

>> +  int nunits_log2 = exact_log2 (nunits);
>> +  gcc_assert (nunits_log2 > 0);
>> +  unsigned int nunits_sq = nunits_log2 * nunits_log2;
> 
>> = 0
> 
> This of course is assuming nunits will always be a power of 2, but I'm
> sure that we have many other places in the compiler assuming that
> already, so that is fine.  And if one day this stops being true we will
> get a nice ICE, pretty much the best we could hope for.
> 

Yeah, exact_log2 returns -1 for non power of 2 input, for example:

input output
0 ->-1
1 ->0
2 ->1
3 ->-1

>> +  unsigned int adjusted_cost = stmt_cost / nunits_sq;
> 
> But this can divide by 0.  Or are we somehow guaranteed that nunits
> will never be 1?  Yes the log2 check above, sure, but that ICEs if this
> is violated; is there anything that actually guarantees it is true?
> 

As I mentioned above, I don't expect we can have nunits 1 strided/ew load,
and the ICE should check this and ensure dividing by zero never happens.  :)

>> +  gcc_assert (adjusted_cost > 0);
> 
> I don't see how you guarantee this, either.
> 

It's mainly to prevent that one day we tweak the cost for construction
in rs6000_builtin_vectorization_cost then make some unexpected values
generated here.  But now these expected values are guaranteed as the
current costs and the formula.

> 
> A magic crazy formula like this is no good.  If you want to make the
> cost of everything but V2D* be the same, and that of V2D* be twice that,
> that is a weird heuristic, but we can live with that perhaps.  But that
> beats completely unexplained (and unexplainable) magic!
> 
> Sorry.
> 

That's all right, thanks for the comments!  let's improve it.  :)
How about just assigning 2 for V2DI and 1 for the others for the
penalized_cost_per_load with some detailed commentary, it should have
the same effect with this "magic crazy formula", but I guess it can
be more clear.

BR,
Kewen

Re: [PATCH] rs6000: Modify the way for extra penalized cost

[Kewen.Lin via Gcc-patches]

Hi Bill,

Thanks for the review!

on 2021/9/18 上午12:34, Bill Schmidt wrote:
> Hi Kewen,
> 
> On 9/15/21 8:14 PM, Kewen.Lin wrote:
>> Hi,
>>
>> This patch follows the discussion here[1], where Segher pointed
>> out the existing way to guard the extra penalized cost for
>> strided/elementwise loads with a magic bound doesn't scale.
>>
>> The way with nunits * stmt_cost can get one much exaggerated
>> penalized cost, such as: for V16QI on P8, it's 16 * 20 = 320,
>> that's why we need one bound.  To make it scale, this patch
>> doesn't use nunits * stmt_cost any more, but it still keeps
>> nunits since there are actually nunits scalar loads there.  So
>> it uses one cost adjusted from stmt_cost, since the current
>> stmt_cost sort of considers nunits, we can stablize the cost
>> for big nunits and retain the cost for small nunits.  After
>> some tries, this patch gets the adjusted cost as:
>>
>>  stmt_cost / (log2(nunits) * log2(nunits))
>>
>> For V16QI, the adjusted cost would be 1 and total penalized
>> cost is 16, it isn't exaggerated.  For V2DI, the adjusted
>> cost would be 2 and total penalized cost is 4, which is the
>> same as before.  btw, I tried to use one single log2(nunits),
>> but the penalized cost is still big enough and can't fix the
>> degraded bmk blender_r.
>>
>> The separated SPEC2017 evaluations on Power8, Power9 and Power10
>> at option sets O2-vect and Ofast-unroll showed this change is
>> neutral (that is same effect as before).
>>
>> Bootstrapped and regress-tested on powerpc64le-linux-gnu Power9.
>>
>> Is it ok for trunk?
>>
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579121.html
>>
>> BR,
>> Kewen
>> -
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000.c (rs6000_update_target_cost_per_stmt): Adjust
>> the way to compute extra penalized cost.
>>
>> ---
>>   gcc/config/rs6000/rs6000.c | 28 +---
>>   1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index 4ab23b0ab33..e08b94c0447 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -5454,17 +5454,23 @@ rs6000_update_target_cost_per_stmt (rs6000_cost_data 
>> *data,
>>   {
>>     tree vectype = STMT_VINFO_VECTYPE (stmt_info);
>>     unsigned int nunits = vect_nunits_for_cost (vectype);
>> -  unsigned int extra_cost = nunits * stmt_cost;
>> -  /* As function rs6000_builtin_vectorization_cost shows, we have
>> - priced much on V16QI/V8HI vector construction as their units,
>> - if we penalize them with nunits * stmt_cost, it can result in
>> - an unreliable body cost, eg: for V16QI on Power8, stmt_cost
>> - is 20 and nunits is 16, the extra cost is 320 which looks
>> - much exaggerated.  So let's use one maximum bound for the
>> - extra penalized cost for vector construction here.  */
>> -  const unsigned int MAX_PENALIZED_COST_FOR_CTOR = 12;
>> -  if (extra_cost > MAX_PENALIZED_COST_FOR_CTOR)
>> -    extra_cost = MAX_PENALIZED_COST_FOR_CTOR;
>> +  /* As function rs6000_builtin_vectorization_cost shows, we
>> + have priced much on V16QI/V8HI vector construction by
>> + considering their units, if we penalize them with nunits
>> + * stmt_cost here, it can result in an unreliable body cost,
> 
> This might be confusing to the reader, since you have deleted the calculation 
> of nunits * stmt_cost.  Could you instead write this to indicate that we used 
> to adjust in this way, and it had this particular downside, so that's why 
> you're choosing this heuristic? It's a minor thing but I think people reading 
> the code will be confused otherwise.
> 

Good point!  I'll update the commentary to explain it, thanks!!

BR,
Kewen 

> I think the heuristic is generally reasonable, and certainly better than what 
> we had before!
> 
> LGTM with adjusted commentary, so recommend maintainers approve.
> 
> Thanks for the patch!
> Bill
>> + eg: for V16QI on Power8, stmt_cost is 20 and nunits is 16,
>> + the penalty will be 320 which looks much exaggerated.  But
>> + there are actually nunits scalar loads, so we try to adopt
>> + one reasonable penalized cost for each load rather than
>> + stmt_cost.  Here, with stmt_cost dividing by log2(nunits)^2,
>> + we can still retain the necessary penalty for small nunits
>> + meanwhile stabilize the penalty for big nunits.  */
>> +  int nunits_log2 = exact_log2 (nunits);
>> +  gcc_assert (nunits_log2 > 0);
>> +  unsigned int nunits_sq = nunits_log2 * nunits_log2;
>> +  unsigned int adjusted_cost = stmt_cost / nunits_sq;
>> +  gcc_assert (adjusted_cost > 0);
>> +  unsigned int extra_cost = nunits * adjusted_cost;
>>     data->extra_ctor_cost += extra_cost;
>>   }
>>   }
>> -- 
>> 2.25.1
> 

Re: [PATCH v2] ipa-inline: Add target info into fn summary [PR102059]

[Kewen.Lin via Gcc-patches]

Hi Martin,

on 2021/9/17 下午7:26, Martin Jambor wrote:
> Hi,
> 
> On Fri, Sep 17 2021, Kewen.Lin wrote:
>> on 2021/9/16 下午9:19, Martin Jambor wrote:
>>> On Thu, Sep 16 2021, Kewen.Lin wrote:
 on 2021/9/15 下午8:51, Martin Jambor wrote:
> On Wed, Sep 08 2021, Kewen.Lin wrote:
>>
>
> [...]
>
>> diff --git a/gcc/ipa-fnsummary.h b/gcc/ipa-fnsummary.h
>> index 78399b0b9bb..300b8da4507 100644
>> --- a/gcc/ipa-fnsummary.h
>> +++ b/gcc/ipa-fnsummary.h
>> @@ -193,6 +194,9 @@ public:
>>vec *loop_strides;
>>/* Parameters tested by builtin_constant_p.  */
>>vec GTY((skip)) builtin_constant_p_parms;
>> +  /* Like fp_expressions, but it's to hold some target specific 
>> information,
>> + such as some target specific isa flags.  */
>> +  auto_vec GTY((skip)) target_info;
>>/* Estimated growth for inlining all copies of the function before 
>> start
>>   of small functions inlining.
>>   This value will get out of date as the callers are duplicated, but
>
> Segher already wrote in the first thread that a vector of HOST_WIDE_INTs
> is an overkill and I agree.  So at least make the new field just a
> HOST_WIDE_INT or better yet, an unsigned int.  But I would even go
> further and make target_info only a 16-bit bit-field, place it after the
> other bit-fields in class ipa_fn_summary and pass it to the hooks as
> uint16_t.  Unless you have plans which require more space, I think we
> should be conservative here.
>

 OK, yeah, the consideration is mainly for the scenario that target has
 a few bits to care about.  I just realized that to avoid inefficient
 bitwise operation for mapping target info bits to isa_flag bits, target
 can rearrange the sparse bits in isa_flag, so it's not a deal.
 Thanks for re-raising this!  I'll use the 16 bits bit-field in v3 as you
 suggested, if you don't mind, I will put it before the existing bit-fields
 to have a good alignment.
>>>
>>> All right.
>>>
>>
>> Sorry that I failed to use 16 bit-fields for this, I figured out that
>> the bit-fields can not be address-taken or passed as non-const reference.
>> The gentype also failed to recognize uint16_t if I used uint16_t directly
>> in ipa-fnsummary.h.  Finally I used unsigned int instead.
>>
> 
> well, you could have used:
> 
>   unsigned int target_info : 16;
> 
> for the field (and uint16_t when passed to hooks).
> 
> But I am not sure if it is that crucial.
> 

I may miss something, specifically I tried with:

1)

  unsigned int target_info : 16;
  unsigned inlinable : 1;
  ...

  update_ipa_fn_target_info (uint16_t &, const gimple *)

2)

  unsigned int target_info : 16;
  unsigned inlinable : 1;
  ...

  update_ipa_fn_target_info (uint16_t *, const gimple *)

The above two ways failed due to:

"Because bit fields do not necessarily begin at the beginning of a byte,
address of a bit field cannot be taken. Pointers and non-const references
to bit fields are not possible." as [1].

Although we can change the hook prototype to

  bool update_ipa_fn_target_info (const uint16_t, const gimple*, uint16_t&)

or

  uint16_t update_ipa_fn_target_info (const uint16_t, const gimple*, bool&)

to workaround bit field limitation, it looks weird and inefficient.

3)

  ...
  unsigned int fp_expressions : 1;
  uint16_t target_info;

  update_ipa_fn_target_info (uint16_t &, const gimple *)

it fails due to gengtype erroring:

gcc/ipa-fnsummary.h:171: undefined type `uint16_t'
gengtype: didn't write state file tmp-gtype.state after errors


Then I gave up and guessed it's not so crucial like you said, 
and used unsigned int instead. :)

[1] https://en.cppreference.com/w/cpp/language/bit_field

BR,
Kewen

Re: [PATCH] ipa-fnsummary: Remove inconsistent bp_pack_value

[Kewen.Lin via Gcc-patches]

Hi Richi,

Thanks for the review!

on 2021/9/17 下午6:04, Richard Biener wrote:
> On Fri, Sep 17, 2021 at 12:03 PM Richard Biener
>  wrote:
>>
>> On Fri, Sep 17, 2021 at 11:43 AM Kewen.Lin  wrote:
>>>
>>> Hi,
>>>
>>> When changing target_info with bitfield, I happened to find this
>>> inconsistent streaming in and out.  We have the streaming in:
>>>
>>>   bp_pack_value (, info->inlinable, 1);
>>>   bp_pack_value (, false, 1);
>>>   bp_pack_value (, info->fp_expressions, 1);
>>>
>>> while the streaming out:
>>>
>>>   info->inlinable = bp_unpack_value (, 1);
>>>   info->fp_expressions = bp_unpack_value (, 1)
>>>
>>> The cleanup of Cilk Plus support seemed to miss to remove the bit
>>> streaming out but change with streaming false.
>>>
>>> By hacking fp_expression_p to return true always, I can see it
>>> reads the wrong fp_expressions value (false) out in wpa dumping.
>>>
>>> Bootstrapped and regress-tested on powerpc64le-linux-gnu Power9.
>>>
>>> Is it ok for trunk?
>>
>> OK for trunk and all affected branches (note we need to bump the
>> LTO minor version there).  The issue comes from the removal
>> of cilk+ in r8-4956 which removed the bp_unpack but replaced
>> the bp_pack ...
>>
>> It's a correctness issue as we'll read fp_expressions as always 'false'
> 
> Btw, on branches we could also simply unpack a dummy bit to avoid
> changing the format.
> 

Committed in r12-3721.  Thanks!

As suggested, the patch for branches is listed below.

Is ok for branches 9, 10 and 11 after some trunk burn in time?

BR,
Kewen
-
gcc/ChangeLog:

* ipa-fnsummary.c (inline_read_section): Unpack a dummy bit
to keep consistent with the side of streaming out.

---
diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 18bbae145b9..bf635c1f78a 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -4403,13 +4403,20 @@ inline_read_section (struct lto_file_decl_data 
*file_data, const char *data,
   bp = streamer_read_bitpack ();
   if (info)
{
-  info->inlinable = bp_unpack_value (, 1);
-  info->fp_expressions = bp_unpack_value (, 1);
+ info->inlinable = bp_unpack_value (, 1);
+ /* On the side of streaming out, there is still one bit
+streamed out between inlinable and fp_expressions bits,
+which was used for cilk+ before but now always false.
+To remove the bit packing need to bump LTO minor version,
+so unpack a dummy bit here to keep consistent instead.  */
+ bp_unpack_value (, 1);
+ info->fp_expressions = bp_unpack_value (, 1);
}
   else
{
-  bp_unpack_value (, 1);
-  bp_unpack_value (, 1);
+ bp_unpack_value (, 1);
+ bp_unpack_value (, 1);
+ bp_unpack_value (, 1);
}

   count2 = streamer_read_uhwi ();

Re: [PATCH] Simplify paradoxical subreg extensions of TRUNCATE

[Segher Boessenkool]

Hi!

On Sun, Sep 19, 2021 at 09:14:55AM -0600, Jeff Law wrote:
> On 9/6/2021 8:24 AM, Segher Boessenkool wrote:
> >On Mon, Sep 06, 2021 at 12:32:13PM +0100, Roger Sayle wrote:
> >>I think the current documentation is sufficient.  During compilation, 
> >>GCC's
> >>combine pass will often substitute a register with an expression defining
> >>it's value, and then attempt to simplify it.  As you point out, this often
> >>produces
> >>(temporary intermediate) expressions with poorly defined semantics.
> >Not "poorly defined".  The documentation says (in rtl.texi)
> >
> >@findex subreg
> >@item (subreg:@var{m1} @var{reg:m2} @var{bytenum})
> >
> >@code{subreg} expressions are used to refer to a register in a machine
> >mode other than its natural one, or to refer to one register of
> >a multi-part @code{reg} that actually refers to several registers.
> >
> >It goes on to say there also are subregs of mem currently; it
> >exhaustively lists all things you can take a subreg of, what the
> >different semantics of those are, how the semantics are further
> >"modified" (read: completely different) if some RTL flags are set, etc.
> >
> >The instruction combiner very often creates invalid RTL (only
> >structurally valid, like, no missing operands).  Invalid RTL is supposed
> >to never match (because backends will not have patterns that match
> >these).  combine even creates invalid RTL on purpose (a clobber of
> >const0_rtx) to ensure its result is deemed invalid, when it wants to
> >abort a combination attempt for some reason.
> >
> >>The
> >>purpose of my patch is to avoid (constructing) SUBREGs that have TRUNCATE
> >>as an argument (that some folks consider undefined) and instead simplify
> >>them to either a single TRUNCATE or a SUBREG of a REG, both of which are
> >>well defined.  I'm making/helping the implementation match the
> >>documentation.
> >But this should never make it to simplify-rtx at all.  You can only
> >validly do such transformations in combine, because you need to know
> >what insns you started with etc.
> >
> >simplify-rtx is a part of combine, sure, but it is used from other
> >contexts as well nowadays.  If you can safely do this from combine,
> >you can do it in combine.
> [ ... ]
> So what I think is missing here is some concrete path forward.  If I'm 
> understanding your objection Segher, you'd like to see Roger look at 
> where these (subreg (truncate)) expressions came from and address them 
> earlier than simplify_rtx?

There is no such thing as "earlier than simplify-rtx", that is the
point.  simplify-rtx is not a pass: it is like a library that is used
from all over the RTL routines.

If something belongs in combine, it should be in combine, not in
simplify-rtx.

> The theory being that the simplification 
> bits could be used from other contexts than just combine and in those 
> other contexts (subreg (truncate)) isn't valid?

Or not actually simplifying (or canonicalising).  Yup.

It also is the case that invalid RTL should simply never be constructed,
we should not (try to) fix it up later.  I'm not sure if that was what
was happening in this case, too much of this stuff recently, everything
blurs together, sorry :-/

I would love to see this all improved.  But if we just poke at it with
a stick we'll be in worse shape instead of better by the time stage 1
closes.  So we need a plan, and/or a topic tree (some devel/* branch or
whatever).


Segher

Re: [PATCH v3] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Marek Polacek via Gcc-patches]

On Mon, Sep 20, 2021 at 09:08:09PM +0200, Jakub Jelinek wrote:
> On Mon, Sep 20, 2021 at 02:37:03PM -0400, Marek Polacek wrote:
> > > So, wouldn't be this better specified as
> > > Wno-attributes=
> > > Common Joined RejectNegative
> > > (not sure if RejectNegative is actually needed for an option
> > > starting with Wno- )?
> > 
> > Looks like RejectNegative is not needed.  I could do that, but I think
> > it regresses the diagnostic:
> > 
> > error: unrecognized command-line option '-Wattributes=attr::'
> > vs
> > error: arguments ignored for ‘-Wattributes=’; use ‘-Wno-attributes=’ instead
> > 
> > I prefer the latter.  (I've changed the warning into error.)
> 
> Ok.
> 
> > > > +/* { dg-additional-options "-std=c++11" { target c++ } } */
> > > > +/* { dg-additional-options "-Wno-attributes=company::,yoyodyne::attr" 
> > > > } */
> > > > +/* { dg-additional-options 
> > > > "-Wno-attributes=c1::attr,c1::attr,c1::__attr__" } */
> > > > +/* { dg-additional-options "-Wno-attributes=clang" } */
> > > > +/* { dg-additional-options "-Wno-attributes=c2::,c2::attr" } */
> > > > +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */
> > > > +/* { dg-additional-options "-Wno-attributes=x::," } */
> > > 
> > > Should the above be accepted (I mean trailing , ?)  What does that mean?
> >  
> > I'm thinking it should: the arguments to -Wno-attributes= could be
> > generated by a tool so accepting the trailing , might help, like in
> > enums etc.
> 
> I don't think we allow that for any other options that accept a list of
> something (except maybe -W{p,c,a,l}, those just pass the args through).
> Generating a list that has comma separation only in between items and not
> at the end is trivial.

Hmm, ok, here's a version with that checking added.  Sadly it needs
to be handled before handle_ignored_attributes_option because of
strtok.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
It is desirable for -Wattributes to warn about e.g.

[[deprecate]] void g(); // typo, should warn

However, -Wattributes also warns about vendor-specific attributes
(that's because lookup_scoped_attribute_spec -> find_attribute_namespace
finds nothing), which, with -Werror, causes grief.  We don't want the
-Wattributes warning for

[[company::attr]] void f();

GCC warns because it doesn't know the "company" namespace; it only knows
the "gnu" and "omp" namespaces.  We could entirely disable warning about
attributes in unknown scopes but then the compiler would also miss typos
like

  [[company::attrx]] void f();

or

  [[gmu::warn_used_result]] int write();

so that is not a viable solution.  A workaround is to use a #pragma:

  #pragma GCC diagnostic push
  #pragma GCC diagnostic ignored "-Wattributes"
  [[company::attr]] void f() {}
  #pragma GCC diagnostic pop

but that's a mouthful and awkward to use and could also hide typos.  In
fact, any macro-based solution doesn't seem like a way forward.

This patch implements -Wno-attributes=, which takes these arguments:

company::attr
company::
clang

The last one is a special option to ignore clang-only attributes; in
this patch it is accepted but currently has no effect.

This option should go well with using @file: the user could have a file
containing
-Wno-attributes=vendor::attr1,vendor::attr2
and then invoke gcc with '@attrs' or similar.

I've also added a new pragma which has the same effect:

The pragma along with the new option should help with various static
analysis tools.

PR c++/101940

gcc/ChangeLog:

* attribs.c (struct scoped_attributes): Add a bool member.
(lookup_scoped_attribute_spec): Forward declare.
(register_scoped_attributes): New bool parameter, defaulted to
false.  Use it.
(handle_ignored_attributes_option): New function.
(free_attr_data): New function.
(init_attributes): Call handle_ignored_attributes_option.
(attr_namespace_ignored_p): New function.
(decl_attributes): Check attr_namespace_ignored_p before
warning.
* attribs.h (free_attr_data): Declare.
(register_scoped_attributes): Adjust declaration.
(handle_ignored_attributes_option): Declare.
* common.opt (Wattributes=): New option with a variable.
* doc/extend.texi: Document #pragma GCC diagnostic ignored_attributes.
* doc/invoke.texi: Document -Wno-attributes=.
* opts.c (common_handle_option) : Handle.
* plugin.h (register_scoped_attributes): Adjust declaration.

gcc/c-family/ChangeLog:

* c-pragma.c (handle_pragma_diagnostic): Handle #pragma GCC diagnostic
ignored_attributes.

gcc/c/ChangeLog:

* c-decl.c (c_parse_final_cleanups): Call free_attr_data.

gcc/cp/ChangeLog:

* decl2.c (c_parse_final_cleanups): Call free_attr_data.

gcc/testsuite/ChangeLog:

* c-c++-common/Wno-attributes-1.c: New test.
* c-c++-common/Wno-attributes-2.c: New test.
---
 gcc/attribs.c   

[COMMITTED] Use EDGE_EXECUTABLE in ranger and return UNDEFINED for those edges.

[Andrew MacLeod via Gcc-patches]

The patch sets the EXECUTABLE property on edges like VRP does, and then 
removes that flag when an edge is determined to be un-executable.


This information is then used to return UNDEFINED for any requests on 
un-executable edges, and to register equivalencies if all executable 
edges of a PHI node are the same SSA_NAME.


This catches up a number of the cases VRP gets that ranger was missing, 
and reduces the EVRP discrepancies to almost 0.


On a side note,  is there any interest/value in reversing the meaning of 
that flag?  It seems to me that we could assume edges are EXECUTABLE by 
default, then set a NON_EXECUTABLE flag when a pass determines the edge 
cannot be executed.  This would rpevent a number fo passes from having 
to loop through all the edges and set the EXECUTABLE property...   It 
just seems backwards to me.


Anyway, bootstrapped on x86_64-pc-linux-gnu with no regressions. pushed.

Andrew

>From 73cf73af2392e00917de042a4692f6a0b6329ee8 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod 
Date: Tue, 24 Aug 2021 12:13:24 -0400
Subject: [PATCH 2/2] Use EDGE_EXECUTABLE in ranger and return UNDEFINED for
 those edges.

If an incoming edge is UNDEFINED, don't process it.  Track if other edges
equate to a single value, and add an equivalence if appropriate.

	gcc/
	* gimple-range-fold.cc (fold_using_range::range_of_phi): Ignore
	undefined edges, apply an equivalence if appropriate.
	* gimple-range-gori.cc (gori_compute::outgoing_edge_range_p): Return
	UNDEFINED if EDGE_EXECUTABLE is not set.
	* gimple-range.cc (gimple_ranger::gimple_ranger): Set all edges
	as EXECUTABLE upon startup.
	(gimple_ranger::range_on_edge): Return UNDEFINED for edges without
	EDGE_EXECUTABLE set.
	* vr-values.c (set_and_propagate_unexecutable): New.
	(simplify_using_ranges::fold_cond): Call set_and_propagate.
	(simplify_using_ranges::simplify_switch_using_ranges): Ditto.
	* vr-values.h: Add prototype.

	gcc/testsuite/
	* gcc.dg/tree-ssa/evrp-ignore.c: New.
---
 gcc/gimple-range-fold.cc| 47 ++---
 gcc/gimple-range-gori.cc| 25 ---
 gcc/gimple-range.cc | 36 +++-
 gcc/testsuite/gcc.dg/tree-ssa/evrp-ignore.c | 28 
 gcc/vr-values.c | 39 -
 gcc/vr-values.h |  1 +
 6 files changed, 140 insertions(+), 36 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/evrp-ignore.c

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 997d02dd4b9..80cc5c0dc0c 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -760,6 +760,10 @@ fold_using_range::range_of_phi (irange , gphi *phi, fur_source )
   if (!type)
 return false;
 
+  // Track if all executable arguments are the same.
+  tree single_arg = NULL_TREE;
+  bool seen_arg = false;
+
   // Start with an empty range, unioning in each argument's range.
   r.set_undefined ();
   for (x = 0; x < gimple_phi_num_args (phi); x++)
@@ -767,19 +771,48 @@ fold_using_range::range_of_phi (irange , gphi *phi, fur_source )
   tree arg = gimple_phi_arg_def (phi, x);
   edge e = gimple_phi_arg_edge (phi, x);
 
-  // Register potential dependencies for stale value tracking.
-  if (gimple_range_ssa_p (arg) && src.gori ())
-	src.gori ()->register_dependency (phi_def, arg);
-
   // Get the range of the argument on its edge.
   src.get_phi_operand (arg_range, arg, e);
-  // If we're recomputing the argument elsewhere, try to refine it.
-  r.union_ (arg_range);
+
+  if (!arg_range.undefined_p ())
+	{
+	  // Register potential dependencies for stale value tracking.
+	  r.union_ (arg_range);
+	  if (gimple_range_ssa_p (arg) && src.gori ())
+	src.gori ()->register_dependency (phi_def, arg);
+
+	  // Track if all arguments are the same.
+	  if (!seen_arg)
+	{
+	  seen_arg = true;
+	  single_arg = arg;
+	}
+	  else if (single_arg != arg)
+	single_arg = NULL_TREE;
+	}
+
   // Once the value reaches varying, stop looking.
-  if (r.varying_p ())
+  if (r.varying_p () && single_arg == NULL_TREE)
 	break;
 }
 
+// If the PHI boils down to a single effective argument, look at it.
+if (single_arg)
+  {
+	// Symbolic arguments are equivalences.
+	if (gimple_range_ssa_p (single_arg))
+	  src.register_relation (phi, EQ_EXPR, phi_def, single_arg);
+	else if (src.get_operand (arg_range, single_arg)
+		 && arg_range.singleton_p ())
+	  {
+	// Numerical arguments that are a constant can be returned as
+	// the constant. This can help fold later cases where even this
+	// constant might have been UNDEFINED via an unreachable edge.
+	r = arg_range;
+	return true;
+	  }
+  }
+
   // If SCEV is available, query if this PHI has any knonwn values.
   if (scev_initialized_p () && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
 {
diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index 

[COMMITTED] Use EDGE_EXECUTABLE in ranger and return UNDEFINED for those edges.

[Andrew MacLeod via Gcc-patches]

The patch sets the EXECUTABLE property on edges like VRP does, and then 
removes that flag when an edge is determined to be un-executable.


This information is then used to return UNDEFINED for any requests on 
un-executable edges, and to register equivalencies if all executable 
edges of a PHI node are the same SSA_NAME.


This catches up a number of the cases VRP gets that ranger was missing, 
and reduces the EVRP discrepancies to almost 0.


On a side note,  is there any interest/value in reversing the meaning of 
that flag?  It seems to me that we could assume edges are EXECUTABLE by 
default, then set a NON_EXECUTABLE flag when a pass determines the edge 
cannot be executed.  This would rpevent a number fo passes from having 
to loop through all the edges and set the EXECUTABLE property...   It 
just seems backwards to me.


Anyway, bootstrapped on x86_64-pc-linux-gnu with no regressions. pushed.

Andrew

>From 73cf73af2392e00917de042a4692f6a0b6329ee8 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod 
Date: Tue, 24 Aug 2021 12:13:24 -0400
Subject: [PATCH 2/2] Use EDGE_EXECUTABLE in ranger and return UNDEFINED for
 those edges.

If an incoming edge is UNDEFINED, don't process it.  Track if other edges
equate to a single value, and add an equivalence if appropriate.

	gcc/
	* gimple-range-fold.cc (fold_using_range::range_of_phi): Ignore
	undefined edges, apply an equivalence if appropriate.
	* gimple-range-gori.cc (gori_compute::outgoing_edge_range_p): Return
	UNDEFINED if EDGE_EXECUTABLE is not set.
	* gimple-range.cc (gimple_ranger::gimple_ranger): Set all edges
	as EXECUTABLE upon startup.
	(gimple_ranger::range_on_edge): Return UNDEFINED for edges without
	EDGE_EXECUTABLE set.
	* vr-values.c (set_and_propagate_unexecutable): New.
	(simplify_using_ranges::fold_cond): Call set_and_propagate.
	(simplify_using_ranges::simplify_switch_using_ranges): Ditto.
	* vr-values.h: Add prototype.

	gcc/testsuite/
	* gcc.dg/tree-ssa/evrp-ignore.c: New.
---
 gcc/gimple-range-fold.cc| 47 ++---
 gcc/gimple-range-gori.cc| 25 ---
 gcc/gimple-range.cc | 36 +++-
 gcc/testsuite/gcc.dg/tree-ssa/evrp-ignore.c | 28 
 gcc/vr-values.c | 39 -
 gcc/vr-values.h |  1 +
 6 files changed, 140 insertions(+), 36 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/evrp-ignore.c

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 997d02dd4b9..80cc5c0dc0c 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -760,6 +760,10 @@ fold_using_range::range_of_phi (irange , gphi *phi, fur_source )
   if (!type)
 return false;
 
+  // Track if all executable arguments are the same.
+  tree single_arg = NULL_TREE;
+  bool seen_arg = false;
+
   // Start with an empty range, unioning in each argument's range.
   r.set_undefined ();
   for (x = 0; x < gimple_phi_num_args (phi); x++)
@@ -767,19 +771,48 @@ fold_using_range::range_of_phi (irange , gphi *phi, fur_source )
   tree arg = gimple_phi_arg_def (phi, x);
   edge e = gimple_phi_arg_edge (phi, x);
 
-  // Register potential dependencies for stale value tracking.
-  if (gimple_range_ssa_p (arg) && src.gori ())
-	src.gori ()->register_dependency (phi_def, arg);
-
   // Get the range of the argument on its edge.
   src.get_phi_operand (arg_range, arg, e);
-  // If we're recomputing the argument elsewhere, try to refine it.
-  r.union_ (arg_range);
+
+  if (!arg_range.undefined_p ())
+	{
+	  // Register potential dependencies for stale value tracking.
+	  r.union_ (arg_range);
+	  if (gimple_range_ssa_p (arg) && src.gori ())
+	src.gori ()->register_dependency (phi_def, arg);
+
+	  // Track if all arguments are the same.
+	  if (!seen_arg)
+	{
+	  seen_arg = true;
+	  single_arg = arg;
+	}
+	  else if (single_arg != arg)
+	single_arg = NULL_TREE;
+	}
+
   // Once the value reaches varying, stop looking.
-  if (r.varying_p ())
+  if (r.varying_p () && single_arg == NULL_TREE)
 	break;
 }
 
+// If the PHI boils down to a single effective argument, look at it.
+if (single_arg)
+  {
+	// Symbolic arguments are equivalences.
+	if (gimple_range_ssa_p (single_arg))
+	  src.register_relation (phi, EQ_EXPR, phi_def, single_arg);
+	else if (src.get_operand (arg_range, single_arg)
+		 && arg_range.singleton_p ())
+	  {
+	// Numerical arguments that are a constant can be returned as
+	// the constant. This can help fold later cases where even this
+	// constant might have been UNDEFINED via an unreachable edge.
+	r = arg_range;
+	return true;
+	  }
+  }
+
   // If SCEV is available, query if this PHI has any knonwn values.
   if (scev_initialized_p () && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
 {
diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index 

[COMMITTED] Relation oracle: Make each def a new equivalency record.

[Andrew MacLeod via Gcc-patches]

The current equivalence oracle is not "killing" back edge equivalence 
sets sometimes.   Missing is that a definition starts a new equivalence 
set at that point, with nothing in it but the def itself.


Creating this new equivalence set will ensure that any subsequent 
comparisons with values live from the back edge will have 
non-symmetrical equivalence sets, and the oracle will no longer pick up 
the equivalence.  This initial set is created lazily, the first time 
that an attempt is made to combine it with another set.


Eventually I may move to a more fine grained location based analysis, 
but for now this takes care of most of the issue.


bootstrapped on x86_64-pc-linux-gnu with no regressions.  pushed.

Andrew

>From 5d110fe90afcd850ea21aee6429f22edd6b1b592 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod 
Date: Fri, 17 Sep 2021 14:58:06 -0400
Subject: [PATCH 1/2] Make each def a new equivalency record.

Create a new equivalency set at each def point killing any equivalencies
coming into the block from back edges.  Do not add equivalences for PHI
arguments defined in this block.

	* value-relation.cc (equiv_oracle::register_initial_def): New.
	(equiv_oracle::register_relation): Call register_initial_def.
	(equiv_oracle::add_equiv_to_block): New.  Split register_relation.
	(relation_oracle::register_stmt): Check def block of PHI arguments.
	* value-relation.h (equiv_oracle): Add new prototypes.
---
 gcc/value-relation.cc | 57 +++
 gcc/value-relation.h  |  3 ++-
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index d370f93d128..ac5f3f9afc0 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -407,6 +407,24 @@ equiv_oracle::register_equiv (basic_block bb, equiv_chain *equiv_1,
   return b;
 }
 
+// Create an equivalency set containing only SSA in its definition block.
+// This is done the first time SSA is registered in an equivalency and blocks
+// any DOM searches past the definition.
+
+void
+equiv_oracle::register_initial_def (tree ssa)
+{
+  if (SSA_NAME_IS_DEFAULT_DEF (ssa))
+return;
+  basic_block bb = gimple_bb (SSA_NAME_DEF_STMT (ssa));
+  gcc_checking_assert (bb && !find_equiv_dom (ssa, bb));
+
+  unsigned v = SSA_NAME_VERSION (ssa);
+  bitmap_set_bit (m_equiv_set, v);
+  bitmap equiv_set = BITMAP_ALLOC (_bitmaps);
+  bitmap_set_bit (equiv_set, v);
+  add_equiv_to_block (bb, equiv_set);
+}
 
 // Register an equivalence between SSA1 and SSA2 in block BB.
 // The equivalence oracle maintains a vector of equivalencies indexed by basic
@@ -425,6 +443,14 @@ equiv_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
 
   unsigned v1 = SSA_NAME_VERSION (ssa1);
   unsigned v2 = SSA_NAME_VERSION (ssa2);
+
+  // If this is the first time an ssa_name has an equivalency registered
+  // create a self-equivalency record in the def block.
+  if (!bitmap_bit_p (m_equiv_set, v1))
+register_initial_def (ssa1);
+  if (!bitmap_bit_p (m_equiv_set, v2))
+register_initial_def (ssa2);
+
   equiv_chain *equiv_1 = find_equiv_dom (ssa1, bb);
   equiv_chain *equiv_2 = find_equiv_dom (ssa2, bb);
 
@@ -456,6 +482,15 @@ equiv_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
   if (!equiv_set)
 return;
 
+  add_equiv_to_block (bb, equiv_set);
+}
+
+// Add an equivalency record in block BB containing bitmap EQUIV_SET.
+// Note the internal caller is responible for allocating EQUIV_SET properly.
+
+void
+equiv_oracle::add_equiv_to_block (basic_block bb, bitmap equiv_set)
+{
   equiv_chain *ptr;
 
   // Check if this is the first time a block has an equivalence added.
@@ -786,6 +821,28 @@ relation_oracle::register_stmt (gimple *stmt, relation_kind k, tree op1,
   print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
 }
 
+  // If an equivalence is being added between a PHI and one of its arguments
+  // make sure that that argument is not defined in the same block.
+  // This can happen along back edges and the equivalence will not be
+  // applicable as it would require a use before def.
+  if (k == EQ_EXPR && is_a (stmt))
+{
+  tree phi_def = gimple_phi_result (stmt);
+  gcc_checking_assert (phi_def == op1 || phi_def == op2);
+  tree arg = op2;
+  if (phi_def == op2)
+	arg = op1;
+  if (gimple_bb (stmt) == gimple_bb (SSA_NAME_DEF_STMT (arg)))
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	{
+	  fprintf (dump_file, "  Not registered due to ");
+	  print_generic_expr (dump_file, arg, TDF_SLIM);
+	  fprintf (dump_file, " being defined in the same block.\n");
+	}
+	  return;
+	}
+}
   register_relation (gimple_bb (stmt), k, op1, op2);
 }
 
diff --git a/gcc/value-relation.h b/gcc/value-relation.h
index 574fdc9efe8..53cefbfa7dc 100644
--- a/gcc/value-relation.h
+++ b/gcc/value-relation.h
@@ -146,7 +146,8 @@ private:
   bitmap register_equiv (basic_block bb, unsigned v, equiv_chain *equiv_1);
   bitmap 

Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]

[Harald Anlauf via Gcc-patches]

Hi Tobias,

Am 20.09.21 um 22:33 schrieb Tobias Burnus:

And v3 – I realized that testcases would be useful. Thus, now with added
testcases. :-)


I was about to recommend a testcase I prepared when your revised patch
arrived.  Will not bother you with it, as you seem to provide really
good coverage.

LGTM.

Thanks for the patch!

Harald


Tobias

On 17.09.21 20:45, Tobias Burnus wrote:

I seemingly messed up a bit in previous patch – corrected version
attached.

OK?

Tobias

PS: Due to now enabling the missing-include-dir warning also for
cpp,the following
warning show up during build. This seems to be specific to libgfortran
building,
libgomp works and real-world code also does not seem to be affected:
: Error: /x86_64-pc-linux-gnu/include: No such file
or directory [-Werror=missing-include-dirs]
: Error: /x86_64-pc-linux-gnu/sys-include: No such
file or directory [-Werror=missing-include-dirs]
: Error: finclude: No such file or directory
[-Werror=missing-include-dirs]

The latter is due to the driver adding '-fintrinsic-modules-path
finclude'
when calling f951. I think the rest is a side effect of running with -B
and other build trickery.

The warnings do not trigger when compiling the Fortran file in libgomp
nor for
a quick real-world case (which uses gfortran in a normal way not with
-B etc.).
Thus, I think it should be fine.
Alternatively, we could think of reducing the noisiness. Thoughts?

PPS: Besides this, the following still applies:

On 17.09.21 15:02, Tobias Burnus wrote:

Short version:
* -Wno-missing-include-dirs  had no effect as the warning was always on
* For CPP-only options like -idirafter, no warning was shown.

This patch tries to address both, i.e. cpp's include-dir diagnostics are
shown as well – and silencing the diagnostic works as well.

OK for mainline?

Tobias

PS:  BACKGROUND and LONG DESCRIPTION

C/C++ by default have disabled the -Wmissing-include-dirs warning.
Fortran by default has that warning enabled.

The value is actually stored at two places (cf. c-family/c.opt):
  Wmissing-include-dirs
  ... CPP(warn_missing_include_dirs)
Var(cpp_warn_missing_include_dirs) Init(0)

For CPP, that value always needs to initialized – and it is used
in gcc/incpath.c as
  cpp_options *opts = cpp_get_options (pfile);
  if (opts->warn_missing_include_dirs &&
cur->user_supplied_p)
    cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s:
%s",

Additionally, there is cpp_warn_missing_include_dirs which is used by
Fortran – and which consists of
  global_options.x_cpp_warn_missing_include_dirs
  global_options_set._cpp_warn_missing_include_dirs

The flag processing happens as described in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11
in short:
  - First gfc_init_options is called
  - Then for reach command-line option gfc_handle_option
  - Finally gfc_post_options

Currently:
- gfc_init_options: Sets cpp_warn_missing_include_dirs
  (unconditionally as unset)
- gfc_handle_option: Always warns about the missing include dir
- before gfc_post_options: set_option is called, which sets
  cpp_warn_missing_include_dirs – but that's too late.

Additionally, as mentioned above – pfile's warn_missing_include_dirs
is never properly set.

 * * *

This patch fixes those issues:
* Now -Wno-missing-include-dirs does silence the warnings
* CPP now also properly does warn.

Example (initial version):
$ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/
-Wmissing-include-dirs
f951: Warning: Nonexistent include directory ‘bar//’
[-Wmissing-include-dirs]
: Warning: /fdaf/: No such file or directory
: Warning: bar/: No such file or directory

In order to avoid the double output for -I, I disabled the Fortran
output if
CPP is enabled. Additionally, I had to use the
cpp_reason_option_codes to
print the flag in brackets.
Fixed/final output is:

: Warning: /fdaf/: No such file or directory
[-Wmissing-include-dirs]
: Warning: bar/: No such file or directory
[-Wmissing-include-dirs]


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 
80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: 
Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; 
Registergericht München, HRB 106955

Re: [PATCH v4] c++: Fix cp_tree_equal for template value args using dependent sizeof/alignof/noexcept expressions

[Jason Merrill via Gcc-patches]

On 9/17/21 12:44, Barrett Adair wrote:

I think the patch is in good shape now, thanks for the help.

canon-type*.C fail with trunk and pass with patch, dependent-name*.C are 
regression tests that pass with both. I removed the dg-ice from 
constexpr-52830.C. I didn't dig much into the churn history there, but 
the test code looks valid to me and clang agrees.


I also returned the copyright assignment form yesterday to 
ass...@gnu.org .



+/* Identify any expressions that use function parms */


A comment should end with a period and two spaces.


+static tree
+find_parm_usage_r (tree tp, int *walk_subtrees, void)
+{
+  tree t = *tp;
+  if (PACK_EXPANSION_P (t) || (TREE_CODE (t) == PARM_DECL))


PACK_EXPANSION_P is wrong here; a pack expansion might not involve a 
function parameter (pack) at all.  And walk_tree should already recurse 
into the pattern of a pack expansion, so handling them specifically here 
shouldn't be necessary.



+ else if (!current_function_decl


This needs a comment about why we're checking current_function_decl 
(because we only need structural comparison for redeclaration comparisons)



+  && dependent_template_arg_p (arg)
+  && cp_walk_tree_without_duplicates (,
+   find_parm_usage_r, NULL))


Here the second line of args needs to line up with the args on the 
previous line.  When that's too far to the right, like here, I tend to 
write e.g.


> + && (cp_walk_tree_without_duplicates
> + (, find_parm_usage_r, NULL)))

Jason

Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]

[Tobias Burnus]

And v3 – I realized that testcases would be useful. Thus, now with added
testcases. :-)

Tobias

On 17.09.21 20:45, Tobias Burnus wrote:

I seemingly messed up a bit in previous patch – corrected version
attached.

OK?

Tobias

PS: Due to now enabling the missing-include-dir warning also for
cpp,the following
warning show up during build. This seems to be specific to libgfortran
building,
libgomp works and real-world code also does not seem to be affected:
: Error: /x86_64-pc-linux-gnu/include: No such file
or directory [-Werror=missing-include-dirs]
: Error: /x86_64-pc-linux-gnu/sys-include: No such
file or directory [-Werror=missing-include-dirs]
: Error: finclude: No such file or directory
[-Werror=missing-include-dirs]

The latter is due to the driver adding '-fintrinsic-modules-path
finclude'
when calling f951. I think the rest is a side effect of running with -B
and other build trickery.

The warnings do not trigger when compiling the Fortran file in libgomp
nor for
a quick real-world case (which uses gfortran in a normal way not with
-B etc.).
Thus, I think it should be fine.
Alternatively, we could think of reducing the noisiness. Thoughts?

PPS: Besides this, the following still applies:

On 17.09.21 15:02, Tobias Burnus wrote:

Short version:
* -Wno-missing-include-dirs  had no effect as the warning was always on
* For CPP-only options like -idirafter, no warning was shown.

This patch tries to address both, i.e. cpp's include-dir diagnostics are
shown as well – and silencing the diagnostic works as well.

OK for mainline?

Tobias

PS:  BACKGROUND and LONG DESCRIPTION

C/C++ by default have disabled the -Wmissing-include-dirs warning.
Fortran by default has that warning enabled.

The value is actually stored at two places (cf. c-family/c.opt):
  Wmissing-include-dirs
  ... CPP(warn_missing_include_dirs)
Var(cpp_warn_missing_include_dirs) Init(0)

For CPP, that value always needs to initialized – and it is used
in gcc/incpath.c as
  cpp_options *opts = cpp_get_options (pfile);
  if (opts->warn_missing_include_dirs &&
cur->user_supplied_p)
cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s:
%s",

Additionally, there is cpp_warn_missing_include_dirs which is used by
Fortran – and which consists of
  global_options.x_cpp_warn_missing_include_dirs
  global_options_set._cpp_warn_missing_include_dirs

The flag processing happens as described in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11
in short:
  - First gfc_init_options is called
  - Then for reach command-line option gfc_handle_option
  - Finally gfc_post_options

Currently:
- gfc_init_options: Sets cpp_warn_missing_include_dirs
  (unconditionally as unset)
- gfc_handle_option: Always warns about the missing include dir
- before gfc_post_options: set_option is called, which sets
  cpp_warn_missing_include_dirs – but that's too late.

Additionally, as mentioned above – pfile's warn_missing_include_dirs
is never properly set.

 * * *

This patch fixes those issues:
* Now -Wno-missing-include-dirs does silence the warnings
* CPP now also properly does warn.

Example (initial version):
$ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/
-Wmissing-include-dirs
f951: Warning: Nonexistent include directory ‘bar//’
[-Wmissing-include-dirs]
: Warning: /fdaf/: No such file or directory
: Warning: bar/: No such file or directory

In order to avoid the double output for -I, I disabled the Fortran
output if
CPP is enabled. Additionally, I had to use the
cpp_reason_option_codes to
print the flag in brackets.
Fixed/final output is:

: Warning: /fdaf/: No such file or directory
[-Wmissing-include-dirs]
: Warning: bar/: No such file or directory
[-Wmissing-include-dirs]


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
Fortran: Fix -Wno-missing-include-dirs handling [PR55534]

gcc/fortran/ChangeLog:

	PR fortran/55534
	* cpp.c: Define GCC_C_COMMON_C for #include "options.h" to make
	cpp_reason_option_codes available.
	(gfc_cpp_register_include_paths): Make static, set pfile's
	warn_missing_include_dirs and move before caller.
	(gfc_cpp_init_cb): New, cb code moved from ...
	(gfc_cpp_init_0): ... here.
	(gfc_cpp_post_options): Call gfc_cpp_init_cb.
	(cb_cpp_diagnostic_cpp_option): New. As implemented in c-family
	to match CppReason flags to -W... names.
	(cb_cpp_diagnostic): Use it to replace single special case.
	* cpp.h (gfc_cpp_register_include_paths): Remove as now static.
	* gfortran.h (gfc_check_include_dirs): New prototype.
	(gfc_add_include_path): Add new bool arg.
	* options.c (gfc_init_options): Don't set -Wmissing-include-dirs.
	(gfc_post_options): Set it here after commandline processing. Call
	gfc_add_include_path with defer_warn=false.
	(gfc_handle_option): Call it with 

Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565]

[Jason Merrill via Gcc-patches]

On 9/17/21 13:31, Michel Morin wrote:

On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill  wrote:


On 9/16/21 11:50, Michel Morin wrote:

On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill  wrote:


On 9/14/21 04:29, Michel Morin via Gcc-patches wrote:

On Tue, Sep 14, 2021 at 7:14 AM David Malcolm  wrote:


On Tue, 2021-09-14 at 03:35 +0900, Michel Morin via Gcc-patches wrote:

Hi,

PR77565 reports that, with the code `typdef int Int;`, GCC emits
"did you mean 'typeof'?" instead of "did you mean 'typedef'?".

This happens because the typo corrector determines that `typeof` is a
candidate for suggestion (through
`cp_keyword_starts_decl_specifier_p`),
but `typedef` is not.

This patch fixes the issue by adding `typedef` as a candidate. The
patch
additionally adds the `inline` specifier and cv-specifiers as a
candidate.
Here is a patch (tests `make check-gcc` pass on darwin):


Thanks for this patch (and for reporting the bug in the first place).

I notice that, as well as being used for fix-it hints by
lookup_name_fuzzy (indirectly via suggest_rid_p),
cp_keyword_starts_decl_specifier_p is also used by
cp_lexer_next_token_is_decl_specifier_keyword, which is used by
cp_parser_lambda_declarator_opt and cp_parser_constructor_declarator_p.


Ah, you're right! Thank you for pointing this out.
I failed to grep those functions somehow.

One thing that confuses me is that cp_keyword_starts_decl_specifier_p
misses many keywords that can start decl-specifiers (e.g.
typedef/inline/cv-qual and friend/explicit/virtual).
So let's wait C++ frontend maintainers ;)


That is strange.  Let's add all the rest of them as well.


Done. Thanks for your help!

One more thing — cp_keyword_starts_decl_specifier_p includes RID_ATTRIBUTE
(from the beginning; see https://gcc.gnu.org/PR28261 ), but attributes are
not decl-specifiers. Would it be reasonable to remove this?


It looks like the place that PR28261 used
cp_lexer_next_token_is_decl_specifier_keyword specifically exempts
attributes:


   && (!cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
   /* GNU attributes can actually appear both at the start of
  a parameter and parenthesized declarator.
  S (__attribute__((unused)) int);
  is a constructor, but
  S (__attribute__((unused)) foo) (int);
  is a function declaration.  */
   || (cp_parser_allow_gnu_extensions_p (parser)
   && cp_next_tokens_can_be_gnu_attribute_p (parser)))


So yes, let's remove RID_ATTRIBUTE and the || clause there.  I'd keep
the comment, but move it to go with the test for C++11 attributes below.


Done. No regressions introduced.


One more thing — cp_keyword_starts_decl_specifier_p includes RID_ATTRIBUTE
(from the beginning; see https://gcc.gnu.org/PR28261 ), but attributes are
not decl-specifiers.


Oh, this is wrong. I thought that, since C++11 attributes are not a
decl-specifier, neither are GNU attributes. But the comment just before
cp_parser_decl_specifier_seq says that GNU attributes are considered as a
decl-specifier. So I'm not confident about the removal of RID_ATTRIBUTE in
cp_keyword_starts_decl_specifier_p...


GNU attributes can appear in lots of places, and the only two callers of 
cp_parser_next_token_is_decl_specifier_keyword don't want to treat 
attributes accordingly.  Let's go with both your patches, and also 
remove the consequently-unnecessary attributes check in 
cp_parser_lambda_declarator_opt:



  if (cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
  && !cp_next_tokens_can_be_gnu_attribute_p (parser))


OK with that change.


I've split the patch into two. The first one is for adding missing keywords to
fix PR77565 and the second one is for removing the "attribute" keyword.
Here is the second patch (if this is not applied, that's no problem ;) )

==
c++: adjust the handling of RID_ATTRIBUTE.

gcc/cp/ChangeLog:

* parser.c (cp_keyword_starts_decl_specifier_p): Do not
handle RID_ATTRIBUTE.
(cp_parser_constructor_declarator_p): Remove now-redundant
checks.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 40308d0d33f..d184a3aca7e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1062,7 +1062,6 @@ cp_keyword_starts_decl_specifier_p (enum rid keyword)
  case RID_TYPEDEF:
  case RID_INLINE:
/* GNU extensions.  */
-case RID_ATTRIBUTE:
  case RID_TYPEOF:
/* C++11 extensions.  */
  case RID_DECLTYPE:
@@ -30798,23 +30797,22 @@ cp_parser_constructor_declarator_p
(cp_parser *parser, cp_parser_flags flags,
 /* A parameter declaration begins with a decl-specifier,
which is either the "attribute" keyword, a storage class
specifier, or (usually) a type-specifier.  */
-   && (!cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
-   /* GNU attributes can actually appear both at the start of
- a parameter and 

Re: [PATCH] c++: consider built-in operator candidates first

[Jason Merrill via Gcc-patches]

On 9/20/21 15:32, Patrick Palka wrote:

On Mon, 20 Sep 2021, Jason Merrill wrote:


On 9/20/21 12:46, Patrick Palka wrote:

During operator overload resolution, we currently consider non-member
candidates before built-in candidates.  This didn't make a difference
before r12-3346, but after this change add_candidates will avoid
computing excess argument conversions if we've already seen a strictly
viable candidate, so it's better to consider built-in candidates first.


Doesn't r12-3346 stop considering conversions after it sees a bad one, and
later return to the bad candidate if there is no strictly viable candidate?
How does this patch change that?


Yes, but add_candidates also looks for a strictly viable candidate among
the already-considered candidates in the 'candidates' list via the line:

   bool seen_strictly_viable = any_strictly_viable (*candidates);

So by considering the built-in candidates first, the subsequent call to
add_candidates that considers the non-member functions in will be aware
of any (built-in) strictly viable candidate.


Ah, I get it, the problem is that the first add_candidates can't see any 
strictly-viable candidates.



Depending on the order of the candidates seems fragile.


Yeah.. :/  I guess in general it'd be better to build up the entire
overload set first and then call add_candidates exactly once (which
would also make the perfect candidate optimization more consistent/effective).
But I'm not sure if we can easily build up such an overload set in this
case since built-in candidates are represented and handled differently
than non-built-in candidates..


Or as another way of getting the same effect, add another possible value 
of shortcut_bad_convs to mean leave bad candidates incomplete in the 
candidates list, and then once we're done adding candidates and still 
don't have a viable one, we can go back and finish processing the bad 
candidates?


Either way, this could also help when there are both member and 
non-member candidates for the operator.



FWIW, although the test case added by this patch is contrived, this
opportunity was found in the real world by instrumenting the 'bad_fns'
mechanism added by r12-3346 to look for situations where we still end up
using it (and thus end up redundantly considering some candidates twice),
and this built-in operator situation was the most common in the
codebases that I tested (although still quite rare in the codebases that
I tested).





Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

* call.c (add_operator_candidates): Consider built-in operator
candidates before considering non-member candidates.

gcc/testsuite/ChangeLog:

* g++.dg/template/conv17.C: Extend test.
---
   gcc/cp/call.c  | 13 +++--
   gcc/testsuite/g++.dg/template/conv17.C |  7 +++
   2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c5601d96ab8..c0da083758f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6321,7 +6321,6 @@ add_operator_candidates (z_candidate **candidates,
 vec *arglist,
 int flags, tsubst_flags_t complain)
   {
-  z_candidate *start_candidates = *candidates;
 bool ismodop = code2 != ERROR_MARK;
 tree fnname = ovl_op_identifier (ismodop, ismodop ? code2 : code);
   @@ -6333,6 +6332,12 @@ add_operator_candidates (z_candidate **candidates,
 if (rewritten && code != EQ_EXPR && code != SPACESHIP_EXPR)
   flags &= ~LOOKUP_REWRITTEN;
   +  /* Add built-in candidates to the candidate set.  The standard says to
+ rewrite built-in candidates, too, but there's no point.  */
+  if (!rewritten)
+add_builtin_candidates (candidates, code, code2, fnname, arglist,
+   flags, complain);
+
 bool memonly = false;
 switch (code)
   {
@@ -6352,6 +6357,7 @@ add_operator_candidates (z_candidate **candidates,
   /* Add namespace-scope operators to the list of functions to
consider.  */
+  z_candidate *start_candidates = *candidates;
 if (!memonly)
   {
 tree fns = lookup_name (fnname, LOOK_where::BLOCK_NAMESPACE);
@@ -6423,11 +6429,6 @@ add_operator_candidates (z_candidate **candidates,
   if (!rewritten)
   {
-  /* The standard says to rewrite built-in candidates, too,
-but there's no point.  */
-  add_builtin_candidates (candidates, code, code2, fnname, arglist,
- flags, complain);
-
 /* Maybe add C++20 rewritten comparison candidates.  */
 tree_code rewrite_code = ERROR_MARK;
 if (cxx_dialect >= cxx20
diff --git a/gcc/testsuite/g++.dg/template/conv17.C
b/gcc/testsuite/g++.dg/template/conv17.C
index f0f10f2ef4f..87ecefb8de3 100644
--- a/gcc/testsuite/g++.dg/template/conv17.C
+++ b/gcc/testsuite/g++.dg/template/conv17.C
@@ -61,3 +61,10 @@ concept E = requires { T().h(nullptr); };
 

Re: [PATCH v2] C++: add type checking for static local vector variable in template

[Jason Merrill via Gcc-patches]

On 9/18/21 01:53, wangpc wrote:

This patch moves verify_type_context from start_decl_1 to cp_finish_decl to
do more type checking such as static local vector variable in C++ template.


Applied, thanks.  Though a couple of issues with the patch formatting:


2021-08-06  wangpc  

gcc/cp/ChangeLog

 * decl.c (start_decl_1): Move verify_type_context to ...
(cp_finish_decl): ... to here.

gcc/testsuite/ChangeLog

 * g++.target/aarch64/sve/static-var-in-template.C: New test.


I needed to adjust the spacing on the ChangeLog entries so that the git 
hook would accept them: lines need to start with a single tab.



diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 90111e4c786..deaa6c56a8f 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5491,13 +5491,6 @@ start_decl_1 (tree decl, bool initialized)
cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
  }
  
-  if (is_global_var (decl))

-{
-  type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
-  ? TCTX_THREAD_STORAGE
-  : TCTX_STATIC_STORAGE);
-  verify_type_context (input_location, context, TREE_TYPE (decl));
-}
if (initialized)


I needed to apply this hunk by hand because there's a missing blank line 
before if (initialized).



  /* Is it valid for this decl to have an initializer at all?  */
  {
@@ -7520,6 +7513,14 @@ cp_finish_decl (tree decl, tree init, bool 
init_const_expr_p,
&& DECL_INITIALIZED_IN_CLASS_P (decl))
  check_static_variable_definition (decl, type);
  
+  if (!processing_template_decl && VAR_P (decl) && is_global_var (decl))

+{
+  type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
+  ? TCTX_THREAD_STORAGE
+  : TCTX_STATIC_STORAGE);
+  verify_type_context (input_location, context, TREE_TYPE (decl));
+}
+
if (init && TREE_CODE (decl) == FUNCTION_DECL)
  {
tree clone;
diff --git a/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C 
b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
new file mode 100644
index 000..74237ff7c57
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+
+#include 
+
+template 
+void f()
+{
+static svbool_t pg = svwhilelt_b64(0, N);
+}
+
+int main(int argc, char **argv)
+{
+f<2>();
+return 0;
+}
+
+/* { dg-error "SVE type 'svbool_t' does not have a fixed size" "" { target 
*-*-* } 0 } */

Re: [PATCH] c++: consider built-in operator candidates first

[Patrick Palka via Gcc-patches]

On Mon, 20 Sep 2021, Jason Merrill wrote:

> On 9/20/21 12:46, Patrick Palka wrote:
> > During operator overload resolution, we currently consider non-member
> > candidates before built-in candidates.  This didn't make a difference
> > before r12-3346, but after this change add_candidates will avoid
> > computing excess argument conversions if we've already seen a strictly
> > viable candidate, so it's better to consider built-in candidates first.
> 
> Doesn't r12-3346 stop considering conversions after it sees a bad one, and
> later return to the bad candidate if there is no strictly viable candidate?
> How does this patch change that?

Yes, but add_candidates also looks for a strictly viable candidate among
the already-considered candidates in the 'candidates' list via the line:

  bool seen_strictly_viable = any_strictly_viable (*candidates);

So by considering the built-in candidates first, the subsequent call to
add_candidates that considers the non-member functions in will be aware
of any (built-in) strictly viable candidate.

> 
> Depending on the order of the candidates seems fragile.

Yeah.. :/  I guess in general it'd be better to build up the entire
overload set first and then call add_candidates exactly once (which
would also make the perfect candidate optimization more consistent/effective).
But I'm not sure if we can easily build up such an overload set in this
case since built-in candidates are represented and handled differently
than non-built-in candidates..

FWIW, although the test case added by this patch is contrived, this
opportunity was found in the real world by instrumenting the 'bad_fns'
mechanism added by r12-3346 to look for situations where we still end up
using it (and thus end up redundantly considering some candidates twice),
and this built-in operator situation was the most common in the
codebases that I tested (although still quite rare in the codebases that
I tested).

> 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > gcc/cp/ChangeLog:
> > 
> > * call.c (add_operator_candidates): Consider built-in operator
> > candidates before considering non-member candidates.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/template/conv17.C: Extend test.
> > ---
> >   gcc/cp/call.c  | 13 +++--
> >   gcc/testsuite/g++.dg/template/conv17.C |  7 +++
> >   2 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> > index c5601d96ab8..c0da083758f 100644
> > --- a/gcc/cp/call.c
> > +++ b/gcc/cp/call.c
> > @@ -6321,7 +6321,6 @@ add_operator_candidates (z_candidate **candidates,
> >  vec *arglist,
> >  int flags, tsubst_flags_t complain)
> >   {
> > -  z_candidate *start_candidates = *candidates;
> > bool ismodop = code2 != ERROR_MARK;
> > tree fnname = ovl_op_identifier (ismodop, ismodop ? code2 : code);
> >   @@ -6333,6 +6332,12 @@ add_operator_candidates (z_candidate **candidates,
> > if (rewritten && code != EQ_EXPR && code != SPACESHIP_EXPR)
> >   flags &= ~LOOKUP_REWRITTEN;
> >   +  /* Add built-in candidates to the candidate set.  The standard says to
> > + rewrite built-in candidates, too, but there's no point.  */
> > +  if (!rewritten)
> > +add_builtin_candidates (candidates, code, code2, fnname, arglist,
> > +   flags, complain);
> > +
> > bool memonly = false;
> > switch (code)
> >   {
> > @@ -6352,6 +6357,7 @@ add_operator_candidates (z_candidate **candidates,
> >   /* Add namespace-scope operators to the list of functions to
> >consider.  */
> > +  z_candidate *start_candidates = *candidates;
> > if (!memonly)
> >   {
> > tree fns = lookup_name (fnname, LOOK_where::BLOCK_NAMESPACE);
> > @@ -6423,11 +6429,6 @@ add_operator_candidates (z_candidate **candidates,
> >   if (!rewritten)
> >   {
> > -  /* The standard says to rewrite built-in candidates, too,
> > -but there's no point.  */
> > -  add_builtin_candidates (candidates, code, code2, fnname, arglist,
> > - flags, complain);
> > -
> > /* Maybe add C++20 rewritten comparison candidates.  */
> > tree_code rewrite_code = ERROR_MARK;
> > if (cxx_dialect >= cxx20
> > diff --git a/gcc/testsuite/g++.dg/template/conv17.C
> > b/gcc/testsuite/g++.dg/template/conv17.C
> > index f0f10f2ef4f..87ecefb8de3 100644
> > --- a/gcc/testsuite/g++.dg/template/conv17.C
> > +++ b/gcc/testsuite/g++.dg/template/conv17.C
> > @@ -61,3 +61,10 @@ concept E = requires { T().h(nullptr); };
> > static_assert(!E);
> >   #endif
> > +
> > +// Verify that the strictly viable built-in operator+ candidate precludes
> > +// us from computing all argument conversions for the below non-strictly
> > +// viable non-member candidate.
> > +enum N { n };
> > +int operator+(N&, B);
> > +int f = n + 42;
> > 
> 
> 

Re: [PATCH] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Jakub Jelinek via Gcc-patches]

On Mon, Sep 20, 2021 at 02:37:03PM -0400, Marek Polacek wrote:
> > So, wouldn't be this better specified as
> > Wno-attributes=
> > Common Joined RejectNegative
> > (not sure if RejectNegative is actually needed for an option
> > starting with Wno- )?
> 
> Looks like RejectNegative is not needed.  I could do that, but I think
> it regresses the diagnostic:
> 
> error: unrecognized command-line option '-Wattributes=attr::'
> vs
> error: arguments ignored for ‘-Wattributes=’; use ‘-Wno-attributes=’ instead
> 
> I prefer the latter.  (I've changed the warning into error.)

Ok.

> > > +/* { dg-additional-options "-std=c++11" { target c++ } } */
> > > +/* { dg-additional-options "-Wno-attributes=company::,yoyodyne::attr" } 
> > > */
> > > +/* { dg-additional-options 
> > > "-Wno-attributes=c1::attr,c1::attr,c1::__attr__" } */
> > > +/* { dg-additional-options "-Wno-attributes=clang" } */
> > > +/* { dg-additional-options "-Wno-attributes=c2::,c2::attr" } */
> > > +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */
> > > +/* { dg-additional-options "-Wno-attributes=x::," } */
> > 
> > Should the above be accepted (I mean trailing , ?)  What does that mean?
>  
> I'm thinking it should: the arguments to -Wno-attributes= could be
> generated by a tool so accepting the trailing , might help, like in
> enums etc.

I don't think we allow that for any other options that accept a list of
something (except maybe -W{p,c,a,l}, those just pass the args through).
Generating a list that has comma separation only in between items and not
at the end is trivial.

Jakub

Re: [PATCH v2] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Marek Polacek via Gcc-patches]

On Mon, Sep 20, 2021 at 02:37:03PM -0400, Marek Polacek wrote:
> > > +/* { dg-additional-options "-Wno-attributes=yoyodyne::attr_new" } */
> > > +/* { dg-additional-options "-Wno-attributes=c4::__attr__" } */
> > 
> > When writing __attr__, does that imply we won't warn about both
> > c4::attr and c4::__attr__ (and __c4__::attr and __c4__::__attr__) like
> > it would when writing -Wno-attributes=c4::attr ?
> 
> "__attr__" and "attr" ought to be interchangeable.  __c4__:: and c4:: 
> currently
> are not, but I guess they should.  I'll post a v2 with that fixed.

Here's v2 with the documentation enhancement, warning -> error, and
which handles __vendor__:: the same as vendor::.

Regtest/bootstrap running.

-- >8 --
It is desirable for -Wattributes to warn about e.g.

[[deprecate]] void g(); // typo, should warn

However, -Wattributes also warns about vendor-specific attributes
(that's because lookup_scoped_attribute_spec -> find_attribute_namespace
finds nothing), which, with -Werror, causes grief.  We don't want the
-Wattributes warning for

[[company::attr]] void f();

GCC warns because it doesn't know the "company" namespace; it only knows
the "gnu" and "omp" namespaces.  We could entirely disable warning about
attributes in unknown scopes but then the compiler would also miss typos
like

  [[company::attrx]] void f();

or

  [[gmu::warn_used_result]] int write();

so that is not a viable solution.  A workaround is to use a #pragma:

  #pragma GCC diagnostic push
  #pragma GCC diagnostic ignored "-Wattributes"
  [[company::attr]] void f() {}
  #pragma GCC diagnostic pop

but that's a mouthful and awkward to use and could also hide typos.  In
fact, any macro-based solution doesn't seem like a way forward.

This patch implements -Wno-attributes=, which takes these arguments:

company::attr
company::
clang

The last one is a special option to ignore clang-only attributes; in
this patch it is accepted but currently has no effect.

This option should go well with using @file: the user could have a file
containing
-Wno-attributes=vendor::attr1,vendor::attr2
and then invoke gcc with '@attrs' or similar.

I've also added a new pragma which has the same effect:

The pragma along with the new option should help with various static
analysis tools.

PR c++/101940

gcc/ChangeLog:

* attribs.c (struct scoped_attributes): Add a bool member.
(lookup_scoped_attribute_spec): Forward declare.
(register_scoped_attributes): New bool parameter, defaulted to
false.  Use it.
(handle_ignored_attributes_option): New function.
(free_attr_data): New function.
(init_attributes): Call handle_ignored_attributes_option.
(attr_namespace_ignored_p): New function.
(decl_attributes): Check attr_namespace_ignored_p before
warning.
* attribs.h (free_attr_data): Declare.
(register_scoped_attributes): Adjust declaration.
(handle_ignored_attributes_option): Declare.
* common.opt (Wattributes=): New option with a variable.
* doc/extend.texi: Document #pragma GCC diagnostic ignored_attributes.
* doc/invoke.texi: Document -Wno-attributes=.
* opts.c (common_handle_option) : Handle.
* plugin.h (register_scoped_attributes): Adjust declaration.

gcc/c-family/ChangeLog:

* c-pragma.c (handle_pragma_diagnostic): Handle #pragma GCC diagnostic
ignored_attributes.

gcc/c/ChangeLog:

* c-decl.c (c_parse_final_cleanups): Call free_attr_data.

gcc/cp/ChangeLog:

* decl2.c (c_parse_final_cleanups): Call free_attr_data.

gcc/testsuite/ChangeLog:

* c-c++-common/Wno-attributes-1.c: New test.
* c-c++-common/Wno-attributes-2.c: New test.
---
 gcc/attribs.c | 126 +-
 gcc/attribs.h |   5 +-
 gcc/c-family/c-pragma.c   |  22 ++-
 gcc/c/c-decl.c|   2 +
 gcc/common.opt|   9 +-
 gcc/cp/decl2.c|   2 +
 gcc/doc/extend.texi   |  19 +++
 gcc/doc/invoke.texi   |  20 +++
 gcc/opts.c|  14 ++
 gcc/plugin.h  |   4 +-
 gcc/testsuite/c-c++-common/Wno-attributes-1.c |  56 
 gcc/testsuite/c-c++-common/Wno-attributes-2.c |  57 
 12 files changed, 327 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-2.c

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 83fafc98b7d..869006e3526 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -87,6 +87,8 @@ struct scoped_attributes
   const char *ns;
   vec attributes;
   hash_table *attribute_hash;
+  /* True if we should not warn about unknown attributes in this NS.  */
+  bool ignored_p;
 };
 
 /* The table of scope 

Re: [PATCH] c++: consider built-in operator candidates first

[Jason Merrill via Gcc-patches]

On 9/20/21 12:46, Patrick Palka wrote:

During operator overload resolution, we currently consider non-member
candidates before built-in candidates.  This didn't make a difference
before r12-3346, but after this change add_candidates will avoid
computing excess argument conversions if we've already seen a strictly
viable candidate, so it's better to consider built-in candidates first.


Doesn't r12-3346 stop considering conversions after it sees a bad one, 
and later return to the bad candidate if there is no strictly viable 
candidate?  How does this patch change that?


Depending on the order of the candidates seems fragile.


Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

* call.c (add_operator_candidates): Consider built-in operator
candidates before considering non-member candidates.

gcc/testsuite/ChangeLog:

* g++.dg/template/conv17.C: Extend test.
---
  gcc/cp/call.c  | 13 +++--
  gcc/testsuite/g++.dg/template/conv17.C |  7 +++
  2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c5601d96ab8..c0da083758f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6321,7 +6321,6 @@ add_operator_candidates (z_candidate **candidates,
 vec *arglist,
 int flags, tsubst_flags_t complain)
  {
-  z_candidate *start_candidates = *candidates;
bool ismodop = code2 != ERROR_MARK;
tree fnname = ovl_op_identifier (ismodop, ismodop ? code2 : code);
  
@@ -6333,6 +6332,12 @@ add_operator_candidates (z_candidate **candidates,

if (rewritten && code != EQ_EXPR && code != SPACESHIP_EXPR)
  flags &= ~LOOKUP_REWRITTEN;
  
+  /* Add built-in candidates to the candidate set.  The standard says to

+ rewrite built-in candidates, too, but there's no point.  */
+  if (!rewritten)
+add_builtin_candidates (candidates, code, code2, fnname, arglist,
+   flags, complain);
+
bool memonly = false;
switch (code)
  {
@@ -6352,6 +6357,7 @@ add_operator_candidates (z_candidate **candidates,
  
/* Add namespace-scope operators to the list of functions to

   consider.  */
+  z_candidate *start_candidates = *candidates;
if (!memonly)
  {
tree fns = lookup_name (fnname, LOOK_where::BLOCK_NAMESPACE);
@@ -6423,11 +6429,6 @@ add_operator_candidates (z_candidate **candidates,
  
if (!rewritten)

  {
-  /* The standard says to rewrite built-in candidates, too,
-but there's no point.  */
-  add_builtin_candidates (candidates, code, code2, fnname, arglist,
- flags, complain);
-
/* Maybe add C++20 rewritten comparison candidates.  */
tree_code rewrite_code = ERROR_MARK;
if (cxx_dialect >= cxx20
diff --git a/gcc/testsuite/g++.dg/template/conv17.C 
b/gcc/testsuite/g++.dg/template/conv17.C
index f0f10f2ef4f..87ecefb8de3 100644
--- a/gcc/testsuite/g++.dg/template/conv17.C
+++ b/gcc/testsuite/g++.dg/template/conv17.C
@@ -61,3 +61,10 @@ concept E = requires { T().h(nullptr); };
  
  static_assert(!E);

  #endif
+
+// Verify that the strictly viable built-in operator+ candidate precludes
+// us from computing all argument conversions for the below non-strictly
+// viable non-member candidate.
+enum N { n };
+int operator+(N&, B);
+int f = n + 42;

Re: [PATCH] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Marek Polacek via Gcc-patches]

On Mon, Sep 20, 2021 at 07:38:59PM +0200, Jakub Jelinek wrote:
> On Mon, Sep 20, 2021 at 01:06:58PM -0400, Marek Polacek via Gcc-patches wrote:
> 
> Not a review, just a few nits:
> 
> I think it would be useful to clarify that -Wno-attributes=list doesn't
> actually imply -Wno-attributes

Agreed, fixed with:

+Note that @option{-Wno-attributes=} does not imply @option{-Wno-attributes}.
 
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -88,6 +88,9 @@ void *flag_instrument_functions_exclude_functions
> >  Variable
> >  void *flag_instrument_functions_exclude_files
> >  
> > +Variable
> > +void *flag_ignored_attributes
> > +
> >  ; Generic structs (e.g. templates not explicitly specialized)
> >  ; may not have a compilation unit associated with them, and so
> >  ; may need to be treated differently from ordinary structs.
> > @@ -546,6 +549,10 @@ Wattributes
> >  Common Var(warn_attributes) Init(1) Warning
> >  Warn about inappropriate attribute usage.
> >  
> > +Wattributes=
> > +Common Joined
> > +Do not warn about specified attributes.
> > +
> 
> So, wouldn't be this better specified as
> Wno-attributes=
> Common Joined RejectNegative
> (not sure if RejectNegative is actually needed for an option
> starting with Wno- )?

Looks like RejectNegative is not needed.  I could do that, but I think
it regresses the diagnostic:

error: unrecognized command-line option '-Wattributes=attr::'
vs
error: arguments ignored for ‘-Wattributes=’; use ‘-Wno-attributes=’ instead

I prefer the latter.  (I've changed the warning into error.)
 
> > +/* { dg-additional-options "-std=c++11" { target c++ } } */
> > +/* { dg-additional-options "-Wno-attributes=company::,yoyodyne::attr" } */
> > +/* { dg-additional-options 
> > "-Wno-attributes=c1::attr,c1::attr,c1::__attr__" } */
> > +/* { dg-additional-options "-Wno-attributes=clang" } */
> > +/* { dg-additional-options "-Wno-attributes=c2::,c2::attr" } */
> > +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */
> > +/* { dg-additional-options "-Wno-attributes=x::," } */
> 
> Should the above be accepted (I mean trailing , ?)  What does that mean?
 
I'm thinking it should: the arguments to -Wno-attributes= could be
generated by a tool so accepting the trailing , might help, like in
enums etc.

> > +/* { dg-additional-options "-Wno-attributes=yoyodyne::attr_new" } */
> > +/* { dg-additional-options "-Wno-attributes=c4::__attr__" } */
> 
> When writing __attr__, does that imply we won't warn about both
> c4::attr and c4::__attr__ (and __c4__::attr and __c4__::__attr__) like
> it would when writing -Wno-attributes=c4::attr ?

"__attr__" and "attr" ought to be interchangeable.  __c4__:: and c4:: currently
are not, but I guess they should.  I'll post a v2 with that fixed.

Thanks!
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Re: [PATCH] Factor out `find_a_program` helper around `find_a_file`

[Gerald Pfeifer]

On Sun, 19 Sep 2021, Jeff Law via Gcc-patches wrote:
> On 8/4/2021 12:21 PM, John Ericson wrote:
>> The helper is for `--print-prog-name` and similar things. Since all
>> executable finding goes through it, we can move the default overrides
>> into that path too. This also ensures that if some is looking for a
>> *non*-program that called `as`, `ld`, etc., weird things don't happen.
>> ---
>>   gcc/gcc.c | 59 ---
>>   1 file changed, 34 insertions(+), 25 deletions(-)
> Thanks.  I added a ChangeLog entry and pushed this to the trunk.

|commit 5fee8a0a9223d030c66d53c104fb0a431369248f
|Author: John Ericson 
|Date:   Sun Sep 19 11:08:32 2021 -0400
|
|[PATCH] Factor out `find_a_program` helper around `find_a_file`
|
|gcc/
|* gcc.c (find_a_program): New function, factored out of...
|(find_a_file): Here.
|(execute): Use find_a_program when looking for programs rather
|than find_a_file.

I'm afraid this broke x86_64-unknown-freebsd12*:

.../GCC-HEAD/gcc/gcc.c: In function 'char* find_a_program(const char*)':
/scratch/tmp/gerald/GCC-HEAD/gcc/gcc.c:3086:59: error: 'mode' was not declared 
in this scope; did you mean 'pmode'?
 3086 |   if (! strcmp (name, "as") && access (DEFAULT_ASSEMBLER, mode) == 0)
  |   ^~~~
  |   pmode
/scratch/tmp/gerald/GCC-HEAD/gcc/gcc.c:3091:56: error: 'mode' was not declared 
in this scope; did you mean 'pmode'?
 3091 |   if (! strcmp (name, "ld") && access (DEFAULT_LINKER, mode) == 0)
  |^~~~
  |pmode

Looking at the code in question I see

  static char*
  find_a_program (const char *name)
  {
/* Do not search if default matches query. */

  #ifdef DEFAULT_ASSEMBLER
if (! strcmp (name, "as") && access (DEFAULT_ASSEMBLER, mode) == 0)
  return xstrdup (DEFAULT_ASSEMBLER);
  #endif

where mode is not defined in that function.

Looks like the factoring out did not take the non-default case into
consideration?


I was just going to commit a fix I tested last night, when that patch 
simply "disappeared". Looks like Ian beat me to it with exactly the same 
patch. :-)

Gerald

Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC

[Fāng-ruì Sòng via Gcc-patches]

PING^5 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html

On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng  wrote:
>
> PING^4 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>
> One major design goal of PIE was to avoid copy relocations.
> The original patch for GCC 5 caused problems for many years.
>
> On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng  wrote:
>>
>> PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>>
>> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng  wrote:
>> >
>> > PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>> >
>> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng  wrote:
>> > >
>> > > Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>> > >
>> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song  wrote:
>> > > >
>> > > > This was introduced in 2014-12 to use local binding for external 
>> > > > symbols
>> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which mostly
>> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
>> > > > should retire now.
>> > > >
>> > > > One design goal of -fPIE was to avoid copy relocations.
>> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal.  With this change, 
>> > > > the
>> > > > -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
>> > > >
>> > > > ---
>> > > >
>> > > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a list
>> > > > of fixed and unfixed (e.g. gold incompatibility with protected
>> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
>> > > >
>> > > > If you prefer a longer write-up, see
>> > > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
>> > > > ---
>> > > >  gcc/config.in |  6 ---
>> > > >  gcc/config/i386/i386.c| 11 +---
>> > > >  gcc/configure | 52 ---
>> > > >  gcc/configure.ac  | 48 -
>> > > >  gcc/doc/sourcebuild.texi  |  3 --
>> > > >  .../gcc.target/i386/pie-copyrelocs-1.c| 14 -
>> > > >  .../gcc.target/i386/pie-copyrelocs-2.c| 14 -
>> > > >  .../gcc.target/i386/pie-copyrelocs-3.c| 14 -
>> > > >  .../gcc.target/i386/pie-copyrelocs-4.c| 17 --
>> > > >  gcc/testsuite/lib/target-supports.exp | 47 -
>> > > >  10 files changed, 2 insertions(+), 224 deletions(-)
>> > > >  delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
>> > > >  delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
>> > > >  delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
>> > > >  delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
>> > > >
>> > > > diff --git a/gcc/config.in b/gcc/config.in
>> > > > index e54f59ce0c3..a65bf5d4176 100644
>> > > > --- a/gcc/config.in
>> > > > +++ b/gcc/config.in
>> > > > @@ -1659,12 +1659,6 @@
>> > > >  #endif
>> > > >
>> > > >
>> > > > -/* Define 0/1 if your linker supports -pie option with copy reloc. */
>> > > > -#ifndef USED_FOR_TARGET
>> > > > -#undef HAVE_LD_PIE_COPYRELOC
>> > > > -#endif
>> > > > -
>> > > > -
>> > > >  /* Define if your PowerPC linker has .gnu.attributes long double 
>> > > > support. */
>> > > >  #ifndef USED_FOR_TARGET
>> > > >  #undef HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE
>> > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> > > > index 915f89f571a..5ec3c6fd0c9 100644
>> > > > --- a/gcc/config/i386/i386.c
>> > > > +++ b/gcc/config/i386/i386.c
>> > > > @@ -10579,11 +10579,7 @@ legitimate_pic_address_disp_p (rtx disp)
>> > > > return true;
>> > > > }
>> > > >   else if (!SYMBOL_REF_FAR_ADDR_P (op0)
>> > > > -  && (SYMBOL_REF_LOCAL_P (op0)
>> > > > -  || (HAVE_LD_PIE_COPYRELOC
>> > > > -  && flag_pie
>> > > > -  && !SYMBOL_REF_WEAK (op0)
>> > > > -  && !SYMBOL_REF_FUNCTION_P (op0)))
>> > > > +  && SYMBOL_REF_LOCAL_P (op0)
>> > > >&& ix86_cmodel != CM_LARGE_PIC)
>> > > > return true;
>> > > >   break;
>> > > > @@ -22892,10 +22888,7 @@ ix86_atomic_assign_expand_fenv (tree *hold, 
>> > > > tree *clear, tree *update)
>> > > >  static bool
>> > > >  ix86_binds_local_p (const_tree exp)
>> > > >  {
>> > > > -  return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
>> > > > - (!flag_pic
>> > > > -  || (TARGET_64BIT
>> > > > -  && HAVE_LD_PIE_COPYRELOC != 
>> > > > 0)));
>> > > > +  return default_binds_local_p_3 (exp, flag_shlib != 0, true, true, 
>> > > > !flag_pic);
>> > > >  }
>> > > >  #endif
>> > > >
>> > > > diff --git a/gcc/configure b/gcc/configure
>> > > > index 

Re: [PATCH] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Jakub Jelinek via Gcc-patches]

On Mon, Sep 20, 2021 at 01:06:58PM -0400, Marek Polacek via Gcc-patches wrote:

Not a review, just a few nits:

I think it would be useful to clarify that -Wno-attributes=list doesn't
actually imply -Wno-attributes

> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -88,6 +88,9 @@ void *flag_instrument_functions_exclude_functions
>  Variable
>  void *flag_instrument_functions_exclude_files
>  
> +Variable
> +void *flag_ignored_attributes
> +
>  ; Generic structs (e.g. templates not explicitly specialized)
>  ; may not have a compilation unit associated with them, and so
>  ; may need to be treated differently from ordinary structs.
> @@ -546,6 +549,10 @@ Wattributes
>  Common Var(warn_attributes) Init(1) Warning
>  Warn about inappropriate attribute usage.
>  
> +Wattributes=
> +Common Joined
> +Do not warn about specified attributes.
> +

So, wouldn't be this better specified as
Wno-attributes=
Common Joined RejectNegative
(not sure if RejectNegative is actually needed for an option
starting with Wno- )?

> +/* { dg-additional-options "-std=c++11" { target c++ } } */
> +/* { dg-additional-options "-Wno-attributes=company::,yoyodyne::attr" } */
> +/* { dg-additional-options "-Wno-attributes=c1::attr,c1::attr,c1::__attr__" 
> } */
> +/* { dg-additional-options "-Wno-attributes=clang" } */
> +/* { dg-additional-options "-Wno-attributes=c2::,c2::attr" } */
> +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */
> +/* { dg-additional-options "-Wno-attributes=x::," } */

Should the above be accepted (I mean trailing , ?)  What does that mean?

> +/* { dg-additional-options "-Wno-attributes=yoyodyne::attr_new" } */
> +/* { dg-additional-options "-Wno-attributes=c4::__attr__" } */

When writing __attr__, does that imply we won't warn about both
c4::attr and c4::__attr__ (and __c4__::attr and __c4__::__attr__) like
it would when writing -Wno-attributes=c4::attr ?

Jakub

Re: [PATCH] Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*

[Jeff Law via Gcc-patches]

On 9/20/2021 10:46 AM, Richard Biener wrote:

On Mon, 20 Sep 2021, Richard Biener wrote:


On Mon, 20 Sep 2021, Jeff Law wrote:



On 9/20/2021 1:00 AM, Richard Biener wrote:

This obsoletes the 32bit hppa-hpux configurations which only support
STABS as debuginfo format.

As discussed, I'm going to push this (and a changes.html entry) when
it was included in a bootstrap/regtest cycle.

2021-09-20  Richard Biener  

gcc/
  * config.gcc: Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*.

contrib/
  * config-list.mk: --enable-obsolete for hppa2.0-hpux10.1 and
  hppa2.0-hpux11.9.

Is this marking hppa2.0w-hp-hpux11 as obsolete?  That platform is using ELF &
DWARF.

If so then that's by mistake - I had the impression that all 32bit
hpux pa targets are STABS only, but that's from trying to decipher
config.gcc and the comments from you and David ... maybe the
matching pattern needs to be "split"?

Btw, I don't see any elfos.h in the hppa[12]*-*-hpux11* case in config.gcc
and it contains a warning that the target doesn't support DWARF.  But yes,
that pattern matches hppa2.0-hpux11.9 and the hppa*64*-*-hpux11* pattern
doesn't - there's hppa64-hpux11.0 in config-list.mk that does though.
Nuts, I think I steered you wrong.  I forgot that while HP used 
"hppa2.0w" to denote their 64 bit ELF platform, we used "hppa64" for the 
target name.


jeff

[PATCH] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Marek Polacek via Gcc-patches]

It is desirable for -Wattributes to warn about e.g.

[[deprecate]] void g(); // typo, should warn

However, -Wattributes also warns about vendor-specific attributes
(that's because lookup_scoped_attribute_spec -> find_attribute_namespace
finds nothing), which, with -Werror, causes grief.  We don't want the
-Wattributes warning for

[[company::attr]] void f();

GCC warns because it doesn't know the "company" namespace; it only knows
the "gnu" and "omp" namespaces.  We could entirely disable warning about
attributes in unknown scopes but then the compiler would also miss typos
like

  [[company::attrx]] void f();

or

  [[gmu::warn_used_result]] int write();

so that is not a viable solution.  A workaround is to use a #pragma:

  #pragma GCC diagnostic push
  #pragma GCC diagnostic ignored "-Wattributes"
  [[company::attr]] void f() {}
  #pragma GCC diagnostic pop

but that's a mouthful and awkward to use and could also hide typos.  In
fact, any macro-based solution doesn't seem like a way forward.

This patch implements -Wno-attributes=, which takes these arguments:

company::attr
company::
clang

The last one is a special option to ignore clang-only attributes; in
this patch it is accepted but currently has no effect.

This option should go well with using @file: the user could have a file
containing
-Wno-attributes=vendor::attr1,vendor::attr2
and then invoke gcc with '@attrs' or similar.

I've also added a new pragma which has the same effect:

The pragma along with the new option should help with various static
analysis tools.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

PR c++/101940

gcc/ChangeLog:

* attribs.c (struct scoped_attributes): Add a bool member.
(lookup_scoped_attribute_spec): Forward declare.
(register_scoped_attributes): New bool parameter, defaulted to
false.  Use it.
(handle_ignored_attributes_option): New function.
(free_attr_data): New function.
(init_attributes): Call handle_ignored_attributes_option.
(attr_namespace_ignored_p): New function.
(decl_attributes): Check attr_namespace_ignored_p before
warning.
* attribs.h (free_attr_data): Declare.
(register_scoped_attributes): Adjust declaration.
(handle_ignored_attributes_option): Declare.
* common.opt (Wattributes=): New option with a variable.
* doc/extend.texi: Document #pragma GCC diagnostic ignored_attributes.
* doc/invoke.texi: Document -Wno-attributes=.
* opts.c (common_handle_option) : Handle.
* plugin.h (register_scoped_attributes): Adjust declaration.

gcc/c-family/ChangeLog:

* c-pragma.c (handle_pragma_diagnostic): Handle #pragma GCC diagnostic
ignored_attributes.

gcc/c/ChangeLog:

* c-decl.c (c_parse_final_cleanups): Call free_attr_data.

gcc/cp/ChangeLog:

* decl2.c (c_parse_final_cleanups): Call free_attr_data.

gcc/testsuite/ChangeLog:

* c-c++-common/Wno-attributes-1.c: New test.
* c-c++-common/Wno-attributes-2.c: New test.
---
 gcc/attribs.c | 122 +-
 gcc/attribs.h |   5 +-
 gcc/c-family/c-pragma.c   |  22 +++-
 gcc/c/c-decl.c|   2 +
 gcc/common.opt|   9 +-
 gcc/cp/decl2.c|   2 +
 gcc/doc/extend.texi   |  19 +++
 gcc/doc/invoke.texi   |  18 +++
 gcc/opts.c|  14 ++
 gcc/plugin.h  |   4 +-
 gcc/testsuite/c-c++-common/Wno-attributes-1.c |  40 ++
 gcc/testsuite/c-c++-common/Wno-attributes-2.c |  41 ++
 12 files changed, 289 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-2.c

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 83fafc98b7d..c85f3aedc4d 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -87,6 +87,8 @@ struct scoped_attributes
   const char *ns;
   vec attributes;
   hash_table *attribute_hash;
+  /* True if we should not warn about unknown attributes in this NS.  */
+  bool ignored_p;
 };
 
 /* The table of scope attributes.  */
@@ -95,6 +97,8 @@ static vec attributes_table;
 static scoped_attributes* find_attribute_namespace (const char*);
 static void register_scoped_attribute (const struct attribute_spec *,
   scoped_attributes *);
+static const struct attribute_spec *lookup_scoped_attribute_spec (const_tree,
+ const_tree);
 
 static bool attributes_initialized = false;
 
@@ -121,12 +125,14 @@ extract_attribute_substring (struct substring *str)
 
 /* Insert an array of attributes ATTRIBUTES into a namespace.  This
array must be NULL terminated.  NS is the name of 

PING^3 [PATCH] x86: Update memcpy/memset inline strategies for -mtune=generic

[H.J. Lu via Gcc-patches]

On Mon, Sep 13, 2021 at 6:38 AM H.J. Lu  wrote:
>
> On Tue, Sep 7, 2021 at 8:01 PM H.J. Lu  wrote:
> >
> > On Sun, Aug 22, 2021 at 8:28 AM H.J. Lu  wrote:
> > >
> > > On Tue, Mar 23, 2021 at 09:19:38AM +0100, Richard Biener wrote:
> > > > On Tue, Mar 23, 2021 at 3:41 AM Hongyu Wang  
> > > > wrote:
> > > > >
> > > > > > Hongyue, please collect code size differences on SPEC CPU 2017 and
> > > > > > eembc.
> > > > >
> > > > > Here is code size difference for this patch
> > > >
> > > > Thanks, nothing too bad although slightly larger impacts than 
> > > > envisioned.
> > > >
> > >
> > > PING.
> > >
> > > OK for master branch?
> > >
> > > Thanks.
> > >
> > > H.J.
> > >  ---
> > > Simplify memcpy and memset inline strategies to avoid branches for
> > > -mtune=generic:
> > >
> > > 1. With MOVE_RATIO and CLEAR_RATIO == 17, GCC will use integer/vector
> > >load and store for up to 16 * 16 (256) bytes when the data size is
> > >fixed and known.
> > > 2. Inline only if data size is known to be <= 256.
> > >a. Use "rep movsb/stosb" with simple code sequence if the data size
> > >   is a constant.
> > >b. Use loop if data size is not a constant.
> > > 3. Use memcpy/memset libray function if data size is unknown or > 256.
> > >
> >
> > PING:
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577889.html
> >
>
> PING.  This should fix:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102294
>

PING.

-- 
H.J.

Re: [PATCH] Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*

[Richard Biener via Gcc-patches]

On Mon, 20 Sep 2021, Richard Biener wrote:

> On Mon, 20 Sep 2021, Jeff Law wrote:
> 
> > 
> > 
> > On 9/20/2021 1:00 AM, Richard Biener wrote:
> > > This obsoletes the 32bit hppa-hpux configurations which only support
> > > STABS as debuginfo format.
> > >
> > > As discussed, I'm going to push this (and a changes.html entry) when
> > > it was included in a bootstrap/regtest cycle.
> > >
> > > 2021-09-20  Richard Biener  
> > >
> > > gcc/
> > >  * config.gcc: Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*.
> > >
> > > contrib/
> > >  * config-list.mk: --enable-obsolete for hppa2.0-hpux10.1 and
> > >  hppa2.0-hpux11.9.
> > Is this marking hppa2.0w-hp-hpux11 as obsolete?  That platform is using ELF 
> > &
> > DWARF.
> 
> If so then that's by mistake - I had the impression that all 32bit
> hpux pa targets are STABS only, but that's from trying to decipher
> config.gcc and the comments from you and David ... maybe the
> matching pattern needs to be "split"?

Btw, I don't see any elfos.h in the hppa[12]*-*-hpux11* case in config.gcc
and it contains a warning that the target doesn't support DWARF.  But yes,
that pattern matches hppa2.0-hpux11.9 and the hppa*64*-*-hpux11* pattern
doesn't - there's hppa64-hpux11.0 in config-list.mk that does though.

Richard.

> Can you maybe take care of any adjustments necessary?  I've also
> edited changes.html already.
> 
> Thanks,
> Richard.
> 
> > jeff
> > 
> > 
> > 
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

[PATCH] c++: consider built-in operator candidates first

[Patrick Palka via Gcc-patches]

During operator overload resolution, we currently consider non-member
candidates before built-in candidates.  This didn't make a difference
before r12-3346, but after this change add_candidates will avoid
computing excess argument conversions if we've already seen a strictly
viable candidate, so it's better to consider built-in candidates first.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

* call.c (add_operator_candidates): Consider built-in operator
candidates before considering non-member candidates.

gcc/testsuite/ChangeLog:

* g++.dg/template/conv17.C: Extend test.
---
 gcc/cp/call.c  | 13 +++--
 gcc/testsuite/g++.dg/template/conv17.C |  7 +++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c5601d96ab8..c0da083758f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6321,7 +6321,6 @@ add_operator_candidates (z_candidate **candidates,
 vec *arglist,
 int flags, tsubst_flags_t complain)
 {
-  z_candidate *start_candidates = *candidates;
   bool ismodop = code2 != ERROR_MARK;
   tree fnname = ovl_op_identifier (ismodop, ismodop ? code2 : code);
 
@@ -6333,6 +6332,12 @@ add_operator_candidates (z_candidate **candidates,
   if (rewritten && code != EQ_EXPR && code != SPACESHIP_EXPR)
 flags &= ~LOOKUP_REWRITTEN;
 
+  /* Add built-in candidates to the candidate set.  The standard says to
+ rewrite built-in candidates, too, but there's no point.  */
+  if (!rewritten)
+add_builtin_candidates (candidates, code, code2, fnname, arglist,
+   flags, complain);
+
   bool memonly = false;
   switch (code)
 {
@@ -6352,6 +6357,7 @@ add_operator_candidates (z_candidate **candidates,
 
   /* Add namespace-scope operators to the list of functions to
  consider.  */
+  z_candidate *start_candidates = *candidates;
   if (!memonly)
 {
   tree fns = lookup_name (fnname, LOOK_where::BLOCK_NAMESPACE);
@@ -6423,11 +6429,6 @@ add_operator_candidates (z_candidate **candidates,
 
   if (!rewritten)
 {
-  /* The standard says to rewrite built-in candidates, too,
-but there's no point.  */
-  add_builtin_candidates (candidates, code, code2, fnname, arglist,
- flags, complain);
-
   /* Maybe add C++20 rewritten comparison candidates.  */
   tree_code rewrite_code = ERROR_MARK;
   if (cxx_dialect >= cxx20
diff --git a/gcc/testsuite/g++.dg/template/conv17.C 
b/gcc/testsuite/g++.dg/template/conv17.C
index f0f10f2ef4f..87ecefb8de3 100644
--- a/gcc/testsuite/g++.dg/template/conv17.C
+++ b/gcc/testsuite/g++.dg/template/conv17.C
@@ -61,3 +61,10 @@ concept E = requires { T().h(nullptr); };
 
 static_assert(!E);
 #endif
+
+// Verify that the strictly viable built-in operator+ candidate precludes
+// us from computing all argument conversions for the below non-strictly
+// viable non-member candidate.
+enum N { n };
+int operator+(N&, B);
+int f = n + 42;
-- 
2.33.0.363.g4c719308ce

Re: [PATCH] Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*

[Richard Biener via Gcc-patches]

On Mon, 20 Sep 2021, Jeff Law wrote:

> 
> 
> On 9/20/2021 1:00 AM, Richard Biener wrote:
> > This obsoletes the 32bit hppa-hpux configurations which only support
> > STABS as debuginfo format.
> >
> > As discussed, I'm going to push this (and a changes.html entry) when
> > it was included in a bootstrap/regtest cycle.
> >
> > 2021-09-20  Richard Biener  
> >
> > gcc/
> >  * config.gcc: Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*.
> >
> > contrib/
> >  * config-list.mk: --enable-obsolete for hppa2.0-hpux10.1 and
> >  hppa2.0-hpux11.9.
> Is this marking hppa2.0w-hp-hpux11 as obsolete?  That platform is using ELF &
> DWARF.

If so then that's by mistake - I had the impression that all 32bit
hpux pa targets are STABS only, but that's from trying to decipher
config.gcc and the comments from you and David ... maybe the
matching pattern needs to be "split"?

Can you maybe take care of any adjustments necessary?  I've also
edited changes.html already.

Thanks,
Richard.

> jeff
> 
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Qing Zhao via Gcc-patches]

> On Sep 20, 2021, at 10:59 AM, Richard Earnshaw 
>  wrote:
> 
> 
> 
> On 20/09/2021 16:51, Qing Zhao via Gcc-patches wrote:
>>> On Sep 20, 2021, at 9:36 AM, Richard Earnshaw 
>>>  wrote:
>>> 
>>> 
>>> 
>>> On 20/09/2021 14:55, Qing Zhao wrote:
> On Sep 20, 2021, at 8:18 AM, Richard Earnshaw 
>  wrote:
> 
> 
> 
> On 20/09/2021 13:47, Qing Zhao wrote:
>>> On Sep 20, 2021, at 5:43 AM, Richard Earnshaw 
>>>  wrote:
>>> 
>>> 
>>> 
>>> On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:
 Hi,
 There are much less issues with aarch64/auto-init-* test cases.
 Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till 
 ‘armv8.6-a’, ‘armv8-r’) do not change the pattern match.
 Only
 1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” 
 and “auto-init-padding-5.c”.
 2. -fstack-clash-protection impact two of the testing cases 
 “auto-init-1.c” and “auto-init-7.c”.
 Naturally the patch for this set is:
 A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
 “auto-init-padding-5.c”;
 B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
 The above A fixed issue 1, however, the above B did not fix issue 2.
 If I fixed “auto-init-1.c” as:
 diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
 b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
 index 0fa4708..a38d91b 100644
 --- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
 +++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
 @@ -1,6 +1,6 @@
  /* Verify zero initialization for integer and pointer type automatic 
 variables.  */
  /* { dg-do compile } */
 -/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
 +/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
 -fno-stack-protector" } */
#ifndef __cplusplus
  # define bool _Bool
 So, I took a look at the log file of the testing, and found that, If I 
 tested it as:
  make check-gcc 
 RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
  aarch64.exp=auto-init*’
 In the log file, I got:
 /home/qinzhao/Work/GCC/build_git/gcc/xgcc 
 -B/home/qinzhao/Work/GCC/build_git/gcc/ 
 /home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
   -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero 
 -fdump-rtl-expand -fno-stack-protector -ffat-lto-objects -S  
 -mabi=lp64 -fstack-clash-protection -fstack-protector-all  -o 
 auto-init-1.s
 From it, we can see, that the options that were passed through 
 RUNTESTFLAGS “mabi-lp64 -fstack-clash-protection 
 -fstack-protector-all” were appended AFTER the options inside the 
 testing case through “dg-options”. As a result, the option 
 “-fno-stack-protector” did not have any impact at all.
 What’s the expected behavior for the order of these options? Should 
 options through RUNTESTFLAGS be appended BEFORE or AFTER the options 
 through testing cases?
 For X86, the options through RUNTESTFLAGS are added BEFORE the options 
 through testing cases. Therefore adding “-fno-stack-protector” has the 
 expected result.
>>> 
>>> Can you check that you are using the same version of dejagnu on both 
>>> platforms?
>> Stupid question:  how to check the version of it?
> 
> $ runtest --version
 Thanks.
 On aarch64:
 qinzhao@gcc116:~/Work/GCC/build_git$ runtest --version
 Expect version is  5.45
 Tcl version is 8.6
 Framework version is   1.5
>>> 
>>> Ok, so this is dejagnu 1.5 (older versions of dejagnu reported the 
>>> 'framework version') ...
>>> 
 On X86:
 [opc@qinzhao-ol8u3-x86 gcc]$ runtest --version
 WARNING: Couldn't find the global config file.
 DejaGnu version1.6.1
 Expect version 5.45.4
 Tcl version8.6
>>> 
>>> While this is dejagnu 1.6.1
>>> 
>>> IIRC there were some changes to the way various options were combined at 
>>> one point and I suspect this may be the source of your problem.  Can you 
>>> update your dejagnu on your aarch64 system to be the same as that on x86?
>> The aarch64 machine I used is a GCC farm machine, looks like it has an older 
>> version of dejagnu. I don’t have permission to update it myself.
>> I will use another aarch64 machine to test this to see whether a higher 
>> version of dejagnu can resolve the issue or not.
> 
> You could probably try installing a local copy of dejagnu in, for example, 
> ~/tools and then adding that to your path before the system version.
Will try this too.
thanks.

Qing
> 
> R.
> 
>> Thanks a lot for your help.
>> 

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Richard Earnshaw via Gcc-patches]

On 20/09/2021 16:51, Qing Zhao via Gcc-patches wrote:




On Sep 20, 2021, at 9:36 AM, Richard Earnshaw  
wrote:



On 20/09/2021 14:55, Qing Zhao wrote:

On Sep 20, 2021, at 8:18 AM, Richard Earnshaw  
wrote:



On 20/09/2021 13:47, Qing Zhao wrote:

On Sep 20, 2021, at 5:43 AM, Richard Earnshaw  
wrote:



On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:

Hi,
There are much less issues with aarch64/auto-init-* test cases.
Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
‘armv8-r’) do not change the pattern match.
Only
1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
“auto-init-padding-5.c”.
2. -fstack-clash-protection impact two of the testing cases “auto-init-1.c” and 
“auto-init-7.c”.
Naturally the patch for this set is:
A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
“auto-init-padding-5.c”;
B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
The above A fixed issue 1, however, the above B did not fix issue 2.
If I fixed “auto-init-1.c” as:
diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
index 0fa4708..a38d91b 100644
--- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
@@ -1,6 +1,6 @@
  /* Verify zero initialization for integer and pointer type automatic 
variables.  */
  /* { dg-do compile } */
-/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
+/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector" } */
#ifndef __cplusplus
  # define bool _Bool
So, I took a look at the log file of the testing, and found that, If I tested 
it as:
  make check-gcc 
RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
 aarch64.exp=auto-init*’
In the log file, I got:
/home/qinzhao/Work/GCC/build_git/gcc/xgcc 
-B/home/qinzhao/Work/GCC/build_git/gcc/ 
/home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
  -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 -fstack-clash-protection 
-fstack-protector-all  -o auto-init-1.s
 From it, we can see, that the options that were passed through RUNTESTFLAGS 
“mabi-lp64 -fstack-clash-protection -fstack-protector-all” were appended AFTER 
the options inside the testing case through “dg-options”. As a result, the 
option “-fno-stack-protector” did not have any impact at all.
What’s the expected behavior for the order of these options? Should options 
through RUNTESTFLAGS be appended BEFORE or AFTER the options through testing 
cases?
For X86, the options through RUNTESTFLAGS are added BEFORE the options through 
testing cases. Therefore adding “-fno-stack-protector” has the expected result.


Can you check that you are using the same version of dejagnu on both platforms?

Stupid question:  how to check the version of it?


$ runtest --version

Thanks.
On aarch64:
qinzhao@gcc116:~/Work/GCC/build_git$ runtest --version
Expect version is   5.45
Tcl version is  8.6
Framework version is1.5


Ok, so this is dejagnu 1.5 (older versions of dejagnu reported the 'framework 
version') ...


On X86:
[opc@qinzhao-ol8u3-x86 gcc]$ runtest --version
WARNING: Couldn't find the global config file.
DejaGnu version 1.6.1
Expect version  5.45.4
Tcl version 8.6


While this is dejagnu 1.6.1

IIRC there were some changes to the way various options were combined at one 
point and I suspect this may be the source of your problem.  Can you update 
your dejagnu on your aarch64 system to be the same as that on x86?


The aarch64 machine I used is a GCC farm machine, looks like it has an older 
version of dejagnu. I don’t have permission to update it myself.

I will use another aarch64 machine to test this to see whether a higher version 
of dejagnu can resolve the issue or not.


You could probably try installing a local copy of dejagnu in, for 
example, ~/tools and then adding that to your path before the system 
version.


R.



Thanks a lot for your help.

Qing


R.


So, is there anything wrong with the installation of runtest on X86?
Qing


R.


Qing


R.


Is this a bug in aarch64 testing suite?
Thanks.
Qing

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Qing Zhao via Gcc-patches]

> On Sep 20, 2021, at 9:36 AM, Richard Earnshaw  
> wrote:
> 
> 
> 
> On 20/09/2021 14:55, Qing Zhao wrote:
>>> On Sep 20, 2021, at 8:18 AM, Richard Earnshaw 
>>>  wrote:
>>> 
>>> 
>>> 
>>> On 20/09/2021 13:47, Qing Zhao wrote:
> On Sep 20, 2021, at 5:43 AM, Richard Earnshaw 
>  wrote:
> 
> 
> 
> On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:
>> Hi,
>> There are much less issues with aarch64/auto-init-* test cases.
>> Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
>> ‘armv8-r’) do not change the pattern match.
>> Only
>> 1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
>> “auto-init-padding-5.c”.
>> 2. -fstack-clash-protection impact two of the testing cases 
>> “auto-init-1.c” and “auto-init-7.c”.
>> Naturally the patch for this set is:
>> A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
>> “auto-init-padding-5.c”;
>> B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
>> The above A fixed issue 1, however, the above B did not fix issue 2.
>> If I fixed “auto-init-1.c” as:
>> diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
>> b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>> index 0fa4708..a38d91b 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>> @@ -1,6 +1,6 @@
>>  /* Verify zero initialization for integer and pointer type automatic 
>> variables.  */
>>  /* { dg-do compile } */
>> -/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
>> +/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
>> -fno-stack-protector" } */
>>#ifndef __cplusplus
>>  # define bool _Bool
>> So, I took a look at the log file of the testing, and found that, If I 
>> tested it as:
>>  make check-gcc 
>> RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
>>  aarch64.exp=auto-init*’
>> In the log file, I got:
>> /home/qinzhao/Work/GCC/build_git/gcc/xgcc 
>> -B/home/qinzhao/Work/GCC/build_git/gcc/ 
>> /home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>>   -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero 
>> -fdump-rtl-expand -fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 
>> -fstack-clash-protection -fstack-protector-all  -o auto-init-1.s
>> From it, we can see, that the options that were passed through 
>> RUNTESTFLAGS “mabi-lp64 -fstack-clash-protection -fstack-protector-all” 
>> were appended AFTER the options inside the testing case through 
>> “dg-options”. As a result, the option “-fno-stack-protector” did not 
>> have any impact at all.
>> What’s the expected behavior for the order of these options? Should 
>> options through RUNTESTFLAGS be appended BEFORE or AFTER the options 
>> through testing cases?
>> For X86, the options through RUNTESTFLAGS are added BEFORE the options 
>> through testing cases. Therefore adding “-fno-stack-protector” has the 
>> expected result.
> 
> Can you check that you are using the same version of dejagnu on both 
> platforms?
 Stupid question:  how to check the version of it?
>>> 
>>> $ runtest --version
>> Thanks.
>> On aarch64:
>> qinzhao@gcc116:~/Work/GCC/build_git$ runtest --version
>> Expect version is5.45
>> Tcl version is   8.6
>> Framework version is 1.5
> 
> Ok, so this is dejagnu 1.5 (older versions of dejagnu reported the 'framework 
> version') ...
> 
>> On X86:
>> [opc@qinzhao-ol8u3-x86 gcc]$ runtest --version
>> WARNING: Couldn't find the global config file.
>> DejaGnu version  1.6.1
>> Expect version   5.45.4
>> Tcl version  8.6
> 
> While this is dejagnu 1.6.1
> 
> IIRC there were some changes to the way various options were combined at one 
> point and I suspect this may be the source of your problem.  Can you update 
> your dejagnu on your aarch64 system to be the same as that on x86?

The aarch64 machine I used is a GCC farm machine, looks like it has an older 
version of dejagnu. I don’t have permission to update it myself.

I will use another aarch64 machine to test this to see whether a higher version 
of dejagnu can resolve the issue or not.

Thanks a lot for your help.

Qing
> 
> R.
> 
>> So, is there anything wrong with the installation of runtest on X86?
>> Qing
>>> 
>>> R.
>>> 
 Qing
> 
> R.
> 
>> Is this a bug in aarch64 testing suite?
>> Thanks.
>> Qing

Re: [PATCH] c: [PR32122] Require pointer types for computed gotos

[Jeff Law via Gcc-patches]

On 9/19/2021 10:14 PM, apinski--- via Gcc-patches wrote:

From: Andrew Pinski 

So GCC has always accepted non-pointer types in computed gotos but
that was wrong based on the documentation:
Any expression of type void * is allowed.

So this fixes the problem by requiring the type to
be a pointer type.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

PR c/32122

gcc/c/ChangeLog:

* c-parser.c (c_parser_statement_after_labels): Pass
the c_expr instead of the tree to c_finish_goto_ptr.
* c-typeck.c (c_finish_goto_ptr): Change the second
argument type to c_expr.
* c-tree.h (c_finish_goto_ptr): Likewise.
Error out if the expression was not of a pointer type.

gcc/testsuite/ChangeLog:

* gcc.dg/comp-goto-5.c: New test.
* gcc.dg/comp-goto-6.c: New test.

OK
jeff

[PATCH] x86: Clean up gcc.target/i386/auto-init-* tests

[H.J. Lu via Gcc-patches]

On Fri, Sep 17, 2021 at 10:32 AM Qing Zhao via Gcc-patches
 wrote:
>
>
>
> > On Sep 17, 2021, at 11:59 AM, Jakub Jelinek  wrote:
> >
> > On Fri, Sep 17, 2021 at 04:55:22PM +, Qing Zhao wrote:
> >> This is the patch to fix gcc.target/i386/auto-init-* tests.
> >>
> >> I have tested the change at X86_64-linux with
> >>
> >> make check-gcc 
> >> RUNTESTFLAGS='--target_board=unix\{-m64,-m64/-march=skylake-avx512,-m64/-fstack-protector-all,-m64/-fstack-clash-protection,-m32/-mno-sse,-m32/-mtune=bonnell,-m32/-march=bonnell,-m32/-fstack-protector-all/-fstack-clash-protection\}
> >>  i386.exp=auto-init*’
> >>
> >> make check-gcc 
> >> RUNTESTFLAGS='--target_board=unix\{-m64,-m64/-march=skylake-avx512/-fPIC,-m64/-fstack-protector-all/-fPIC,-m64/-fstack-clash-protection/-fPIC,-m32/-mno-sse/-fPIC,-m32/-mtune=bonnell/-fPIC,-m32/-march=bonnell/-fPIC,-m32/-fstack-protector-all/-fstack-clash-protection/-fPIC\}
> >>  i386.exp=auto-init*’
> >>
> >> Everything works fine.
> >>
> >> Okay for commit?
> >
> > LGTM.
>
> Thank you.
>
> I will commit the change soon.
>
> For the aarch64 tests, do you have a suggestion on what the option 
> combination I should test?
>

Here is the followup patch to clean up these tests:

1. Replace ia32 with { ! lp64 } to enable ILP32 tests for -mx32.
2. Replace lp64 with { ! ia32 } to enable x86-64 ISA tests for -mx32.
3. For auto-init-3.c, add -msse and -mfpmath=387 for ia32.

Any comments?

-- 
H.J.
From 79831005e6d63e17d077d3c94b6f3c1e097c3da8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Mon, 20 Sep 2021 07:48:05 -0700
Subject: [PATCH] x86: Clean up gcc.target/i386/auto-init-* tests

1. Replace ia32 with { ! lp64 } to enable ILP32 tests for -mx32.
2. Replace lp64 with { ! ia32 } to enable x86-64 ISA tests for -mx32.
3. For auto-init-3.c, add -msse and -mfpmath=387 for ia32.

	* gcc.target/i386/auto-init-2.c: Replace ia32 with { ! lp64 }.
	* gcc.target/i386/auto-init-3.c (dg-options): Add -msse.
	(dg-additional-options): Add -mfpmath=387 for ia32.
	Replace lp64 with { ! ia32 }. Add a space after ia32.
	* gcc.target/i386/auto-init-4.c: Replace lp64 with { ! ia32 }.
	* gcc.target/i386/auto-init-5.c: Likewise.
	* gcc.target/i386/auto-init-padding-3.c: Likewise.
	* gcc.target/i386/auto-init-padding-7.c: Likewise.
	* gcc.target/i386/auto-init-padding-8.c: Likewise.
	* gcc.target/i386/auto-init-padding-9.c: Likewise.
---
 gcc/testsuite/gcc.target/i386/auto-init-2.c | 5 ++---
 gcc/testsuite/gcc.target/i386/auto-init-3.c | 7 ---
 gcc/testsuite/gcc.target/i386/auto-init-4.c | 7 +++
 gcc/testsuite/gcc.target/i386/auto-init-5.c | 4 +---
 gcc/testsuite/gcc.target/i386/auto-init-padding-3.c | 6 ++
 gcc/testsuite/gcc.target/i386/auto-init-padding-7.c | 4 +---
 gcc/testsuite/gcc.target/i386/auto-init-padding-8.c | 2 +-
 gcc/testsuite/gcc.target/i386/auto-init-padding-9.c | 4 ++--
 8 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/gcc/testsuite/gcc.target/i386/auto-init-2.c b/gcc/testsuite/gcc.target/i386/auto-init-2.c
index b23f733a403..e22930ae89b 100644
--- a/gcc/testsuite/gcc.target/i386/auto-init-2.c
+++ b/gcc/testsuite/gcc.target/i386/auto-init-2.c
@@ -33,6 +33,5 @@ void foo()
 /* { dg-final { scan-rtl-dump-times "0xfefe" 1 "expand" } } */
 /* { dg-final { scan-rtl-dump-times "0xfefefefe" 2 "expand" { target lp64 } } } */
 /* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 3 "expand" { target lp64 } } } */
-/* { dg-final { scan-rtl-dump-times "0xfefefefe" 4 "expand" { target ia32 } } } */
-/* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 1 "expand" { target ia32 } } } */
-
+/* { dg-final { scan-rtl-dump-times "0xfefefefe" 4 "expand" { target { ! lp64 } } } } */
+/* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 1 "expand" { target { ! lp64 } } } } */
diff --git a/gcc/testsuite/gcc.target/i386/auto-init-3.c b/gcc/testsuite/gcc.target/i386/auto-init-3.c
index df317616db3..891eab1c40b 100644
--- a/gcc/testsuite/gcc.target/i386/auto-init-3.c
+++ b/gcc/testsuite/gcc.target/i386/auto-init-3.c
@@ -1,6 +1,7 @@
 /* Verify zero initialization for floating point type automatic variables.  */
 /* { dg-do compile } */
-/* { dg-options "-ftrivial-auto-var-init=zero -march=x86-64 -mtune=generic" } */
+/* { dg-options "-ftrivial-auto-var-init=zero -march=x86-64 -mtune=generic -msse" } */
+/* { dg-additional-options "-mfpmath=387" { target ia32 } } */
 
 long double result;
 
@@ -14,5 +15,5 @@ long double foo()
   return result;
 }
 
-/* { dg-final { scan-assembler-times "pxor\t\\\%xmm0, \\\%xmm0" 3  { target lp64 } } } */
-/* { dg-final { scan-assembler-times "fldz" 3  { target ia32} } } */
+/* { dg-final { scan-assembler-times "pxor\t\\\%xmm0, \\\%xmm0" 3  { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "fldz" 3  { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/auto-init-4.c b/gcc/testsuite/gcc.target/i386/auto-init-4.c
index 554a2c57eb6..7b46c74a073 100644
--- 

Re: [Patch]GCC11 - Fortran: combined directives - order(concurrent) not on distribute (was: Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier)

[Jakub Jelinek via Gcc-patches]

On Mon, Sep 20, 2021 at 05:01:32PM +0200, Tobias Burnus wrote:
> On 20.09.21 11:55, Jakub Jelinek via Fortran wrote:
> > So the FE was splitting the order clause to distribute already before,
> > perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
> > support.
> > The difference is e.g. the distribute parallel do order(concurrent) 
> > copyin(thr)
> > case which used to be ok in 5.0 and is not in 5.1.
> 
> Well, if I try with GCC 11:
> 
> void f(int *a)
> {
> int  i;
> static int thr;
> #pragma omp threadprivate (thr)
> #pragma omp distribute parallel for order(concurrent) copyin(thr)
>   for (i = 0; i < 10; ++i)
>{
> thr = 5;
> a[i] = thr;
>}
> }
> 
> I get with gcc (+ gfortran):
>   error: threadprivate variable ‘thr’ used in a region with 
> ‘order(concurrent)’ clause
> I might have misunderstood the example.

Sure, even before you couldn't use it in the region body, because
order(concurrent) was split to worksharing-loop.
The testcase that used to be accepted and is now rejected is e.g.

int thr;
#pragma omp threadprivate (thr)

void
foo (void)
{
  int i;
#pragma omp distribute parallel for order(concurrent) copyin(thr)
  for (i = 0; i < 10; ++i)
   ;
}

While copyin without actually using the threadprivate var in the body
might look weird, in some cases it might be useful if the threadprivate
variable is used in some following parallel region.

> OK for GCC 11, only?
> 
> Tobias
> 
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> GCC11 - Fortran: combined directives - order(concurrent) not on distribute
> 
> While OpenMP 5.1 and GCC 12 permits 'order(concurrent)' on distribute,
> OpenMP 5.0 and GCC 11 don't. This patch for GCC 11 ensures the clause also
> does not end up on 'distribute' when splitting combined directives.
> 
> gcc/fortran/ChangeLog:
> 
>   * trans-openmp.c (gfc_split_omp_clauses): Don't put 'order(concurrent)'
>   on 'distribute' for combined directives, matching OpenMP 5.0
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/distribute-order-concurrent.f90: New test.

Ok, thanks.

Jakub

[Patch]GCC11 - Fortran: combined directives - order(concurrent) not on distribute (was: Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier)

[Tobias Burnus]

On 20.09.21 11:55, Jakub Jelinek via Fortran wrote:

So the FE was splitting the order clause to distribute already before,
perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
support.
The difference is e.g. the distribute parallel do order(concurrent) copyin(thr)
case which used to be ok in 5.0 and is not in 5.1.


Well, if I try with GCC 11:

void f(int *a)
{
int  i;
static int thr;
#pragma omp threadprivate (thr)
#pragma omp distribute parallel for order(concurrent) copyin(thr)
  for (i = 0; i < 10; ++i)
   {
thr = 5;
a[i] = thr;
   }
}

I get with gcc (+ gfortran):
  error: threadprivate variable ‘thr’ used in a region with ‘order(concurrent)’ 
clause
I might have misunderstood the example.

 * * *

In any case, for GCC 11, I have now fixed the splitting and added a testcase 
which
relies on -fdump-tree-original scanning and does not use threadprivate.

OK for GCC 11, only?

Tobias

-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
GCC11 - Fortran: combined directives - order(concurrent) not on distribute

While OpenMP 5.1 and GCC 12 permits 'order(concurrent)' on distribute,
OpenMP 5.0 and GCC 11 don't. This patch for GCC 11 ensures the clause also
does not end up on 'distribute' when splitting combined directives.

gcc/fortran/ChangeLog:

	* trans-openmp.c (gfc_split_omp_clauses): Don't put 'order(concurrent)'
	on 'distribute' for combined directives, matching OpenMP 5.0

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/distribute-order-concurrent.f90: New test.

 gcc/fortran/trans-openmp.c |  2 --
 .../gomp/distribute-order-concurrent.f90   | 25 ++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 7e931bf4bc7..973d916b4a2 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -5176,8 +5176,6 @@ gfc_split_omp_clauses (gfc_code *code,
 	  /* Duplicate collapse.  */
 	  clausesa[GFC_OMP_SPLIT_DISTRIBUTE].collapse
 	= code->ext.omp_clauses->collapse;
-	  clausesa[GFC_OMP_SPLIT_DISTRIBUTE].order_concurrent
-	= code->ext.omp_clauses->order_concurrent;
 	}
   if (mask & GFC_OMP_MASK_PARALLEL)
 	{
diff --git a/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90 b/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90
new file mode 100644
index 000..9597d913684
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90
@@ -0,0 +1,25 @@
+! { dg-additional-options "-fdump-tree-original" }
+!
+! In OpenMP 5.0, 'order(concurrent)' does not apply to distribute
+! Ensure that it is rejected in GCC 11.
+! 
+! Note: OpenMP 5.1 allows it; the GCC 12 testcase for it is gfortran.dg/gomp/order-5.f90
+
+subroutine f(a)
+implicit none
+integer :: i, thr
+!save :: thr
+integer :: a(:)
+
+!$omp distribute parallel do order(concurrent) private(thr)
+  do i = 1, 10
+thr = 5
+a(i) = thr
+  end do
+!$omp end distribute parallel do
+end
+
+! { dg-final { scan-tree-dump-not "omp distribute\[^\n\r]*order" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp distribute\[\n\r\]" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp parallel private\\(thr\\)" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp for nowait order\\(concurrent\\)" "original" } }

Re: [PATCH] Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*

[Jeff Law via Gcc-patches]

On 9/20/2021 1:00 AM, Richard Biener wrote:

This obsoletes the 32bit hppa-hpux configurations which only support
STABS as debuginfo format.

As discussed, I'm going to push this (and a changes.html entry) when
it was included in a bootstrap/regtest cycle.

2021-09-20  Richard Biener  

gcc/
* config.gcc: Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*.

contrib/
* config-list.mk: --enable-obsolete for hppa2.0-hpux10.1 and
hppa2.0-hpux11.9.
Is this marking hppa2.0w-hp-hpux11 as obsolete?  That platform is using 
ELF & DWARF.


jeff

Re: [pushed] Driver: Fix bootstrap with DEFAULT_{ASSEMBLER,LINKER,DSYMUTIL}.

[Jeff Law via Gcc-patches]

On 9/20/2021 12:50 AM, Iain Sandoe wrote:

Hi

The patch at r12-3662-g5fee8a0a9223d factored the code for
printing the names of programes into a separate function.
However the moved editions that print out the names of the
assembler, linker (and dsymutil on Darwin) when those are
specified at configure-time were not adjusted accordingly,
leading to a bootstrap fail.

Fixed by testing specifically for execute OK, since we know
these are programs.

tested on i686-darwin9, x86_64-darwin20 configured with
—with-{as,ld,dsymutil}= and on x86_64-darwin18 without.

confirmed that the build and installed versions print the right
thing (and, of course, that bootstrap succeeds).

pushed to master as bootstrap fix, thanks
Iain

Signed-off-by: Iain Sandoe 

gcc/ChangeLog:

* gcc.c: Test for execute OK when we find the
programs for assembler linker and dsymutil and those
were specified at configure-time.

Thanks for taking care of this.  Sorry for the breakage.

jeff

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Richard Earnshaw via Gcc-patches]

On 20/09/2021 14:55, Qing Zhao wrote:




On Sep 20, 2021, at 8:18 AM, Richard Earnshaw  
wrote:



On 20/09/2021 13:47, Qing Zhao wrote:

On Sep 20, 2021, at 5:43 AM, Richard Earnshaw  
wrote:



On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:

Hi,
There are much less issues with aarch64/auto-init-* test cases.
Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
‘armv8-r’) do not change the pattern match.
Only
1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
“auto-init-padding-5.c”.
2. -fstack-clash-protection impact two of the testing cases “auto-init-1.c” and 
“auto-init-7.c”.
Naturally the patch for this set is:
A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
“auto-init-padding-5.c”;
B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
The above A fixed issue 1, however, the above B did not fix issue 2.
If I fixed “auto-init-1.c” as:
diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
index 0fa4708..a38d91b 100644
--- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
@@ -1,6 +1,6 @@
  /* Verify zero initialization for integer and pointer type automatic 
variables.  */
  /* { dg-do compile } */
-/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
+/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector" } */
#ifndef __cplusplus
  # define bool _Bool
So, I took a look at the log file of the testing, and found that, If I tested 
it as:
  make check-gcc 
RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
 aarch64.exp=auto-init*’
In the log file, I got:
/home/qinzhao/Work/GCC/build_git/gcc/xgcc 
-B/home/qinzhao/Work/GCC/build_git/gcc/ 
/home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
  -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 -fstack-clash-protection 
-fstack-protector-all  -o auto-init-1.s
 From it, we can see, that the options that were passed through RUNTESTFLAGS 
“mabi-lp64 -fstack-clash-protection -fstack-protector-all” were appended AFTER 
the options inside the testing case through “dg-options”. As a result, the 
option “-fno-stack-protector” did not have any impact at all.
What’s the expected behavior for the order of these options? Should options 
through RUNTESTFLAGS be appended BEFORE or AFTER the options through testing 
cases?
For X86, the options through RUNTESTFLAGS are added BEFORE the options through 
testing cases. Therefore adding “-fno-stack-protector” has the expected result.


Can you check that you are using the same version of dejagnu on both platforms?

Stupid question:  how to check the version of it?


$ runtest --version


Thanks.

On aarch64:
qinzhao@gcc116:~/Work/GCC/build_git$ runtest --version
Expect version is   5.45
Tcl version is  8.6
Framework version is1.5


Ok, so this is dejagnu 1.5 (older versions of dejagnu reported the 
'framework version') ...




On X86:
[opc@qinzhao-ol8u3-x86 gcc]$ runtest --version
WARNING: Couldn't find the global config file.
DejaGnu version 1.6.1
Expect version  5.45.4
Tcl version 8.6


While this is dejagnu 1.6.1

IIRC there were some changes to the way various options were combined at 
one point and I suspect this may be the source of your problem.  Can you 
update your dejagnu on your aarch64 system to be the same as that on x86?


R.



So, is there anything wrong with the installation of runtest on X86?

Qing




R.


Qing


R.


Is this a bug in aarch64 testing suite?
Thanks.
Qing

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Qing Zhao via Gcc-patches]

> On Sep 20, 2021, at 8:18 AM, Richard Earnshaw  
> wrote:
> 
> 
> 
> On 20/09/2021 13:47, Qing Zhao wrote:
>>> On Sep 20, 2021, at 5:43 AM, Richard Earnshaw 
>>>  wrote:
>>> 
>>> 
>>> 
>>> On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:
 Hi,
 There are much less issues with aarch64/auto-init-* test cases.
 Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
 ‘armv8-r’) do not change the pattern match.
 Only
 1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
 “auto-init-padding-5.c”.
 2. -fstack-clash-protection impact two of the testing cases 
 “auto-init-1.c” and “auto-init-7.c”.
 Naturally the patch for this set is:
 A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
 “auto-init-padding-5.c”;
 B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
 The above A fixed issue 1, however, the above B did not fix issue 2.
 If I fixed “auto-init-1.c” as:
 diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
 b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
 index 0fa4708..a38d91b 100644
 --- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
 +++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
 @@ -1,6 +1,6 @@
  /* Verify zero initialization for integer and pointer type automatic 
 variables.  */
  /* { dg-do compile } */
 -/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
 +/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
 -fno-stack-protector" } */
#ifndef __cplusplus
  # define bool _Bool
 So, I took a look at the log file of the testing, and found that, If I 
 tested it as:
  make check-gcc 
 RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
  aarch64.exp=auto-init*’
 In the log file, I got:
 /home/qinzhao/Work/GCC/build_git/gcc/xgcc 
 -B/home/qinzhao/Work/GCC/build_git/gcc/ 
 /home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
   -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero 
 -fdump-rtl-expand -fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 
 -fstack-clash-protection -fstack-protector-all  -o auto-init-1.s
 From it, we can see, that the options that were passed through 
 RUNTESTFLAGS “mabi-lp64 -fstack-clash-protection -fstack-protector-all” 
 were appended AFTER the options inside the testing case through 
 “dg-options”. As a result, the option “-fno-stack-protector” did not have 
 any impact at all.
 What’s the expected behavior for the order of these options? Should 
 options through RUNTESTFLAGS be appended BEFORE or AFTER the options 
 through testing cases?
 For X86, the options through RUNTESTFLAGS are added BEFORE the options 
 through testing cases. Therefore adding “-fno-stack-protector” has the 
 expected result.
>>> 
>>> Can you check that you are using the same version of dejagnu on both 
>>> platforms?
>> Stupid question:  how to check the version of it?
> 
> $ runtest --version

Thanks.

On aarch64:
qinzhao@gcc116:~/Work/GCC/build_git$ runtest --version
Expect version is   5.45
Tcl version is  8.6
Framework version is1.5

On X86:
[opc@qinzhao-ol8u3-x86 gcc]$ runtest --version
WARNING: Couldn't find the global config file.
DejaGnu version 1.6.1
Expect version  5.45.4
Tcl version 8.6

So, is there anything wrong with the installation of runtest on X86?

Qing


> 
> R.
> 
>> Qing
>>> 
>>> R.
>>> 
 Is this a bug in aarch64 testing suite?
 Thanks.
 Qing

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Richard Earnshaw via Gcc-patches]

On 20/09/2021 13:47, Qing Zhao wrote:




On Sep 20, 2021, at 5:43 AM, Richard Earnshaw  
wrote:



On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:

Hi,
There are much less issues with aarch64/auto-init-* test cases.
Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
‘armv8-r’) do not change the pattern match.
Only
1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
“auto-init-padding-5.c”.
2. -fstack-clash-protection impact two of the testing cases “auto-init-1.c” and 
“auto-init-7.c”.
Naturally the patch for this set is:
A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
“auto-init-padding-5.c”;
B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
The above A fixed issue 1, however, the above B did not fix issue 2.
If I fixed “auto-init-1.c” as:
diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
index 0fa4708..a38d91b 100644
--- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
@@ -1,6 +1,6 @@
  /* Verify zero initialization for integer and pointer type automatic 
variables.  */
  /* { dg-do compile } */
-/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
+/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector" } */
#ifndef __cplusplus
  # define bool _Bool
So, I took a look at the log file of the testing, and found that, If I tested 
it as:
  make check-gcc 
RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
 aarch64.exp=auto-init*’
In the log file, I got:
/home/qinzhao/Work/GCC/build_git/gcc/xgcc 
-B/home/qinzhao/Work/GCC/build_git/gcc/ 
/home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
  -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 -fstack-clash-protection 
-fstack-protector-all  -o auto-init-1.s
 From it, we can see, that the options that were passed through RUNTESTFLAGS 
“mabi-lp64 -fstack-clash-protection -fstack-protector-all” were appended AFTER 
the options inside the testing case through “dg-options”. As a result, the 
option “-fno-stack-protector” did not have any impact at all.
What’s the expected behavior for the order of these options? Should options 
through RUNTESTFLAGS be appended BEFORE or AFTER the options through testing 
cases?
For X86, the options through RUNTESTFLAGS are added BEFORE the options through 
testing cases. Therefore adding “-fno-stack-protector” has the expected result.


Can you check that you are using the same version of dejagnu on both platforms?


Stupid question:  how to check the version of it?



$ runtest --version

R.


Qing


R.


Is this a bug in aarch64 testing suite?
Thanks.
Qing

[Ada] Present and No functions for type Uint

[Pierre-Marie de Rodat]

Declare Present and No functions for type Uint, analogous to other types
such as Node_Id, and use them as appropriate.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* uintp.ads, uintp.adb (Present, No): New functions for
comparing with No_Uint.
* checks.adb, einfo-utils.adb, exp_aggr.adb, exp_attr.adb,
exp_ch3.adb, exp_ch4.adb, exp_dbug.adb, exp_disp.adb,
exp_util.adb, repinfo.adb, repinfo-input.adb, scn.adb,
sem_attr.adb, sem_ch13.adb, sem_eval.adb, sem_util.adb,
sinfo-utils.adb, treepr.adb: Use Present (...) instead of "...
/= No_Uint", and No (...) instead of "... = No_Uint".

patch.diff.gz
Description: application/gzip

[Ada] Fix shadowing in conditions for inlining

[Pierre-Marie de Rodat]

Cleanup related to inlining-for-proof and detection of overlaying actual
parameters in GNATprove; semantics is unaffected.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* inline.adb (Has_Excluded_Declaration): Rename and reduce scope
of a local variable.diff --git a/gcc/ada/inline.adb b/gcc/ada/inline.adb
--- a/gcc/ada/inline.adb
+++ b/gcc/ada/inline.adb
@@ -4215,8 +4215,6 @@ package body Inline is
  (Subp  : Entity_Id;
   Decls : List_Id) return Boolean
is
-  D : Node_Id;
-
   function Is_Unchecked_Conversion (D : Node_Id) return Boolean;
   --  Nested subprograms make a given body ineligible for inlining, but
   --  we make an exception for instantiations of unchecked conversion.
@@ -4250,6 +4248,10 @@ package body Inline is
and then Is_Intrinsic_Subprogram (Conv);
   end Is_Unchecked_Conversion;
 
+  --  Local variables
+
+  Decl : Node_Id;
+
--  Start of processing for Has_Excluded_Declaration
 
begin
@@ -4259,19 +4261,19 @@ package body Inline is
  return False;
   end if;
 
-  D := First (Decls);
-  while Present (D) loop
+  Decl := First (Decls);
+  while Present (Decl) loop
 
  --  First declarations universally excluded
 
- if Nkind (D) = N_Package_Declaration then
+ if Nkind (Decl) = N_Package_Declaration then
 Cannot_Inline
-  ("cannot inline & (nested package declaration)?", D, Subp);
+  ("cannot inline & (nested package declaration)?", Decl, Subp);
 return True;
 
- elsif Nkind (D) = N_Package_Instantiation then
+ elsif Nkind (Decl) = N_Package_Instantiation then
 Cannot_Inline
-  ("cannot inline & (nested package instantiation)?", D, Subp);
+  ("cannot inline & (nested package instantiation)?", Decl, Subp);
 return True;
  end if;
 
@@ -4280,51 +4282,52 @@ package body Inline is
  if Back_End_Inlining then
 null;
 
- elsif Nkind (D) = N_Task_Type_Declaration
-   or else Nkind (D) = N_Single_Task_Declaration
+ elsif Nkind (Decl) = N_Task_Type_Declaration
+   or else Nkind (Decl) = N_Single_Task_Declaration
  then
 Cannot_Inline
-  ("cannot inline & (nested task type declaration)?", D, Subp);
+  ("cannot inline & (nested task type declaration)?", Decl, Subp);
 return True;
 
- elsif Nkind (D) = N_Protected_Type_Declaration
-   or else Nkind (D) = N_Single_Protected_Declaration
+ elsif Nkind (Decl) in N_Protected_Type_Declaration
+ | N_Single_Protected_Declaration
  then
 Cannot_Inline
   ("cannot inline & (nested protected type declaration)?",
-   D, Subp);
+   Decl, Subp);
 return True;
 
- elsif Nkind (D) = N_Subprogram_Body then
+ elsif Nkind (Decl) = N_Subprogram_Body then
 Cannot_Inline
-  ("cannot inline & (nested subprogram)?", D, Subp);
+  ("cannot inline & (nested subprogram)?", Decl, Subp);
 return True;
 
- elsif Nkind (D) = N_Function_Instantiation
-   and then not Is_Unchecked_Conversion (D)
+ elsif Nkind (Decl) = N_Function_Instantiation
+   and then not Is_Unchecked_Conversion (Decl)
  then
 Cannot_Inline
-  ("cannot inline & (nested function instantiation)?", D, Subp);
+  ("cannot inline & (nested function instantiation)?", Decl, Subp);
 return True;
 
- elsif Nkind (D) = N_Procedure_Instantiation then
+ elsif Nkind (Decl) = N_Procedure_Instantiation then
 Cannot_Inline
-  ("cannot inline & (nested procedure instantiation)?", D, Subp);
+  ("cannot inline & (nested procedure instantiation)?",
+   Decl, Subp);
 return True;
 
  --  Subtype declarations with predicates will generate predicate
  --  functions, i.e. nested subprogram bodies, so inlining is not
  --  possible.
 
- elsif Nkind (D) = N_Subtype_Declaration
-   and then Present (Aspect_Specifications (D))
+ elsif Nkind (Decl) = N_Subtype_Declaration
+   and then Present (Aspect_Specifications (Decl))
  then
 declare
A: Node_Id;
A_Id : Aspect_Id;
 
 begin
-   A := First (Aspect_Specifications (D));
+   A := First (Aspect_Specifications (Decl));
while Present (A) loop
   A_Id := Get_Aspect_Id (Chars (Identifier (A)));
 
@@ -4334,7 +4337,7 @@ package body Inline is
   then
  Cannot_Inline
("cannot inline & (subtype declaration with "
-& "predicate)?", D, 

[Ada] Remove redundant checks for non-empty list of aspects

[Pierre-Marie de Rodat]

Cleanup related to inlining-for-proof and detection of overlaying actual
parameters in GNATprove; semantics is unaffected.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* inline.adb (Has_Excluded_Declaration): Remove redundant guard;
the guarded code will call First on a No_List, which is
well-defined and gives Empty.diff --git a/gcc/ada/inline.adb b/gcc/ada/inline.adb
--- a/gcc/ada/inline.adb
+++ b/gcc/ada/inline.adb
@@ -4319,9 +4319,7 @@ package body Inline is
  --  functions, i.e. nested subprogram bodies, so inlining is not
  --  possible.
 
- elsif Nkind (Decl) = N_Subtype_Declaration
-   and then Present (Aspect_Specifications (Decl))
- then
+ elsif Nkind (Decl) = N_Subtype_Declaration then
 declare
A: Node_Id;
A_Id : Aspect_Id;

[Ada] Clean up Uint fields, remove unused routines

[Pierre-Marie de Rodat]

Remove unused routines.

Remove 2-parameter versions of Init_Alignment and friends.  Replace
calls with direct calls to Set_Alignment and friends.  These routines
aren't really doing anything worth an extra abstraction.

Change remaining Init_ routines to Reinit_, because these are not
usually being used to initialize.

Reinit_Alignment correctly calls Reinit_Field_To_Zero.  The other two
(Reinit_Esize and Reinit_RM_Size) are still setting the field to Uint_0;
this will be changed to Reinit_Field_To_Zero later.

Add Copy_Esize and Copy_RM_Size, not yet implemented.  These will be
implemented when Reinit_Esize and Reinit_RM_Size are corrected.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* einfo-utils.ads, einfo-utils.adb, fe.h, einfo.ads,
gen_il-fields.ads: Remove unused and no-longer-used routines.
Move related routines together.  Rewrite incorrect
documentation, and documentation that will be incorrect when
e.g. Esize-related routines are fixed.  Remove unused field
Normalized_Position_Max.
* cstand.adb, exp_pakd.adb, freeze.adb,
gen_il-gen-gen_entities.adb, itypes.adb, layout.adb,
sem_ch10.adb, sem_ch12.adb, sem_ch13.adb, sem_ch3.adb,
sem_ch7.adb, sem_ch8.adb, sem_ch9.adb, sem_prag.adb,
sem_util.adb, ttypes.ads: Update calls to routines removed from
or renamed in Einfo.Utils.
* uintp.ads (Upos): Fix this subtype, which was unintentionally
declared to include Uint_0.

patch.diff.gz
Description: application/gzip

[Ada] Accept volatile properties on constant objects

[Pierre-Marie de Rodat]

Aspects Volatile and its related properties, i.e. Async_Readers,
Async_Writers, Effective_Reads, Effective_Writes and No_Caching, are now
allowed on stand-alone constant objects in SPARK.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* contracts.adb (Add_Contract_Item): Accept volatile-related
properties on constants.
(Analyze_Object_Contract): Check external properties on
constants; accept volatile constants.
(Check_Type_Or_Object_External_Properties): Replace "variable"
with "object" in error messages; replace Decl_Kind with a local
constant.
* sem_prag.adb (Analyze_Pragma): Accept volatile-related
properties on constants.diff --git a/gcc/ada/contracts.adb b/gcc/ada/contracts.adb
--- a/gcc/ada/contracts.adb
+++ b/gcc/ada/contracts.adb
@@ -144,7 +144,13 @@ package body Contracts is
   --Part_Of
 
   if Ekind (Id) = E_Constant then
- if Prag_Nam = Name_Part_Of then
+ if Prag_Nam in Name_Async_Readers
+  | Name_Async_Writers
+  | Name_Effective_Reads
+  | Name_Effective_Writes
+  | Name_No_Caching
+  | Name_Part_Of
+ then
 Add_Classification;
 
  --  The pragma is not a proper contract item
@@ -778,25 +784,9 @@ package body Contracts is
procedure Check_Type_Or_Object_External_Properties
  (Type_Or_Obj_Id : Entity_Id)
is
-  function Decl_Kind (Is_Type : Boolean;
-  Object_Kind : String) return String;
-  --  Returns "type" or Object_Kind, depending on Is_Type
-
-  ---
-  -- Decl_Kind --
-  ---
-
-  function Decl_Kind (Is_Type : Boolean;
-  Object_Kind : String) return String is
-  begin
- if Is_Type then
-return "type";
- else
-return Object_Kind;
- end if;
-  end Decl_Kind;
-
   Is_Type_Id : constant Boolean := Is_Type (Type_Or_Obj_Id);
+  Decl_Kind  : constant String :=
+(if Is_Type_Id then "type" else "object");
 
   --  Local variables
 
@@ -923,8 +913,7 @@ package body Contracts is
 if not Is_Library_Level_Entity (Type_Or_Obj_Id) then
Error_Msg_N
  ("effectively volatile "
-& Decl_Kind (Is_Type => Is_Type_Id,
- Object_Kind => "variable")
+& Decl_Kind
 & " & must be declared at library level "
 & "(SPARK RM 7.1.3(3))", Type_Or_Obj_Id);
 
@@ -935,10 +924,7 @@ package body Contracts is
   and then not Is_Protected_Type (Obj_Typ)
 then
Error_Msg_N
-("discriminated "
-   & Decl_Kind (Is_Type => Is_Type_Id,
-Object_Kind => "object")
-   & " & cannot be volatile",
+("discriminated " & Decl_Kind & " & cannot be volatile",
  Type_Or_Obj_Id);
 end if;
 
@@ -1019,7 +1005,7 @@ package body Contracts is
   Saved_SMP : constant Node_Id := SPARK_Mode_Pragma;
   --  Save the SPARK_Mode-related data to restore on exit
 
-  NC_Val   : Boolean := False;
+  NC_Val   : Boolean;
   Items: Node_Id;
   Prag : Node_Id;
   Ref_Elmt : Elmt_Id;
@@ -1056,6 +1042,19 @@ package body Contracts is
  Set_SPARK_Mode (Obj_Id);
   end if;
 
+  --  Checks related to external properties, same for constants and
+  --  variables.
+
+  Check_Type_Or_Object_External_Properties (Type_Or_Obj_Id => Obj_Id);
+
+  --  Analyze the non-external volatility property No_Caching
+
+  Prag := Get_Pragma (Obj_Id, Pragma_No_Caching);
+
+  if Present (Prag) then
+ Analyze_External_Property_In_Decl_Part (Prag, NC_Val);
+  end if;
+
   --  Constant-related checks
 
   if Ekind (Obj_Id) = E_Constant then
@@ -1071,35 +1070,10 @@ package body Contracts is
 Check_Missing_Part_Of (Obj_Id);
  end if;
 
- --  A constant cannot be effectively volatile (SPARK RM 7.1.3(4)).
- --  This check is relevant only when SPARK_Mode is on, as it is not
- --  a standard Ada legality rule. Internally-generated constants that
- --  map generic formals to actuals in instantiations are allowed to
- --  be volatile.
-
- if SPARK_Mode = On
-   and then Comes_From_Source (Obj_Id)
-   and then Is_Effectively_Volatile (Obj_Id)
-   and then No (Corresponding_Generic_Association (Parent (Obj_Id)))
- then
-Error_Msg_N ("constant cannot be volatile", Obj_Id);
- end if;
-
   --  Variable-related checks
 
   else pragma Assert (Ekind (Obj_Id) = E_Variable);
 
- Check_Type_Or_Object_External_Properties
-   

[Ada] Accept volatile expressions as non-scalar actual parameters

[Pierre-Marie de Rodat]

This change removes an old, incomplete and duplicated code that
implemented the very first wording of a SPARK RM rule related to
volatile expressions acting as actual parameters.

Current the rule says: "[a name denoting] an effectively volatile object
for reading [can be] an actual parameter in a call for which the
corresponding formal parameter is of a non-scalar effectively volatile
type for reading".

This wording is implemented in Is_OK_Volatile_Context and enforced when
this routine is called by Resolve_Actuals via
Flag_Effectively_Volatile_Objects with Check_Actuals parameter being
True.

In particular, the removed code was incorrectly only looking at
procedure calls and their parameters of mode IN; the rule applies to
also to function and entry calls and their parameters of modes IN OUT
and OUT too.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_res.adb (Resolve_Actual): Removediff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -3454,7 +3454,6 @@ package body Sem_Res is
procedure Resolve_Actuals (N : Node_Id; Nam : Entity_Id) is
   Loc: constant Source_Ptr := Sloc (N);
   A  : Node_Id;
-  A_Id   : Entity_Id;
   A_Typ  : Entity_Id := Empty; -- init to avoid warning
   F  : Entity_Id;
   F_Typ  : Entity_Id;
@@ -4969,31 +4968,6 @@ package body Sem_Res is
--  must be resolved first.
 
Flag_Effectively_Volatile_Objects (A);
-
-   --  An effectively volatile variable cannot act as an actual
-   --  parameter in a procedure call when the variable has enabled
-   --  property Effective_Reads and the corresponding formal is of
-   --  mode IN (SPARK RM 7.1.3(10)).
-
-   if Ekind (Nam) = E_Procedure
- and then Ekind (F) = E_In_Parameter
- and then Is_Entity_Name (A)
-   then
-  A_Id := Entity (A);
-
-  if Ekind (A_Id) = E_Variable
-and then Is_Effectively_Volatile_For_Reading (Etype (A_Id))
-and then Effective_Reads_Enabled (A_Id)
-  then
- Error_Msg_NE
-   ("effectively volatile variable & cannot appear as "
-& "actual in procedure call", A, A_Id);
-
- Error_Msg_Name_1 := Name_Effective_Reads;
- Error_Msg_N ("\\variable has enabled property %", A);
- Error_Msg_N ("\\corresponding formal has mode IN", A);
-  end if;
-   end if;
 end if;
 
 --  A formal parameter of a specific tagged type whose related

[Ada] Use OS_Time for interface to TZ functions.

[Pierre-Marie de Rodat]

A recent regression caused by the parameterization of time_t was due to
the unusual declaration used for time_t in the interface to TZ functions
in sysdep.c. The root cause was the Long_Integer size of 32 bits used on
x86_64-windows. The incident was temporarily fixed by reverting the
declaration to its former self.

This however will break vxworks SR0660 use of 64-bit time_t on 32-bit
targets. The proper fix below is to use OS_Time for the interface to
ensure compatibility independent of Long_Integer size.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/a-calend.adb: Remove time_t, replace with OS_Time.
* libgnat/s-os_lib.ads: Fix comments regarding time_t conversion
functions to reflect the use of To_Ada in in Ada.Calendar
package body.
* sysdep.c (__gnat_localtime_tzoff): Use OS_Time instead of
time_t.diff --git a/gcc/ada/libgnat/a-calend.adb b/gcc/ada/libgnat/a-calend.adb
--- a/gcc/ada/libgnat/a-calend.adb
+++ b/gcc/ada/libgnat/a-calend.adb
@@ -35,6 +35,8 @@ with Interfaces.C;
 
 with System.OS_Primitives;
 
+with System.OS_Lib;
+
 package body Ada.Calendar with
   SPARK_Mode => Off
 is
@@ -685,13 +687,10 @@ is
   type int_Pointer  is access all Interfaces.C.int;
   type long_Pointer is access all Interfaces.C.long;
 
-  type time_t is
-range -(2 ** (Standard'Address_Size - Integer'(1))) ..
-  +(2 ** (Standard'Address_Size - Integer'(1)) - 1);
-  type time_t_Pointer is access all time_t;
+  type OS_Time_Pointer is access all System.OS_Lib.OS_Time;
 
   procedure localtime_tzoff
-(timer   : time_t_Pointer;
+(timer   : OS_Time_Pointer;
  is_historic : int_Pointer;
  off : long_Pointer);
   pragma Import (C, localtime_tzoff, "__gnat_localtime_tzoff");
@@ -708,7 +707,7 @@ is
   Date_N   : Time_Rep;
   Flag : aliased Interfaces.C.int;
   Offset   : aliased Interfaces.C.long;
-  Secs_T   : aliased time_t;
+  Secs_T   : aliased System.OS_Lib.OS_Time;
 
--  Start of processing for UTC_Time_Offset
 
@@ -745,7 +744,7 @@ is
 
   --  Convert the date into seconds
 
-  Secs_T := time_t (Date_N / Nano);
+  Secs_T := System.OS_Lib.To_Ada (Long_Long_Integer (Date_N / Nano));
 
   --  Determine whether to treat the input date as historical or not. A
   --  value of "0" signifies that the date is NOT historic.


diff --git a/gcc/ada/libgnat/s-os_lib.ads b/gcc/ada/libgnat/s-os_lib.ads
--- a/gcc/ada/libgnat/s-os_lib.ads
+++ b/gcc/ada/libgnat/s-os_lib.ads
@@ -169,16 +169,15 @@ package System.OS_Lib is
--
 
--  Note: Do not use time_t in the compiler and host-based tools; instead
-   --  use OS_Time. These 3 declarations are intended for use only by consumers
-   --  of the GNAT.OS_Lib renaming of this package.
+   --  use OS_Time.
 
subtype time_t is Long_Long_Integer;
-   --  C time_t can be either long or long long, but this is a subtype not used
-   --  in the compiler or tools, but only for user applications, so we choose
-   --  the Ada equivalent of the latter because eventually that will be the
+   --  C time_t can be either long or long long, so we choose the Ada
+   --  equivalent of the latter because eventually that will be the
--  type used out of necessity. This may affect some user code on 32-bit
--  targets that have not yet migrated to the Posix 2008 standard,
-   --  particularly pre version 5 32-bit Linux.
+   --  particularly pre version 5 32-bit Linux. Do not change this
+   --  declaration without coordinating it with conversions in Ada.Calendar.
 
function To_C (Time : OS_Time) return time_t;
--  Convert OS_Time to C time_t type


diff --git a/gcc/ada/sysdep.c b/gcc/ada/sysdep.c
--- a/gcc/ada/sysdep.c
+++ b/gcc/ada/sysdep.c
@@ -643,11 +643,11 @@ long __gnat_invalid_tzoff = 259273;
 /* Reentrant localtime for Windows. */
 
 extern void
-__gnat_localtime_tzoff (const time_t *, const int *, long *);
+__gnat_localtime_tzoff (const OS_Time *, const int *, long *);
 
 static const unsigned long long w32_epoch_offset = 11644473600ULL;
 void
-__gnat_localtime_tzoff (const time_t *timer, const int *is_historic, long *off)
+__gnat_localtime_tzoff (const OS_Time *timer, const int *is_historic, long *off)
 {
   TIME_ZONE_INFORMATION tzi;
 
@@ -737,10 +737,10 @@ __gnat_localtime_tzoff (const time_t *timer, const int *is_historic, long *off)
the Lynx convention when building against the legacy API. */
 
 extern void
-__gnat_localtime_tzoff (const time_t *, const int *, long *);
+__gnat_localtime_tzoff (const OS_Time *, const int *, long *);
 
 void
-__gnat_localtime_tzoff (const time_t *timer, const int *is_historic, long *off)
+__gnat_localtime_tzoff (const OS_Time *timer, const int *is_historic, long *off)
 {
   *off = 0;
 }
@@ -756,21 +756,22 @@ extern void (*Lock_Task) (void);
 extern void (*Unlock_Task) (void);
 
 extern void
-__gnat_localtime_tzoff 

[Ada] Cleanups related to building of dispatch tables

[Pierre-Marie de Rodat]

Code cleanup only; semantics is unaffected.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_ch7.adb (Expand_N_Package_Declaration): Fix wording in
comment.
* exp_disp.adb (Mark_DT): Remove unnecessary initialization of
I_Depth.diff --git a/gcc/ada/exp_ch7.adb b/gcc/ada/exp_ch7.adb
--- a/gcc/ada/exp_ch7.adb
+++ b/gcc/ada/exp_ch7.adb
@@ -6067,7 +6067,7 @@ package body Exp_Ch7 is
  Pop_Scope;
   end if;
 
-  --  Build dispatch tables of library level tagged types
+  --  Build dispatch tables of library-level tagged types
 
   if Tagged_Type_Expansion
 and then (Is_Compilation_Unit (Id)


diff --git a/gcc/ada/exp_disp.adb b/gcc/ada/exp_disp.adb
--- a/gcc/ada/exp_disp.adb
+++ b/gcc/ada/exp_disp.adb
@@ -4712,7 +4712,7 @@ package body Exp_Disp is
   Exname : Entity_Id;
   HT_Link: Entity_Id;
   ITable : Node_Id;
-  I_Depth: Nat := 0;
+  I_Depth: Nat;
   Iface_Table_Node   : Node_Id;
   Name_ITable: Name_Id;
   Nb_Prim: Nat := 0;

[Ada] Add support for PE-COFF PIE to System.Dwarf_Line

[Pierre-Marie de Rodat]

This makes it possible for System.Dwarf_Line to handle Position-Independent
Executables on Windows systems by translating the run-time addresses it is
provided with into addresses in the executable.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* adaint.c (__gnat_get_executable_load_address): Add Win32 support.
* libgnat/s-objrea.ads (Get_Xcode_Bounds): Fix typo in comment.
(Object_File): Minor reformatting.
(ELF_Object_File): Uncomment predicate.
(PECOFF_Object_File): Likewise.
(XCOFF32_Object_File): Likewise.
* libgnat/s-objrea.adb: Minor reformatting throughout.
(Get_Load_Address): Implement for PE-COFF.
* libgnat/s-dwalin.ads: Remove clause for System.Storage_Elements
and use consistent wording in comments.
(Dwarf_Context): Set type of Low, High and Load_Address to Address.
* libgnat/s-dwalin.adb (Get_Load_Displacement): New function.
(Is_Inside): Call Get_Load_Displacement.
(Low_Address): Likewise.
(Open): Adjust to type change.
(Aranges_Lookup): Change type of Addr to Address.
(Read_Aranges_Entry): Likewise for Start and adjust.
(Enable_Cach): Adjust to type change.
(Symbolic_Address): Change type of Addr to Address.
(Symbolic_Traceback): Call Get_Load_Displacement.diff --git a/gcc/ada/adaint.c b/gcc/ada/adaint.c
--- a/gcc/ada/adaint.c
+++ b/gcc/ada/adaint.c
@@ -3542,6 +3542,9 @@ __gnat_get_executable_load_address (void)
 
   return (const void *)map->l_addr;
 
+#elif defined (_WIN32)
+  return GetModuleHandle (NULL);
+
 #else
   return NULL;
 #endif


diff --git a/gcc/ada/libgnat/s-dwalin.adb b/gcc/ada/libgnat/s-dwalin.adb
--- a/gcc/ada/libgnat/s-dwalin.adb
+++ b/gcc/ada/libgnat/s-dwalin.adb
@@ -47,6 +47,10 @@ package body System.Dwarf_Lines is
 
SSU : constant := System.Storage_Unit;
 
+   function Get_Load_Displacement (C : Dwarf_Context) return Storage_Offset;
+   --  Return the displacement between the load address present in the binary
+   --  and the run-time address at which it is loaded (i.e. non-zero for PIE).
+
function String_Length (Str : Str_Access) return Natural;
--  Return the length of the C string Str
 
@@ -74,7 +78,7 @@ package body System.Dwarf_Lines is
 
procedure Read_Aranges_Entry
  (C : in out Dwarf_Context;
-  Start :out Storage_Offset;
+  Start :out Address;
   Len   :out Storage_Count);
--  Read a single .debug_aranges pair
 
@@ -86,7 +90,7 @@ package body System.Dwarf_Lines is
 
procedure Aranges_Lookup
  (C   : in out Dwarf_Context;
-  Addr:Storage_Offset;
+  Addr:Address;
   Info_Offset :out Offset;
   Success :out Boolean);
--  Search for Addr in .debug_aranges and return offset Info_Offset in
@@ -151,7 +155,7 @@ package body System.Dwarf_Lines is
 
procedure Symbolic_Address
  (C   : in out Dwarf_Context;
-  Addr:Storage_Offset;
+  Addr:Address;
   Dir_Name:out Str_Access;
   File_Name   :out Str_Access;
   Subprg_Name :out String_Ptr_Len;
@@ -368,6 +372,19 @@ package body System.Dwarf_Lines is
   end loop;
end For_Each_Row;
 
+   ---
+   -- Get_Load_Displacement --
+   ---
+
+   function Get_Load_Displacement (C : Dwarf_Context) return Storage_Offset is
+   begin
+  if C.Load_Address /= Null_Address then
+ return C.Load_Address - Address (Get_Load_Address (C.Obj.all));
+  else
+ return 0;
+  end if;
+   end Get_Load_Displacement;
+
-
-- Initialize_Pass --
-
@@ -403,18 +420,19 @@ package body System.Dwarf_Lines is
---
 
function Is_Inside (C : Dwarf_Context; Addr : Address) return Boolean is
+  Disp : constant Storage_Offset := Get_Load_Displacement (C);
+
begin
-  return (Addr >= C.Low + C.Load_Address
-and then Addr <= C.High + C.Load_Address);
+  return Addr >= C.Low + Disp and then Addr <= C.High + Disp;
end Is_Inside;
 
-
-- Low_Address --
-
 
-   function Low_Address (C : Dwarf_Context) return System.Address is
+   function Low_Address (C : Dwarf_Context) return Address is
begin
-  return C.Load_Address + C.Low;
+  return C.Low + Get_Load_Displacement (C);
end Low_Address;
 
--
@@ -448,12 +466,12 @@ package body System.Dwarf_Lines is
 
   Success := True;
 
-  --  Get memory bounds for executable code.  Note that such code
+  --  Get address bounds for executable code. Note that such code
   --  might come from multiple sections.
 
   Get_Xcode_Bounds (C.Obj.all, Lo, Hi);
-  C.Low  := Storage_Offset (Lo);
-  C.High := Storage_Offset (Hi);
+  C.Low  := Address (Lo);
+  C.High := 

[Ada] Adjust latest change for ELF platforms

[Pierre-Marie de Rodat]

Shared libraries effectively have a "static" load address of zero in ELF.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/s-objrea.adb (Get_Load_Address): Return 0 for ELF.diff --git a/gcc/ada/libgnat/s-objrea.adb b/gcc/ada/libgnat/s-objrea.adb
--- a/gcc/ada/libgnat/s-objrea.adb
+++ b/gcc/ada/libgnat/s-objrea.adb
@@ -1656,12 +1656,11 @@ package body System.Object_Reader is
 
function Get_Load_Address (Obj : Object_File) return uint64 is
begin
-  if Obj.Format in Any_PECOFF then
- return Obj.ImageBase;
-
-  else
- raise Format_Error with "Get_Load_Address not implemented";
-  end if;
+  case Obj.Format is
+ when ELF=> return 0;
+ when Any_PECOFF => return Obj.ImageBase;
+ when XCOFF32=> raise Format_Error;
+  end case;
end Get_Load_Address;
 
-

[Ada] SPARK proof of the Ada.Strings.Fixed library

[Pierre-Marie de Rodat]

Introduced pragmas to prove with SPARK the behaviours of most of the
functions and procedures from Ada.Strings.Fixed. Procedure Move and all
procedures that rely on it (Insert, Delete, Overwrite, Replace_Slice)
have incomplete contracts and can have runtime errors. Function Count is
given without a postcondition because it would be hard to express, but
absence of runtime errors is ensured.  The private package
Ada.Strings.Search has also been made public, to allow the use of Match
in the contracts of Ada.Strings.Fixed.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/a-strfix.adb ("*"): Added loop invariants and lemmas
for proof.
(Delete): Added assertions for proof, and conditions to avoid
overflow.
(Head): Added loop invariant.
(Insert): Same as Delete.
(Move): Declared with SPARK_Mode Off.
(Overwrite): Added assertions for proof, and conditions to avoid
overflow.
(Replace_Slice): Added assertions for proof, and conditions to
avoid overflow.
(Tail): Added loop invariant and avoided overflows.
(Translate): Added loop invariants.
(Trim): Ensured empty strings returned start at 1.
* libgnat/a-strfix.ads (Index): Rewrote contract cases for
easier proof.
(Index_Non_Blank): Separated the null string case.
(Count): Specified Mapping shouldn't be null.
(Find_Token): Specified Source'First should be Positive when no
From is given.
(Translate): Specified Mapping shouldn't be null.
("*"): Rewrote postcondition for easier proof.
* libgnat/a-strsea.adb (Belongs): Added postcondition.
(Count): Rewrote loops and added loop invariants to avoid
overflows.
(Find_Token): Added loop invariants.
(Index): Rewrote loops to avoid overflows and added loop
invariants for proof.
(Index_Non_Blank): Added loop invariants.
(Is_Identity): New function isolated without SPARK_Mode.
* libgnat/a-strsea.ads: Fix starting comment as package is no
longer private.
(Match): Declared ghost expression function Match.
(Is_Identity): Described identity in the postcondition.
(Index, Index_Non_Blank, Count, Find_Token): Added contract from
a-strfix.ads.

patch.diff.gz
Description: application/gzip

[Ada] Fix repeated generation of dispatch tables in CodePeer mode

[Pierre-Marie de Rodat]

Routine Make_DT that generates dispatch tables for tagged types might be
called twice: when the tagged type is frozen (if it requires freezing)
and once the enclosing package is fully analyzed. The Has_Dispatch_Table
flag on a type prevents dispatch tables being generated twice. However,
this flag was only set in ordinary compilation mode, not in the CodePeer
mode.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_disp.adb (Make_DT): Move call to Set_Has_Dispatch_Table,
so it is executed regardless of the Generate_SCIL mode.diff --git a/gcc/ada/exp_disp.adb b/gcc/ada/exp_disp.adb
--- a/gcc/ada/exp_disp.adb
+++ b/gcc/ada/exp_disp.adb
@@ -6610,7 +6610,6 @@ package body Exp_Disp is
   Append_Elmt (DT, DT_Decl);
 
   Analyze_List (Result, Suppress => All_Checks);
-  Set_Has_Dispatch_Table (Typ);
 
   --  Mark entities containing dispatch tables. Required by the backend to
   --  handle them properly.
@@ -6643,6 +6642,8 @@ package body Exp_Disp is
 
<>
 
+  Set_Has_Dispatch_Table (Typ);
+
   --  Register the tagged type in the call graph nodes table
 
   Register_CG_Node (Typ);

[Ada] Fix condition in op interpretation resolution

[Pierre-Marie de Rodat]

A previous patch fixed crashes on comparisons of string literals with
access to strings by making sure that resolution of operations was only
performed when operand types are actually compatible.

However, the check was incomplete. Indeed, using only
Has_Compatible_Type does not cover the case where the right operand's
type covers the left operand's, which caused programs such as the
following to fail:

procedure tmp is
   type Root is tagged null record;
   type Child is new Root with null record;
   type Grandchild is new Child with null record;

   GC : access Grandchild;
   CC : access Child'Class;
begin
   if GC = CC then
 null;
   end if;
end tmp;

The fix is trivial: when the type of the right operand covers the type
of the left one, allow resolution of the operation.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_ch4.adb (Finc_Non_Universal_Interpretations): Fix check.diff --git a/gcc/ada/sem_ch4.adb b/gcc/ada/sem_ch4.adb
--- a/gcc/ada/sem_ch4.adb
+++ b/gcc/ada/sem_ch4.adb
@@ -6626,7 +6626,7 @@ package body Sem_Ch4 is
Get_Next_Interp (Index, It);
 end loop;
  end if;
-  elsif Has_Compatible_Type (R, T1) then
+  elsif Has_Compatible_Type (R, T1) or else Covers (Etype (R), T1) then
  Add_One_Interp (N, Op_Id, Standard_Boolean, Base_Type (T1));
   end if;
end Find_Non_Universal_Interpretations;

[Ada] Spurious link error with child unit and different Assertion modes.

[Pierre-Marie de Rodat]

This patch fixes a spurious link error on a compilation that involves
a child unit that must be compiled with assertions enabled, and a parent
that is compiled without. The error occurs when the parent includes
instantiations that involve constructs such as predicates or pre/
postconditions, and object declarations for discriminated types with
complex discriminant constraints.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_util.ads (Force_Evaluation): Add formal parameter
Discr_Number, to indicate discriminant expression for which an
external name must be created.
(Remove_Side_Effects): Ditto.
* exp_util.adb (Force_Evaluation): Call Remove_Side_Effects with
added parameter.
(Remove_Side_Effects, Build_Temporary): If Discr_Number is
positive, create an external name with suffix DISCR and the
given discriminant number, analogous to what is done for
temporaries for array type bounds.
* sem_ch3.adb (Process_Discriminant_Expressions): If the
constraint is for an object or component declaration and the
corresponding entity may be visible in another unit, invoke
Force_Evaluation with the new parameter.diff --git a/gcc/ada/exp_util.adb b/gcc/ada/exp_util.adb
--- a/gcc/ada/exp_util.adb
+++ b/gcc/ada/exp_util.adb
@@ -6589,6 +6589,7 @@ package body Exp_Util is
   Related_Id: Entity_Id := Empty;
   Is_Low_Bound  : Boolean   := False;
   Is_High_Bound : Boolean   := False;
+  Discr_Number  : Int   := 0;
   Mode  : Force_Evaluation_Mode := Relaxed)
is
begin
@@ -6600,6 +6601,7 @@ package body Exp_Util is
  Related_Id => Related_Id,
  Is_Low_Bound   => Is_Low_Bound,
  Is_High_Bound  => Is_High_Bound,
+ Discr_Number   => Discr_Number,
  Check_Side_Effects =>
Is_Static_Expression (Exp)
  or else Mode = Relaxed);
@@ -11623,6 +11625,7 @@ package body Exp_Util is
   Related_Id : Entity_Id := Empty;
   Is_Low_Bound   : Boolean   := False;
   Is_High_Bound  : Boolean   := False;
+  Discr_Number   : Int   := 0;
   Check_Side_Effects : Boolean   := True)
is
   function Build_Temporary
@@ -11653,13 +11656,28 @@ package body Exp_Util is
  Temp_Nam : Name_Id;
 
   begin
- --  The context requires an external symbol
+ --  The context requires an external symbol : expression is
+ --  the bound of an array, or a discriminant value. We create
+ --  a unique string using the related entity and an appropriate
+ --  suffix, rather than a numeric serial number (used for internal
+ --  entities) that may vary depending on compilation options, in
+ --  particular on the Assertions_Enabled mode. This avoids spurious
+ --  link errors.
 
  if Present (Related_Id) then
 if Is_Low_Bound then
Temp_Nam := New_External_Name (Chars (Related_Id), "_FIRST");
-else pragma Assert (Is_High_Bound);
+
+elsif Is_High_Bound then
Temp_Nam := New_External_Name (Chars (Related_Id), "_LAST");
+
+else
+   pragma Assert (Discr_Number > 0);
+   --  Use fully qualified name to avoid ambiguities.
+
+   Temp_Nam :=
+  New_External_Name
+   (Get_Qualified_Name (Related_Id), "_DISCR", Discr_Number);
 end if;
 
 Temp_Id := Make_Defining_Identifier (Loc, Temp_Nam);


diff --git a/gcc/ada/exp_util.ads b/gcc/ada/exp_util.ads
--- a/gcc/ada/exp_util.ads
+++ b/gcc/ada/exp_util.ads
@@ -668,6 +668,7 @@ package Exp_Util is
   Related_Id: Entity_Id := Empty;
   Is_Low_Bound  : Boolean   := False;
   Is_High_Bound : Boolean   := False;
+  Discr_Number  : Int   := 0;
   Mode  : Force_Evaluation_Mode := Relaxed);
--  Force the evaluation of the expression right away. Similar behavior
--  to Remove_Side_Effects when Variable_Ref is set to TRUE. That is to
@@ -688,6 +689,12 @@ package Exp_Util is
--  of the Is_xxx_Bound flags must be set. For use of these parameters see
--  the warning in the body of Sem_Ch3.Process_Range_Expr_In_Decl.
 
+   --  Discr_Number is positive when the expression is a discriminant value
+   --  in an object or component declaration. In that case Discr_Number is
+   --  the position of the corresponding discriminant in the corresponding
+   --  type declaration, and the name for the evaluated expression is built
+   --  out of the Related_Id and the Discr_Number.
+
function Fully_Qualified_Name_String
  (E  : Entity_Id;
   Append_NUL : Boolean := True) return String_Id;
@@ -1004,6 +1011,7 @@ package Exp_Util is
   Related_Id : Entity_Id := Empty;
   Is_Low_Bound   : Boolean   := False;
   Is_High_Bound  : 

[Ada] Refine types of local constants that store Etype results

[Pierre-Marie de Rodat]

Calls to Etype return entities, even though the signature of the Etype
routine says it returns nodes. Fixed automatically with:

  $ sed -i 's/ Node_Id := Etype/ Entity_Id := Etype/' *.adb

Found while reviewing changes in GNATprove related to aliasing checks.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_aggr.adb, exp_ch4.adb, exp_ch5.adb, sprint.adb: Refine
types of local constants.diff --git a/gcc/ada/exp_aggr.adb b/gcc/ada/exp_aggr.adb
--- a/gcc/ada/exp_aggr.adb
+++ b/gcc/ada/exp_aggr.adb
@@ -4003,7 +4003,7 @@ package body Exp_Aggr is
  and then Present (First_Index (Etype (Expr_Q)))
then
   declare
- Expr_Q_Type : constant Node_Id := Etype (Expr_Q);
+ Expr_Q_Type : constant Entity_Id := Etype (Expr_Q);
   begin
  Append_List_To (L,
Build_Array_Aggr_Code


diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb
--- a/gcc/ada/exp_ch4.adb
+++ b/gcc/ada/exp_ch4.adb
@@ -7763,8 +7763,8 @@ package body Exp_Ch4 is
 
  if Is_Unchecked_Union (Op_Type) then
 declare
-   Lhs_Type : constant Node_Id := Etype (L_Exp);
-   Rhs_Type : constant Node_Id := Etype (R_Exp);
+   Lhs_Type : constant Entity_Id := Etype (L_Exp);
+   Rhs_Type : constant Entity_Id := Etype (R_Exp);
 
Lhs_Discr_Vals : Elist_Id;
--  List of inferred discriminant values for left operand.


diff --git a/gcc/ada/exp_ch5.adb b/gcc/ada/exp_ch5.adb
--- a/gcc/ada/exp_ch5.adb
+++ b/gcc/ada/exp_ch5.adb
@@ -742,8 +742,8 @@ package body Exp_Ch5 is
   --  in the front end.
 
   declare
- L_Index_Typ : constant Node_Id := Etype (First_Index (L_Type));
- R_Index_Typ : constant Node_Id := Etype (First_Index (R_Type));
+ L_Index_Typ : constant Entity_Id := Etype (First_Index (L_Type));
+ R_Index_Typ : constant Entity_Id := Etype (First_Index (R_Type));
 
  Left_Lo  : constant Node_Id := Type_Low_Bound  (L_Index_Typ);
  Left_Hi  : constant Node_Id := Type_High_Bound (L_Index_Typ);
@@ -1382,8 +1382,8 @@ package body Exp_Ch5 is
 
   Loc  : constant Source_Ptr := Sloc (N);
 
-  L_Index_Typ : constant Node_Id := Etype (First_Index (L_Type));
-  R_Index_Typ : constant Node_Id := Etype (First_Index (R_Type));
+  L_Index_Typ : constant Entity_Id := Etype (First_Index (L_Type));
+  R_Index_Typ : constant Entity_Id := Etype (First_Index (R_Type));
   Left_Lo  : constant Node_Id := Type_Low_Bound  (L_Index_Typ);
   Right_Lo : constant Node_Id := Type_Low_Bound  (R_Index_Typ);
 


diff --git a/gcc/ada/sprint.adb b/gcc/ada/sprint.adb
--- a/gcc/ada/sprint.adb
+++ b/gcc/ada/sprint.adb
@@ -4222,7 +4222,7 @@ package body Sprint is
  --  Itype to be printed
 
  declare
-B : constant Node_Id := Etype (Typ);
+B : constant Entity_Id := Etype (Typ);
 P : constant Node_Id := Parent (Typ);
 S : constant Saved_Output_Buffer := Save_Output_Buffer;
 --  Save current output buffer

[Ada] Implementation of Preelaborable_Initialization attribute for AI12-0409

[Pierre-Marie de Rodat]

This set of changes implements the Preelaborable_Initialization
attribute, corresponding to the existing aspect/pragma, as defined by
AI12-0409 (RM2022 10.2.1(11.6/5-11.8/5). This includes semantic checking
of restrictions on the prefix, and support for the aspect expression
being given by an expression with one or more P_I attributes applied to
formal private or derived types, when the type with the aspect is
specified on types within a generic package declaration (the value of
the aspect in instantiations can be different depending on the actual
types), as well as applying preelaborable-initialization restrictions on
full types when the partial type has such aspects.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_attr.adb (Expand_N_Attribute_Reference): Fold
Preelaborable_Initialization attribute in cases where it hasn't
been folded by the analyzer.
* exp_disp.adb (Original_View_In_Visible_Part): This function is
removed and moved to sem_util.adb.
* sem_attr.adb (Attribute_22): Add
Attribute_Preelaborable_Initialization as an Ada 2022 attribute.
(Analyze_Attribute, Attribute_Preelaborable_Initialization):
Check that the prefix of the attribute is either a formal
private or derived type, or a composite type declared within the
visible part of a package or generic package.
(Eval_Attribute): Perform folding of
Preelaborable_Initialization attribute based on
Has_Preelaborable_Initialization applied to the prefix type.
* sem_ch3.adb (Resolve_Aspects): Add specialized code for
Preelaborable_Initialization used at the end of a package
visible part for setting Known_To_Have_Preelab_Init on types
that are specified with True or that have a conjunction of one
or more P_I attributes applied to formal types.
* sem_ch7.adb (Analyze_Package_Specification): On call to
Has_Preelaborable_Initialization, pass True for new formal
Formal_Types_Have_Preelab_Init, so that error checking treats
subcomponents that are declared within types in generics as
having preelaborable initialization when the subcomponents are
of formal types.
* sem_ch13.adb (Analyze_Aspects_At_Freeze_Point): Add test for
P_I to prevent calling Make_Pragma_From_Boolean_Aspect, since
this aspect is handled specially and the
Known_To_Have_Preelab_Init flag will get set on types that have
the aspect by other means.
(Analyze_Aspect_Specifications.Analyze_One_Aspect): Add test for
Aspect_Preelaborable_Initialization for allowing the aspect to
be specified on formal type declarations.
(Is_Operational_Item): Treat Attribute_Put_Image as an
operational attribute.  The need for this was encountered while
working on these changes.
* sem_util.ads (Has_Preelaborable_Initialization): Add
Formal_Types_Have_Preelab_Init as a new formal parameter that
defaults to False.
(Is_Conjunction_Of_Formal_Preelab_Init_Attributes): New
function.
(Original_View_In_Visible_Part): Moved here from exp_disp.adb,
so it can be called by Analyze_Attribute.
* sem_util.adb (Has_Preelaborable_Initialization): Return True
for formal private and derived types when new formal
Formal_Types_Have_Preelab_Init is True, and pass along the
Formal_Types_Have_Preelab_Init flag in the array component case.
(Check_Components): Pass along Formal_Types_Have_Preelab_Init
flag on call to Has_Preelaborable_Initialization.
(Is_Conjunction_Of_Formal_Preelab_Init_Attributes): New function
that returns True when passed an expression that includes one or
more attributes for Preelaborable_Initialization applied to
prefixes that denote formal types.
(Is_Formal_Preelab_Init_Attribute): New utility function nested
within Is_Conjunction_Of_Formal_Preelab_Init_Attributes that
determines whether a node is a P_I attribute applied to a
generic formal type.
(Original_View_In_Visible_Part): Moved here from exp_util.adb,
so it can be called by Analyze_Attribute.
* snames.ads-tmpl: Add note near the start of spec giving
details about what needs to be done when adding a name that
corresponds to both an attribute and a pragma.  Delete existing
occurrence of Name_Preelaborable_Initialization, and add a note
comment in the list of Name_* constants at that place,
indicating that it's included in type Pragma_Id, etc., echoing
other such comments for names that are both an attribute and a
pragma.  Insert Name_Preelaborable_Initialization in the
alphabetized set of Name_* constants corresponding to
attributes (between First_Attribute_Name and
Last_Attribute_Name).
 

[Ada] Small cleanup in System.Dwarf_Line

[Pierre-Marie de Rodat]

The unit has got "with" and "use" clauses both for Ada.Exceptions.Traceback
and System.Traceback_Entries, but the former is essentially a forwarder for
the latter so can be eliminated.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* libgnat/s-dwalin.ads: Remove clause for Ada.Exceptions.Traceback,
add clause for System.Traceback_Entries and alphabetize.
(AET): Delete.
(STE): New package renaming.
(Symbolic_Traceback): Adjust.
* libgnat/s-dwalin.adb: Remove clauses for Ada.Exceptions.Traceback
and System.Traceback_Entries.
(Symbolic_Traceback): Adjust.diff --git a/gcc/ada/libgnat/s-dwalin.adb b/gcc/ada/libgnat/s-dwalin.adb
--- a/gcc/ada/libgnat/s-dwalin.adb
+++ b/gcc/ada/libgnat/s-dwalin.adb
@@ -31,7 +31,6 @@
 
 with Ada.Characters.Handling;
 with Ada.Containers.Generic_Array_Sort;
-with Ada.Exceptions.Traceback; use Ada.Exceptions.Traceback;
 with Ada.Unchecked_Deallocation;
 
 with Interfaces; use Interfaces;
@@ -42,7 +41,6 @@ with System.Bounded_Strings;   use System.Bounded_Strings;
 with System.IO;use System.IO;
 with System.Mmap;  use System.Mmap;
 with System.Object_Reader; use System.Object_Reader;
-with System.Traceback_Entries; use System.Traceback_Entries;
 with System.Storage_Elements;  use System.Storage_Elements;
 
 package body System.Dwarf_Lines is
@@ -1864,7 +1862,7 @@ package body System.Dwarf_Lines is
 
procedure Symbolic_Traceback
  (Cin  :Dwarf_Context;
-  Traceback:AET.Tracebacks_Array;
+  Traceback:STE.Tracebacks_Array;
   Suppress_Hex :Boolean;
   Symbol_Found :out Boolean;
   Res  : in out System.Bounded_Strings.Bounded_String)
@@ -1893,7 +1891,7 @@ package body System.Dwarf_Lines is
  --  If the buffer is full, no need to do any useless work
  exit when Is_Full (Res);
 
- Addr_In_Traceback := PC_For (Traceback (J));
+ Addr_In_Traceback := STE.PC_For (Traceback (J));
 
  Offset_To_Lookup := Addr_In_Traceback - C.Load_Address;
 


diff --git a/gcc/ada/libgnat/s-dwalin.ads b/gcc/ada/libgnat/s-dwalin.ads
--- a/gcc/ada/libgnat/s-dwalin.ads
+++ b/gcc/ada/libgnat/s-dwalin.ads
@@ -35,15 +35,14 @@
 --
 --  Files must be compiled with at least minimal debugging information (-g1).
 
-with Ada.Exceptions.Traceback;
-
+with System.Bounded_Strings;
 with System.Object_Reader;
 with System.Storage_Elements;
-with System.Bounded_Strings;
+with System.Traceback_Entries;
 
 package System.Dwarf_Lines is
 
-   package AET renames Ada.Exceptions.Traceback;
+   package STE renames System.Traceback_Entries;
package SOR renames System.Object_Reader;
 
type Dwarf_Context (In_Exception : Boolean := False) is private;
@@ -83,7 +82,7 @@ package System.Dwarf_Lines is
 
procedure Symbolic_Traceback
  (Cin  :Dwarf_Context;
-  Traceback:AET.Tracebacks_Array;
+  Traceback:STE.Tracebacks_Array;
   Suppress_Hex :Boolean;
   Symbol_Found :out Boolean;
   Res  : in out System.Bounded_Strings.Bounded_String);

[Ada] Work around CodePeer bug by declaring variable

[Pierre-Marie de Rodat]

This commit works around a CodePeer bug where CodePeer thinks
Get_32_Bit_Val returns something uninitialized.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* atree.adb (Get_32_Bit_Field): Declare result before returning.diff --git a/gcc/ada/atree.adb b/gcc/ada/atree.adb
--- a/gcc/ada/atree.adb
+++ b/gcc/ada/atree.adb
@@ -513,8 +513,13 @@ package body Atree is
 
  function Cast is new
Unchecked_Conversion (Field_Size_32_Bit, Field_Type);
+
+ Result : constant Field_Type := Cast (Get_32_Bit_Val (N, Offset));
+ --  Note: declaring Result here instead of directly returning
+ --  Cast (...) helps CodePeer understand that there are no issues
+ --  around uninitialized variables.
   begin
- return Cast (Get_32_Bit_Val (N, Offset));
+ return Result;
   end Get_32_Bit_Field;
 
   function Get_32_Bit_Field_With_Default

[Ada] Don't examine all discriminants when looking for the first one

[Pierre-Marie de Rodat]

A minor performance improvement; semantics is unaffected.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_ch3.adb (Build_Discriminant_Constraints): Exit once a
first discriminant is found and the Discrim_Present flag is set.diff --git a/gcc/ada/sem_ch3.adb b/gcc/ada/sem_ch3.adb
--- a/gcc/ada/sem_ch3.adb
+++ b/gcc/ada/sem_ch3.adb
@@ -10392,6 +10392,7 @@ package body Sem_Ch3 is
   (Discr_Expr (J), Check_Concurrent => True)
  then
 Discrim_Present := True;
+exit;
  end if;
   end loop;
 

[Ada] Fix assertion in GNATprove_Mode

[Pierre-Marie de Rodat]

Avoid calling List_Rep_Info in Generate_SCIL and GNATprove_Mode, because
the representation info is not there. Otherwise, we fail an assertion.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* gnat1drv.adb (Gnat1drv): Avoid calling List_Rep_Info in
Generate_SCIL and GNATprove_Mode.
* repinfo.adb (List_Common_Type_Info): Fix comment.diff --git a/gcc/ada/gnat1drv.adb b/gcc/ada/gnat1drv.adb
--- a/gcc/ada/gnat1drv.adb
+++ b/gcc/ada/gnat1drv.adb
@@ -1616,7 +1616,14 @@ begin
 
   Errout.Finalize (Last_Call => True);
   Errout.Output_Messages;
-  Repinfo.List_Rep_Info (Ttypes.Bytes_Big_Endian);
+
+  --  Back annotation of representation info is not done in CodePeer and
+  --  SPARK modes.
+
+  if not (Generate_SCIL or GNATprove_Mode) then
+ Repinfo.List_Rep_Info (Ttypes.Bytes_Big_Endian);
+  end if;
+
   Inline.List_Inlining_Info;
 
   --  Only write the library if the backend did not generate any error


diff --git a/gcc/ada/repinfo.adb b/gcc/ada/repinfo.adb
--- a/gcc/ada/repinfo.adb
+++ b/gcc/ada/repinfo.adb
@@ -422,7 +422,8 @@ package body Repinfo is
 Write_Line (";");
  end if;
 
-  --  Alignment is not always set for task and protected types
+  --  Alignment is not always set for task, protected, and class-wide
+  --  types.
 
   else
  pragma Assert

[Ada] Spurious accessibility error on allocator in generic instance

[Pierre-Marie de Rodat]

This patch fixes an error in the compiler whereby an allocator for a
limited type within a generic instance may cause spurious compile-time
warnings and run-time errors.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_ch4.adb (Expand_N_Type_Conversion): Add guard to protect
against calculating accessibility levels against internal
compiler-generated types.diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb
--- a/gcc/ada/exp_ch4.adb
+++ b/gcc/ada/exp_ch4.adb
@@ -12361,10 +12361,16 @@ package body Exp_Ch4 is
  --  an instantiation, otherwise the conversion will already have been
  --  rejected as illegal.
 
- --  Note: warnings are issued by the analyzer for the instance cases
+ --  Note: warnings are issued by the analyzer for the instance cases,
+ --  and, since we are late in expansion, a check is performed to
+ --  verify that neither the target type nor the operand type are
+ --  internally generated - as this can lead to spurious errors when,
+ --  for example, the operand type is a result of BIP expansion.
 
  elsif In_Instance_Body
and then Statically_Deeper_Relation_Applies (Target_Type)
+   and then not Is_Internal (Target_Type)
+   and then not Is_Internal (Operand_Type)
and then
  Type_Access_Level (Operand_Type) > Type_Access_Level (Target_Type)
  then

[Ada] Refactor scan_backend_switch to share logic across backends

[Pierre-Marie de Rodat]

This commit refactors scan_backend_switch to share logic across
adabkend.adb and back_end.adb. A side effect of this refactor is that
`-fdump-diagnostics-format` is now available with other backends.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* adabkend.adb (Scan_Back_End_Switches): Replace switch-scanning
logic with call to Backend_Utils.Scan_Common_Back_End_Switches.
* back_end.adb (Scan_Back_End_Switches): Replace switch-scanning
logic with call to Backend_Utils.Scan_Common_Back_End_Switches.
* backend_utils.adb: New file.
* backend_utils.ads: New file.
* gcc-interface/Make-lang.in: Add ada/backend_utils.o.diff --git a/gcc/ada/adabkend.adb b/gcc/ada/adabkend.adb
--- a/gcc/ada/adabkend.adb
+++ b/gcc/ada/adabkend.adb
@@ -22,15 +22,16 @@
 
 --  This is the version of the Back_End package for back ends written in Ada
 
-with Atree;use Atree;
+with Atree; use Atree;
+with Backend_Utils; use Backend_Utils;
 with Debug;
 with Lib;
-with Opt;  use Opt;
-with Output;   use Output;
-with Osint;use Osint;
-with Osint.C;  use Osint.C;
-with Switch.C; use Switch.C;
-with Types;use Types;
+with Opt;   use Opt;
+with Output;use Output;
+with Osint; use Osint;
+with Osint.C;   use Osint.C;
+with Switch.C;  use Switch.C;
+with Types; use Types;
 
 with System.OS_Lib; use System.OS_Lib;
 
@@ -182,48 +183,11 @@ package body Adabkend is
 
 return;
 
- --  Special check, the back-end switch -fno-inline also sets the
- --  front end flags to entirely inhibit all inlining. So we store it
- --  and set the appropriate flags.
-
- elsif Switch_Chars (First .. Last) = "fno-inline" then
-Lib.Store_Compilation_Switch (Switch_Chars);
-Opt.Disable_FE_Inline := True;
-return;
-
- --  Similar processing for -fpreserve-control-flow
-
- elsif Switch_Chars (First .. Last) = "fpreserve-control-flow" then
-Lib.Store_Compilation_Switch (Switch_Chars);
-Opt.Suppress_Control_Flow_Optimizations := True;
-return;
-
- --  Recognize -gxxx switches
-
- elsif Switch_Chars (First) = 'g' then
-Debugger_Level := 2;
-
-if First < Last then
-   case Switch_Chars (First + 1) is
-  when '0' =>
- Debugger_Level := 0;
-  when '1' =>
- Debugger_Level := 1;
-  when '2' =>
- Debugger_Level := 2;
-  when '3' =>
- Debugger_Level := 3;
-  when others =>
- null;
-   end case;
-end if;
-
- elsif Switch_Chars (First .. Last) = "S" then
-Generate_Asm := True;
-
  --  Ignore all other back-end switches
 
- elsif Is_Back_End_Switch (Switch_Chars) then
+ elsif Scan_Common_Back_End_Switch (Switch_Chars)
+or else Is_Back_End_Switch (Switch_Chars)
+ then
 null;
 
  --  Give error for junk switch


diff --git a/gcc/ada/back_end.adb b/gcc/ada/back_end.adb
--- a/gcc/ada/back_end.adb
+++ b/gcc/ada/back_end.adb
@@ -25,23 +25,24 @@
 
 --  This is the version of the Back_End package for GCC back ends
 
-with Atree;use Atree;
-with Debug;use Debug;
-with Elists;   use Elists;
-with Errout;   use Errout;
-with Lib;  use Lib;
-with Osint;use Osint;
-with Opt;  use Opt;
-with Osint.C;  use Osint.C;
-with Namet;use Namet;
-with Nlists;   use Nlists;
-with Stand;use Stand;
-with Sinput;   use Sinput;
-with Stringt;  use Stringt;
-with Switch;   use Switch;
-with Switch.C; use Switch.C;
-with System;   use System;
-with Types;use Types;
+with Atree; use Atree;
+with Backend_Utils; use Backend_Utils;
+with Debug; use Debug;
+with Elists;use Elists;
+with Errout;use Errout;
+with Lib;   use Lib;
+with Osint; use Osint;
+with Opt;   use Opt;
+with Osint.C;   use Osint.C;
+with Namet; use Namet;
+with Nlists;use Nlists;
+with Stand; use Stand;
+with Sinput;use Sinput;
+with Stringt;   use Stringt;
+with Switch;use Switch;
+with Switch.C;  use Switch.C;
+with System;use System;
+with Types; use Types;
 
 with System.OS_Lib; use System.OS_Lib;
 
@@ -266,52 +267,20 @@ package body Back_End is
  --  specific switches that the Ada front-end knows about.
 
  else
-Store_Compilation_Switch (Switch_Chars);
-
---  For gcc back ends, -fno-inline disables Inline pragmas only,
---  not Inline_Always to remain consistent with the always_inline
---  attribute behavior.
-
-if Switch_Chars (First .. Last) = "fno-inline" then
-   Opt.Disable_FE_Inline := True;
-
-  

[Ada] Only assign type to op if compatible

[Pierre-Marie de Rodat]

Before this commit, the following program would make the compiler crash:

procedure Main is
   ConstantString1 : aliased String := "Class1";
   My_Access : access String := ConstantString1'Access;
begin
   if "Class1" = My_Access then
  null;
   end if;
end Main;

This was because when an access type was given on the right side of an
operator, GNAT assumed that an interpretation for the operator existed.
This assumption resulted in no error being thrown and Gigi crashing when
encountering the malformed tree.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_ch4.adb (Find_Non_Universal_Interpretations): Check if
types are compatible before adding interpretation.diff --git a/gcc/ada/sem_ch4.adb b/gcc/ada/sem_ch4.adb
--- a/gcc/ada/sem_ch4.adb
+++ b/gcc/ada/sem_ch4.adb
@@ -6626,7 +6626,7 @@ package body Sem_Ch4 is
Get_Next_Interp (Index, It);
 end loop;
  end if;
-  else
+  elsif Has_Compatible_Type (R, T1) then
  Add_One_Interp (N, Op_Id, Standard_Boolean, Base_Type (T1));
   end if;
end Find_Non_Universal_Interpretations;

[Ada] Move Build_And_Insert_Cuda_Initialization to Expand_CUDA_Package

[Pierre-Marie de Rodat]

This commit makes Build_And_Insert_Cuda_Initialization an internal
procedure and creates a new Expand_CUDA_Package procedure which calls
Build_And_Insert_Cuda_Initialization.

This is a small, self-contained refactoring that does not impact any
feature or fix any bug - it just makes future commits that do add new
features smaller and easier to review.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* exp_ch7.adb (Expand_N_Package_Body): Replace
Build_And_Insert_Cuda_Initialization with Expand_CUDA_Package.
* gnat_cuda.adb (Expand_CUDA_Package): New procedure.
(Build_And_Insert_Cuda_Initialization): Make internal.
* gnat_cuda.ads (Expand_CUDA_Package): New procedure.
(Build_And_Insert_Cuda_Initialization): Remove from spec.diff --git a/gcc/ada/exp_ch7.adb b/gcc/ada/exp_ch7.adb
--- a/gcc/ada/exp_ch7.adb
+++ b/gcc/ada/exp_ch7.adb
@@ -5918,12 +5918,7 @@ package body Exp_Ch7 is
 Build_Static_Dispatch_Tables (N);
  end if;
 
- --  If procedures marked with CUDA_Global have been defined within N,
- --  we need to register them with the CUDA runtime at program startup.
- --  This requires multiple declarations and function calls which need
- --  to be appended to N's declarations.
-
- Build_And_Insert_CUDA_Initialization (N);
+ Expand_CUDA_Package (N);
 
  Build_Task_Activation_Call (N);
 


diff --git a/gcc/ada/gnat_cuda.adb b/gcc/ada/gnat_cuda.adb
--- a/gcc/ada/gnat_cuda.adb
+++ b/gcc/ada/gnat_cuda.adb
@@ -66,6 +66,25 @@ package body GNAT_CUDA is
--  least one procedure marked with aspect CUDA_Global. The values are
--  Elists of the marked procedures.
 
+   procedure Build_And_Insert_CUDA_Initialization (N : Node_Id);
+   --  Builds declarations necessary for CUDA initialization and inserts them
+   --  in N, the package body that contains CUDA_Global nodes. These
+   --  declarations are:
+   --
+   --* A symbol to hold the pointer P to the CUDA fat binary.
+   --
+   --* A type definition T for a wrapper that contains the pointer to the
+   --  CUDA fat binary.
+   --
+   --* An object of the aforementioned type to hold the aforementioned
+   --  pointer.
+   --
+   --* For each CUDA_Global procedure in the package, a declaration of a C
+   --  string containing the function's name.
+   --
+   --* A procedure that takes care of calling CUDA functions that register
+   --  CUDA_Global procedures with the runtime.
+
function Get_CUDA_Kernels (Pack_Id : Entity_Id) return Elist_Id;
--  Returns an Elist of all procedures marked with pragma CUDA_Global that
--  are declared within package body Pack_Body. Returns No_Elist if Pack_Id
@@ -94,6 +113,23 @@ package body GNAT_CUDA is
   Append_Elmt (Kernel, Kernels);
end Add_CUDA_Kernel;
 
+   procedure Expand_CUDA_Package (N : Node_Id) is
+   begin
+
+  --  If not compiling for the host, do not do anything.
+
+  if not Debug_Flag_Underscore_C then
+ return;
+  end if;
+
+  --  If procedures marked with CUDA_Global have been defined within N,
+  --  we need to register them with the CUDA runtime at program startup.
+  --  This requires multiple declarations and function calls which need
+  --  to be appended to N's declarations.
+
+  Build_And_Insert_CUDA_Initialization (N);
+   end Expand_CUDA_Package;
+
--
-- Hash --
--
@@ -524,7 +560,7 @@ package body GNAT_CUDA is
--  Start of processing for Build_And_Insert_CUDA_Initialization
 
begin
-  if CUDA_Node_List = No_Elist or not Debug_Flag_Underscore_C then
+  if CUDA_Node_List = No_Elist then
  return;
   end if;
 


diff --git a/gcc/ada/gnat_cuda.ads b/gcc/ada/gnat_cuda.ads
--- a/gcc/ada/gnat_cuda.ads
+++ b/gcc/ada/gnat_cuda.ads
@@ -82,26 +82,8 @@ package GNAT_CUDA is
--  Kernel is a procedure entity marked with CUDA_Global, Pack_Id is the
--  entity of its parent package body.
 
-   procedure Build_And_Insert_CUDA_Initialization (N : Node_Id);
-   --  Builds declarations necessary for CUDA initialization and inserts them
-   --  in N, the package body that contains CUDA_Global nodes. These
-   --  declarations are:
-   --
-   --* A symbol to hold the pointer to the CUDA fat binary
-   --
-   --* A type definition for a wrapper that contains the pointer to the
-   --  CUDA fat binary
-   --
-   --* An object of the aforementioned type to hold the aforementioned
-   --  pointer.
-   --
-   --* For each CUDA_Global procedure in the package, a declaration of a C
-   --  string containing the function's name.
-   --
-   --* A function that takes care of calling CUDA functions that register
-   --  CUDA_Global procedures with the runtime.
-   --
-   --* A boolean that holds the result of the call to the aforementioned
-   --  function.
+   procedure Expand_CUDA_Package (N : 

[Ada] usage.adb: make -gnatw.c description clearer

[Pierre-Marie de Rodat]

The term "unrepped" can be hard to understand for users.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* usage.adb (Usage): Update -gnatw.c messages.diff --git a/gcc/ada/usage.adb b/gcc/ada/usage.adb
--- a/gcc/ada/usage.adb
+++ b/gcc/ada/usage.adb
@@ -483,8 +483,10 @@ begin
Write_Line (".B   turn off warnings for biased representation");
Write_Line ("c+   turn on warnings for constant conditional");
Write_Line ("C*   turn off warnings for constant conditional");
-   Write_Line (".c+  turn on warnings for unrepped components");
-   Write_Line (".C*  turn off warnings for unrepped components");
+   Write_Line (".c+  turn on warnings for components without " &
+ "representation clauses");
+   Write_Line (".C*  turn off warnings for components without " &
+ "representation clauses");
Write_Line ("_c*  turn on warnings for unknown " &
  "Compile_Time_Warning");
Write_Line ("_C   turn off warnings for unknown " &

[Ada] Remove inappropriate test from Is_By_Reference_Type

[Pierre-Marie de Rodat]

The result returned by the predicate may change depending on whether an
error was posted on the type, which complicates further error reporting.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

* sem_aux.adb (Is_By_Reference_Type): Do not test Error_Posted.diff --git a/gcc/ada/sem_aux.adb b/gcc/ada/sem_aux.adb
--- a/gcc/ada/sem_aux.adb
+++ b/gcc/ada/sem_aux.adb
@@ -846,10 +846,7 @@ package body Sem_Aux is
   Btype : constant Entity_Id := Base_Type (Ent);
 
begin
-  if Error_Posted (Ent) or else Error_Posted (Btype) then
- return False;
-
-  elsif Is_Private_Type (Btype) then
+  if Is_Private_Type (Btype) then
  declare
 Utyp : constant Entity_Id := Underlying_Type (Btype);
  begin

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Qing Zhao via Gcc-patches]

> On Sep 20, 2021, at 5:43 AM, Richard Earnshaw  
> wrote:
> 
> 
> 
> On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:
>> Hi,
>> There are much less issues with aarch64/auto-init-* test cases.
>> Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
>> ‘armv8-r’) do not change the pattern match.
>> Only
>> 1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
>> “auto-init-padding-5.c”.
>> 2. -fstack-clash-protection impact two of the testing cases “auto-init-1.c” 
>> and “auto-init-7.c”.
>> Naturally the patch for this set is:
>> A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
>> “auto-init-padding-5.c”;
>> B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”
>> The above A fixed issue 1, however, the above B did not fix issue 2.
>> If I fixed “auto-init-1.c” as:
>> diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
>> b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>> index 0fa4708..a38d91b 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>> @@ -1,6 +1,6 @@
>>  /* Verify zero initialization for integer and pointer type automatic 
>> variables.  */
>>  /* { dg-do compile } */
>> -/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
>> +/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
>> -fno-stack-protector" } */
>>#ifndef __cplusplus
>>  # define bool _Bool
>> So, I took a look at the log file of the testing, and found that, If I 
>> tested it as:
>>  make check-gcc 
>> RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
>>  aarch64.exp=auto-init*’
>> In the log file, I got:
>> /home/qinzhao/Work/GCC/build_git/gcc/xgcc 
>> -B/home/qinzhao/Work/GCC/build_git/gcc/ 
>> /home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
>>   -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero -fdump-rtl-expand 
>> -fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 
>> -fstack-clash-protection -fstack-protector-all  -o auto-init-1.s
>> From it, we can see, that the options that were passed through RUNTESTFLAGS 
>> “mabi-lp64 -fstack-clash-protection -fstack-protector-all” were appended 
>> AFTER the options inside the testing case through “dg-options”. As a result, 
>> the option “-fno-stack-protector” did not have any impact at all.
>> What’s the expected behavior for the order of these options? Should options 
>> through RUNTESTFLAGS be appended BEFORE or AFTER the options through testing 
>> cases?
>> For X86, the options through RUNTESTFLAGS are added BEFORE the options 
>> through testing cases. Therefore adding “-fno-stack-protector” has the 
>> expected result.
> 
> Can you check that you are using the same version of dejagnu on both 
> platforms?

Stupid question:  how to check the version of it?

Qing
> 
> R.
> 
>> Is this a bug in aarch64 testing suite?
>> Thanks.
>> Qing

Re: [PATCH] [og10] OpenACC: Shared memory layout optimisation

[Julian Brown]

On Fri, 17 Sep 2021 16:26:50 +0200
Thomas Schwinge  wrote:

> > @@ -1449,8 +1634,120 @@ oacc_do_neutering (void)  
> 
> > +  addr_range ar
> > + = first_fit_range (conflicts, size, align,
> > _shm_bounds); +
> > +  splay_tree_delete (conflicts);
> > +
> > +  if (ar.invalid ())
> > + {
> > +   unsigned HOST_WIDE_INT base;
> > +   base = bounds_lo + random () % 512;
> > +   base = (base + align - 1) & ~(align - 1);
> > +   if (base + size > bounds_hi)
> > + error_at (UNKNOWN_LOCATION, "shared-memory region
> > overflow");  
> 
> My dice doesn't have 512 faces -- what am I to read into the
> expression 'random () % 512' here?  (Surely this must offend the
> folks of .)  ;-)

For this one -- the randomness shouldn't be necessary for the
correctness of the patch (i.e. it could just be "base = bounds_lo", or
indeed folded into the line after).

The "ar.invalid ()" case happens when we fail to allocate a block of
memory in LDS space for broadcasting a particular set of variables,
and trigger a fall-back path in the broadcasting code that adds extra
barriers around the broadcast in question. I imagine I was thinking
that adding randomness could mean we can "get lucky" sometimes and
avoid needing those barriers in some cases, but in fact I don't think
that was implemented, so the randomness is useless. (Or it could just
have been leftover debug code... oops).

Thanks,

Julian

Re: [PATCH] analyzer: Impose recursion limit on indirect calls.

[Ankur Saini via Gcc-patches]

[ Sorry for very late response ]

> On 27-Aug-2021, at 6:28 PM, Martin Liška  wrote:
> 
> On 8/27/21 14:44, Ankur Saini wrote:
>> While working on patch to convert most of the 8 whitespace characters to 
>> tabs in the analyzer’s source, I see this weird behaviour where at some 
>> places indentation looks weird when viewed in patch file ( generated via 
>> “git format-patch” )  but looks alright when viewed in text editor.
> 
> Yes, I can confirm the patch fixed the white spaces, you can verify that with:
> $ git diff | ./contrib/check_GNU_style.py -
> ...
> 
> Note there are some other formatting issues reported by the script.

Yes, I see that too, and have fixed most of them 

Unfortunately, I can’t seem to find a way to avoid the following report.
---
=== ERROR type #1: lines should not exceed 80 characters (1 error(s)) ===
gcc/analyzer/diagnostic-manager.cc:2152:80:= 
cg_superedge.map_expr_from_caller_to_callee (caller_var,
—

Also, does these kinds of patches require some special changelog or should I 
proceed in usual fashion ?

==
Here is updated patch : 

==

Thanks
- Ankur 

[PATCH] Allow different vector types for stmt groups

[Richard Biener via Gcc-patches]

This allows vectorization (in practice non-loop vectorization) to
have a stmt participate in different vector type vectorizations.
It allows us to remove vect_update_shared_vectype and replace it
by pushing/popping STMT_VINFO_VECTYPE from SLP_TREE_VECTYPE around
vect_analyze_stmt and vect_transform_stmt.

For data-ref the situation is a bit more complicated since we
analyze alignment info with a specific vector type in mind which
doesn't play well when that changes.

So the bulk of the change is passing down the actual vector type
used for a vectorized access to the various accessors of alignment
info, first and foremost dr_misalignment but also aligned_access_p,
known_alignment_for_access_p, vect_known_alignment_in_bytes and
vect_supportable_dr_alignment.  I took the liberty to replace
ALL_CAPS macro accessors with the lower-case function invocations.

The actual changes to the behavior are in dr_misalignment which now
is the place factoring in the negative step adjustment as well as
handling alignment queries for a vector type with bigger alignment
requirements than what we can (or have) analyze(d).

vect_slp_analyze_node_alignment makes use of this and upon receiving
a vector type with a bigger alingment desire re-analyzes the DR
with respect to it but keeps an older more precise result if possible.
In this context it might be possible to do the analysis just once
but instead of analyzing with respect to a specific desired alignment
look for the biggest alignment we can compute a not unknown alignment.

The ChangeLog includes the functional changes but not the bulk due
to the alignment accessor API changes - I hope that's something good.

Bootstrapped and tested on x86_64-unknown-linux-gnu, testing on SPEC
CPU 2017 in progress (for stats and correctness).

Any comments?

Thanks,
Richard.

2021-09-17  Richard Biener  

PR tree-optimization/97351
PR tree-optimization/97352
PR tree-optimization/82426
* tree-vectorizer.h (dr_misalignment): Add vector type
argument.
(aligned_access_p): Likewise.
(known_alignment_for_access_p): Likewise.
(vect_supportable_dr_alignment): Likewise.
(vect_known_alignment_in_bytes): Likewise.  Refactor.
(DR_MISALIGNMENT): Remove.
(vect_update_shared_vectype): Likewise.
* tree-vect-data-refs.c (dr_misalignment): Refactor, handle
a vector type with larger alignment requirement and apply
the negative step adjustment here.
(vect_calculate_target_alignment): Remove.
(vect_compute_data_ref_alignment): Get explicit vector type
argument, do not apply a negative step alignment adjustment
here.
(vect_slp_analyze_node_alignment): Re-analyze alignment
when we re-visit the DR with a bigger desired alignment but
keep more precise results from smaller alignments.
* tree-vect-slp.c (vect_update_shared_vectype): Remove.
(vect_slp_analyze_node_operations_1): Do not update the
shared vector type on stmts.
* tree-vect-stmts.c (vect_analyze_stmt): Push/pop the
vector type of an SLP node to the representative stmt-info.
(vect_transform_stmt): Likewise.

* gcc.target/i386/vect-pr82426.c: New testcase.
* gcc.target/i386/vect-pr97352.c: Likewise.
---
 gcc/testsuite/gcc.target/i386/vect-pr82426.c |  32 +++
 gcc/testsuite/gcc.target/i386/vect-pr97352.c |  22 ++
 gcc/tree-vect-data-refs.c| 217 ++-
 gcc/tree-vect-slp.c  |  59 -
 gcc/tree-vect-stmts.c|  77 ---
 gcc/tree-vectorizer.h|  32 ++-
 6 files changed, 231 insertions(+), 208 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/vect-pr82426.c
 create mode 100644 gcc/testsuite/gcc.target/i386/vect-pr97352.c

diff --git a/gcc/testsuite/gcc.target/i386/vect-pr82426.c 
b/gcc/testsuite/gcc.target/i386/vect-pr82426.c
new file mode 100644
index 000..741a1d14d36
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/vect-pr82426.c
@@ -0,0 +1,32 @@
+/* i?86 does not have V2SF, x32 does though.  */
+/* { dg-do compile { target { lp64 || x32 } } } */
+/* ???  With AVX512 we only realize one FMA opportunity.  */
+/* { dg-options "-O3 -mavx -mfma -mno-avx512f" } */
+
+struct Matrix
+{
+  float m11;
+  float m12;
+  float m21;
+  float m22;
+  float dx;
+  float dy;
+};
+
+struct Matrix multiply(const struct Matrix *a, const struct Matrix *b)
+{
+  struct Matrix out;
+  out.m11 = a->m11*b->m11 + a->m12*b->m21;
+  out.m12 = a->m11*b->m12 + a->m12*b->m22;
+  out.m21 = a->m21*b->m11 + a->m22*b->m21;
+  out.m22 = a->m21*b->m12 + a->m22*b->m22;
+
+  out.dx = a->dx*b->m11  + a->dy*b->m21 + b->dx;
+  out.dy = a->dx*b->m12  + a->dy*b->m22 + b->dy;
+  return out;
+}
+
+/* The whole kernel should be vectorized with V4SF and V2SF operations.  */
+/* { dg-final { scan-assembler-times "vadd" 1 } } */
+/* { dg-final { 

Re: [PATCH, Fortran] Fixes for F2018 C838 (PR fortran/101334)

[Thomas Koenig via Gcc-patches]

Hi Sandra,

This patch fixes some bugs in handling of assumed-rank arguments 
revealed by the TS29113 testsuite, allowing xfails to be removed from 
those testcases.  It was previously failing to diagnose an error when 
passing an assumed-rank argument to a procedure via a non-assumed-rank 
dummy, and giving a bogus error when passing one as the first argument 
to the ASSOCIATED intrinsic.  Both fixes turned out to be 1-liners.  OK 
to commit?


OK.

Thanks for the patch!

Best regards

Thomas

Re: [HELP Needed!][PATCH] testsuite: Fix gcc.target/aarch64/auto-init-* tests.

[Richard Earnshaw via Gcc-patches]

On 17/09/2021 20:48, Qing Zhao via Gcc-patches wrote:

Hi,

There are much less issues with aarch64/auto-init-* test cases.
Different -march values (from ‘armv8-a’, ‘armv8.1-a’, till ‘armv8.6-a’, 
‘armv8-r’) do not change the pattern match.

Only

1. -mabi=ilp32/lp64 impact two of the testing cases “auto-init-2.c” and 
“auto-init-padding-5.c”.
2. -fstack-clash-protection impact two of the testing cases “auto-init-1.c” and 
“auto-init-7.c”.

Naturally the patch for this set is:

A. Adjust the patterns for ilp32 or lp64 in “auto-init-2.c” and 
“auto-init-padding-5.c”;
B. Add -fno-stack-protector to “auto-init-1.c” and “auto-init-7.c”


The above A fixed issue 1, however, the above B did not fix issue 2.

If I fixed “auto-init-1.c” as:

diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c 
b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
index 0fa4708..a38d91b 100644
--- a/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
@@ -1,6 +1,6 @@
  /* Verify zero initialization for integer and pointer type automatic 
variables.  */
  /* { dg-do compile } */
-/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand" } */
+/* { dg-options "-ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector" } */
  
  #ifndef __cplusplus

  # define bool _Bool

So, I took a look at the log file of the testing, and found that, If I tested 
it as:

  make check-gcc 
RUNTESTFLAGS='--target_board=unix\{-mabi=lp64/-fstack-clash-protection/-fstack-protector-all\}
 aarch64.exp=auto-init*’

In the log file, I got:

/home/qinzhao/Work/GCC/build_git/gcc/xgcc 
-B/home/qinzhao/Work/GCC/build_git/gcc/ 
/home/qinzhao/Work/GCC/latest_gcc_git/gcc/testsuite/gcc.target/aarch64/auto-init-1.c
  -fdiagnostics-plain-output  -ftrivial-auto-var-init=zero -fdump-rtl-expand 
-fno-stack-protector -ffat-lto-objects -S  -mabi=lp64 -fstack-clash-protection 
-fstack-protector-all  -o auto-init-1.s

 From it, we can see, that the options that were passed through RUNTESTFLAGS 
“mabi-lp64 -fstack-clash-protection -fstack-protector-all” were appended AFTER 
the options inside the testing case through “dg-options”. As a result, the 
option “-fno-stack-protector” did not have any impact at all.

What’s the expected behavior for the order of these options? Should options 
through RUNTESTFLAGS be appended BEFORE or AFTER the options through testing 
cases?

For X86, the options through RUNTESTFLAGS are added BEFORE the options through 
testing cases. Therefore adding “-fno-stack-protector” has the expected result.



Can you check that you are using the same version of dejagnu on both 
platforms?


R.


Is this a bug in aarch64 testing suite?

Thanks.

Qing

[PATCH] Use the proper vectype

[Richard Biener via Gcc-patches]

The following uses the SLP node vectype rather than the vectype
stored in the DR group.

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

2021-09-17  Richard Biener  

* tree-vect-stmts.c (vectorizable_load): Use the vectype
from the SLP node.
---
 gcc/tree-vect-stmts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index ce79d883dbf..17849b575b7 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -8650,7 +8650,7 @@ vectorizable_load (vec_info *vinfo,
  FOR_EACH_VEC_ELT (SLP_TREE_LOAD_PERMUTATION (slp_node), j, k)
if (k > maxk)
  maxk = k;
- tree vectype = STMT_VINFO_VECTYPE (group_info);
+ tree vectype = SLP_TREE_VECTYPE (slp_node);
  if (!TYPE_VECTOR_SUBPARTS (vectype).is_constant ()
  || maxk >= (DR_GROUP_SIZE (group_info) & ~(nunits - 1)))
{
-- 
2.31.1

Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier

[Jakub Jelinek via Gcc-patches]

On Fri, Sep 17, 2021 at 11:42:13PM +0200, Tobias Burnus wrote:
> Fortran/OpenMP: unconstrained/reproducible ordered modifier
> 
> gcc/fortran/ChangeLog:
> 
>   * gfortran.h (gfc_omp_clauses): Add order_unconstrained.
>   * dump-parse-tree.c (show_omp_clauses): Dump it.
>   * openmp.c (gfc_match_omp_clauses): Match unconstrained/reproducible
>   modifiers to ordered(concurrent).
>   (OMP_DISTRIBUTE_CLAUSES): Accept ordered clause.
>   (resolve_omp_clauses): Reject ordered + order on same directive.
>   * trans-openmp.c (gfc_trans_omp_clauses, gfc_split_omp_clauses): Pass
>   on unconstrained modifier of ordered(concurrent).
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/order-5.f90: New test.
>   * gfortran.dg/gomp/order-6.f90: New test.
>   * gfortran.dg/gomp/order-7.f90: New test.
>   * gfortran.dg/gomp/order-8.f90: New test.
>   * gfortran.dg/gomp/order-9.f90: New test.

Ok, thanks.

> @@ -5892,6 +5893,8 @@ gfc_split_omp_clauses (gfc_code *code,
>   = code->ext.omp_clauses->collapse;
> clausesa[GFC_OMP_SPLIT_DISTRIBUTE].order_concurrent
>   = code->ext.omp_clauses->order_concurrent;

So the FE was splitting the order clause to distribute already before,
perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
support.
The difference is e.g. the distribute parallel do order(concurrent) copyin(thr)
case which used to be ok in 5.0 and is not in 5.1.

Jakub

[PATCH] vect alignmet enhance TLC

[Richard Biener via Gcc-patches]

This properly marks the loop as for a runtime alias peel rather
than (pointlessly) going through DR_MISALIGNMENT.

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

2021-09-20  Richard Biener  

* tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
Store -1 for runtime alias peeling iterations.
---
 gcc/tree-vect-data-refs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 967d43726db..1a7abe3ff29 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -2268,8 +2268,7 @@ vect_enhance_data_refs_alignment (loop_vec_info 
loop_vinfo)
   if (npeel)
 LOOP_VINFO_PEELING_FOR_ALIGNMENT (loop_vinfo) = npeel;
   else
-LOOP_VINFO_PEELING_FOR_ALIGNMENT (loop_vinfo)
- = DR_MISALIGNMENT (dr0_info);
+   LOOP_VINFO_PEELING_FOR_ALIGNMENT (loop_vinfo) = -1;
  SET_DR_MISALIGNMENT (dr0_info, 0);
  if (dump_enabled_p ())
 {
-- 
2.31.1

[PATCH] IBM Z: Provide rawmemchr{qi,hi,si} expander

[Stefan Schulze Frielinghaus via Gcc-patches]

This patch implements the rawmemchr expander as introduced in
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579649.html

Bootstrapped and regtested in conjunction with the patch from above on
IBM Z.  Ok for mainline?
>From 551362cda54048dc1a51588112f11c070ed52020 Mon Sep 17 00:00:00 2001
From: Stefan Schulze Frielinghaus 
Date: Mon, 8 Feb 2021 10:35:39 +0100
Subject: [PATCH 2/2] IBM Z: Provide rawmemchr{qi,hi,si} expander

gcc/ChangeLog:

* config/s390/s390-protos.h (s390_rawmemchrqi): Add prototype.
(s390_rawmemchrhi): Add prototype.
(s390_rawmemchrsi): Add prototype.
* config/s390/s390.c (s390_rawmemchr): New function.
(s390_rawmemchrqi): New function.
(s390_rawmemchrhi): New function.
(s390_rawmemchrsi): New function.
* config/s390/s390.md (rawmemchr): New expander.
(rawmemchr): New expander.
* config/s390/vector.md (vec_vfees): Basically a copy of
the pattern vfees from vx-builtins.md.
* config/s390/vx-builtins.md (*vfees): Remove.

gcc/testsuite/ChangeLog:

* gcc.target/s390/rawmemchr-1.c: New test.
---
 gcc/config/s390/s390-protos.h   |  4 +
 gcc/config/s390/s390.c  | 89 ++
 gcc/config/s390/s390.md | 20 +
 gcc/config/s390/vector.md   | 26 ++
 gcc/config/s390/vx-builtins.md  | 26 --
 gcc/testsuite/gcc.target/s390/rawmemchr-1.c | 99 +
 6 files changed, 238 insertions(+), 26 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/rawmemchr-1.c

diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
index 4b03c6e99f5..0d9619e8254 100644
--- a/gcc/config/s390/s390-protos.h
+++ b/gcc/config/s390/s390-protos.h
@@ -66,6 +66,10 @@ s390_asm_declare_function_size (FILE *asm_out_file,
const char *fnname ATTRIBUTE_UNUSED, tree decl);
 #endif
 
+extern void s390_rawmemchrqi(rtx dst, rtx src, rtx pat);
+extern void s390_rawmemchrhi(rtx dst, rtx src, rtx pat);
+extern void s390_rawmemchrsi(rtx dst, rtx src, rtx pat);
+
 #ifdef RTX_CODE
 extern int s390_extra_constraint_str (rtx, int, const char *);
 extern int s390_const_ok_for_constraint_p (HOST_WIDE_INT, int, const char *);
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 54dd6332c3a..1435ce156e2 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -16559,6 +16559,95 @@ s390_excess_precision (enum excess_precision_type type)
 }
 #endif
 
+template 
+static void
+s390_rawmemchr(rtx dst, rtx src, rtx pat) {
+  rtx lens = gen_reg_rtx (V16QImode);
+  rtx pattern = gen_reg_rtx (vec_mode);
+  rtx loop_start = gen_label_rtx ();
+  rtx loop_end = gen_label_rtx ();
+  rtx addr = gen_reg_rtx (Pmode);
+  rtx offset = gen_reg_rtx (Pmode);
+  rtx tmp = gen_reg_rtx (Pmode);
+  rtx loadlen = gen_reg_rtx (SImode);
+  rtx matchlen = gen_reg_rtx (SImode);
+  rtx mem;
+
+  pat = GEN_INT (trunc_int_for_mode (INTVAL (pat), elt_mode));
+  emit_insn (gen_rtx_SET (pattern, gen_rtx_VEC_DUPLICATE (vec_mode, pat)));
+
+  emit_move_insn (addr, XEXP (src, 0));
+
+  // alignment
+  emit_insn (gen_vlbb (lens, gen_rtx_MEM (BLKmode, addr), GEN_INT (6)));
+  emit_insn (gen_lcbb (loadlen, addr, GEN_INT (6)));
+  lens = convert_to_mode (vec_mode, lens, 1);
+  emit_insn (gen_vec_vfees (lens, lens, pattern, GEN_INT (0)));
+  lens = convert_to_mode (V4SImode, lens, 1);
+  emit_insn (gen_vec_extractv4sisi (matchlen, lens, GEN_INT (1)));
+  lens = convert_to_mode (vec_mode, lens, 1);
+  emit_cmp_and_jump_insns (matchlen, loadlen, LT, NULL_RTX, SImode, 1, 
loop_end);
+  force_expand_binop (Pmode, and_optab, addr, GEN_INT (15), tmp, 1, 
OPTAB_DIRECT);
+  force_expand_binop (Pmode, sub_optab, GEN_INT (16), tmp, tmp, 1, 
OPTAB_DIRECT);
+  force_expand_binop (Pmode, add_optab, addr, tmp, addr, 1, OPTAB_DIRECT);
+  // now, addr is 16-byte aligned
+
+  mem = gen_rtx_MEM (vec_mode, addr);
+  set_mem_align (mem, 128);
+  emit_move_insn (lens, mem);
+  emit_insn (gen_vec_vfees (lens, lens, pattern, GEN_INT (VSTRING_FLAG_CS)));
+  add_int_reg_note (s390_emit_ccraw_jump (4, EQ, loop_end),
+   REG_BR_PROB,
+   profile_probability::very_unlikely ().to_reg_br_prob_note 
());
+
+  emit_label (loop_start);
+  LABEL_NUSES (loop_start) = 1;
+
+  force_expand_binop (Pmode, add_optab, addr, GEN_INT (16), addr, 1, 
OPTAB_DIRECT);
+  mem = gen_rtx_MEM (vec_mode, addr);
+  set_mem_align (mem, 128);
+  emit_move_insn (lens, mem);
+  emit_insn (gen_vec_vfees (lens, lens, pattern, GEN_INT (VSTRING_FLAG_CS)));
+  add_int_reg_note (s390_emit_ccraw_jump (4, NE, loop_start),
+   REG_BR_PROB,
+   profile_probability::very_likely ().to_reg_br_prob_note ());
+
+  emit_label (loop_end);
+  LABEL_NUSES (loop_end) = 1;
+
+  if (TARGET_64BIT)
+{
+  lens = convert_to_mode (V2DImode, lens, 1);
+  emit_insn (gen_vec_extractv2didi (offset, 

[PATCH] Avoid premature alignment setting in vect_duplicate_ssa_name_ptr_info

[Richard Biener via Gcc-patches]

This removes adjusting alignment based on the vectorized accesses
and instead keeps what was set on the original access.  The
code generating the actual accesses make sure to properly align
the vectorized accesses based on the generated pointer already
and the vectorizers alignment is always based of the desired
alignment of a vector type and thus will reset alignment to
unknown this way for example when doing strided accesses.

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

2021-09-20  Richard Biener  

* tree-vect-data-refs.c (vect_duplicate_ssa_name_ptr_info):
Do not compute alignment of the vectorized access here.
---
 gcc/tree-vect-data-refs.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 1a7abe3ff29..a57700f2c1b 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -4635,13 +4635,6 @@ static void
 vect_duplicate_ssa_name_ptr_info (tree name, dr_vec_info *dr_info)
 {
   duplicate_ssa_name_ptr_info (name, DR_PTR_INFO (dr_info->dr));
-  int misalign = DR_MISALIGNMENT (dr_info);
-  if (misalign == DR_MISALIGNMENT_UNKNOWN)
-mark_ptr_info_alignment_unknown (SSA_NAME_PTR_INFO (name));
-  else
-set_ptr_info_alignment (SSA_NAME_PTR_INFO (name),
-   known_alignment (DR_TARGET_ALIGNMENT (dr_info)),
-   misalign);
 }
 
 /* Function vect_create_addr_base_for_vector_ref.
-- 
2.31.1

Re: [PATCH 00/13] ARM/MVE use vectors of boolean for predicates

[Christophe LYON via Gcc-patches]

Ping?

On 13/09/2021 10:33, Christophe LYON via Gcc-patches wrote:

ping?


On 07/09/2021 11:15, Christophe Lyon wrote:

This patch series addresses PR 100757 and 101325 by representing
vectors of predicates (MVE VPR.P0 register) as vectors of booleans
rather than using HImode.

As this implies a lot of mostly mechanical changes, I have tried to
split the patches in a way that should help reviewers, but the split
is a bit artificial.

Patches 1-3 add new tests.

Patches 4-6 are small independent improvements.

Patch 7 implements the predicate qualifier, but does not change any
builtin yet.

Patch 8 is the first of the two main patches, and uses the new
qualifier to describe the vcmp and vpsel builtins that are useful for
auto-vectorization of comparisons.

Patch 9 is the second main patch, which fixes the vcond_mask expander.

Patches 10-13 convert almost all the remaining builtins with HI
operands to use the predicate qualifier.  After these, there are still
a few builtins with HI operands left, about which I am not sure: vctp,
vpnot, load-gather and store-scatter with v2di operands.  In fact,
patches 11/12 update some STR/LDR qualifiers in a way that breaks
these v2di builtins although existing tests still pass.

Christophe Lyon (13):
   arm: Add new tests for comparison vectorization with Neon and MVE
   arm: Add tests for PR target/100757
   arm: Add test for PR target/101325
   arm: Add GENERAL_AND_VPR_REGS regclass
   arm: Add support for VPR_REG in arm_class_likely_spilled_p
   arm: Fix mve_vmvnq_n_ argument mode
   arm: Implement MVE predicates as vectors of booleans
   arm: Implement auto-vectorized MVE comparisons with vectors of 
boolean

 predicates
   arm: Fix vcond_mask expander for MVE (PR target/100757)
   arm: Convert remaining MVE vcmp builtins to predicate qualifiers
   arm: Convert more MVE builtins to predicate qualifiers
   arm: Convert more load/store MVE builtins to predicate qualifiers
   arm: Convert more MVE/CDE builtins to predicate qualifiers

  gcc/config/arm/arm-builtins.c | 228 +++--
  gcc/config/arm/arm-modes.def  |   5 +
  gcc/config/arm/arm-protos.h   |   3 +-
  gcc/config/arm/arm-simd-builtin-types.def |   4 +
  gcc/config/arm/arm.c  | 128 ++-
  gcc/config/arm/arm.h  |   5 +-
  gcc/config/arm/arm_mve_builtins.def   | 746 
  gcc/config/arm/iterators.md   |   5 +
  gcc/config/arm/mve.md | 823 ++
  gcc/config/arm/neon.md    |  39 +
  gcc/config/arm/vec-common.md  |  52 --
  gcc/simplify-rtx.c    |   7 +
  .../arm/acle/cde-mve-full-assembly.c  | 264 +++---
  .../gcc.target/arm/simd/mve-vcmp-f32-2.c  |  32 +
  .../gcc.target/arm/simd/neon-compare-1.c  |  78 ++
  .../gcc.target/arm/simd/neon-compare-2.c  |  13 +
  .../gcc.target/arm/simd/neon-compare-3.c  |  14 +
  .../arm/simd/neon-compare-scalar-1.c  |  57 ++
  .../gcc.target/arm/simd/neon-vcmp-f16.c   |  12 +
  .../gcc.target/arm/simd/neon-vcmp-f32-2.c |  15 +
  .../gcc.target/arm/simd/neon-vcmp-f32-3.c |  12 +
  .../gcc.target/arm/simd/neon-vcmp-f32.c   |  12 +
  gcc/testsuite/gcc.target/arm/simd/neon-vcmp.c |  22 +
  .../gcc.target/arm/simd/pr100757-2.c  |  20 +
  .../gcc.target/arm/simd/pr100757-3.c  |  20 +
  .../gcc.target/arm/simd/pr100757-4.c  |  19 +
  gcc/testsuite/gcc.target/arm/simd/pr100757.c  |  19 +
  gcc/testsuite/gcc.target/arm/simd/pr101325.c  |  14 +
  28 files changed, 1581 insertions(+), 1087 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/mve-vcmp-f32-2.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-compare-1.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-compare-2.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-compare-3.c
  create mode 100644 
gcc/testsuite/gcc.target/arm/simd/neon-compare-scalar-1.c

  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-vcmp-f16.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-vcmp-f32-2.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-vcmp-f32-3.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-vcmp-f32.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/neon-vcmp.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/pr100757-2.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/pr100757-3.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/pr100757-4.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/pr100757.c
  create mode 100644 gcc/testsuite/gcc.target/arm/simd/pr101325.c

Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

[Richard Biener via Gcc-patches]

On Fri, Sep 17, 2021 at 5:52 PM Thomas Schwinge  wrote:
>
> Hi!
>
> On 2021-09-17T15:03:18+0200, Richard Biener  
> wrote:
> > On Fri, Sep 17, 2021 at 2:39 PM Jonathan Wakely  
> > wrote:
> >> On Fri, 17 Sept 2021 at 13:08, Richard Biener
> >>  wrote:
> >> > On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge 
> >> >  wrote:
> >> > > On 2021-09-10T10:00:25+0200, I wrote:
> >> > > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches 
> >> > > >  wrote:
> >> > > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
> >> > > >>> Ping -- we still need to plug the memory leak; see patch attached, 
> >> > > >>> [...]
>
> >> > > > Ping for formal approval (and review for using proper
> >> > > > C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source 
> >> > > > code
> >> > > > comment that I'm adding).  Patch again attached, for easy reference.
>
> >> > I'm happy when a C++ literate approves the main change which I quote as
> >> >
> >> >   new ((void*) q) value_type (std::move (x));
> >> > +
> >> > + /* Manually invoke destructor of original object, to 
> >> > counterbalance
> >> > +object constructed via placement new.  */
> >> > + x.~value_type ();
> >> >
> >> > but I had the impression that std::move already "moves away" from the 
> >> > source?
> >>
> >> It just casts the argument to an rvalue reference, which allows the
> >> value_type constructor to steal its guts.
> >>
> >> > That said, the dance above looks iffy, there must be a nicer way to 
> >> > "move"
> >> > an object in C++?
> >>
> >> The code above is doing two things: transfer the resources from x to a
> >> new object at location *q, and then destroy x.
> >>
> >> The first part (moving its resources) has nothing to do with
> >> destruction. An object still needs to be destroyed, even if its guts
> >> have been moved to another object.
> >>
> >> The second part is destroying the object, to end its lifetime. You
> >> wouldn't usually call a destructor explicitly, because it would be
> >> done automatically at the end of scope for objects on the stack, or
> >> done by delete when you free obejcts on the heap. This is a special
> >> case where the object's lifetime is manually managed in storage that
> >> is manually managed.
>
> ACK, and happy that I understood this correctly.
>
> And, thanks for providing some proper C++-esque wording, which I
> summarized to replace my original source code comment.
>
> >> > What happens if the dtor is deleted btw?
> >>
> >> If the destructor is deleted you have created an unusable type that
> >> cannot be stored in containers. It can only be created using new, and
> >> then never destroyed. If you play stupid games, you win stupid prizes.
> >> Don't do that.
>
> Haha!  ;-)
>
> And, by the way, as I understood this: if the destructor is "trivial"
> (which includes POD types, for example), the explicit destructor call
> here is a no-op.
>
> >> I haven't read the rest of the patch, but the snippet above looks fine.
> >
> > OK, thanks for clarifying.
> >
> > The patch is OK then.
>
> Thanks, pushed to master branch
> commit 89be17a1b231ade643f28fbe616d53377e069da8
> "Fix 'hash_table::expand' to destruct stale Value objects".
>
> Should this be backported to release branches, after a while?

You'd have to cross-check the status of C++ support in our containers there.
If it is a memory leak fix then yes, but as said, something older than
GCC 11 needs
double-checking if it is a) affected and b) has other bits already.

Richard.

>
> Grüße
>  Thomas
>
>
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

Re: [PATCH] darwin: support aarch64-darwin host

[Iain Sandoe]

Hello Yuta

thanks for your patch and interest.

> On 17 Sep 2021, at 16:26, Yuta Saito via Gcc-patches 
>  wrote:

> Currently, building gcc for aarch64-darwin host fails due to missing
> host_hooks definition.
> 
> This patch adds host_hooks definition for aarch64-darwin.
> aarch64-darwin is not supported as a target yet, but this allows using
> gcc cross-compiler on aarch64-darwin.

1. The development prototype for aarch64-darwin is here:
   https://github.com/iains/gcc-darwin-arm64

we need to phase the work into master with the approval of the Arm
maintainers - so I would recommend that if you have fixes or improvements
in the short-term, to make pull requests against the development branch.

2.
The patch you have presented is identical in the host-only content to
the existing development one:

https://github.com/iains/gcc-darwin-arm64/commit/2190f7bda7bc0e6d0b74c7bd41c97510a685b06b

So, that aspect has already been handled in the development.

> I confirmed linking gcc-cross succeed on aarch64 darwin.

Patches need more testing than that - specifically, that they do not
regress other targets (unlikely, in this case, but it is good form to
test at least that aarch64-linux-gnu is unaffected).



There are two other small changes I’d like to check before enabling host support
1/
https://github.com/iains/gcc-darwin-arm64/commit/98c8f79929db1bf29ac52f748137b08c21976483
might be needed for cross tools too.

2/
We need to ensure that PCH is defaulted to “off” and that there is a proper 
warning if 
the user tries to configure it “on” see:
https://github.com/iains/gcc-darwin-arm64/issues/2
(which we do not plan to fix in the short term).



There is clearly interest in building cross-compilers on aarch64-darwin, so I 
will try
to phase the host support sooner rather than later,

thanks again for the patch,
Iain

[ping] Fix PR C++/64697

[Eric Botcazou]

This fixes a regression for MinGW with recent binutils:
  https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578756.html

Thanks in advance.

-- 
Eric Botcazou

[PATCH] libgcc, emutls: Allow building weak definitions of the emutls functions.

[Iain Sandoe]

Hi,

The non-Darwin part of this patch is trivial but raises a couple of questions

A/
We define builtins to support emulated TLS.
These are defined with void * pointers
The implementation (in libgcc) uses the correct type (struct __emutls_object *)
in both a forward declaration of the functions and in thier eventual 
implementation.

This leads to a (long-standing, nothing new) complaint at build-time about
the mismatch in the builtin/implementation decls.

AFAICT, there’s no way to fix that unless we introduce struct __emutls_object *
as a built-in type?

B/ 
It seems that a consequence of the mismatch in decls means that if I apply
attributes to the decl (in the implementation file), they are ignored and I have
to apply them to the definition in order for this to work.

This (B) is what the patch below does.

tested on powerpc,i686,x86_64-darwin, x86_64-linux
OK for master?
thanks,
Iain

If the current situation is that A or B indicates “there’s a bug”, please could 
that
be considered as distinct from the current patch (which doesn’t alter this in 
any
way) so that we can make progress on fixing Darwin libgcc issues.

= commit log

In order to better support use of the emulated TLS between objects with
DSO dependencies and static-linked libgcc, allow a target to make weak
definitions.

Signed-off-by: Iain Sandoe 

libgcc/ChangeLog:

* config.host: Add weak-defined emutls crt.
* config/t-darwin: Build weak-defined emutls objects.
* emutls.c (__emutls_get_address): Add optional attributes.
(__emutls_register_common): Likewise.
(EMUTLS_ATTR): New.
---
 libgcc/config.host |  2 +-
 libgcc/config/t-darwin | 13 +
 libgcc/emutls.c| 17 +++--
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/libgcc/config.host b/libgcc/config.host
index 6c34b13d611..a447ac7ae30 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -215,7 +215,7 @@ case ${host} in
 *-*-darwin*)
   asm_hidden_op=.private_extern
   tmake_file="$tmake_file t-darwin ${cpu_type}/t-darwin t-libgcc-pic 
t-slibgcc-darwin"
-  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o"
+  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a"
   ;;
 *-*-dragonfly*)
   tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip"
diff --git a/libgcc/config/t-darwin b/libgcc/config/t-darwin
index 14ae6b35a4e..d6f688d66d5 100644
--- a/libgcc/config/t-darwin
+++ b/libgcc/config/t-darwin
@@ -15,6 +15,19 @@ crttme.o: $(srcdir)/config/darwin-crt-tm.c
 LIB2ADDEH = $(srcdir)/unwind-dw2.c $(srcdir)/config/unwind-dw2-fde-darwin.c \
   $(srcdir)/unwind-sjlj.c $(srcdir)/unwind-c.c
 
+# Make emutls weak so that we can deal with -static-libgcc, override the
+# hidden visibility when this is present in libgcc_eh.
+emutls.o: HOST_LIBGCC2_CFLAGS += \
+  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
+emutls_s.o: HOST_LIBGCC2_CFLAGS += \
+  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
+
+# Make the emutls crt as a convenience lib so that it can be linked
+# optionally, use the shared version so that we can link with DSO.
+libemutls_w.a: emutls_s.o
+   $(AR_CREATE_FOR_TARGET) $@ $<
+   $(RANLIB_FOR_TARGET) $@
+
 # Patch to __Unwind_Find_Enclosing_Function for Darwin10.
 d10-uwfef.o: $(srcdir)/config/darwin10-unwind-find-enc-func.c
$(crt_compile) -mmacosx-version-min=10.6 -c $<
diff --git a/libgcc/emutls.c b/libgcc/emutls.c
index ed2658170f5..d553a74728f 100644
--- a/libgcc/emutls.c
+++ b/libgcc/emutls.c
@@ -50,7 +50,16 @@ struct __emutls_array
   void **data[];
 };
 
+/* EMUTLS_ATTR is provided to allow targets to build the emulated tls
+   routines as weak definitions, for example.
+   If there is no definition, fall back to the default.  */
+#ifndef EMUTLS_ATTR
+#  define EMUTLS_ATTR
+#endif
+
+EMUTLS_ATTR
 void *__emutls_get_address (struct __emutls_object *);
+EMUTLS_ATTR
 void __emutls_register_common (struct __emutls_object *, word, word, void *);
 
 #ifdef __GTHREADS
@@ -123,7 +132,11 @@ emutls_alloc (struct __emutls_object *obj)
   return ret;
 }
 
-void *
+/* Despite applying the attribute to the declaration, in this case the mis-
+   match between the builtin's declaration [void * (*)(void *)] and the
+   implementation here, causes the decl. attributes to be discarded.  */
+
+EMUTLS_ATTR void *
 __emutls_get_address (struct __emutls_object *obj)
 {
   if (! __gthread_active_p ())
@@ -187,7 +200,7 @@ __emutls_get_address (struct __emutls_object *obj)
 #endif
 }
 
-void
+EMUTLS_ATTR void
 __emutls_register_common (struct __emutls_object *obj,
  word size, word align, void *templ)
 {
-- 

Re: [PATCH, Fortran] Fix testcases that violate C838, + revealed ICE

[Tobias Burnus]

On 20.09.21 06:02, Sandra Loosemore wrote:

This patch fixes 3 testcases that violate F2018 C838 by passing an
assumed-rank argument to a procedure via an assumed-sized dummy, by
wrapping the call in a SELECT RANK construct.  But wait, there's more!
This triggered an ICE due to a null pointer dereference in the code
that handles the associated variable in the SELECT RANK.  I fixed that
by copying the idiom used in other places for
GFC_DECL_SAVED_DESCRIPTOR, so now all the tests pass again.

Is this OK to commit?


LGTM – as mentioned in the other patch, please commit this patch first
to avoid intermittent testsuite fails.

Tobias


I confess I'm not certain whether adding the SELECT RANK causes the
testcases now to do something different from what they were originally
trying to test, but they never should have worked as originally
written anyway.  We were just not previously diagnosing the C838
violations without the other patch I just posted to do that.

-Sandra

-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955

Re: [PATCH, Fortran] Fixes for F2018 C838 (PR fortran/101334)

[Tobias Burnus]

On 20.09.21 06:01, Sandra Loosemore wrote:

This patch fixes some bugs in handling of assumed-rank arguments
revealed by the TS29113 testsuite, allowing xfails to be removed from
those testcases.  It was previously failing to diagnose an error when
passing an assumed-rank argument to a procedure via a non-assumed-rank
dummy, and giving a bogus error when passing one as the first argument
to the ASSOCIATED intrinsic.  Both fixes turned out to be 1-liners.
OK to commit?


OK - however, I think you should first commit your follow-up/second patch (fix 
testsuite)
to avoid intermittent test-suite fails.

Additionally, if I try the following testcase, which is now permitted, I get
two ICEs. Can you check?

* The first one seems to be a bug in gfc_conv_intrinsic_function, which
  assumes also for assumed rank that if the first argument is an array,
  the second argument must also be an array.

* For the second one, I see in the dump:
p->dim[p->dtype.rank + -1].stride
  is seems as '-1' is gfc_array_index_type while 'dtype.rank' is 
signed_char_type_node.


(Disclaimer: I don't have a clean tree, but I think this issue not affected
by my patches.)

subroutine foo(p, lp, lpd)
  use iso_c_binding
  implicit none (type, external)
  real, pointer :: p(..)
  real, pointer :: lp
  real, pointer :: lpd(:,:)

! gfc_conv_expr_descriptor, at fortran/trans-array.c:7324
  if (associated(p, lp)) stop 1

! verify_gimple: type mismatch in binary expression - signed char, signed char, 
integer(kind=8), _4 = _3 + -1;
  if (associated(p, lpd)) stop 1
end



Tobias

-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955

[PATCH] Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*

[Richard Biener via Gcc-patches]

This obsoletes the 32bit hppa-hpux configurations which only support
STABS as debuginfo format.

As discussed, I'm going to push this (and a changes.html entry) when
it was included in a bootstrap/regtest cycle.

2021-09-20  Richard Biener  

gcc/
* config.gcc: Obsolete hppa[12]*-*-hpux10* and hppa[12]*-*-hpux11*.

contrib/
* config-list.mk: --enable-obsolete for hppa2.0-hpux10.1 and
hppa2.0-hpux11.9.
---
 contrib/config-list.mk | 5 +++--
 gcc/config.gcc | 2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/contrib/config-list.mk b/contrib/config-list.mk
index 28c0db7a26c..3e1d1321861 100644
--- a/contrib/config-list.mk
+++ b/contrib/config-list.mk
@@ -45,8 +45,9 @@ LIST = aarch64-elf aarch64-linux-gnu aarch64-rtems \
   epiphany-elf epiphany-elfOPT-with-stack-offset=16 fido-elf \
   fr30-elf frv-elf frv-linux ft32-elf h8300-elf hppa-linux-gnu \
   hppa-linux-gnuOPT-enable-sjlj-exceptions=yes hppa64-linux-gnu \
-  hppa2.0-hpux10.1 hppa64-hpux11.3 \
-  hppa64-hpux11.0OPT-enable-sjlj-exceptions=yes hppa2.0-hpux11.9 \
+  hppa2.0-hpux10.1OPT-enable-obsolete hppa64-hpux11.3 \
+  hppa64-hpux11.0OPT-enable-sjlj-exceptions=yes \
+  hppa2.0-hpux11.9OPT-enable-obsolete \
   i686-pc-linux-gnu i686-apple-darwin i686-apple-darwin9 i686-apple-darwin10 \
   i486-freebsd4 i686-freebsd6 i686-kfreebsd-gnu \
   i686-netbsdelf9 \
diff --git a/gcc/config.gcc b/gcc/config.gcc
index c3a8b27f819..498c51e619d 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -250,6 +250,8 @@ md_file=
 case ${target} in
   tile*-*-*\
  | cr16-*-*\
+ | hppa[12]*-*-hpux10* \
+ | hppa[12]*-*-hpux11* \
  )
 if test "x$enable_obsolete" != xyes; then
   echo "*** Configuration ${target} is obsolete." >&2
-- 
2.31.1

Re: [PATCH] Factor out `find_a_program` helper around `find_a_file`

[Iain Sandoe]

Hi Folks

> On 19 Sep 2021, at 23:41, Iain Sandoe  wrote:
> 
>> On 19 Sep 2021, at 16:10, Jeff Law via Gcc-patches  
>> wrote:
>> 
>> On 8/4/2021 12:21 PM, John Ericson wrote:
>>> The helper is for `--print-prog-name` and similar things. Since all
>>> executable finding goes through it, we can move the default overrides
>>> into that path too. This also ensures that if some is looking for a
>>> *non*-program that called `as`, `ld`, etc., weird things don't happen.
>>> ---
>>> gcc/gcc.c | 59 ---
>>> 1 file changed, 34 insertions(+), 25 deletions(-)
>> Thanks.  I added a ChangeLog entry and pushed this to the trunk.
> 
> This doesn’t appear to have been tested with —with-{as,ld,dsymutil}= where
> bootstrap fails since ‘mode' is undefined in the reorganised code.
> 
> I’m testing the patch below and will apply it if all goes OK,

https://gcc.gnu.org/pipermail/gcc-cvs/2021-September/353756.html
Iain

[pushed] Driver: Fix bootstrap with DEFAULT_{ASSEMBLER,LINKER,DSYMUTIL}.

[Iain Sandoe]

Hi

The patch at r12-3662-g5fee8a0a9223d factored the code for
printing the names of programes into a separate function.
However the moved editions that print out the names of the
assembler, linker (and dsymutil on Darwin) when those are
specified at configure-time were not adjusted accordingly,
leading to a bootstrap fail.

Fixed by testing specifically for execute OK, since we know
these are programs.

tested on i686-darwin9, x86_64-darwin20 configured with
—with-{as,ld,dsymutil}= and on x86_64-darwin18 without.

confirmed that the build and installed versions print the right
thing (and, of course, that bootstrap succeeds).

pushed to master as bootstrap fix, thanks
Iain

Signed-off-by: Iain Sandoe 

gcc/ChangeLog:

* gcc.c: Test for execute OK when we find the
programs for assembler linker and dsymutil and those
were specified at configure-time.
---
 gcc/gcc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 1a74bf92f7a..506c2acc282 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3083,17 +3083,17 @@ find_a_program (const char *name)
   /* Do not search if default matches query. */
 
 #ifdef DEFAULT_ASSEMBLER
-  if (! strcmp (name, "as") && access (DEFAULT_ASSEMBLER, mode) == 0)
+  if (! strcmp (name, "as") && access (DEFAULT_ASSEMBLER, X_OK) == 0)
 return xstrdup (DEFAULT_ASSEMBLER);
 #endif
 
 #ifdef DEFAULT_LINKER
-  if (! strcmp (name, "ld") && access (DEFAULT_LINKER, mode) == 0)
+  if (! strcmp (name, "ld") && access (DEFAULT_LINKER, X_OK) == 0)
 return xstrdup (DEFAULT_LINKER);
 #endif
 
 #ifdef DEFAULT_DSYMUTIL
-  if (! strcmp (name, "dsymutil") && access (DEFAULT_DSYMUTIL, mode) == 0)
+  if (! strcmp (name, "dsymutil") && access (DEFAULT_DSYMUTIL, X_OK) == 0)
 return xstrdup (DEFAULT_DSYMUTIL);
 #endif
 
-- 

Re: [PATCH] tree-optimization/65206 - dependence analysis on mixed pointer/array

[Richard Biener via Gcc-patches]

On Fri, 17 Sep 2021, Richard Biener wrote:

> On September 17, 2021 6:36:10 PM GMT+02:00, Richard Sandiford 
>  wrote:
> >Richard Biener via Gcc-patches  writes:
> >> On Fri, 17 Sep 2021, Richard Biener wrote:
> >>
> >>> On Fri, 17 Sep 2021, Richard Sandiford wrote:
> >>> 
> >>> > Richard Biener  writes:
> >>> > > This adds the capability to analyze the dependence of mixed
> >>> > > pointer/array accesses.  The example is from where using a masked
> >>> > > load/store creates the pointer-based access when an otherwise
> >>> > > unconditional access is array based.  Other examples would include
> >>> > > accesses to an array mixed with accesses from inlined helpers
> >>> > > that work on pointers.
> >>> > >
> >>> > > The idea is quite simple and old - analyze the data-ref indices
> >>> > > as if the reference was pointer-based.  The following change does
> >>> > > this by changing dr_analyze_indices to work on the indices
> >>> > > sub-structure and storing an alternate indices substructure in
> >>> > > each data reference.  That alternate set of indices is analyzed
> >>> > > lazily by initialize_data_dependence_relation when it fails to
> >>> > > match-up the main set of indices of two data references.
> >>> > > initialize_data_dependence_relation is refactored into a head
> >>> > > and a tail worker and changed to work on one of the indices
> >>> > > structures and thus away from using DR_* access macros which
> >>> > > continue to reference the main indices substructure.
> >>> > >
> >>> > > There are quite some vectorization and loop distribution opportunities
> >>> > > unleashed in SPEC CPU 2017, notably 520.omnetpp_r, 548.exchange2_r,
> >>> > > 510.parest_r, 511.povray_r, 521.wrf_r, 526.blender_r, 527.cam4_r and
> >>> > > 544.nab_r see amendments in what they report with -fopt-info-loop 
> >>> > > while
> >>> > > the rest of the specrate set sees no changes there.  Measuring runtime
> >>> > > for the set where changes were reported reveals nothing off-noise
> >>> > > besides 511.povray_r which seems to regress slightly for me
> >>> > > (on a Zen2 machine with -Ofast -march=native).
> >>> > >
> >>> > > Changes from the [RFC] version are properly handling bitfields
> >>> > > that we cannot take the address of and optimization of refs
> >>> > > that already are MEM_REFs and thus won't see any change.  I've
> >>> > > also elided changing the set of vect_masked_stores targets in
> >>> > > favor of explicitely listing avx (but I did not verify if the
> >>> > > testcase passes on aarch64-sve or amdgcn).
> >>> > >
> >>> > > The improves cases like the following from Povray:
> >>> > >
> >>> > >for(i = 0; i < Sphere_Sweep->Num_Modeling_Spheres; i++)
> >>> > >  {
> >>> > > VScaleEq(Sphere_Sweep->Modeling_Sphere[i].Center, Vector[X]);
> >>> > > Sphere_Sweep->Modeling_Sphere[i].Radius *= Vector[X];
> >>> > >  }
> >>> > >
> >>> > > where there is a plain array access mixed with abstraction
> >>> > > using T[] or T* arguments.  That should be a not too uncommon
> >>> > > situation in the wild.  The loop above is now vectorized and was not
> >>> > > without the change.
> >>> > >
> >>> > > Bootstrapped and tested on x86_64-unknown-linux-gnu and I've
> >>> > > built and run SPEC CPU 2017 successfully.
> >>> > >
> >>> > > OK?
> >>> > 
> >>> > Took a while to page this stuff back in :-/
> >>> > 
> >>> > I guess if we're adding alt_indices to the main data_reference,
> >>> > we'll need to free the access_fns in free_data_ref.  It looked like
> >>> > the patch as posted might have a memory leak.
> >>> 
> >>> Doh, yes - thanks for noticing.
> >>> 
> >>> > Perhaps instead we could use local indices structures for the
> >>> > alt_indices and pass them in to initialize_data_dependence_relation?
> >>> > Not that that's very elegant?
> >>> 
> >>> Yeah, I had that but then since for N data references we possibly
> >>> call initialize_data_dependence_relation N^2 times we'd do this
> >>> alternate analysis N^2 times at worst instead of N so it looked worth
> >>> caching it in the data reference.  Since we have no idea why the
> >>> first try fails we're going to try this fallback in the majority
> >>> of cases that we cannot figure out otherwise so I didn't manage
> >>> to hand-wave the quadraticness away ;)  OTOH one might argue
> >>> it's just a constant factor ontop of the number of
> >>> initialize_data_dependence_relation invocations.
> >>> 
> >>> So I can probably be convinced either way - guess I'll gather
> >>> some statistics.
> >>
> >> I built SPEC 2017 CPU rate with -Ofast -march=znver2, overall there
> >> are
> >>
> >>  4433976 calls to the first stage initialize_data_dependence_relation
> >>  (skipping the cases dr_may_alias returned false)
> >>  360248 (8%) ended up recursing with a set of alt_indices
> >>  83512   times we computed alt_indices of a DR (that's with the cache)
> >>  14905 (0.3%) times the recursive invocation ended with != chrec_dont_know
> >>
> >> thus 

Re: [committed] avoid assuming cfun is nonnull [PR102243]

[Richard Biener via Gcc-patches]

On Mon, Sep 20, 2021 at 1:33 AM Martin Sebor via Gcc-patches
 wrote:
>
> After the front end passes control to the middle end cfun is never
> null (I'm pretty sure)

cfun is NULL during IPA pass execution, it's set to non-NULL by
the pass manager when we execute a non-IPA pass on a specific
function only.

Richard.

> but when a middle end helper is called
> from the C++ front end cfun can be null for a namespace scope
> initializer expression.  A recent change of mine to the helper
> introduced an assumption to the contrary, causing an ICE.  In
> r12-3673 I've committed the trivial fix below to correct this
> mistake
>
> Martin
>
> Handle null cfun [PR102243].
>
> Resolves:
> PR middle-end/102243 - ICE on placement new at global scope
>
> gcc/ChangeLog:
> PR middle-end/102243
> * tree-ssa-strlen.c (get_range): Handle null cfun.
>
> gcc/testsuite/ChangeLog:
> PR middle-end/102243
> * g++.dg/warn/Wplacement-new-size-10.C: New test.
> ---
>   gcc/testsuite/g++.dg/warn/Wplacement-new-size-10.C | 13 +
>   gcc/tree-ssa-strlen.c  | 14 ++
>   2 files changed, 23 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wplacement-new-size-10.C
>
> diff --git a/gcc/testsuite/g++.dg/warn/Wplacement-new-size-10.C
> b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-10.C
> new file mode 100644
> index 000..6b71a832a30
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-10.C
> @@ -0,0 +1,13 @@
> +/* PR middle-end/102243 - ICE on placement new at global scope
> +   { dg-do compile }
> +   { dg-options "-Wall" } */
> +
> +void *operator new (__SIZE_TYPE__, void *);
> +
> +char a[2][sizeof (int)];
> +
> +int *p = new (a[1]) int;
> +
> +void *operator new[] (__SIZE_TYPE__, void *p) { return p; }
> +
> +int *q = new (a[1]) int[1];
> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
> index 7c93958e9ad..7c568a42d49 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -199,16 +199,22 @@ static bool handle_assign (gimple_stmt_iterator *,
> tree, bool *,
>
>   /* Sets MINMAX to either the constant value or the range VAL is in
>  and returns either the constant value or VAL on success or null
> -   when the range couldn't be determined.  Uses RVALS when nonnull
> -   to determine the range, otherwise uses CFUN or global range info,
> -   whichever is nonnull.  */
> +   when the range couldn't be determined.  Uses RVALS or CFUN for
> +   range info, whichever is nonnull.  */
>
>   tree
>   get_range (tree val, gimple *stmt, wide_int minmax[2],
>range_query *rvals /* = NULL */)
>   {
> if (!rvals)
> -rvals = get_range_query (cfun);
> +{
> +  if (!cfun)
> +   /* When called from front ends for global initializers CFUN
> +  may be null.  */
> +   return NULL_TREE;
> +
> +  rvals = get_range_query (cfun);
> +}
>
> value_range vr;
> if (!rvals->range_of_expr (vr, val, stmt))
> --
> 2.27.0
>