Re: [stage1][PATCH] Change semantics of -frecord-gcc-switches and add -frecord-gcc-switches-format.
On 4/3/20 8:15 PM, Egeyar Bagcioglu wrote: Hi Martin, Hello. I noticed that some comments in the patch were still referring to --record-gcc-command-line, the option I suggested earlier. I updated those comments to mention -frecord-gcc-switches-format instead and also added my name to the patch as you agreed above. Yes, thank you for it. I attached the updated patch. We are starting to use this patch in the specific domain where we need its functionality. Great, that would allow us to have the patch tested. Martin Regards Egeyar
Re: [PATCH] Generalize -fuse-ld= to support absolute path or arbitrary ld.linker
On 4/6/20 12:32 AM, Fangrui Song wrote:
On 2020-03-11, Martin Liška wrote:
On 2/10/20 1:02 AM, Fangrui Song via gcc-patches wrote:
Hello.
Thank you for the patch. You haven't received a review because we are right now
in stage4 of the development cycle:
https://gcc.gnu.org/develop.html#stage4
Thanks for the review!
According to https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543028.html "GCC
10.0 Status Report (2020-04-01)",
I guess GCC is not open for a new development cycle yet.
Yes, it's not opened, but I expect to be opened in 3 weeks from now.
I will just answer a few questions instead of uploading a new patch.
Sure, but don't hesitate to send a patch. It can sit here and wait for next
stage1 ;)
Anyway, I'm going to provide a review (even though I'm not maintainer of that).
Can you please describe your test-case why you need such option? When using
a different ld, I typically export PATH=/path/to/binutils and then run configure
and make.
I would hope -fuse-ld=ld.bfd and -fuse-ld=ld.gold were used instead of
-fuse-ld=bfd and -fuse-ld=gold, then it would be more natural to have
-fuse-ld=/abs/path/to/ld . https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470
Well, problem with that is that the option values are used and we want to
preserve
backward compatibility of options (if possible). I mean we can't just rename
-fuse-ld=bfd to -fuse-ld=ld.bfd.
-fuse-ld=bfd, -fuse-ld=gold and -fuse-ld=lld are hard-coded in numerous
places. It is too late to change that.
One idea is to make
-fuse-ld=bfd
-fuse-ld=gold
-fuse-ld=lld
special. For any other value, e.g. -fuse-ld=foo or -fuse-ld=/abs/path, just
searches the
executable named "foo" (instead of "ld.foo") or /abs/path .
Yes, that seems feasible to me.
Does the scheme sound good? If it is agreed, I can make a similar change to
clang.
Yes, please send a patch and we can make another round of review process.
Thanks,
Martin
I noticed not ideal error message:
$ gcc -fuse-ld=pes /tmp/foo.c
collect2: fatal error: cannot find ‘ld’
compilation terminated.
while clang prints:
$clang -fuse-ld=pes /tmp/foo.c
clang-9.0: error: invalid linker name in argument '-fuse-ld=pes'
The rest of the patch is comment inline...
Thanks for all the comments.
PR driver/93645
* common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
* opts.c (common_handle_option): Handle OPT_fuse_ld_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
---
gcc/ChangeLog | 8 ++
gcc/collect2.c | 67 -
gcc/common.opt | 14 ++
gcc/doc/invoke.texi | 15 +++---
gcc/gcc.c | 14 --
gcc/opts.c | 4 +--
6 files changed, 57 insertions(+), 65 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index feb2d066d0b..6bcec12d841 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-09 Fangrui Song
+
+ PR driver/93645
+ * common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
+ * opts.c (common_handle_option): Handle OPT_fuse_ld_.
+ * gcc.c (driver_handle_option): Likewise.
+ * collect2.c (main): Likewise.
+
2020-02-09 Uroš Bizjak
* recog.c: Move pass_split_before_sched2 code in front of
diff --git a/gcc/collect2.c b/gcc/collect2.c
index 502d629141c..a3ef525a93b 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -859,18 +859,12 @@ main (int argc, char **argv)
{
USE_DEFAULT_LD,
USE_PLUGIN_LD,
- USE_GOLD_LD,
- USE_BFD_LD,
- USE_LLD_LD,
- USE_LD_MAX
+ USE_LD
} selected_linker = USE_DEFAULT_LD;
- static const char *const ld_suffixes[USE_LD_MAX] =
+ static const char *const ld_suffixes[USE_LD] =
{
"ld",
- PLUGIN_LD_SUFFIX,
- "ld.gold",
- "ld.bfd",
- "ld.lld"
+ PLUGIN_LD_SUFFIX
};
static const char *const real_ld_suffix = "real-ld";
static const char *const collect_ld_suffix = "collect-ld";
@@ -882,7 +876,7 @@ main (int argc, char **argv)
static const char *const strip_suffix = "strip";
static const char *const gstrip_suffix = "gstrip";
- const char *full_ld_suffixes[USE_LD_MAX];
+ const char *full_ld_suffixes[USE_LD];
#ifdef CROSS_DIRECTORY_STRUCTURE
/* If we look for a program in the compiler directories, we just use
the short name, since these directories are already system-specific.
@@ -924,6 +918,7 @@ main (int argc, char **argv)
const char **ld1;
bool use_plugin = false;
bool use_collect_ld = false;
+ const char *use_ld = NULL;
/* The kinds of symbols we will have to consider when scanning the
outcome of a first pass link. This is ALL to start with, then might
@@ -948,7 +943,7 @@ main (int argc, char **argv)
#endif
int i;
- for (i = 0; i < USE_LD_MAX; i++)
+ for (i = 0; i < USE_LD; i++)
full_ld_suffixes[i]
#ifdef CROSS_DIRECTORY_STRUCTURE
= concat (target_machine, "-", ld_suffixes[i], NULL);
[stage1][PATCH] Make TOPN counter dynamically allocated.
Hi. We've started discussion the patch with Honza when we started working on reproducibility of -fprofile-generate/use. The patch replaces pre-allocated TOP N counters with a dynamical linked list allocation that happens during profiling. The similar approach is used by Clang and it provides these benefits: 1) addition to linked list is being done atomically, we should not end up with corrupted profiles 2) we waste a pointer per each key-value-pair, but we reduce memory footprint for counters that are not used 3) the method provide better stability results, there are collected stats for GCC PGO build: Covered threshold: 0.25 == Stats for /home/marxin/Programming/gcc/objdir/gcc == stats for indirect_call: Total: 9240, total freq: 5999706513, covered freq: 4309848999 (71.83%), missing freq: 14248293 (0.24%) Total tuples: 8759 (size before: 9*N=83160, after: 2*N + (2*TUPLE_COUNT)=35998 Histogram: 0 tracked: 6278 (67.94%), >=0.25:0 (cov. freq:0 (0.00%)) 1 tracked: 1784 (19.31%), >=0.25: 1784 (cov. freq: 2276692405 (37.95%)) 2 tracked: 222 (2.40%), >=0.25: 300 (cov. freq:575634854 (9.59%)) 3 tracked:79 (0.85%), >=0.25: 143 (cov. freq:220498760 (3.68%)) 4 tracked:78 (0.84%), >=0.25: 135 (cov. freq:160196399 (2.67%)) 5 tracked: 105 (1.14%), >=0.25: 162 (cov. freq: 48169245 (0.80%)) 6 tracked: 239 (2.59%), >=0.25: 369 (cov. freq:115649188 (1.93%)) 7 tracked: 135 (1.46%), >=0.25: 210 (cov. freq: 11660200 (0.19%)) 8 tracked: 224 (2.42%), >=0.25: 260 (cov. freq:313294659 (5.22%)) 9 tracked:62 (0.67%), >=0.25: 50 (cov. freq:222127899 (3.70%)) 10 tracked: 2 (0.02%), >=0.25:3 (cov. freq: 334914 (0.01%)) 11 tracked: 2 (0.02%), >=0.25:1 (cov. freq: 2454696 (0.04%)) 12 tracked: 2 (0.02%), >=0.25:3 (cov. freq: 981658 (0.02%)) 13 tracked: 4 (0.04%), >=0.25:3 (cov. freq: 23617640 (0.39%)) 14 tracked: 4 (0.04%), >=0.25:4 (cov. freq: 9493736 (0.16%)) 17 tracked: 2 (0.02%), >=0.25:3 (cov. freq: 8416719 (0.14%)) 20 tracked: 1 (0.01%), >=0.25:1 (cov. freq: 5041813 (0.08%)) 21 tracked: 3 (0.03%), >=0.25:4 (cov. freq: 23487922 (0.39%)) 27 tracked: 1 (0.01%), >=0.25:1 (cov. freq: 99561333 (1.66%)) 28 tracked: 1 (0.01%), >=0.25:2 (cov. freq: 64012144 (1.07%)) 30 tracked: 1 (0.01%), >=0.25:1 (cov. freq:114573384 (1.91%)) 32 tracked:11 (0.12%), >=0.25:8 (cov. freq: 13949431 (0.23%)) stats for topn: Total: 1519, total freq: 1660520369, covered freq: 767273373 (46.21%), missing freq: 84286328 (5.08%) Total tuples: 5365 (size before: 9*N=13671, after: 2*N + (2*TUPLE_COUNT)=13768 Histogram: 0 tracked: 1032 (67.94%), >=0.25:0 (cov. freq:0 (0.00%)) 1 tracked: 154 (10.14%), >=0.25: 154 (cov. freq:240913892 (14.51%)) 2 tracked:43 (2.83%), >=0.25: 62 (cov. freq: 28671331 (1.73%)) 3 tracked:38 (2.50%), >=0.25: 56 (cov. freq: 72179614 (4.35%)) 4 tracked:23 (1.51%), >=0.25: 38 (cov. freq: 9598177 (0.58%)) 5 tracked:36 (2.37%), >=0.25: 48 (cov. freq: 89840680 (5.41%)) 6 tracked:13 (0.86%), >=0.25: 17 (cov. freq: 4548625 (0.27%)) 7 tracked: 8 (0.53%), >=0.25: 10 (cov. freq: 23456524 (1.41%)) 8 tracked: 6 (0.39%), >=0.25:6 (cov. freq: 357781 (0.02%)) 9 tracked: 2 (0.13%), >=0.25:2 (cov. freq: 266881 (0.02%)) 10 tracked: 5 (0.33%), >=0.25:5 (cov. freq:68985 (0.00%)) 11 tracked: 6 (0.39%), >=0.25:7 (cov. freq: 6216337 (0.37%)) 12 tracked: 1 (0.07%), >=0.25:1 (cov. freq: 892 (0.00%)) 13 tracked:11 (0.72%), >=0.25: 11 (cov. freq: 70639704 (4.25%)) 14 tracked: 3 (0.20%), >=0.25:1 (cov. freq:16524 (0.00%)) 15 tracked: 1 (0.07%), >=0.25:0 (cov. freq:0 (0.00%)) 16 tracked: 1 (0.07%), >=0.25:1 (cov. freq: 106363 (0.01%)) 17 tracked: 1 (0.07%), >=0.25:0 (cov. freq:0 (0.00%)) 18 tracked: 2 (0.13%), >=0.25:3 (cov. freq:32033 (0.00%)) 20 tracked: 1 (0.07%), >=0.25:0 (cov. freq:0 (0.00%)) 21 tracked: 3 (0.20%), >=0.25:2 (cov. freq: 355742 (0.02%)) 22 tracked: 1 (0.07%), >=0.25:1 (cov. freq: 624 (0.00%)) 24 tracked: 3 (0.20%), >=0.25:2 (cov. freq: 1684949 (0.10%)) 25 tracked: 1 (0.07%), >=0.25:1 (cov. freq: 655 (0.00%)) 26 tracked: 2 (0.13%), >=0.25:2 (cov. freq:72971 (0.00%)) 27 tracked: 1 (0.07%), >=0.25:0 (cov. freq:0 (0.00%)) 28 tracked: 1 (0.07%), >=0.25:1 (cov. freq:14053 (0.00%)) 30 tracked: 3 (0.20%), >=0.25:
Re: [PATCH] Check DECL_CONTEXT of new/delete operators.
On 4/3/20 5:26 PM, Jan Hubicka wrote:
On 3/31/20 2:29 PM, Jan Hubicka wrote:
Well, I basically went through all pointers and tried to get rid of as
many of them as possible. CONTEXT pointers do increase size of SCCs
that increases chance they will not get merged and also processing time
of merging algorithm. I guess if we need to stream more contexts we
could do that (and check the effect on merging and average SCC size)
Ok, do we want to stream contexts just for the new/delete operators?
Can you please prepare a streaming patch?
Hi,
I am still not convinced that context is useful here.
It took me a while to understand what the code does and why it fails,
but here is a testcase.
It fails for me with your patch and -O2 --param early-inlining-insns=100
The invalid transform is to remove pair
base:new and B:delete
B:new gets inlined and we get count out of sync.
Honza
#include
volatile int idx;
struct base
{
__attribute__((malloc,noinline))
static void* operator new(unsigned long sz)
{
return ::operator new(sz);
}
__attribute__((malloc,noinline))
static void operator delete(void* ptr)
{
--count[idx];
::operator delete(ptr);
}
volatile static int count[2];
};
volatile int base::count[2] = {0,0};
struct B:base
{
static void* operator new(unsigned long sz)
{
++count[idx];
return base::operator new(sz);
}
};
volatile int c=1;
int main(){
for (int i; i
May I please ping Jason, Nathan and Jonathan who can help us here?
Thanks,
Martin
Re: ipa: Fix wrong code with failed propagation to builtin_constant_p [PR93940]
On Sat, 4 Apr 2020 at 11:47, Jan Hubicka wrote:
>
> Hi,
> this patch fixes wrong code on a testcase where inline predicts
> builtin_constant_p to be true but we fail to optimize its parameter to
> constant
> becuase FRE is not run and the value is passed by an aggregate.
>
> This patch makes the inline predicates to disable aggregate tracking
> when FRE is not going to be run and similarly value range when VRP is not
> going to be run.
>
> This is just partial fix. Even with it we can arrange FRE/VRP to fail and
> produce wrong code, unforutnately.
>
> I think for GCC11 I will need to implement transformation in ipa-inline
> but this is bit hard to do: predicates only tracks that value will be constant
> and do not track what constant to be.
>
> Optimizing builtin_constant_p in a conditional is not going to do good job
> when the value is used later in a place that expects it to be constant.
> This is pre-existing problem that is not limited to inline tracking. For
> example,
> FRE may do the transofrm at one place but not in another due to alias oracle
> walking limits.
>
> So I am not sure what full fix would be :(
>
> gcc/ChangeLog:
>
> 2020-04-04 Jan Hubicka
>
> PR ipa/93940
> * ipa-fnsummary.c (vrp_will_run_p): New function.
> (fre_will_run_p): New function.
> (evaluate_properties_for_edge): Use it.
> * ipa-inline.c (can_inline_edge_by_limits_p): Do not inline
> !optimize_debug to optimize_debug.
>
> gcc/testsuite/ChangeLog:
>
> 2020-04-04 Jan Hubicka
>
> * g++.dg/tree-ssa/pr93940.C: New test.
>
> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> index b411bc4d660..279ac8f7cc9 100644
> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -503,6 +503,32 @@ evaluate_conditions_for_known_args (struct cgraph_node
> *node,
> *ret_nonspec_clause = nonspec_clause;
> }
>
> +/* Return true if VRP will be exectued on the function.
> + We do not want to anticipate optimizations that will not happen.
> +
> + FIXME: This can be confused with -fdisable and debug counters and thus
> + it should not be used for correctness (only to make heuristics work).
> + This means that inliner should do its own optimizations of expressions
> + that it predicts to be constant so wrong code can not be triggered by
> + builtin_constant_p. */
> +
> +static bool
> +vrp_will_run_p (struct cgraph_node *node)
> +{
> + return (opt_for_fn (node->decl, optimize)
> + && !opt_for_fn (node->decl, optimize_debug)
> + && opt_for_fn (node->decl, flag_tree_vrp));
> +}
> +
> +/* Similarly about FRE. */
> +
> +static bool
> +fre_will_run_p (struct cgraph_node *node)
> +{
> + return (opt_for_fn (node->decl, optimize)
> + && !opt_for_fn (node->decl, optimize_debug)
> + && opt_for_fn (node->decl, flag_tree_fre));
> +}
>
> /* Work out what conditions might be true at invocation of E.
> Compute costs for inlined edge if INLINE_P is true.
> @@ -594,6 +620,7 @@ evaluate_properties_for_edge (struct cgraph_edge *e, bool
> inline_p,
>
> /* If we failed to get simple constant, try value range. */
> if ((!cst || TREE_CODE (cst) != INTEGER_CST)
> + && vrp_will_run_p (caller)
> && ipa_is_param_used_by_ipa_predicates (callee_pi, i))
> {
> value_range vr
> @@ -609,14 +636,17 @@ evaluate_properties_for_edge (struct cgraph_edge *e,
> bool inline_p,
> }
>
> /* Determine known aggregate values. */
> - ipa_agg_value_set agg
> - = ipa_agg_value_set_from_jfunc (caller_parms_info,
> - caller, &jf->agg);
> - if (agg.items.length ())
> + if (vrp_will_run_p (caller))
> {
> - if (!known_aggs_ptr->length ())
> - vec_safe_grow_cleared (known_aggs_ptr, count);
> - (*known_aggs_ptr)[i] = agg;
> + ipa_agg_value_set agg
> + = ipa_agg_value_set_from_jfunc (caller_parms_info,
> + caller, &jf->agg);
> + if (agg.items.length ())
> + {
> + if (!known_aggs_ptr->length ())
> + vec_safe_grow_cleared (known_aggs_ptr, count);
> + (*known_aggs_ptr)[i] = agg;
> + }
> }
> }
>
> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
> index 302ce16a646..f71443feff7 100644
> --- a/gcc/ipa-inline.c
> +++ b/gcc/ipa-inline.c
> @@ -485,6 +485,7 @@ can_inline_edge_by_limits_p (struct cgraph_edge *e, bool
> report,
> else if (check_match (flag_wrapv)
> || check_match (flag_trapv)
> || check_match (flag_pcc_struct_return)
> + || check_maybe_down (optimize_
Re: [RFA/RFC] [PR tree-optimization/80635] Optimize some V_C_Es with limited ranges into NOP_EXPRs
On Sun, Apr 5, 2020 at 5:25 PM Jeff Law via Gcc-patches
wrote:
>
>
> So here's an approach to try and address PR80635.
>
> In this BZ we're getting a false positive uninitialized warning using
> std::optional.
>
> As outlined in the BZ this stems from SRA using a VIEW_CONVERT_EXPR which
> isn't
> handled terribly well by the various optimizers/analysis passes.
>
> We have these key blocks:
>
> ;; basic block 5, loop depth 0
> ;;pred: 3
> ;;2
> # maybe_a$m_6 = PHI <_5(3), maybe_a$m_4(D)(2)>
> # maybe_a$4_7 = PHI <1(3), 0(2)>
> :
> _8 = maybe_b.live;
> if (_8 != 0)
> goto ; [0.00%]
> else
> goto ; [0.00%]
> ;;succ: 6
> ;;7
>
> ;; basic block 6, loop depth 0
> ;;pred: 5
> B::~B (&maybe_b.D.2512.m_item);
> ;;succ: 7
>
> ;; basic block 7, loop depth 0
> ;;pred: 5
> ;;6
> maybe_b ={v} {CLOBBER};
> resx 3
> ;;succ: 8
>
> ;; basic block 8, loop depth 0
> ;;pred: 7
> :
> _9 = VIEW_CONVERT_EXPR(maybe_a$4_7);
So this is a reg-reg copy. But if you replace it with a NOP_EXPR
it becomes a truncation which is less optimal.
Testcase:
char y;
_Bool x;
void __GIMPLE(ssa) foo()
{
_Bool _1;
char _2;
__BB(2):
_2 = y;
_1 = (_Bool)_2;
x = _1;
return;
}
void __GIMPLE(ssa) bar()
{
_Bool _1;
char _2;
__BB(2):
_2 = y;
_1 = __VIEW_CONVERT <_Bool> (_2);
x = _1;
return;
}
where assembly is
foo:
.LFB0:
.cfi_startproc
movzbl y(%rip), %eax
andl$1, %eax
movb%al, x(%rip)
ret
vs.
bar:
.LFB1:
.cfi_startproc
movzbl y(%rip), %eax
movb%al, x(%rip)
ret
so the reverse transformation is what should be done ...
Which means other analyses have to improve their handling
of VIEW_CONVERT_EXPR instead.
Richard.
> if (_9 != 0)
> goto ; [0.00%]
> else
> goto ; [0.00%]
> ;;succ: 9
> ;;10
>
> Where there is a use of maybe_a$m_6 in block #9.
>
> Of course maybe_a$m_6 only takes the value of maybe_a$m_4(D) when we traverse
> the
> edge 2->5 but in that case maybe_a$4_7 will always have the value zero and
> thus
> we can not reach bb #9.. But the V_C_E gets in the way of the analysis and we
> issue the false positive warning. Martin Jambor has indicated that he doesn't
> see a way to avoid the V_C_E from SRA without reintroducing PR52244.
>
> This patch optimizes the V_C_E into a NOP_EXPR by verifying that the V_C_E
> folds
> to a constant value for the min & max values of the range of the input operand
> and the result of folding is equal to the original input. We do some
> additional
> checking beyond just that original value and converted value are equal
> according
> to operand_equal_p.
>
> Eventually the NOP_EXPR also gets removed as well and the conditional in bb8
> tests maybe_a$4_7 against 0 directly.
>
> That in turn allows the uninit analysis to determine the use of maybe_a$_m_6
> in
> block #9 is properly guarded and the false positive is avoided.
>
> The optimization of a V_C_E into a NOP_EXPR via this patch occurs a couple
> hundred times during a bootstrap, so this isn't a horribly narrow change just
> to
> fix a false positive warning.
>
> Bootstrapped and regression tested on x86_64. I've also put it through its
> paces
> in the tester. The tester's current failures (aarch64, mips, h8) are
> unrelated
> to this patch.
>
>
> Thoughts? OK for the trunk? Alternately I wouldn't lose sleep moving this to
> gcc-11.
>
> jeff
>
>
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
Martin Liška writes:
> Hello.
>
> This is second attempt to get rid of tcc_comparison GENERIC trees
> to be used as the first argument of VEC_COND_EXPR.
>
> The patch attempts achieves that in the following steps:
> 1) veclower pass expands all tcc_comparison expression into a SSA_NAME
> 2) since that tcc_comparsion can't be used as the first argument of
> VEC_COND_EXPR
> (done in GIMPLE verifier)
> 3) I exposed new internal functions with:
> DEF_INTERNAL_OPTAB_FN (VCOND, 0, vcond, vec_cond)
> DEF_INTERNAL_OPTAB_FN (VCONDU, 0, vcondu, vec_condu)
> DEF_INTERNAL_OPTAB_FN (VCONDEQ, 0, vcondeq, vec_condeq)
> DEF_INTERNAL_OPTAB_FN (VCOND_MASK, 0, vcond_mask, vec_cond_mask)
>
> 4) logic of expand_vec_cond_expr is moved into the new pass_gimple_isel pass
> 5) the pass expands VEC_COND_EXPR into one of the internal functions defined
> in 3)
> 6) moreover, I've added a new logic that prefers expand_vec_cmp_expr_p when
> a SSA_NAME is being used in multiple (2+) VEC_COND_EXPR statements
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> Moreover, I run SPEC2006 and SPEC2017 benchmarks on znver1, znver2 and skylake
> target and I don't see any reasonable change.
>
> Achieved benefits of the patch:
> - removal of a GENERIC expression being used in GIMPLE statements
> - extraction into SSA_NAMEs can enable proper tree optimizer (FRE, DOM, PRE)
> - possibility to expand smarter based on number of uses
> (expand_vec_cmp_expr_p)
>
> Future plans:
> - tcc_comparison removal just during gimplification
> - removal of a code where these expressions are handled for VEC_COND_EXPR
> - do the similar thing for COND_EXPR?
>
> The task was guided by Richi (Biener) and I bet he can help with both further
> questions
> and reasoning.
Thanks for doing this. It definitely seems more friendly than the
four-operand version to targets where separate comparisons are the norm.
Just a couple of comments about the implementation:
> diff --git a/gcc/passes.def b/gcc/passes.def
> index 2bf2cb78fc5..d654e5ee9fe 100644
> --- a/gcc/passes.def
> +++ b/gcc/passes.def
> @@ -397,6 +397,7 @@ along with GCC; see the file COPYING3. If not see
>NEXT_PASS (pass_cleanup_eh);
>NEXT_PASS (pass_lower_resx);
>NEXT_PASS (pass_nrv);
> + NEXT_PASS (pass_gimple_isel);
>NEXT_PASS (pass_cleanup_cfg_post_optimizing);
>NEXT_PASS (pass_warn_function_noreturn);
>NEXT_PASS (pass_gen_hsail);
What was the reason for making this a separate pass, rather than doing
it as part of veclower? If we do them separately, then it's harder for
veclower to know which VEC_COND_EXPRs it needs to open-code. (OK, so
that's a general problem between veclower and expand already, but it
seems like the new approach could help to move away from that by
doing the instruction selection directly in veclower.)
> +/* Expand all VEC_COND_EXPR gimple assignments into calls to internal
> + function based on type of selected expansion. */
> +
> +static gimple *
> +gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi)
> +{
> + tree lhs, op0a = NULL_TREE, op0b = NULL_TREE;
> + enum tree_code code;
> + enum tree_code tcode;
> + machine_mode cmp_op_mode;
> + bool unsignedp;
> + enum insn_code icode;
> + imm_use_iterator imm_iter;
> +
> + /* Only consider code == GIMPLE_ASSIGN. */
> + gassign *stmt = dyn_cast (gsi_stmt (*gsi));
> + if (!stmt)
> +return NULL;
> +
> + code = gimple_assign_rhs_code (stmt);
> + if (code != VEC_COND_EXPR)
> +return NULL;
> +
> + tree op0 = gimple_assign_rhs1 (stmt);
> + tree op1 = gimple_assign_rhs2 (stmt);
> + tree op2 = gimple_assign_rhs3 (stmt);
> + lhs = gimple_assign_lhs (stmt);
> + machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
> +
> + gcc_assert (!COMPARISON_CLASS_P (op0));
> + if (TREE_CODE (op0) == SSA_NAME)
> +{
> + unsigned int used_vec_cond_exprs = 0;
> + gimple *use_stmt;
> + FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, op0)
> + {
> + gassign *assign = dyn_cast (use_stmt);
> + if (assign != NULL && gimple_assign_rhs_code (assign) == VEC_COND_EXPR
> + && gimple_assign_rhs1 (assign) == op0)
> + used_vec_cond_exprs++;
> + }
This looks like it's quadratic in the worst case. Could we check
this in a different way?
> +
> + gassign *def_stmt = dyn_cast (SSA_NAME_DEF_STMT (op0));
> + if (def_stmt)
> + {
> + tcode = gimple_assign_rhs_code (def_stmt);
> + op0a = gimple_assign_rhs1 (def_stmt);
> + op0b = gimple_assign_rhs2 (def_stmt);
> +
> + tree op0a_type = TREE_TYPE (op0a);
> + if (used_vec_cond_exprs >= 2
It would be good if targets were able to provide only vcond_mask.
In that case I guess we should go this path if the later one would fail.
> + && (get_vcond_mask_icode (mode, TYPE_MODE (op0a_type))
> + != CODE_FOR_nothing)
> + && expand_vec_cmp_expr_p (op0a_type, TREE_TYPE (lhs), tcode))
> + {
> + /* Keep the SSA n
Re: [PATCH][PPC64] [PR88877]
On Mon, 6 Apr 2020, kamlesh kumar wrote: > Segher, > Please provide your suggestion/thought on the fix. There is not a single line of explanation what the change does so what do you expect? Richard. > On Mon, Mar 23, 2020 at 8:16 PM kamlesh kumar wrote: > > > > Attached patch fixes. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88877. > > ChangeLog Entry. > > > > 2020-03-23 Kamlesh Kumar > > > > * rtl.h : Defined Tuple for bundling rtx, mode and > > unsignedness default as 0 > > Added Extra argument (unsignedp) in emit_library_call and > > emit_library_call_value. > > * except.c : Likewise. > > * explow.c : Likewise. > > * expmed.c : Likewise. > > * expr.c : Likewise. > > * optabs.c : Likewise. > > * asan.c : Likewise. > > * builtins.c : Likewise. > > * calls.c : Likewise. > > * cfgexpand.c : Likewise. > > * config/aarch64/aarch64.c : Likewise. > > * config/aarch64/aarch64.h : Likewise. > > * config/aarch64/atomics.md : Likewise. > > * config/alpha/alpha.c : Likewise. > > * config/arc/arc.c : Likewise. > > * config/arc/elf.h : Likewise. > > * config/arc/linux.h : Likewise. > > * config/arm/arm.c : Likewise. > > * config/bfin/bfin.md : Likewise. > > * config/c6x/c6x.c : Likewise. > > * config/csky/csky.c : Likewise. > > * config/frv/frv.c : Likewise. > > * config/i386/i386-expand.c : Likewise. > > * config/i386/i386.c : Likewise. > > * config/ia64/ia64.c : Likewise. > > * config/ia64/ia64.md : Likewise. > > * config/m32r/m32r.c : Likewise. > > * config/m68k/linux.h : Likewise. > > * config/m68k/m68k.c : Likewise. > > * config/microblaze/microblaze.c : Likewise. > > * config/mips/mips.h : Likewise. > > * config/mips/sdemtk.h : Likewise. > > * config/nds32/nds32.h : Likewise. > > * config/nios2/nios2.c : Likewise. > > * config/or1k/or1k.c : Likewise. > > * config/pa/pa.c : Likewise. > > * config/pa/pa.md : Likewise. > > * config/pru/pru.c : Likewise. > > * config/riscv/riscv.h : Likewise. > > * config/riscv/riscv.md : Likewise. > > * config/rl78/rl78.c : Likewise. > > * config/rs6000/rs6000-string.c : Likewise. > > * config/rs6000/rs6000.c : Likewise. > > * config/rs6000/rs6000.md : Likewise. > > * config/rs6000/vsx.md : Likewise. > > * config/sh/sh.c : Likewise. > > * config/sparc/sparc.c : Likewise. > > * config/tilegx/tilegx.c : Likewise. > > * config/tilepro/tilepro.c : Likewise. > > * config/visium/visium.c : Likewise. > > * config/xtensa/xtensa.c : Likewise. > > * testsuite/gcc.target/powerpc/pr88877.c : Newtest > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH v2 00/11] aarch64: Implement TImode comparisons
On 03/04/2020 16:03, Richard Sandiford wrote: > "Richard Earnshaw (lists)" writes: >> On 03/04/2020 13:27, Richard Sandiford wrote: >>> "Richard Earnshaw (lists)" writes: On 02/04/2020 19:53, Richard Henderson via Gcc-patches wrote: > This is attacking case 3 of PR 94174. > > In v2, I unify the various subtract-with-borrow and add-with-carry > patterns that also output flags with unspecs. As suggested by > Richard Sandiford during review of v1. It does seem cleaner. > Really? I didn't need to use any unspecs for the Arm version of this. R. >>> >>> See https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543063.html >>> (including quoted context) for how we got here. >>> >>> The same problem affects the existing aarch64 patterns like >>> *usub3_carryinC. Although that pattern avoids unspecs, >>> the compare:CC doesn't seem to be correct. >>> >>> Richard >>> >> >> But I don't think you can use ANY_EXTEND in these comparisons. It >> doesn't describe what the instruction does, since the instruction does >> not really extend the values first. > > Yeah, that was the starting point in the thread above too. And using > zero_extend in the existing *usub3_carryinC pattern: > > (define_insn "*usub3_carryinC" > [(set (reg:CC CC_REGNUM) > (compare:CC > (zero_extend: > (match_operand:GPI 1 "register_operand" "r")) > (plus: > (zero_extend: > (match_operand:GPI 2 "register_operand" "r")) > (match_operand: 3 "aarch64_borrow_operation" "" >(set (match_operand:GPI 0 "register_operand" "=r") > (minus:GPI > (minus:GPI (match_dup 1) (match_dup 2)) > (match_operand:GPI 4 "aarch64_borrow_operation" "")))] >"" >"sbcs\\t%0, %1, %2" > [(set_attr "type" "adc_reg")] > ) > > looks wrong for the same reason. But the main problem IMO isn't how the > inputs to the compare:CC are represented, but that we're using compare:CC > at all. Using compare doesn't accurately model the effect of SBCS on NZCV > for all inputs, so if we're going to use a compare here, it can't be :CC. > >> I would really expect this patch series to be pretty much a dual of this >> series that I posted last year for Arm. >> >> https://gcc.gnu.org/pipermail/gcc-patches/2019-October/532180.html > > That series uses compares with modes like CC_V and CC_B, so I think > you're saying that given the choice in the earlier thread between adding > a new CC mode or using unspecs, you would have preferred a new CC mode, > is that right? > Yes. It surprised me, when doing the aarch32 version, just how often the mid-end parts of the compiler were able to reason about parts of the parallel insn and optimize things accordingly (eg propagating the truth of the comparison). If you use an unspec that can never happen. R. > Richard >
Re: [PATCH] Check DECL_CONTEXT of new/delete operators.
On Sat, Apr 4, 2020 at 1:53 PM Jan Hubicka wrote:
>
> Hi,
> thinking a bit of the problem, I guess we could match in addition to
> DECL_CONTEXT the whole inline stack of both statements and see if there
> are inlined new/delete operators and if so if they are always in
> matching pairs.
>
> The inline stack is available as
> for (tree block = gimple_block (call); block && TREE_CODE (block) == BLOCK;
> block = BLOCK_SUPERCONTEXT (block))
> {
> tree fn = block_ultimate_origin (block);
> if (fn != NULL && TREE_CODE (fn) == FUNCTION_DECL)
> do the checking htere.
> }
>
> But I do not understand what C++ promises here and in what conditions
> the new/delete pair can be removed.
But if the inline stack matches in pairs then the ultimate new/delete
call should also
match, no? When there's a mismatch in inlining we can't DCE since we
can't remove
the extra inlined stmts.
Your failing testcase suggests we never can remove new/delete pairs though
unless the DECL_CONTEXT is 'final'. Also the user could have chosen to
"inline" the side-effect of the new operation manually but not the
delete one, so
operator delete() { count-- }
ptr = new A;
count++;
delete ptr;
is it valid to elide the new/delete pair here?
Richard.
> Honza
Re: [RFA] skip gcc.target/arm/div64-unwinding.c on vxworks_kernel targets
On 04/04/2020 02:12, Joel Brobecker wrote: > Hello, > > This test verifies, by using a weak reference to _Unwind_RaiseException, > that performing division by zero does not cause that symbol to get > indirectly pulled into our closure. > > The testing methodology unfortunately does not work on VxWorks targets > when building in kernel mode. This is inherent to how kernel mode > on VxWorks works: The link is only partial and the remaining symbols > which have not been resolved already get automatically resolved by > the VxWorks loader at the moment the module is loaded onto the target, > prior to execution. The resolution includes weak symbols too, which > defeats the purpose of this test. > > gcc/testsuite/ > > * gcc.target/arm/div64-unwinding.c: Skip on vxworks_kernel targets. > > Tested by running the test on ARM VxWorks 7 SR0640, both in kernel > mode as well as RTP mode (verifying it only gets disabled when in > kernel mode). Also run on ARM ELF to verify that this still runs on > non-VxWorks targets. > > OK to push to master? > > Thank you! > OK. R.
Re: [PATCH][PPC64] [PR88877]
Hi Richard, Here is a discussion we did some time ago https://gcc.gnu.org/pipermail/gcc/2019-January/227834.html please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88877 for more info regarding the bug. We incorporated below Jakub's suggestion in this patch. Jakub wrote: "" Yeah, all the callers of emit_library_call* would need to be changed to pass triplets rtx, machine_mode, int/bool /*unsignedp*/, instead of just rtx_mode_t pair. Jakub "" On Mon, Apr 6, 2020 at 2:47 PM Richard Biener wrote: > > On Mon, 6 Apr 2020, kamlesh kumar wrote: > > > Segher, > > Please provide your suggestion/thought on the fix. > > There is not a single line of explanation what the change does so what > do you expect? > > Richard. > > > On Mon, Mar 23, 2020 at 8:16 PM kamlesh kumar > > wrote: > > > > > > Attached patch fixes. > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88877. > > > ChangeLog Entry. > > > > > > 2020-03-23 Kamlesh Kumar > > > > > > * rtl.h : Defined Tuple for bundling rtx, mode and > > > unsignedness default as 0 > > > Added Extra argument (unsignedp) in emit_library_call and > > > emit_library_call_value. > > > * except.c : Likewise. > > > * explow.c : Likewise. > > > * expmed.c : Likewise. > > > * expr.c : Likewise. > > > * optabs.c : Likewise. > > > * asan.c : Likewise. > > > * builtins.c : Likewise. > > > * calls.c : Likewise. > > > * cfgexpand.c : Likewise. > > > * config/aarch64/aarch64.c : Likewise. > > > * config/aarch64/aarch64.h : Likewise. > > > * config/aarch64/atomics.md : Likewise. > > > * config/alpha/alpha.c : Likewise. > > > * config/arc/arc.c : Likewise. > > > * config/arc/elf.h : Likewise. > > > * config/arc/linux.h : Likewise. > > > * config/arm/arm.c : Likewise. > > > * config/bfin/bfin.md : Likewise. > > > * config/c6x/c6x.c : Likewise. > > > * config/csky/csky.c : Likewise. > > > * config/frv/frv.c : Likewise. > > > * config/i386/i386-expand.c : Likewise. > > > * config/i386/i386.c : Likewise. > > > * config/ia64/ia64.c : Likewise. > > > * config/ia64/ia64.md : Likewise. > > > * config/m32r/m32r.c : Likewise. > > > * config/m68k/linux.h : Likewise. > > > * config/m68k/m68k.c : Likewise. > > > * config/microblaze/microblaze.c : Likewise. > > > * config/mips/mips.h : Likewise. > > > * config/mips/sdemtk.h : Likewise. > > > * config/nds32/nds32.h : Likewise. > > > * config/nios2/nios2.c : Likewise. > > > * config/or1k/or1k.c : Likewise. > > > * config/pa/pa.c : Likewise. > > > * config/pa/pa.md : Likewise. > > > * config/pru/pru.c : Likewise. > > > * config/riscv/riscv.h : Likewise. > > > * config/riscv/riscv.md : Likewise. > > > * config/rl78/rl78.c : Likewise. > > > * config/rs6000/rs6000-string.c : Likewise. > > > * config/rs6000/rs6000.c : Likewise. > > > * config/rs6000/rs6000.md : Likewise. > > > * config/rs6000/vsx.md : Likewise. > > > * config/sh/sh.c : Likewise. > > > * config/sparc/sparc.c : Likewise. > > > * config/tilegx/tilegx.c : Likewise. > > > * config/tilepro/tilepro.c : Likewise. > > > * config/visium/visium.c : Likewise. > > > * config/xtensa/xtensa.c : Likewise. > > > * testsuite/gcc.target/powerpc/pr88877.c : Newtest > > > > -- > Richard Biener > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: issue with behavior change of gcc -r between gcc-8 and gcc-9
Hello Allan, Thanks for following up. > On 02 Apr 2020, at 10:37, Allan Sandfeld Jensen wrote: > -r is used for relinking. The idea behind the change was to make it directly > suitable for that. In my mind, it was just a means to convey "I will relink this somehow later on, don't complain if the closure is incomplete", while you control what goes in the closure or not through means intended for that (-nostdlib, -nostartfiles). > It takes object files and relinks them into a new object > file. It gives the caller complete control. It's true that the caller can resort to -Wl if he doesn't like what -r now does, which is more than before. > It sounds like you are missing some way to add startfiles? A reverse of > -nostartfiles? Sort of, but I'm not sure this would be a better way out than using -Wl. > But hopefully you can just use the linker directly? I don't know yet. > Unless you have LTO > enabled object files you dont need the compiler to link. Part of the issue is all the scripts, makefiles and IDEs driving operations that might implicitly or explicitly rely on a behavior which suddenly changes. Or pieces of documentation here and there. I'm afraid we might have no other choice but to arrange to come back to the previous behavior somehow, at least for VxWorks. I still have at least a few experiments to conduct. Olivier
Re: [PATCH v2 00/11] aarch64: Implement TImode comparisons
"Richard Earnshaw (lists)" writes: > On 03/04/2020 16:03, Richard Sandiford wrote: >> "Richard Earnshaw (lists)" writes: >>> On 03/04/2020 13:27, Richard Sandiford wrote: "Richard Earnshaw (lists)" writes: > On 02/04/2020 19:53, Richard Henderson via Gcc-patches wrote: >> This is attacking case 3 of PR 94174. >> >> In v2, I unify the various subtract-with-borrow and add-with-carry >> patterns that also output flags with unspecs. As suggested by >> Richard Sandiford during review of v1. It does seem cleaner. >> > > Really? I didn't need to use any unspecs for the Arm version of this. > > R. See https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543063.html (including quoted context) for how we got here. The same problem affects the existing aarch64 patterns like *usub3_carryinC. Although that pattern avoids unspecs, the compare:CC doesn't seem to be correct. Richard >>> >>> But I don't think you can use ANY_EXTEND in these comparisons. It >>> doesn't describe what the instruction does, since the instruction does >>> not really extend the values first. >> >> Yeah, that was the starting point in the thread above too. And using >> zero_extend in the existing *usub3_carryinC pattern: >> >> (define_insn "*usub3_carryinC" >> [(set (reg:CC CC_REGNUM) >> (compare:CC >>(zero_extend: >> (match_operand:GPI 1 "register_operand" "r")) >>(plus: >> (zero_extend: >>(match_operand:GPI 2 "register_operand" "r")) >> (match_operand: 3 "aarch64_borrow_operation" "" >>(set (match_operand:GPI 0 "register_operand" "=r") >> (minus:GPI >>(minus:GPI (match_dup 1) (match_dup 2)) >>(match_operand:GPI 4 "aarch64_borrow_operation" "")))] >>"" >>"sbcs\\t%0, %1, %2" >> [(set_attr "type" "adc_reg")] >> ) >> >> looks wrong for the same reason. But the main problem IMO isn't how the >> inputs to the compare:CC are represented, but that we're using compare:CC >> at all. Using compare doesn't accurately model the effect of SBCS on NZCV >> for all inputs, so if we're going to use a compare here, it can't be :CC. >> >>> I would really expect this patch series to be pretty much a dual of this >>> series that I posted last year for Arm. >>> >>> https://gcc.gnu.org/pipermail/gcc-patches/2019-October/532180.html >> >> That series uses compares with modes like CC_V and CC_B, so I think >> you're saying that given the choice in the earlier thread between adding >> a new CC mode or using unspecs, you would have preferred a new CC mode, >> is that right? >> > > Yes. It surprised me, when doing the aarch32 version, just how often > the mid-end parts of the compiler were able to reason about parts of the > parallel insn and optimize things accordingly (eg propagating the truth > of the comparison). If you use an unspec that can never happen. That could be changed though. E.g. we could add something like a simplify_unspec target hook if this becomes a problem (either here or for other unspecs). A fancy implementation could even use match.pd-style rules in the .md file. The reason I'm not keen on using special modes for this case is that they'd describe one way in which the result can be used rather than describing what the instruction actually does. The instruction really does set all four flags to useful values. The "problem" is that they're not the values associated with a compare between two values, so representing them that way will always lose information. Thanks, Richard
Re: [PATCH v2 00/11] aarch64: Implement TImode comparisons
On Mon, Apr 6, 2020 at 1:20 PM Richard Sandiford wrote: > > "Richard Earnshaw (lists)" writes: > > On 03/04/2020 16:03, Richard Sandiford wrote: > >> "Richard Earnshaw (lists)" writes: > >>> On 03/04/2020 13:27, Richard Sandiford wrote: > "Richard Earnshaw (lists)" writes: > > On 02/04/2020 19:53, Richard Henderson via Gcc-patches wrote: > >> This is attacking case 3 of PR 94174. > >> > >> In v2, I unify the various subtract-with-borrow and add-with-carry > >> patterns that also output flags with unspecs. As suggested by > >> Richard Sandiford during review of v1. It does seem cleaner. > >> > > > > Really? I didn't need to use any unspecs for the Arm version of this. > > > > R. > > See https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543063.html > (including quoted context) for how we got here. > > The same problem affects the existing aarch64 patterns like > *usub3_carryinC. Although that pattern avoids unspecs, > the compare:CC doesn't seem to be correct. > > Richard > > >>> > >>> But I don't think you can use ANY_EXTEND in these comparisons. It > >>> doesn't describe what the instruction does, since the instruction does > >>> not really extend the values first. > >> > >> Yeah, that was the starting point in the thread above too. And using > >> zero_extend in the existing *usub3_carryinC pattern: > >> > >> (define_insn "*usub3_carryinC" > >> [(set (reg:CC CC_REGNUM) > >> (compare:CC > >>(zero_extend: > >> (match_operand:GPI 1 "register_operand" "r")) > >>(plus: > >> (zero_extend: > >>(match_operand:GPI 2 "register_operand" "r")) > >> (match_operand: 3 "aarch64_borrow_operation" "" > >>(set (match_operand:GPI 0 "register_operand" "=r") > >> (minus:GPI > >>(minus:GPI (match_dup 1) (match_dup 2)) > >>(match_operand:GPI 4 "aarch64_borrow_operation" "")))] > >>"" > >>"sbcs\\t%0, %1, %2" > >> [(set_attr "type" "adc_reg")] > >> ) > >> > >> looks wrong for the same reason. But the main problem IMO isn't how the > >> inputs to the compare:CC are represented, but that we're using compare:CC > >> at all. Using compare doesn't accurately model the effect of SBCS on NZCV > >> for all inputs, so if we're going to use a compare here, it can't be :CC. > >> > >>> I would really expect this patch series to be pretty much a dual of this > >>> series that I posted last year for Arm. > >>> > >>> https://gcc.gnu.org/pipermail/gcc-patches/2019-October/532180.html > >> > >> That series uses compares with modes like CC_V and CC_B, so I think > >> you're saying that given the choice in the earlier thread between adding > >> a new CC mode or using unspecs, you would have preferred a new CC mode, > >> is that right? > >> > > > > Yes. It surprised me, when doing the aarch32 version, just how often > > the mid-end parts of the compiler were able to reason about parts of the > > parallel insn and optimize things accordingly (eg propagating the truth > > of the comparison). If you use an unspec that can never happen. > > That could be changed though. E.g. we could add something like a > simplify_unspec target hook if this becomes a problem (either here > or for other unspecs). A fancy implementation could even use > match.pd-style rules in the .md file. > > The reason I'm not keen on using special modes for this case is that > they'd describe one way in which the result can be used rather than > describing what the instruction actually does. The instruction really > does set all four flags to useful values. The "problem" is that they're > not the values associated with a compare between two values, so representing > them that way will always lose information. Can't you recover the pieces by using a parallel with multiple set:CC_X that tie together the pieces in the "correct" way? Richard. > Thanks, > Richard
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
On Mon, Apr 6, 2020 at 11:18 AM Richard Sandiford
wrote:
>
> Martin Liška writes:
> > Hello.
> >
> > This is second attempt to get rid of tcc_comparison GENERIC trees
> > to be used as the first argument of VEC_COND_EXPR.
> >
> > The patch attempts achieves that in the following steps:
> > 1) veclower pass expands all tcc_comparison expression into a SSA_NAME
> > 2) since that tcc_comparsion can't be used as the first argument of
> > VEC_COND_EXPR
> > (done in GIMPLE verifier)
> > 3) I exposed new internal functions with:
> > DEF_INTERNAL_OPTAB_FN (VCOND, 0, vcond, vec_cond)
> > DEF_INTERNAL_OPTAB_FN (VCONDU, 0, vcondu, vec_condu)
> > DEF_INTERNAL_OPTAB_FN (VCONDEQ, 0, vcondeq, vec_condeq)
> > DEF_INTERNAL_OPTAB_FN (VCOND_MASK, 0, vcond_mask, vec_cond_mask)
> >
> > 4) logic of expand_vec_cond_expr is moved into the new pass_gimple_isel pass
> > 5) the pass expands VEC_COND_EXPR into one of the internal functions
> > defined in 3)
> > 6) moreover, I've added a new logic that prefers expand_vec_cmp_expr_p when
> > a SSA_NAME is being used in multiple (2+) VEC_COND_EXPR statements
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > Moreover, I run SPEC2006 and SPEC2017 benchmarks on znver1, znver2 and
> > skylake
> > target and I don't see any reasonable change.
> >
> > Achieved benefits of the patch:
> > - removal of a GENERIC expression being used in GIMPLE statements
> > - extraction into SSA_NAMEs can enable proper tree optimizer (FRE, DOM, PRE)
> > - possibility to expand smarter based on number of uses
> > (expand_vec_cmp_expr_p)
> >
> > Future plans:
> > - tcc_comparison removal just during gimplification
> > - removal of a code where these expressions are handled for VEC_COND_EXPR
> > - do the similar thing for COND_EXPR?
> >
> > The task was guided by Richi (Biener) and I bet he can help with both
> > further questions
> > and reasoning.
>
> Thanks for doing this. It definitely seems more friendly than the
> four-operand version to targets where separate comparisons are the norm.
>
> Just a couple of comments about the implementation:
>
> > diff --git a/gcc/passes.def b/gcc/passes.def
> > index 2bf2cb78fc5..d654e5ee9fe 100644
> > --- a/gcc/passes.def
> > +++ b/gcc/passes.def
> > @@ -397,6 +397,7 @@ along with GCC; see the file COPYING3. If not see
> >NEXT_PASS (pass_cleanup_eh);
> >NEXT_PASS (pass_lower_resx);
> >NEXT_PASS (pass_nrv);
> > + NEXT_PASS (pass_gimple_isel);
> >NEXT_PASS (pass_cleanup_cfg_post_optimizing);
> >NEXT_PASS (pass_warn_function_noreturn);
> >NEXT_PASS (pass_gen_hsail);
>
> What was the reason for making this a separate pass, rather than doing
> it as part of veclower? If we do them separately, then it's harder for
> veclower to know which VEC_COND_EXPRs it needs to open-code. (OK, so
> that's a general problem between veclower and expand already, but it
> seems like the new approach could help to move away from that by
> doing the instruction selection directly in veclower.)
As the name of the pass suggests it was supposed to be the starting point
of doing all the "complex" (multi-GIMPLE-stmt matching) RTL expansion tricks.
But most importantly veclower is too early to catch CSE opportunities from
loop opts on the conditions and if veclower lowers things then we also want
CSE to cleanup its mess. I guess we also do not want veclower to be done
before vectorization since it should be easier to re-vectorize from unsupported
vector code than from what veclower makes out of it ... catch-22.
So I consider pass placement a secondary issue for now.
> > +/* Expand all VEC_COND_EXPR gimple assignments into calls to internal
> > + function based on type of selected expansion. */
> > +
> > +static gimple *
> > +gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi)
> > +{
> > + tree lhs, op0a = NULL_TREE, op0b = NULL_TREE;
> > + enum tree_code code;
> > + enum tree_code tcode;
> > + machine_mode cmp_op_mode;
> > + bool unsignedp;
> > + enum insn_code icode;
> > + imm_use_iterator imm_iter;
> > +
> > + /* Only consider code == GIMPLE_ASSIGN. */
> > + gassign *stmt = dyn_cast (gsi_stmt (*gsi));
> > + if (!stmt)
> > +return NULL;
> > +
> > + code = gimple_assign_rhs_code (stmt);
> > + if (code != VEC_COND_EXPR)
> > +return NULL;
> > +
> > + tree op0 = gimple_assign_rhs1 (stmt);
> > + tree op1 = gimple_assign_rhs2 (stmt);
> > + tree op2 = gimple_assign_rhs3 (stmt);
> > + lhs = gimple_assign_lhs (stmt);
> > + machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
> > +
> > + gcc_assert (!COMPARISON_CLASS_P (op0));
> > + if (TREE_CODE (op0) == SSA_NAME)
> > +{
> > + unsigned int used_vec_cond_exprs = 0;
> > + gimple *use_stmt;
> > + FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, op0)
> > + {
> > + gassign *assign = dyn_cast (use_stmt);
> > + if (assign != NULL && gimple_assign_rhs_code (assign) ==
> > VEC_COND_EXPR
> > + && gimple_assign_rhs1 (assig
Re: [Patch] HSA: omp-grid.c – access proper clause code (was: Re: [Patch] omp-grid.c – add cast to silence different enumeration types warning)
On Fri, Apr 03, 2020 at 12:43:42PM +0200, Tobias Burnus wrote:
> HSA: omp-grid.c – access proper clause code
>
> * omp-grid.c (grid_eliminate_combined_simd_part): Use
> OMP_CLAUSE_CODE to access the omp clause code.
>
> diff --git a/gcc/omp-grid.c b/gcc/omp-grid.c
> index b98e45de6a0..ba635fd3ea2 100644
> --- a/gcc/omp-grid.c
> +++ b/gcc/omp-grid.c
> @@ -1065,7 +1065,7 @@ grid_eliminate_combined_simd_part (gomp_for *parloop)
>while (*pc)
> {
>tree c = *pc;
> - switch (TREE_CODE (c))
> + switch (OMP_CLAUSE_CODE (c))
> {
> case OMP_CLAUSE_LINEAR:
> {
It looks good to me, but I have no way to actually test it, nor understand
why it worked (or isn't covered by testsuite) in the HSAIL offloading case.
Martin?
Jakub
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
On Mon, Apr 6, 2020 at 11:18 AM Richard Sandiford
wrote:
>
> Martin Liška writes:
> > Hello.
> >
> > This is second attempt to get rid of tcc_comparison GENERIC trees
> > to be used as the first argument of VEC_COND_EXPR.
> >
> > The patch attempts achieves that in the following steps:
> > 1) veclower pass expands all tcc_comparison expression into a SSA_NAME
> > 2) since that tcc_comparsion can't be used as the first argument of
> > VEC_COND_EXPR
> > (done in GIMPLE verifier)
> > 3) I exposed new internal functions with:
> > DEF_INTERNAL_OPTAB_FN (VCOND, 0, vcond, vec_cond)
> > DEF_INTERNAL_OPTAB_FN (VCONDU, 0, vcondu, vec_condu)
> > DEF_INTERNAL_OPTAB_FN (VCONDEQ, 0, vcondeq, vec_condeq)
> > DEF_INTERNAL_OPTAB_FN (VCOND_MASK, 0, vcond_mask, vec_cond_mask)
> >
> > 4) logic of expand_vec_cond_expr is moved into the new pass_gimple_isel pass
> > 5) the pass expands VEC_COND_EXPR into one of the internal functions
> > defined in 3)
> > 6) moreover, I've added a new logic that prefers expand_vec_cmp_expr_p when
> > a SSA_NAME is being used in multiple (2+) VEC_COND_EXPR statements
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > Moreover, I run SPEC2006 and SPEC2017 benchmarks on znver1, znver2 and
> > skylake
> > target and I don't see any reasonable change.
> >
> > Achieved benefits of the patch:
> > - removal of a GENERIC expression being used in GIMPLE statements
> > - extraction into SSA_NAMEs can enable proper tree optimizer (FRE, DOM, PRE)
> > - possibility to expand smarter based on number of uses
> > (expand_vec_cmp_expr_p)
> >
> > Future plans:
> > - tcc_comparison removal just during gimplification
> > - removal of a code where these expressions are handled for VEC_COND_EXPR
> > - do the similar thing for COND_EXPR?
> >
> > The task was guided by Richi (Biener) and I bet he can help with both
> > further questions
> > and reasoning.
>
> Thanks for doing this. It definitely seems more friendly than the
> four-operand version to targets where separate comparisons are the norm.
>
> Just a couple of comments about the implementation:
>
> > diff --git a/gcc/passes.def b/gcc/passes.def
> > index 2bf2cb78fc5..d654e5ee9fe 100644
> > --- a/gcc/passes.def
> > +++ b/gcc/passes.def
> > @@ -397,6 +397,7 @@ along with GCC; see the file COPYING3. If not see
> >NEXT_PASS (pass_cleanup_eh);
> >NEXT_PASS (pass_lower_resx);
> >NEXT_PASS (pass_nrv);
> > + NEXT_PASS (pass_gimple_isel);
> >NEXT_PASS (pass_cleanup_cfg_post_optimizing);
> >NEXT_PASS (pass_warn_function_noreturn);
> >NEXT_PASS (pass_gen_hsail);
>
> What was the reason for making this a separate pass, rather than doing
> it as part of veclower? If we do them separately, then it's harder for
> veclower to know which VEC_COND_EXPRs it needs to open-code. (OK, so
> that's a general problem between veclower and expand already, but it
> seems like the new approach could help to move away from that by
> doing the instruction selection directly in veclower.)
>
> > +/* Expand all VEC_COND_EXPR gimple assignments into calls to internal
> > + function based on type of selected expansion. */
> > +
> > +static gimple *
> > +gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi)
> > +{
> > + tree lhs, op0a = NULL_TREE, op0b = NULL_TREE;
> > + enum tree_code code;
> > + enum tree_code tcode;
> > + machine_mode cmp_op_mode;
> > + bool unsignedp;
> > + enum insn_code icode;
> > + imm_use_iterator imm_iter;
> > +
> > + /* Only consider code == GIMPLE_ASSIGN. */
> > + gassign *stmt = dyn_cast (gsi_stmt (*gsi));
> > + if (!stmt)
> > +return NULL;
> > +
> > + code = gimple_assign_rhs_code (stmt);
> > + if (code != VEC_COND_EXPR)
> > +return NULL;
> > +
> > + tree op0 = gimple_assign_rhs1 (stmt);
> > + tree op1 = gimple_assign_rhs2 (stmt);
> > + tree op2 = gimple_assign_rhs3 (stmt);
> > + lhs = gimple_assign_lhs (stmt);
> > + machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
> > +
> > + gcc_assert (!COMPARISON_CLASS_P (op0));
> > + if (TREE_CODE (op0) == SSA_NAME)
> > +{
> > + unsigned int used_vec_cond_exprs = 0;
> > + gimple *use_stmt;
> > + FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, op0)
> > + {
> > + gassign *assign = dyn_cast (use_stmt);
> > + if (assign != NULL && gimple_assign_rhs_code (assign) ==
> > VEC_COND_EXPR
> > + && gimple_assign_rhs1 (assign) == op0)
> > + used_vec_cond_exprs++;
> > + }
>
> This looks like it's quadratic in the worst case. Could we check
> this in a different way?
We could remember a SSA names cond-expr-uses and thus only compute it
once.
> > +
> > + gassign *def_stmt = dyn_cast (SSA_NAME_DEF_STMT (op0));
> > + if (def_stmt)
> > + {
> > + tcode = gimple_assign_rhs_code (def_stmt);
> > + op0a = gimple_assign_rhs1 (def_stmt);
> > + op0b = gimple_assign_rhs2 (def_stmt);
> > +
> > + tree op0a_type = TREE_TYPE (op0a);
> > + if (used_vec_con
Re: [PATCH] Check DECL_CONTEXT of new/delete operators.
On 4/6/20 4:34 AM, Martin Liška wrote:
May I please ping Jason, Nathan and Jonathan who can help us here?
On IRC Martin clarified the question as essentially 'how do you pair up
operator new and operator delete calls?' (so you may delete them if the
object is not used).
I am not sure you're permitted to remove those calls in general. All I
can find is [expr.new]/12
'An implementation is allowed to omit a call to a replaceable global
allocation function (17.6.2.1, 17.6.2.2). When it does so, the storage
is instead provided by the implementation or provided by extending the
allocation of another new-expression.'
But, I suspect the optimization is safe, in that either no one counts
objects by their allocation, or if they do, they don't actually care
that the std-conforming number of allocations happen.
The both operator new and operator delete are looked up in the same
manner. The std does not require a 'matching pair' be found. but it is
extremely poor form for a class to declare exactly one of operator
{new,delete}.
The following is well formed:
struct PoorForm {
void *operator new (size_t s) {count++; return ::operator new (s)};
static unsigned count;
};
Have you considered throwing ctors?
struct Obj {
Obj (); // might throw
};
'obj = new Obj (); ... delete obj;' sequence expand to something like ...
// obj = new Obj ();
void *p = ::operator new (sizeof (Obj));
try {
Obj::ctor(p);
}
catch (...) // cleanup code
{
::operator delete (p); // #1
throw;
}
obj = (Obj*)p;
user code
// delete obj;
Obj::dtor (obj);
::operator delete (obj); // #2
calls 1 & 2 matchup to the operator new call
Notice that para I quoted allows one to coalesce allocations using the
global operator new/deletes. The rules are pretty much as you can guess
-- one lifetime must be entirely within the other. If inner one's ctor
throws, the exception path must destroy the outer.
does that help?
nathan
--
Nathan Sidwell
ICE on wrong code [PR94192]
Hi all,
I'm new to gcc and this is my first patch. The idea is not have another
resolution of a pointer if an error has occurred previously. I hope this
meets all the criteria for a patch. In case anything is missing or
wrong, I'm open to add to or change the patch.
Best regards,
Linus König
2020-04-06 Linus Koenig
PR fortran/94192
* resolve.c (resolve_fl_var_and_proc): Set flag "error" to 1 if
pointer is found to not have an assumed rank or a deferred shape.
* simplify.c (simplify_bound): If an error has been issued for a
given pointer, one should not attempt to find its bounds.
2020-04-06 Linus Koenig
PR fortran/94192
* gfortran.dg/bound_resolve_after_error_1.f90: New test.
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 23b5a2b4439..ca149c0dd00 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12607,6 +12607,7 @@ resolve_fl_var_and_proc (gfc_symbol *sym, int mp_flag)
{
gfc_error ("Array pointer %qs at %L must have a deferred shape or "
"assumed rank", sym->name, &sym->declared_at);
+ sym->error = 1;
return false;
}
}
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 66ed925c10d..54af85bcab8 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4089,10 +4089,19 @@ returnNull:
static gfc_expr *
simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr *kind, int upper)
{
+ gfc_symbol *array_sym;
gfc_ref *ref;
gfc_array_spec *as;
int d;
+ /* Do not attempt to resolve if error has already been issued. */
+ if (array&&array->symtree&&array->symtree->n.sym)
+{
+ array_sym = array->symtree->n.sym;
+ if (array_sym->error && array_sym->resolved)
+ return NULL;
+}
+
if (array->ts.type == BT_CLASS)
return NULL;
! Testcase for bound check after issued error
! See PR 94192
! { dg-do compile }
program bound_for_illegal
contains
subroutine bnds(a) ! { dg-error "must have a deferred shape or assumed rank"
}
integer, pointer, intent(in) :: a(1:2)
print *,lbound(a)
end subroutine bnds
end program bound_for_illegal
Re: [PATCH v2] c++: Fix crash in gimplifier with paren init of aggregates [PR94155]
On 4/4/20 1:56 PM, Marek Polacek wrote:
On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via Gcc-patches wrote:
On 4/3/20 9:08 PM, Marek Polacek wrote:
On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches wrote:
On 3/30/20 4:28 PM, Marek Polacek wrote:
Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
expect null indexes for a constructor:
/* ??? Here's to hoping the front end fills in all of the indices,
so we don't have to figure out what's missing ourselves. */
gcc_assert (purpose);
The indexes weren't filled because we never called reshape_init: for
a constructor that represents parenthesized initialization of an
aggregate we don't allow brace elision or designated initializers. So
fill in the indexes manually, here we have an array, and we can simply
assign indexes starting from 0.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
Shouldn't digest_init fill in the indexes? In
process_init_constructor_array I see
if (!ce->index)
ce->index = size_int (i);
Yes, that works too. Thus:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
-- >8 --
Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
expect null indexes for a constructor:
/* ??? Here's to hoping the front end fills in all of the indices,
so we don't have to figure out what's missing ourselves. */
gcc_assert (purpose);
The indexes weren't filled because we never called reshape_init: for
a constructor that represents parenthesized initialization of an
aggregate we don't allow brace elision or designated initializers. So
call digest_init to fill in the indexes.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
PR c++/94155 - crash in gimplifier with paren init of aggregates.
* decl.c (check_initializer): Call digest_init.
* g++.dg/cpp2a/paren-init22.C: New test.
---
gcc/cp/decl.c | 5 +
gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 69a238997b4..63e7bda09f5 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int flags,
vec **cleanups)
init = build_constructor_from_list (init_list_type_node, init);
CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
CONSTRUCTOR_IS_PAREN_INIT (init) = true;
+ /* The gimplifier expects that the front end fills in all of the
+indices. Normally, reshape_init_array fills these in, but we
+don't call reshape_init because that does nothing when it gets
+CONSTRUCTOR_IS_PAREN_INIT. */
+ init = digest_init (type, init, tf_warning_or_error);
But why weren't we already calling digest_init in store_init_value? Was the
CONSTRUCTOR making it all the way to gimplification still having
init_list_type_node?
It's because we set LOOKUP_ALREADY_DIGESTED a few lines below:
6813 /* Don't call digest_init; it's unnecessary and will
complain
6814 about aggregate initialization of non-aggregate
classes. */
6815 flags |= LOOKUP_ALREADY_DIGESTED;
and so store_init_value doesn't digest. Given the comment I'd be nervous about
not setting that flag here.
OK, then why isn't it called by build_aggr_init? How is the CONSTRUCTOR
getting a type without this being fixed up?
...
Ah, because build_vec_init builds up a new CONSTRUCTOR and gives it a
type without setting the indexes like process_init_constructor_array does:
Jason
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 27623cf4db1..ea95a3bc910 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree init,
errors = true;
if (try_const)
{
+ if (!field)
+ field = size_int (idx);
tree e = maybe_constant_init (one_init);
if (reduced_constant_expression_p (e))
{
Re: [PATCH] Check DECL_CONTEXT of new/delete operators.
On Mon, Apr 6, 2020 at 5:27 AM Richard Biener via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:
> On Sat, Apr 4, 2020 at 1:53 PM Jan Hubicka wrote:
> >
> > Hi,
> > thinking a bit of the problem, I guess we could match in addition to
> > DECL_CONTEXT the whole inline stack of both statements and see if there
> > are inlined new/delete operators and if so if they are always in
> > matching pairs.
> >
> > The inline stack is available as
> > for (tree block = gimple_block (call); block && TREE_CODE (block) ==
> BLOCK; block = BLOCK_SUPERCONTEXT (block))
> > {
> > tree fn = block_ultimate_origin (block);
> > if (fn != NULL && TREE_CODE (fn) == FUNCTION_DECL)
> > do the checking htere.
> > }
> >
> > But I do not understand what C++ promises here and in what conditions
> > the new/delete pair can be removed.
>
> But if the inline stack matches in pairs then the ultimate new/delete
> call should also
> match, no? When there's a mismatch in inlining we can't DCE since we
> can't remove
> the extra inlined stmts.
>
> Your failing testcase suggests we never can remove new/delete pairs though
> unless the DECL_CONTEXT is 'final'. Also the user could have chosen to
> "inline" the side-effect of the new operation manually but not the
> delete one, so
>
> operator delete() { count-- }
>
> ptr = new A;
> count++;
> delete ptr;
>
> is it valid to elide the new/delete pair here?
>
The C++ constraints are (deliberately, I think) vague. As Nathan quotes,
it just says that a call to a replaceable operator new can be omitted, and
that if it is, the matching delete-expression should not call operator
delete. This example seems to demonstrate that we should also only
consider the replaceable delete operators, not any operator delete.
Jason
[testsuite][arm] Fix cmse-15.c expected output
Hi,
While checking Martin's fix for PR ipa/94445, he made me realize that
the cmse-15.c testcase still fails at -Os because ICF means that we
generate
nonsecure2:
b nonsecure0
which is OK, but does not match the currently expected
nonsecure2:
...
bl __gnu_cmse_nonsecure_call
(see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html)
The test has already different expectations for v8-M and v8.1-M.
I've decided to try to use check-function-bodies to account for the
different possibilities:
- v8-M vs v8.1-M via two different prefixes
- code generation variants (-0?) via multiple regexps
I've tested that the test now passes with --target-board=-march=armv8-m.main
and --target-board=-march=armv8.1-m.main.
I feel this a bit too much of a burden for the purpose, maybe there's
a better way of handling all these alternatives (in particular,
there's a lot of duplication since the expected code for the secure*
functions is the same for v8-M and v8.1-M).
OK?
Thanks,
Christophe
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
index 0e37b50..603c456 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
@@ -1,5 +1,9 @@
/* { dg-do compile } */
/* { dg-options "-mcmse" } */
+/* ARMv8-M expectation. */
+/* { dg-final { check-function-bodies "*Noclear" "" "" { target { !
arm_cmse_clear_ok } } } } */
+/* ARMv8.1-M expectation. */
+/* { dg-final { check-function-bodies "*Clear" "" "" { target
arm_cmse_clear_ok } } } */
int __attribute__ ((cmse_nonsecure_call)) (*ns_foo) (void);
int (*s_bar) (void);
@@ -11,67 +15,242 @@ typedef int s_bar_t (void);
typedef int __attribute__ ((cmse_nonsecure_call)) (* ns_foo_ptr) (void);
typedef int (*s_bar_ptr) (void);
+/*
+*Clear nonsecure0:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure0:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure0 (ns_foo_t * ns_foo_p)
{
return ns_foo_p ();
}
+/*
+*Clear nonsecure1:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure1:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure1 (ns_foo_t ** ns_foo_p)
{
return (*ns_foo_p) ();
}
+/*
+*Clear nonsecure2:
+*Clear ...
+*Clear (
+*Clear blxns r[0-3]
+*Clear |
+*Clear b nonsecure0
+*Clear )
+*Clear ...
+*Noclear nonsecure2:
+*Noclear ...
+*Noclear (
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear |
+*Noclear b nonsecure0
+*Noclear )
+*Noclear ...
+*/
int nonsecure2 (ns_foo_ptr ns_foo_p)
{
return ns_foo_p ();
}
+
+/*
+*Clear nonsecure3:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure3:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure3 (ns_foo_ptr * ns_foo_p)
{
return (*ns_foo_p) ();
}
+/*
+*Clear secure0:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear )
+*Clear ...
+*Noclear secure0:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear )
+*Noclear ...
+*/
int secure0 (s_bar_t * s_bar_p)
{
return s_bar_p ();
}
+/*
+*Clear secure1:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear )
+*Clear ...
+*Noclear secure1:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear )
+*Noclear ...
+*/
int secure1 (s_bar_t ** s_bar_p)
{
return (*s_bar_p) ();
}
+/*
+*Clear secure2:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear |
+*Clear b secure0
+*Clear )
+*Clear ...
+*Noclear secure2:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear |
+*Noclear b secure0
+*Noclear )
+*Noclear ...
+*/
int secure2 (s_bar_ptr s_bar_p)
{
return s_bar_p ();
}
+/*
+*Clear secure3:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear )
+*Clear ...
+*Noclear secure3:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear )
+*Noclear ...
+*/
int secure3 (s_bar_ptr * s_bar_p)
{
return (*s_bar_p) ();
}
+/*
+*Clear nonsecure4:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure4:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure4 (void)
{
return ns_foo ();
}
+/*
+*Clear nonsecure5:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure5:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure5 (void)
{
return (*ns_foo2) ();
}
+/*
+*Clear
Re: ICE on wrong code [PR94192]
On Mon, Apr 6, 2020 at 8:51 AM Linus König wrote:
>
> Hi all,
>
> I'm new to gcc and this is my first patch. The idea is not have another
> resolution of a pointer if an error has occurred previously. I hope this
> meets all the criteria for a patch. In case anything is missing or
> wrong, I'm open to add to or change the patch.
First, thanks for your work.
In principle the patch looks fine. I was surprised that I could find
no other code that uses gfc_symbol::error; however, your usage of it
seems appropriate.
A note about style, there should be spaces between logical operators:
/* Do not attempt to resolve if error has already been issued. */
- if (array&&array->symtree&&array->symtree->n.sym)
+ if (array && array->symtree && array->symtree->n.sym)
{
Additionally, in simplify.c (simplify_bound) you can see the
surrounding code does not check array or array->symtree->n.sym for
NULL. One can assume this precondition holds and factor the code
slightly simpler. I believe also it is sufficient to check only "if
(array_sym->error)". By the time sym->error is set in
resolve_fl_var_and_proc, sym->resolved must already be set to 1
because this is the first step in resolve_symbol.
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 66ed925c10d..84aeafc1e8b 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4090,6 +4090,7 @@ static gfc_expr *
simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr *kind, int upper)
{
gfc_ref *ref;
+ gfc_symbol *array_sym;
gfc_array_spec *as;
int d;
@@ -4103,8 +4104,13 @@ simplify_bound (gfc_expr *array, gfc_expr *dim,
gfc_expr *kind, int upper)
goto done;
}
+ /* Do not attempt to resolve if error has already been issued. */
+ array_sym = array->symtree->n.sym;
+ if (array_sym->error)
+return NULL;
+
/* Follow any component references. */
- as = array->symtree->n.sym->as;
+ as = array_sym->as;
for (ref = array->ref; ref; ref = ref->next)
{
switch (ref->type)
Thanks again for your patch.
---
Fritz Reese
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Wed, 1 Apr 2020, Jason Merrill wrote:
> On 4/1/20 6:29 PM, Jason Merrill wrote:
> > On 3/31/20 3:50 PM, Patrick Palka wrote:
> > > On Tue, 31 Mar 2020, Jason Merrill wrote:
> > >
> > > > On 3/30/20 6:46 PM, Patrick Palka wrote:
> > > > > On Mon, 30 Mar 2020, Jason Merrill wrote:
> > > > > > On 3/30/20 3:58 PM, Patrick Palka wrote:
> > > > > > > On Thu, 26 Mar 2020, Jason Merrill wrote:
> > > > > > >
> > > > > > > > On 3/22/20 9:21 PM, Patrick Palka wrote:
> > > > > > > > > This patch relaxes an assertion in tsubst_default_argument
> > > > > > > > > that
> > > > > > > > > exposes
> > > > > > > > > a
> > > > > > > > > latent
> > > > > > > > > bug in how we substitute an array type into a cv-qualified
> > > > > > > > > wildcard
> > > > > > > > > function
> > > > > > > > > parameter type. Concretely, the latent bug is that given the
> > > > > > > > > function
> > > > > > > > > template
> > > > > > > > >
> > > > > > > > > template void foo(const T t);
> > > > > > > > >
> > > > > > > > > one would expect the type of foo to be void(const
> > > > > > > > > int*), but
> > > > > > > > > we
> > > > > > > > > (seemingly prematurely) strip function parameter types of
> > > > > > > > > their
> > > > > > > > > top-level
> > > > > > > > > cv-qualifiers when building the function's TYPE_ARG_TYPES, and
> > > > > > > > > instead
> > > > > > > > > end
> > > > > > > > > up
> > > > > > > > > obtaining void(int*) as the type of foo after
> > > > > > > > > substitution
> > > > > > > > > and
> > > > > > > > > decaying.
> > > > > > > > >
> > > > > > > > > We still however correctly substitute into and decay the
> > > > > > > > > formal
> > > > > > > > > parameter
> > > > > > > > > type,
> > > > > > > > > obtaining const int* as the type of t after substitution. But
> > > > > > > > > this
> > > > > > > > > then
> > > > > > > > > leads
> > > > > > > > > to us tripping over the assert in tsubst_default_argument that
> > > > > > > > > verifies
> > > > > > > > > the
> > > > > > > > > formal parameter type and the function type are consistent.
> > > > > > > > >
> > > > > > > > > Assuming it's too late at this stage to fix the substitution
> > > > > > > > > bug, we
> > > > > > > > > can
> > > > > > > > > still
> > > > > > > > > relax the assertion like so. Tested on x86_64-pc-linux-gnu,
> > > > > > > > > does
> > > > > > > > > this
> > > > > > > > > look
> > > > > > > > > OK?
> > > > > > > >
> > > > > > > > This is core issues 1001/1322, which have not been resolved.
> > > > > > > > Clang
> > > > > > > > does
> > > > > > > > the
> > > > > > > > substitution the way you suggest; EDG rejects the testcase
> > > > > > > > because the
> > > > > > > > two
> > > > > > > > substitutions produce different results. I think it would make
> > > > > > > > sense
> > > > > > > > to
> > > > > > > > follow the EDG behavior until this issue is actually resolved.
> > > > > > >
> > > > > > > Here is what I have so far towards that end. When substituting
> > > > > > > into the
> > > > > > > PARM_DECLs of a function decl, we now additionally check if the
> > > > > > > aforementioned Core issues are relevant and issue a (fatal)
> > > > > > > diagnostic
> > > > > > > if so. This patch checks this in tsubst_decl
> > > > > > > rather
> > > > > > > than in tsubst_function_decl for efficiency reasons, so that we
> > > > > > > don't
> > > > > > > have to perform another traversal over the DECL_ARGUMENTS /
> > > > > > > TYPE_ARG_TYPES just to implement this check.
> > > > > >
> > > > > > Hmm, this seems like writing more complicated code for a very
> > > > > > marginal
> > > > > > optimization; how many function templates have so many parameters
> > > > > > that
> > > > > > walking
> > > > > > over them once to compare types will have any effect on compile
> > > > > > time?
> > > > >
> > > > > Good point... though I just tried implementing this check in
> > > > > tsubst_function_decl, and it seems it might be just as complicated to
> > > > > implement it there instead, at least if we want to handle function
> > > > > parameter packs correctly.
> > > > >
> > > > > If we were to implement this check in tsubst_function_decl, then since
> > > > > we have access to the instantiated function, it would presumably
> > > > > suffice
> > > > > to compare its substituted DECL_ARGUMENTS with its substituted
> > > > > TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
> > > > > catch the original testcase, i.e.
> > > > >
> > > > > template
> > > > > void foo(const T);
> > > > > int main() { foo(0); }
> > > > >
> > > > > because the DECL_ARGUMENTS of foo would be {const int*} and its
> > > > > TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
> > > > > corresponding testcase that uses a function parameter pack, i.e.
> > > > >
> > > > > template
> > > > > void foo(const Ts...);
> > > > > int main() { foo(0); }
> > > > >
> > > > > because it turns out we don't strip top-level cv-qualif
Re: [PATCH v3] c++: Fix crash in gimplifier with paren init of aggregates [PR94155]
On Mon, Apr 06, 2020 at 10:47:49AM -0400, Jason Merrill via Gcc-patches wrote:
> On 4/4/20 1:56 PM, Marek Polacek wrote:
> > On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via Gcc-patches
> > wrote:
> > > On 4/3/20 9:08 PM, Marek Polacek wrote:
> > > > On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches
> > > > wrote:
> > > > > On 3/30/20 4:28 PM, Marek Polacek wrote:
> > > > > > Here we crash in the gimplifier because gimplify_init_ctor_eval
> > > > > > doesn't
> > > > > > expect null indexes for a constructor:
> > > > > >
> > > > > > /* ??? Here's to hoping the front end fills in all of the
> > > > > > indices,
> > > > > > so we don't have to figure out what's missing
> > > > > > ourselves. */
> > > > > > gcc_assert (purpose);
> > > > > >
> > > > > > The indexes weren't filled because we never called reshape_init: for
> > > > > > a constructor that represents parenthesized initialization of an
> > > > > > aggregate we don't allow brace elision or designated initializers.
> > > > > > So
> > > > > > fill in the indexes manually, here we have an array, and we can
> > > > > > simply
> > > > > > assign indexes starting from 0.
> > > > > >
> > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > >
> > > > > Shouldn't digest_init fill in the indexes? In
> > > > > process_init_constructor_array I see
> > > > >
> > > > > if (!ce->index)
> > > > > ce->index = size_int (i);
> > > >
> > > > Yes, that works too. Thus:
> > > >
> > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > >
> > > > -- >8 --
> > > > Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> > > > expect null indexes for a constructor:
> > > >
> > > > /* ??? Here's to hoping the front end fills in all of the
> > > > indices,
> > > >so we don't have to figure out what's missing ourselves. */
> > > > gcc_assert (purpose);
> > > >
> > > > The indexes weren't filled because we never called reshape_init: for
> > > > a constructor that represents parenthesized initialization of an
> > > > aggregate we don't allow brace elision or designated initializers. So
> > > > call digest_init to fill in the indexes.
> > > >
> > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > >
> > > > PR c++/94155 - crash in gimplifier with paren init of
> > > > aggregates.
> > > > * decl.c (check_initializer): Call digest_init.
> > > >
> > > > * g++.dg/cpp2a/paren-init22.C: New test.
> > > > ---
> > > >gcc/cp/decl.c | 5 +
> > > >gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++
> > > >2 files changed, 20 insertions(+)
> > > >create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> > > >
> > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > > > index 69a238997b4..63e7bda09f5 100644
> > > > --- a/gcc/cp/decl.c
> > > > +++ b/gcc/cp/decl.c
> > > > @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int
> > > > flags, vec **cleanups)
> > > > init = build_constructor_from_list (init_list_type_node,
> > > > init);
> > > > CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
> > > > CONSTRUCTOR_IS_PAREN_INIT (init) = true;
> > > > + /* The gimplifier expects that the front end fills in all
> > > > of the
> > > > +indices. Normally, reshape_init_array fills these in,
> > > > but we
> > > > +don't call reshape_init because that does nothing when
> > > > it gets
> > > > +CONSTRUCTOR_IS_PAREN_INIT. */
> > > > + init = digest_init (type, init, tf_warning_or_error);
> > >
> > > But why weren't we already calling digest_init in store_init_value? Was
> > > the
> > > CONSTRUCTOR making it all the way to gimplification still having
> > > init_list_type_node?
> >
> > It's because we set LOOKUP_ALREADY_DIGESTED a few lines below:
> > 6813 /* Don't call digest_init; it's unnecessary and will
> > complain
> > 6814 about aggregate initialization of non-aggregate
> > classes. */
> > 6815 flags |= LOOKUP_ALREADY_DIGESTED;
> > and so store_init_value doesn't digest. Given the comment I'd be nervous
> > about
> > not setting that flag here.
>
> OK, then why isn't it called by build_aggr_init? How is the CONSTRUCTOR
> getting a type without this being fixed up?
>
> ...
>
> Ah, because build_vec_init builds up a new CONSTRUCTOR and gives it a type
> without setting the indexes like process_init_constructor_array does:
>
> Jason
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index 27623cf4db1..ea95a3bc910 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree init,
> errors = true;
> if (try_const)
> {
> + if (!field)
> + fi
Re: [PATCH v3] c++: Fix crash in gimplifier with paren init of aggregates [PR94155]
Ok. On Mon, Apr 6, 2020, 11:57 AM Marek Polacek wrote: > On Mon, Apr 06, 2020 at 10:47:49AM -0400, Jason Merrill via Gcc-patches > wrote: > > On 4/4/20 1:56 PM, Marek Polacek wrote: > > > On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via > Gcc-patches wrote: > > > > On 4/3/20 9:08 PM, Marek Polacek wrote: > > > > > On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via > Gcc-patches wrote: > > > > > > On 3/30/20 4:28 PM, Marek Polacek wrote: > > > > > > > Here we crash in the gimplifier because > gimplify_init_ctor_eval doesn't > > > > > > > expect null indexes for a constructor: > > > > > > > > > > > > > > /* ??? Here's to hoping the front end fills in all of > the indices, > > > > > > > so we don't have to figure out what's missing > ourselves. */ > > > > > > > gcc_assert (purpose); > > > > > > > > > > > > > > The indexes weren't filled because we never called > reshape_init: for > > > > > > > a constructor that represents parenthesized initialization of > an > > > > > > > aggregate we don't allow brace elision or designated > initializers. So > > > > > > > fill in the indexes manually, here we have an array, and we > can simply > > > > > > > assign indexes starting from 0. > > > > > > > > > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > > > > > > > > Shouldn't digest_init fill in the indexes? In > > > > > > process_init_constructor_array I see > > > > > > > > > > > > if (!ce->index) > > > > > > ce->index = size_int (i); > > > > > > > > > > Yes, that works too. Thus: > > > > > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > > > > > > -- >8 -- > > > > > Here we crash in the gimplifier because gimplify_init_ctor_eval > doesn't > > > > > expect null indexes for a constructor: > > > > > > > > > > /* ??? Here's to hoping the front end fills in all of the > indices, > > > > >so we don't have to figure out what's missing > ourselves. */ > > > > > gcc_assert (purpose); > > > > > > > > > > The indexes weren't filled because we never called reshape_init: > for > > > > > a constructor that represents parenthesized initialization of an > > > > > aggregate we don't allow brace elision or designated > initializers. So > > > > > call digest_init to fill in the indexes. > > > > > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > > > > > > PR c++/94155 - crash in gimplifier with paren init of > aggregates. > > > > > * decl.c (check_initializer): Call digest_init. > > > > > > > > > > * g++.dg/cpp2a/paren-init22.C: New test. > > > > > --- > > > > >gcc/cp/decl.c | 5 + > > > > >gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++ > > > > >2 files changed, 20 insertions(+) > > > > >create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C > > > > > > > > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > > > > > index 69a238997b4..63e7bda09f5 100644 > > > > > --- a/gcc/cp/decl.c > > > > > +++ b/gcc/cp/decl.c > > > > > @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, > int flags, vec **cleanups) > > > > > init = build_constructor_from_list > (init_list_type_node, init); > > > > > CONSTRUCTOR_IS_DIRECT_INIT (init) = true; > > > > > CONSTRUCTOR_IS_PAREN_INIT (init) = true; > > > > > + /* The gimplifier expects that the front end fills > in all of the > > > > > +indices. Normally, reshape_init_array fills > these in, but we > > > > > +don't call reshape_init because that does nothing > when it gets > > > > > +CONSTRUCTOR_IS_PAREN_INIT. */ > > > > > + init = digest_init (type, init, tf_warning_or_error); > > > > > > > > But why weren't we already calling digest_init in store_init_value? > Was the > > > > CONSTRUCTOR making it all the way to gimplification still having > > > > init_list_type_node? > > > > > > It's because we set LOOKUP_ALREADY_DIGESTED a few lines below: > > > 6813 /* Don't call digest_init; it's unnecessary and > will complain > > > 6814 about aggregate initialization of > non-aggregate classes. */ > > > 6815 flags |= LOOKUP_ALREADY_DIGESTED; > > > and so store_init_value doesn't digest. Given the comment I'd be > nervous about > > > not setting that flag here. > > > > OK, then why isn't it called by build_aggr_init? How is the CONSTRUCTOR > > getting a type without this being fixed up? > > > > ... > > > > Ah, because build_vec_init builds up a new CONSTRUCTOR and gives it a > type > > without setting the indexes like process_init_constructor_array does: > > > > Jason > > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > > index 27623cf4db1..ea95a3bc910 100644 > > --- a/gcc/cp/init.c > > +++ b/gcc/cp/init.c > > @@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree
[PATCH, V4] PowerPC Turn on -mpcrel by default for -mcpu=future
Commit message:
Enable -mpcrel for -mcpu=future if it is allowed by the ABI.
2020-04-06 Michael Meissner
* config/rs6000/linux64.h (PCREL_SUPPORTED_BY_OS): Enable
prefixed PC-relative addressing if the ABI supports it.
* config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do not
set OPTION_MASK_PREFIXED here.
* config/rs6000/rs6000.c (rs6000_option_override_internal): Enable
OPTION_MASK_PREFIXED and OPTION_MASK_PCREL on -mcpu=future by
default if the current ABI allows the options.
I tested this on a little endian PowerPC power8 system, doing bootstrap and
make check. There were no regressions. I tested by hand the various
conditions where -mpcrel is not enabled, and they all used the normal power9
TOC references.
-mcpu=power9generates TOC;
-mcpu=future -mcmodel=large generates TOC;
-mcpu=future -mcmode=small generates TOC;
-mcpu=future -mno-prefixed generates TOC;
-mcpu=future -mno-pcrel generates TOC;
-mcpu=futuregenerates PC-relative.
--- /tmp/apbaWN_linux64.h 2020-04-03 17:15:05.059677000 -0400
+++ gcc/config/rs6000/linux64.h 2020-04-03 17:01:05.580426937 -0400
@@ -640,3 +640,10 @@ extern int dot_symbols;
enabling the __float128 keyword. */
#undef TARGET_FLOAT128_ENABLE_TYPE
#define TARGET_FLOAT128_ENABLE_TYPE 1
+
+/* Enable using prefixed PC-relative addressing on the 'future' machine if the
+ ABI supports it. The ELF v2 ABI only supports PC-relative relocations for
+ the medium code model. */
+#define PCREL_SUPPORTED_BY_OS (TARGET_FUTURE && TARGET_PREFIXED \
+&& ELFv2_ABI_CHECK \
+&& (TARGET_CMODEL == CMODEL_MEDIUM))
--- /tmp/XzRKno_rs6000-cpus.def 2020-04-03 17:15:05.068676928 -0400
+++ gcc/config/rs6000/rs6000-cpus.def 2020-04-03 17:00:50.115550614 -0400
@@ -75,10 +75,14 @@
| OPTION_MASK_P8_VECTOR\
| OPTION_MASK_P9_VECTOR)
-/* Support for a future processor's features. Do not enable -mpcrel until it
- is fully functional. */
+/* Support for a future processor's features. Do not set OPTION_MASK_PREFIXED
+ or OPTION_MASK_PCREL here. Those options are enabled in the function
+ rs6000_option_override if the ABI supports them. */
#define ISA_FUTURE_MASKS_SERVER(ISA_3_0_MASKS_SERVER
\
-| OPTION_MASK_FUTURE \
+| OPTION_MASK_FUTURE)
+
+/* Flags that need to be turned off if -mno-future. */
+#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \
| OPTION_MASK_PREFIXED)
/* Flags that need to be turned off if -mno-future. */
--- /tmp/nyxSRY_rs6000.c2020-04-03 17:15:05.081676823 -0400
+++ gcc/config/rs6000/rs6000.c 2020-04-03 17:03:19.846353197 -0400
@@ -4020,6 +4020,12 @@ rs6000_option_override_internal (bool gl
rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
}
+ /* Enable -mprefixed by default on 64-bit 'future' systems. */
+ if (TARGET_FUTURE && TARGET_POWERPC64
+ && (rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED) == 0)
+rs6000_isa_flags |= OPTION_MASK_PREFIXED;
+
+
/* -mprefixed (and hence -mpcrel) requires -mcpu=future. */
if (TARGET_PREFIXED && !TARGET_FUTURE)
{
@@ -4171,6 +4177,14 @@ rs6000_option_override_internal (bool gl
SUB3TARGET_OVERRIDE_OPTIONS;
#endif
+#ifdef PCREL_SUPPORTED_BY_OS
+ /* If the ABI has support for PC-relative relocations, enable it by
+ default. */
+ if (PCREL_SUPPORTED_BY_OS
+ && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
+rs6000_isa_flags |= OPTION_MASK_PCREL;
+#endif
+
/* -mpcrel requires -mcmodel=medium, but we can't check TARGET_CMODEL until
after the subtarget override options are done. */
if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
[committed] wwwdocs: Merge tilegx and tilepro entries, remove broken doc link.
Pushed. Walter, I could not find current doc links on mellanox.com; do you have any? Gerald --- htdocs/readings.html | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/htdocs/readings.html b/htdocs/readings.html index da37af6a..2d0a4275 100644 --- a/htdocs/readings.html +++ b/htdocs/readings.html @@ -292,14 +292,8 @@ names. Acronym stands for: Scalable Processor ARChitecture - tilegx + tilegx, tilepro Manufacturer: Mellanox - http://www.mellanox.com/repository/solutions/tile-scm/docs/";>Documentation - - - tilepro - Manufacturer: Mellanox - http://www.mellanox.com/repository/solutions/tile-scm/docs/";>Documentation v850 -- 2.26.0
[PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c
All, The attached patch fixes PR 87923 while also simplifying the code in io.c. I do say this patch simplifies io.c because it is true. This patch causes more deletions than insertions to the file -- a rare sight: gcc/fortran/io.c | 859 --- 1 file changed, 381 insertions(+), 478 deletions(-) Over the years various special cases have been introduced which are not necessary. The constraints for I/O statements are verified in several different places, and in fact some constraints (like whether an I/O tag is a scalar default-kind character) are checked as many as three times. This patches simplifies the code by moving all checks not necessary for matching out of the matching phase and into the resolution phase. The resolve_tag function already checks several constraints for I/O tags, including type and kind, which were previously checked redundantly in other places. This patch also improves error reporting by providing the correct locus for I/O statement error messages, and by providing a more detailed error message when a tag which requires an init-expr is not a valid init-expr. The patch also increases test coverage of I/O statements, especially for I/O tags, by incorporating testcases provided in PRs from the past which the removed code originally addressed: specifically, PRs 66724 and 66725. With the patch applied on the current master (c72a1b6f8b2...) all regression tests with check-fortran pass, including the new ones. OK for master? commit 5a403f4e8e77123994ca1ed05e8f10877423fe55 Author: Fritz Reese Date: Mon Apr 6 12:13:48 2020 -0400 Fix fortran/87923 -- ICE(s) when resolving I/O tags. 2020-04-06 Fritz Reese This patch also reorganizes I/O checking code. Checks which were done in the matching phase which do not affect the match result are moved to the resolution phase. Checks which were duplicated in both the matching phase and resolution phase have been reduced to one check in the resolution phase. Another section of code which used a global async_io_dt flag to check for and assign the asynchronous attribute to variables used in asynchronous I/O has been simplified. Furthermore, this patch improves error reporting and expands test coverage of I/O tags: - "TAG must be an initialization expression" reported by io.c (check_io_constraints) is replaced with an error from expr.c (gfc_reduce_init_expr) indicating _why_ the expression is not a valid initialization expression. - Several distinct error messages regarding the check for scalar + character + default kind have been unified to one message reported by resolve_tag or check_*_constraints. gcc/fortran/ChangeLog: PR fortran/87923 * gfortran.h (gfc_resolve_open, gfc_resolve_close): Add locus parameter. (gfc_resolve_dt): Add code parameter. * io.c (async_io_dt, check_char_variable, is_char_type): Removed. (resolve_tag_format): Add locus to error message regarding zero-sized array in FORMAT tag. (check_open_constraints, check_close_constraints): New functions called at resolution time. (gfc_match_open, gfc_match_close, match_io): Move checks which don't affect the match result to new functions check_open_constraints, check_close_constraints, check_io_constraints. (gfc_resolve_open, gfc_resolve_close): Call new functions check_open_constraints, check_close_constraints after all tags have been independently resolved. Remove duplicate constraints which are already verified by resolve_tag. Explicitly pass locus to all error reports. (compare_to_allowed_values): Add locus parameter and provide explicit locus all error reports. (match_open_element, match_close_element, match_file_element, match_dt_element, match_inquire_element): Remove redundant special cases for ASYNCHRONOUS and IOMSG tags. (gfc_resolve_dt): Remove redundant special case for format expression. Call check_io_constraints, forwarding an I/O list as the io_code parameter if present. (check_io_constraints): Change return type to bool. Pass explicit locus to error reports. Move generic checks after tag-specific checks, since errors are no longer buffered. Move simplification of format string to match_io. Remove redundant checks which are verified by resolve_tag. Remove usage of async_io_dt flag and explicitly mark symbols used in asynchronous I/O with the asynchronous attribute. * resolve.c (resolve_transfer, resolve_fl_namelist): Remove checks for async_io_dt flag. This is now done in io.c (check_io_constraints). (gfc_resolve_code): Pass code
[PATCH] aarch64: Fix {ash[lr],lshr}3 expanders [PR94488]
Hi!
The following testcase ICEs on aarch64 apparently since the introduction of
the aarch64 port. The reason is that the {ashl,ashr,lshr}3 expanders
completely unnecessarily FAIL; if operands[2] is something other than
a CONST_INT or REG or MEM and the middle-end code can't cope with the
pattern giving up in these cases. All the expanders use general_operand
predicate for the shift amount operand, but then have just a special case
for CONST_INT (if in-bound, emit an immediate shift, otherwise force into
REG), or MEM (force into REG), or REG (that is the case it handles).
In the testcase, operands[2] is a lowpart SUBREG of a REG, which is valid
general_operand.
I don't see any reason what is magic about MEMs that it should be forced
into REG and others like SUBREGs that it shouldn't, there isn't even a
reason to check for !REG_P because force_reg will do nothing if the operand
is already a REG, and otherwise can handle general_operand just fine.
Fixed thusly, bootstrapped/regtested on aarch64-linux, ok for trunk and
after a while for backports?
2020-04-06 Jakub Jelinek
PR target/94488
* config/aarch64/aarch64-simd.md (ashl3, lshr3,
ashr3): Force operands[2] into reg whenever it is not CONST_INT.
Assume it is a REG after that instead of testing it and doing FAIL
otherwise. Formatting fix.
* gcc.c-torture/compile/pr94488.c: New test.
--- gcc/config/aarch64/aarch64-simd.md.jj 2020-03-09 12:43:00.742038043
+0100
+++ gcc/config/aarch64/aarch64-simd.md 2020-04-06 08:28:35.527116650 +0200
@@ -1106,30 +1106,18 @@ (define_expand "ashl3"
DONE;
}
else
-{
- operands[2] = force_reg (SImode, operands[2]);
-}
-}
- else if (MEM_P (operands[2]))
-{
- operands[2] = force_reg (SImode, operands[2]);
-}
-
- if (REG_P (operands[2]))
-{
- rtx tmp = gen_reg_rtx (mode);
- emit_insn (gen_aarch64_simd_dup (tmp,
-convert_to_mode (mode,
- operands[2],
- 0)));
- emit_insn (gen_aarch64_simd_reg_sshl (operands[0], operands[1],
- tmp));
- DONE;
+ operands[2] = force_reg (SImode, operands[2]);
}
else
-FAIL;
-}
-)
+operands[2] = force_reg (SImode, operands[2]);
+
+ rtx tmp = gen_reg_rtx (mode);
+ emit_insn (gen_aarch64_simd_dup (tmp, convert_to_mode (mode,
+ operands[2],
+ 0)));
+ emit_insn (gen_aarch64_simd_reg_sshl (operands[0], operands[1], tmp));
+ DONE;
+})
(define_expand "lshr3"
[(match_operand:VDQ_I 0 "register_operand")
@@ -1155,28 +1143,18 @@ (define_expand "lshr3"
else
operands[2] = force_reg (SImode, operands[2]);
}
- else if (MEM_P (operands[2]))
-{
- operands[2] = force_reg (SImode, operands[2]);
-}
-
- if (REG_P (operands[2]))
-{
- rtx tmp = gen_reg_rtx (SImode);
- rtx tmp1 = gen_reg_rtx (mode);
- emit_insn (gen_negsi2 (tmp, operands[2]));
- emit_insn (gen_aarch64_simd_dup (tmp1,
-convert_to_mode (mode,
- tmp, 0)));
- emit_insn (gen_aarch64_simd_reg_shl_unsigned (operands[0],
- operands[1],
- tmp1));
- DONE;
-}
else
-FAIL;
-}
-)
+operands[2] = force_reg (SImode, operands[2]);
+
+ rtx tmp = gen_reg_rtx (SImode);
+ rtx tmp1 = gen_reg_rtx (mode);
+ emit_insn (gen_negsi2 (tmp, operands[2]));
+ emit_insn (gen_aarch64_simd_dup (tmp1,
+convert_to_mode (mode, tmp, 0)));
+ emit_insn (gen_aarch64_simd_reg_shl_unsigned (operands[0], operands[1],
+ tmp1));
+ DONE;
+})
(define_expand "ashr3"
[(match_operand:VDQ_I 0 "register_operand")
@@ -1202,28 +1180,18 @@ (define_expand "ashr3"
else
operands[2] = force_reg (SImode, operands[2]);
}
- else if (MEM_P (operands[2]))
-{
- operands[2] = force_reg (SImode, operands[2]);
-}
-
- if (REG_P (operands[2]))
-{
- rtx tmp = gen_reg_rtx (SImode);
- rtx tmp1 = gen_reg_rtx (mode);
- emit_insn (gen_negsi2 (tmp, operands[2]));
- emit_insn (gen_aarch64_simd_dup (tmp1,
-convert_to_mode (mode,
- tmp, 0)));
- emit_insn (gen_aarch64_simd_reg_shl_signed (operands[0],
- operands[1],
- tmp1));
- DONE;
-}
else
-F
[committed] libstdc++: Make string_view::copy usable in constant expressions (PR 94498)
PR libstdc++/94498
* include/bits/char_traits.h (__gnu_cxx::char_traits::move): Make it
usable in constant expressions for C++20.
(__gnu_cxx::char_traits::copy, __gnu_cxx::char_traits::assign): Add
_GLIBCXX20_CONSTEXPR.
(std::char_traits, std::char_traits)
(std::char_traits): Make move, copy and assign usable in
constant expressions for C++20.
(std::char_traits, std::char_traits): Make move
and copy usable in constant expressions for C++20.
* include/std/string_view (basic_string_view::copy): Add
_GLIBCXX20_CONSTEXPR.
* testsuite/21_strings/basic_string_view/operations/copy/char/
constexpr.cc: New test.
* testsuite/21_strings/basic_string_view/operations/copy/wchar_t/
constexpr.cc: New test.
Tested powerpc64le-linux, committed to master.
I'd like to backport this to gcc-9 too.
commit b696698767ba45b4d61a93205167e2f1f744d3f1
Author: Jonathan Wakely
Date: Mon Apr 6 18:30:53 2020 +0100
libstdc++: Make string_view::copy usable in constant expressions (PR 94498)
PR libstdc++/94498
* include/bits/char_traits.h (__gnu_cxx::char_traits::move): Make it
usable in constant expressions for C++20.
(__gnu_cxx::char_traits::copy, __gnu_cxx::char_traits::assign): Add
_GLIBCXX20_CONSTEXPR.
(std::char_traits, std::char_traits)
(std::char_traits): Make move, copy and assign usable in
constant expressions for C++20.
(std::char_traits, std::char_traits): Make move
and copy usable in constant expressions for C++20.
* include/std/string_view (basic_string_view::copy): Add
_GLIBCXX20_CONSTEXPR.
* testsuite/21_strings/basic_string_view/operations/copy/char/
constexpr.cc: New test.
* testsuite/21_strings/basic_string_view/operations/copy/wchar_t/
constexpr.cc: New test.
diff --git a/libstdc++-v3/include/bits/char_traits.h
b/libstdc++-v3/include/bits/char_traits.h
index 2edb84ca3b8..65031d3ac83 100644
--- a/libstdc++-v3/include/bits/char_traits.h
+++ b/libstdc++-v3/include/bits/char_traits.h
@@ -113,13 +113,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static _GLIBCXX14_CONSTEXPR const char_type*
find(const char_type* __s, std::size_t __n, const char_type& __a);
- static char_type*
+ static _GLIBCXX20_CONSTEXPR char_type*
move(char_type* __s1, const char_type* __s2, std::size_t __n);
- static char_type*
+ static _GLIBCXX20_CONSTEXPR char_type*
copy(char_type* __s1, const char_type* __s2, std::size_t __n);
- static char_type*
+ static _GLIBCXX20_CONSTEXPR char_type*
assign(char_type* __s, std::size_t __n, char_type __a);
static _GLIBCXX_CONSTEXPR char_type
@@ -179,17 +179,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template
+_GLIBCXX20_CONSTEXPR
typename char_traits<_CharT>::char_type*
char_traits<_CharT>::
move(char_type* __s1, const char_type* __s2, std::size_t __n)
{
if (__n == 0)
return __s1;
+#ifdef __cpp_lib_is_constant_evaluated
+ if (std::is_constant_evaluated())
+ {
+ if (__s1 > __s2 && __s1 < __s2 + __n)
+ std::copy_backward(__s2, __s2 + __n, __s1);
+ else
+ std::copy(__s2, __s2 + __n, __s1);
+ return __s1;
+ }
+#endif
return static_cast<_CharT*>(__builtin_memmove(__s1, __s2,
__n * sizeof(char_type)));
}
template
+_GLIBCXX20_CONSTEXPR
typename char_traits<_CharT>::char_type*
char_traits<_CharT>::
copy(char_type* __s1, const char_type* __s2, std::size_t __n)
@@ -200,6 +212,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template
+_GLIBCXX20_CONSTEXPR
typename char_traits<_CharT>::char_type*
char_traits<_CharT>::
assign(char_type* __s, std::size_t __n, char_type __a)
@@ -349,27 +362,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return static_cast(__builtin_memchr(__s, __a, __n));
}
- static char_type*
+ static _GLIBCXX20_CONSTEXPR char_type*
move(char_type* __s1, const char_type* __s2, size_t __n)
{
if (__n == 0)
return __s1;
+#ifdef __cpp_lib_is_constant_evaluated
+ if (std::is_constant_evaluated())
+ return __gnu_cxx::char_traits::move(__s1, __s2, __n);
+#endif
return static_cast(__builtin_memmove(__s1, __s2, __n));
}
- static char_type*
+ static _GLIBCXX20_CONSTEXPR char_type*
copy(char_type* __s1, const char_type* __s2, size_t __n)
{
if (__n == 0)
return __s1;
+#ifdef __cpp_lib_is_constant_evaluated
+ if (std::is_constant_evaluated())
+ return __gnu_cxx::char_traits::copy(__s1, __s2, __n);
+#endif
return static_cast(__builtin_memcpy(__s1, __s2, __n));
}
Re: [PATCH] lra: Stop eh_return data regs being incorrectly marked live [PR92989]
On Sun, 2020-04-05 at 08:45 +0100, Richard Sandiford wrote: > lra_assign has an assert to make sure that no pseudo is allocated > to a conflicting hard register. It used to be restricted to > !flag_ipa_ra, but in g:a1e6ee38e708ef2bdef4 I'd enabled it for > flag_ipa_ra too. It then tripped a few times while building > libstdc++ for mips-mti-linux. > > Previous patches fixed one of the problems: registers clobbered > by the taking of an exception were being treated as live at the > beginning of the EH receiver, and this got propagated to predecessor > blocks. But it turns out that there was a second problem: eh_return > data registers were also being marked live in the same way. > > These registers are defined by the unwinder and so in reality they > really are live on entry to the EH receiver. But definitions can > only happen in blocks, not on edges, so for liveness purposes > we use artificial definitions at the start of the EH receiver. > process_bb_lives should therefore model the effect of a definition, > not a plain use. > > Tested this time by building a full mips-mti-linux-gnu toolchain > (including the prerequisite 26 glibc sysroots). Also boostrapped > & regression tested on aarch64-linux-gnu and x86_64-linux-gnu. > OK to install? > > Richard > > > 2020-04-05 Richard Sandiford > > gcc/ > PR rtl-optimization/92989 > * lra-lives.c (process_bb_lives): Do not treat eh_return data > registers as being live at the beginning of the EH receiver. OK jeff >
Re: [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU
On Fri, 2020-04-03 at 23:55 +0100, Maciej W. Rozycki via Gcc-patches wrote: > Use an Autoconf template rather an inline piece of scriptery to set > DejaGNU's $CC_FOR_TARGET and $CXX_FOR_TARGET variables from $CC and $CXX > respectively, making it easier to maintain and making it take advantage > of Automake's dependency and rule generation. Relocate the generated > `local.exp' file to within testsuite/ so as to make its regeneration > rule to actually work, i.e. (in testsuite/Makefile.in): > > EXTRA_DEJAGNU_SITE_CONFIG = local.exp > site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG) > [...] > local.exp: $(top_builddir)/config.status $(srcdir)/local.exp.in > cd $(top_builddir) && $(SHELL) ./config.status $(subdir)/$@ > > It wouldn't work if the regeneration rule was placed in the top-level > Makefile.in, which is what Automake would faithfully do if `local.exp' > stayed in the top level directory. > --- > Hi, > > I think having individual AC_CONFIG_FILES macro invocations for each > output file or group of files would make this change (and code itself) > more readable, however it hasn't been done before and I decided not to > change the style on this occasion. It may make sense as a follow-up > clean-up. > > Maciej > --- > Makefile.am|3 --- > configure.ac |7 +-- > testsuite/Makefile.am |2 +- > testsuite/local.exp.in |2 ++ > 4 files changed, 4 insertions(+), 10 deletions(-) So from the cover letter I got the impression this series was supposed to be pulling down some bits from upstream libffi into gcc's copy. But AFAICT that's not the case. I don't see anything inherently wrong here. It was just a bit confusing. It's actually follow-on patches where you're cherry picking from the upstream libffi repository. OK for the trunk. jeff >
Re: [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor to DejaGNU
On Fri, 2020-04-03 at 23:55 +0100, Maciej W. Rozycki via Gcc-patches wrote: > Use Autoconf substitution in the template used for extra DejaGNU site > configuration, which is a documented supported way to pass information > from the `configure' script, rather than resorting to a hack with > extracting an undocumented internal setting from `config.log' to pass > the compiler vendor to DejaGNU. No functional change (and no risk of > breakage with some Autoconf version anymore). > > Use AM_SUBST_NOTMAKE with the new substitution so as not to place it in > Makefile.in files, where it is not needed, and set the Autmoake version > requirement accordingly. > --- > Hi, > > I chose to use AM_SUBST_NOTMAKE so as not to clutter Makefile.in files > with the new variable as Automake does that by default. That however > requires the use of Automake 1.11 or newer. By the look of our sources > that shouldn't be an issue as far as I can tell, but the macro invocation > can be dropped along with the requirement if it would. > > Maciej > --- > Makefile.am |3 ++- > configure.ac |2 ++ > testsuite/lib/libffi.exp |4 > testsuite/local.exp.in |1 + > 4 files changed, 5 insertions(+), 5 deletions(-) OK jeff >
Re: [PATCH libffi 4/4] Correct indentation throughout `libffi-init'
On Fri, 2020-04-03 at 23:56 +0100, Maciej W. Rozycki via Gcc-patches wrote: > --- > testsuite/lib/libffi.exp |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) OK when prereqs have been committed. jeff >
Re: [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET
On Fri, 2020-04-03 at 23:56 +0100, Maciej W. Rozycki via Gcc-patches wrote:
> Update code in `libffi-init' to use $CC_FOR_TARGET in determining the
> value of $ld_library_path, as using a different compiler location from
> one actually used in testing may have odd consequences.
>
> As this obviously loses the setting of $gccdir provide a replacement way
> to determine the directory if feasible, however prefer the location of
> shared libgcc over static libgcc. This helps in configurations where
> shared libgcc is, unlike libgcc, a location that is not specific to the
> compiler version, a common scenario. It does not address the scenario
> however where there is a shared libgcc symlink installed pointing to the
> actual run-time library installed elsewhere; this would have to be dealt
> with in a board description file specific to the installation.
>
> Use `remote_exec host' rather than `exec' to invoke the compiler so as
> to support remote configurations and also avoid the latter procedure's
> path length limitation that prevents execution in some actual scenarios.
>
> As using `remote_exec host' precludes the use of our existing file name
> globbing to examine directory contents, reuse, with minor modifications
> needed to adjust to our structure, the piece I previously contributed to
> GCC with commit d42b84f427e4 ("testsuite: Fix run-time tracking down
> of `libgcc_s'").
> ---
> Hi,
>
> This has its limitation in that it still doesn't default to the same
> compiler as `target_compile' (`default_target_compile') from target.exp in
> DejaGNU does, but I believe it is a step in the right direction. That
> will only affect standalone (e.g. installed) testing iff $CC_FOR_TARGET
> hasn't been set.
>
> Also for C++ compilation our carefully crafted $ld_library_path is
> unfortunately overridden by `g++_link_flags' from libgloss.exp in DejaGNU
> called in `default_target_compile'. This actually does cause test
> failures in my `riscv64-linux-gnu' cross-compilation setup:
>
> FAIL: libffi.closures/unwindtest.cc -W -Wall -Wno-psabi -O0 execution test
> FAIL: libffi.closures/unwindtest.cc -W -Wall -Wno-psabi -O2 execution test
> FAIL: libffi.closures/unwindtest_ffi_call.cc -W -Wall -Wno-psabi -O0 execution
> test
> FAIL: libffi.closures/unwindtest_ffi_call.cc -W -Wall -Wno-psabi -O2 execution
> test
>
> and I am currently not sure how to best address it, i.e. whether to change
> DejaGNU's `g++_link_flags' or to take advantage of a TCL's feature and
> provide our own copy of the procedure. Something to consider sometime.
>
> NB I chose not to correct obvious indentation issues with lines not
> touched by this change so as not to obfuscate the patch unnecessarily. A
> complementing formatting change follows.
>
> Maciej
> ---
> testsuite/lib/libffi.exp | 68
> +++---
> -
> 1 file changed, 51 insertions(+), 17 deletions(-)
OK. Note that all 4 patches in the series need ChangeLog entries.
jeff
>
Re: [PATCH v4 GCC 2/5] libffi/test: Fix compilation for build sysroot
On Sat, 2020-04-04 at 00:01 +0100, Maciej W. Rozycki wrote: > Fix a problem with the libffi testsuite using a method to determine the > compiler to use resulting in the tool being different from one the > library has been built with, and causing a catastrophic failure from the > inability to actually choose any compiler at all in a cross-compilation > configuration. > > Address this problem by providing a DejaGNU configuration file defining > the compiler to use, via the CC_FOR_TARGET TCL variable, set from $CC by > autoconf, which will have all the required options set for the target > compiler to build executables in the environment configured, removing > failures like: > > FAIL: libffi.call/closure_fn0.c -W -Wall -Wno-psabi -O0 (test for excess > errors) > Excess errors: > default_target_compile: No compiler to compile with > UNRESOLVED: libffi.call/closure_fn0.c -W -Wall -Wno-psabi -O0 compilation > failed to produce executable > > and bringing overall test results for the `riscv64-linux-gnu' target > (here with the `x86_64-linux-gnu' host and RISC-V QEMU in the Linux user > emulation mode as the target board) from: > > === libffi Summary === > > # of unexpected failures 708 > # of unresolved testcases 708 > # of unsupported tests30 > > to: > > === libffi Summary === > > # of expected passes 1934 > # of unsupported tests28 > > libffi/ > * configure.ac: Add testsuite/local.exp to output files. > * configure: Regenerate. > * testsuite/local.exp.in: New file. > * testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New > variable. > * testsuite/Makefile.in: Regenerate. Oh, I see it now. THe patches I ack'd were actually for upstream libffi. You should actually wait for a libffi maintainer to ack those, not me :-) Sorry for the confusion. Both backports are OK once they're upstreamed. jeff >
Re: [PATCH v4 1/5] libatomic/test: Fix compilation for build sysroot
On Sat, 2020-04-04 at 00:00 +0100, Maciej W. Rozycki wrote: > Fix a problem with the libatomic testsuite using a method to determine > the compiler to use resulting in the tool being different from one the > library has been built with, and causing a catastrophic failure from the > lack of a suitable `--sysroot=' option where the `--with-build-sysroot=' > configuration option has been used to build the compiler resulting in > the inability to link executables. > > Address this problem by providing a DejaGNU configuration file defining > the compiler to use, via the GCC_UNDER_TEST TCL variable, set from $CC > by autoconf, which will have all the required options set for the target > compiler to build executables in the environment configured, removing > failures like: > > .../bin/riscv64-linux-gnu-ld: cannot find crt1.o: No such file or directory > .../bin/riscv64-linux-gnu-ld: cannot find -lm > collect2: error: ld returned 1 exit status > compiler exited with status 1 > FAIL: libatomic.c/atomic-compare-exchange-1.c (test for excess errors) > Excess errors: > .../bin/riscv64-linux-gnu-ld: cannot find crt1.o: No such file or directory > .../bin/riscv64-linux-gnu-ld: cannot find -lm > > UNRESOLVED: libatomic.c/atomic-compare-exchange-1.c compilation failed to > produce executable > > and bringing overall test results for the `riscv64-linux-gnu' target > (here with the `x86_64-linux-gnu' host and RISC-V QEMU in the Linux user > emulation mode as the target board) from: > > === libatomic Summary === > > # of unexpected failures 27 > # of unresolved testcases 27 > > to: > > === libatomic Summary === > > # of expected passes 54 > > libatomic/ > * configure.ac: Add testsuite/libatomic-site-extra.exp to output > files. > * configure: Regenerate. > * libatomic/testsuite/libatomic-site-extra.exp.in: New file. > * testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New > variable. > * testsuite/Makefile.in: Regenerate. OK jeff >
Re: [PATCH v4 5/5] libgomp/test: Remove a build sysroot fix regression
On Sat, 2020-04-04 at 00:01 +0100, Maciej W. Rozycki wrote:
> Fix a problem with commit c8e759b4215b ("libgomp/test: Fix compilation
> for build sysroot") that caused a regression in some standalone test
> environments where testsuite/libgomp-test-support.exp is used, but the
> compiler is expected to be determined by `[find_gcc]', and set the
> GCC_UNDER_TEST TCL variable in testsuite/libgomp-site-extra.exp instead.
>
> libgomp/
> * configure.ac: Add testsuite/libgomp-site-extra.exp to output
> files.
> * configure: Regenerate.
> * testsuite/libgomp-site-extra.exp.in: New file.
> * testsuite/libgomp-test-support.exp.in (GCC_UNDER_TEST): Remove
> variable.
> * testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New
> variable.
> * testsuite/Makefile.in: Regenerate.
OK
jeff
>
[PATCH] c++: Fix usage of CONSTRUCTOR_PLACEHOLDER_BOUNDARY inside array initializers [90996]
This PR reports that since the introduction of the
CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag, we are sometimes failing to resolve
PLACEHOLDER_EXPRs inside array initializers that refer to some inner
constructor. In the testcase in the PR, we have as the initializer for "S c[];"
the following
{{.a=(int &) &_ZGR1c_, .b={*(&)->a}}}
where CONSTRUCTOR_PLACEHOLDER_BOUNDARY is set on the second outermost
constructor. However, we pass the whole initializer to replace_placeholders in
store_init_value, and so due to the flag being set on the second outermost ctor
it avoids recursing into the innermost constructor and we fail to resolve the
PLACEHOLDER_EXPR within.
To fix this, we could perhaps either call replace_placeholders in more places,
or we could change where we set CONSTRUCTOR_PLACEHOLDER_BOUNDARY. This patch
takes the latter approach -- when building up an array initializer, it bubbles
any CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set on the element initializers up to
the array initializer. Doing so shouldn't create any new PLACEHOLDER_EXPR
resolution ambiguities because we don't deal with PLACEHOLDER_EXPRs of array
type in the frontend, as far as I can tell.
Does this look OK to comit after testing?
gcc/cp/ChangeLog:
PR c++/90996
* tree.c (replace_placeholders): Look through all handled components,
not just COMPONENT_REFs.
* typeck2.c (process_init_constructor_array): Propagate
CONSTRUCTOR_PLACEHOLDER_BOUNDARY up from each element initializer to
the array initializer.
gcc/testsuite/ChangeLog:
PR c++/90996
* g++.dg/cpp1y/pr90996.C: New test.
---
gcc/cp/tree.c| 2 +-
gcc/cp/typeck2.c | 18 ++
gcc/testsuite/g++.dg/cpp1y/pr90996.C | 17 +
3 files changed, 36 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr90996.C
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 5eb0dcd717a..d1192b7e094 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3247,7 +3247,7 @@ replace_placeholders (tree exp, tree obj, bool *seen_p
/*= NULL*/)
/* If the object isn't a (member of a) class, do nothing. */
tree op0 = obj;
- while (TREE_CODE (op0) == COMPONENT_REF)
+ while (handled_component_p (op0))
op0 = TREE_OPERAND (op0, 0);
if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0
return exp;
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index cf1cb5ace66..fe844bc08bb 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1488,6 +1488,17 @@ process_init_constructor_array (tree type, tree init,
int nested, int flags,
= massage_init_elt (TREE_TYPE (type), ce->value, nested, flags,
complain);
+ if (TREE_CODE (ce->value) == CONSTRUCTOR
+ && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value))
+ {
+ /* Shift CONSTRUCTOR_PLACEHOLDER_BOUNDARY from the element initializer
+up to the array initializer, so that the call to
+replace_placeholders from store_init_value can resolve any
+PLACEHOLDER_EXPRs within this element initializer. */
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value) = 0;
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1;
+ }
+
gcc_checking_assert
(ce->value == error_mark_node
|| (same_type_ignoring_top_level_qualifiers_p
@@ -1516,6 +1527,13 @@ process_init_constructor_array (tree type, tree init,
int nested, int flags,
/* The default zero-initialization is fine for us; don't
add anything to the CONSTRUCTOR. */
next = NULL_TREE;
+ else if (TREE_CODE (next) == CONSTRUCTOR
+&& CONSTRUCTOR_PLACEHOLDER_BOUNDARY (next))
+ {
+ /* As above. */
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (next) = 0;
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1;
+ }
}
else if (!zero_init_p (TREE_TYPE (type)))
next = build_zero_init (TREE_TYPE (type),
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr90996.C
b/gcc/testsuite/g++.dg/cpp1y/pr90996.C
new file mode 100644
index 000..780cbb4e3ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr90996.C
@@ -0,0 +1,17 @@
+// PR c++/90996
+// { dg-do compile { target c++14 } }
+
+struct S
+{
+ int &&a = 2;
+ int b[1] {a};
+};
+
+S c[2][2] {{{5}}};
+
+struct T
+{
+ S c[2][2] {{{7}}};
+};
+
+T d {};
--
2.26.0.106.g9fadedd637
Re: [RFA] skip gcc.target/arm/div64-unwinding.c on vxworks_kernel targets
> > gcc/testsuite/ > > > > * gcc.target/arm/div64-unwinding.c: Skip on vxworks_kernel targets. > OK. Thank you, Richard. Pushed to master. -- Joel
Re: Fix an ICE found in PR93686
On Sat, Apr 4, 2020 at 2:58 PM Steve Kargl via Fortran wrote: > > This patch fixes the ICE found in PR93686. > > > Index: gcc/fortran/decl.c > === > --- gcc/fortran/decl.c (revision 280157) > +++ gcc/fortran/decl.c (working copy) > @@ -696,6 +696,10 @@ gfc_match_data (void) > /* F2008:C567 (R536) A data-i-do-object or a variable that appears > as a data-stmt-object shall not be an object designator in which > a pointer appears other than as the entire rightmost part-ref. > */ > + if (!e->ref && e->ts.type == BT_DERIVED > + && e->symtree->n.sym->attr.pointer) > + goto partref; > + > ref = e->ref; > if (e->symtree->n.sym->ts.type == BT_DERIVED > && e->symtree->n.sym->attr.pointer > > -- > Steve LGTM, thanks for the patch. I will commit along with the testcases from the PR. --- Fritz
Re: [PATCH v4 GCC 2/5] libffi/test: Fix compilation for build sysroot
On Mon, 6 Apr 2020, Jeff Law wrote: > > libffi/ > > * configure.ac: Add testsuite/local.exp to output files. > > * configure: Regenerate. > > * testsuite/local.exp.in: New file. > > * testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New > > variable. > > * testsuite/Makefile.in: Regenerate. > Oh, I see it now. THe patches I ack'd were actually for upstream libffi. > > You should actually wait for a libffi maintainer to ack those, not me :-) > Sorry > for the confusion. Sorry to make it unclear. I chose to cc the other mailing list with the libffi part of both submissions so as to give a chance to chime in to members of both communities. We are quite tightly coupled with each other here after all and my experience over the many years I have been involved has been that the bits related to Autotools are often quite tricky and hard to get right (Autotools are fine tools in my experience, it's just you need to be thorough with them as they tend not to forgive a cursory approach). > Both backports are OK once they're upstreamed. Thanks for your ack; it was my intent to get these upstream first, and I realise there can be changes requested that will require the GCC backport to be adjusted accordingly (and reviewed again). Maciej
Re: [RS6000] Don't pass -many to the assembler
Hello, I am sorry to come back to this thread after such a long time. I recently noticed that one of RTEMS multilibs is broken (for whatever reason it didn't show up in my regular build): /build/git-build/b-gcc-git-powerpc-rtems5/powerpc-rtems5/m8540/nof/libgcc (master) > make # If this is the top-level multilib, build all the other # multilibs. /build/git-build/b-gcc-git-powerpc-rtems5/./gcc/xgcc -B/build/git-build/b-gcc-git-powerpc-rtems5/./gcc/ -nostdinc -B/build/git-build/b-gcc-git-powerpc-rtems5/powerpc-rtems5/m8540/nof/newlib/ -isystem /build/git-build/b-gcc-git-powerpc-rtems5/powerpc-rtems5/m8540/nof/newlib/targ-include -isystem /home/EB/sebastian_h/archive/gcc-git/newlib/libc/include -B/opt/rtems/5/powerpc-rtems5/bin/ -B/opt/rtems/5/powerpc-rtems5/lib/ -isystem /opt/rtems/5/powerpc-rtems5/include -isystem /opt/rtems/5/powerpc-rtems5/sys-include -mcpu=8540 -msoft-float -g -O2 -O2 -I/home/EB/sebastian_h/archive/gcc-git/libgcc/../newlib/libc/sys/rtems/include -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../../.././gcc -I/home/EB/sebastian_h/archive/gcc-git/libgcc -I/home/EB/sebastian_h/archive/gcc-git/libgcc/. -I/home/EB/sebastian_h/archive/gcc-git/libgcc/../gcc -I/home/EB/sebastian_h/archive/gcc-git/libgcc/../include -DHAVE_CC_TLS -o crtsavfpr_s.o -MT crtsavfpr_s.o -MD -MP -MF crtsavfpr_s.dep -DSHARED -c -xassembler-with-cpp /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S: Assembler messages: /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:41: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:42: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:43: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:44: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:45: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:46: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:47: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:48: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:49: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:50: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:51: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:52: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:53: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:54: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:55: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:56: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:57: Error: unrecognized opcode: `stfd' /home/EB/sebastian_h/archive/gcc-git/libgcc/config/rs6000/crtsavfpr.S:58: Error: unrecognized opcode: `stfd' The assembler is called via: /build/git-build/b-gcc-git-powerpc-rtems5/./gcc/as --gdwarf2 -I /home/EB/sebastian_h/archive/gcc-git/libgcc/../newlib/libc/sys/rtems/include -I . -I . -I ../../../.././gcc -I /home/EB/sebastian_h/archive/gcc-git/libgcc -I /home/EB/sebastian_h/archive/gcc-git/libgcc/. -I /home/EB/sebastian_h/archive/gcc-git/libgcc/../gcc -I /home/EB/sebastian_h/archive/gcc-git/libgcc/../include -a32 -me500 -mbig -o crtsavfpr_s.o /tmp/cc6Fy8nn.s The e500 has no standard FPU. For this reason the multilib flags are: -mcpu=8540 -msoft-float What do you think about the attached patch? >From 7329c85c0e7bcda52ab0a55092747512731e38cd Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Mon, 6 Apr 2020 22:30:21 +0200 Subject: [PATCH] RS6000: Disable some CRT functions libgcc/ * config/rs6000/crtresfpr.S: Disable all functions if _SOFT_DOUBLE is defined. * config/rs6000/crtresxfpr.S: Likewise. * config/rs6000/crtsavfpr.S: Likewise. --- libgcc/config/rs6000/crtresfpr.S | 5 +++-- libgcc/config/rs6000/crtresxfpr.S | 5 +++-- libgcc/config/rs6000/crtsavfpr.S | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/libgcc/config/rs6000/crtresfpr.S b/libgcc/config/rs6000/crtresfpr
Re: [PATCH] gcc/gcc.c: add outfiles spec
On Sat, 4 Apr 2020, Rasmus Villemoes wrote: > +#ifndef OUTFILES_SPEC > +#define OUTFILES_SPEC "%o" > +#endif A new target macro needs to be documented in tm.texi.in, with tm.texi then being regenerated. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 4/6/20 11:45 AM, Patrick Palka wrote:
On Wed, 1 Apr 2020, Jason Merrill wrote:
On 4/1/20 6:29 PM, Jason Merrill wrote:
On 3/31/20 3:50 PM, Patrick Palka wrote:
On Tue, 31 Mar 2020, Jason Merrill wrote:
On 3/30/20 6:46 PM, Patrick Palka wrote:
On Mon, 30 Mar 2020, Jason Merrill wrote:
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument
that
exposes
a
latent
bug in how we substitute an array type into a cv-qualified
wildcard
function
parameter type. Concretely, the latent bug is that given the
function
template
template void foo(const T t);
one would expect the type of foo to be void(const
int*), but
we
(seemingly prematurely) strip function parameter types of
their
top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and
instead
end
up
obtaining void(int*) as the type of foo after
substitution
and
decaying.
We still however correctly substitute into and decay the
formal
parameter
type,
obtaining const int* as the type of t after substitution. But
this
then
leads
to us tripping over the assert in tsubst_default_argument that
verifies
the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution
bug, we
can
still
relax the assertion like so. Tested on x86_64-pc-linux-gnu,
does
this
look
OK?
This is core issues 1001/1322, which have not been resolved.
Clang
does
the
substitution the way you suggest; EDG rejects the testcase
because the
two
substitutions produce different results. I think it would make
sense
to
follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting
into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal)
diagnostic
if so. This patch checks this in tsubst_decl
rather
than in tsubst_function_decl for efficiency reasons, so that we
don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very
marginal
optimization; how many function templates have so many parameters
that
walking
over them once to compare types will have any effect on compile
time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then since
we have access to the instantiated function, it would presumably
suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time, as
we
do with regular function parameters. So in this second testcase both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
and yet we would (presumably) want to reject this instantiation too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to do
a
variant of the trick that's done in this patch, i.e. substitute into
each dependent parameter type stripped of its top-level cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to detect
this?
I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS;
the
problem comes when they disagree. If we're handling pack expansions
wrong,
that's a separate issue.
Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems
to be exposing a latent bug with how we handle lambdas that appear in
function parameter types. Take g++.dg/cpp2a/lambda-uneval3.C for
example:
template void spam(decltype([]{}) (*s)[sizeof(T)]) {}
int main() { spam(nullptr); }
According to tsubst_function_decl in current trunk, the type of the
function paremeter 's' of spam according to its TYPE_ARG_TYPES is
struct ._anon_4[1] *
and according to its DECL_ARGUMENTS the type of 's' is
struct ._anon_5[1] *
The disagreement happens because we call tsubst_lambda_expr twice during
substitution and thereby generate two distinct lambda types, one when
substituting into the TYPE_ARG_TYPES and another when substituting into
the DECL_
Re: [PATCH] c++: Fix usage of CONSTRUCTOR_PLACEHOLDER_BOUNDARY inside array initializers [90996]
On 4/6/20 3:07 PM, Patrick Palka wrote:
This PR reports that since the introduction of the
CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag, we are sometimes failing to resolve
PLACEHOLDER_EXPRs inside array initializers that refer to some inner
constructor. In the testcase in the PR, we have as the initializer for "S c[];"
the following
{{.a=(int &) &_ZGR1c_, .b={*(&)->a}}}
where CONSTRUCTOR_PLACEHOLDER_BOUNDARY is set on the second outermost
constructor. However, we pass the whole initializer to replace_placeholders in
store_init_value, and so due to the flag being set on the second outermost ctor
it avoids recursing into the innermost constructor and we fail to resolve the
PLACEHOLDER_EXPR within.
To fix this, we could perhaps either call replace_placeholders in more places,
or we could change where we set CONSTRUCTOR_PLACEHOLDER_BOUNDARY. This patch
takes the latter approach -- when building up an array initializer, it bubbles
any CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set on the element initializers up to
the array initializer. Doing so shouldn't create any new PLACEHOLDER_EXPR
resolution ambiguities because we don't deal with PLACEHOLDER_EXPRs of array
type in the frontend, as far as I can tell.
Interesting. Yes, that sounds like it should work.
Does this look OK to comit after testing?
Yes.
Though I'm seeing "after testing" a lot; what testing are you doing
before sending patches?
gcc/cp/ChangeLog:
PR c++/90996
* tree.c (replace_placeholders): Look through all handled components,
not just COMPONENT_REFs.
* typeck2.c (process_init_constructor_array): Propagate
CONSTRUCTOR_PLACEHOLDER_BOUNDARY up from each element initializer to
the array initializer.
gcc/testsuite/ChangeLog:
PR c++/90996
* g++.dg/cpp1y/pr90996.C: New test.
---
gcc/cp/tree.c| 2 +-
gcc/cp/typeck2.c | 18 ++
gcc/testsuite/g++.dg/cpp1y/pr90996.C | 17 +
3 files changed, 36 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr90996.C
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 5eb0dcd717a..d1192b7e094 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3247,7 +3247,7 @@ replace_placeholders (tree exp, tree obj, bool *seen_p
/*= NULL*/)
/* If the object isn't a (member of a) class, do nothing. */
tree op0 = obj;
- while (TREE_CODE (op0) == COMPONENT_REF)
+ while (handled_component_p (op0))
op0 = TREE_OPERAND (op0, 0);
if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0
return exp;
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index cf1cb5ace66..fe844bc08bb 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1488,6 +1488,17 @@ process_init_constructor_array (tree type, tree init,
int nested, int flags,
= massage_init_elt (TREE_TYPE (type), ce->value, nested, flags,
complain);
+ if (TREE_CODE (ce->value) == CONSTRUCTOR
+ && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value))
+ {
+ /* Shift CONSTRUCTOR_PLACEHOLDER_BOUNDARY from the element initializer
+up to the array initializer, so that the call to
+replace_placeholders from store_init_value can resolve any
+PLACEHOLDER_EXPRs within this element initializer. */
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value) = 0;
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1;
+ }
+
gcc_checking_assert
(ce->value == error_mark_node
|| (same_type_ignoring_top_level_qualifiers_p
@@ -1516,6 +1527,13 @@ process_init_constructor_array (tree type, tree init,
int nested, int flags,
/* The default zero-initialization is fine for us; don't
add anything to the CONSTRUCTOR. */
next = NULL_TREE;
+ else if (TREE_CODE (next) == CONSTRUCTOR
+&& CONSTRUCTOR_PLACEHOLDER_BOUNDARY (next))
+ {
+ /* As above. */
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (next) = 0;
+ CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1;
+ }
}
else if (!zero_init_p (TREE_TYPE (type)))
next = build_zero_init (TREE_TYPE (type),
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr90996.C
b/gcc/testsuite/g++.dg/cpp1y/pr90996.C
new file mode 100644
index 000..780cbb4e3ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr90996.C
@@ -0,0 +1,17 @@
+// PR c++/90996
+// { dg-do compile { target c++14 } }
+
+struct S
+{
+ int &&a = 2;
+ int b[1] {a};
+};
+
+S c[2][2] {{{5}}};
+
+struct T
+{
+ S c[2][2] {{{7}}};
+};
+
+T d {};
Re: Fix an ICE found in PR93686
On Mon, Apr 06, 2020 at 04:10:24PM -0400, Fritz Reese wrote: > On Sat, Apr 4, 2020 at 2:58 PM Steve Kargl via Fortran > wrote: > > > > This patch fixes the ICE found in PR93686. > > > > > > Index: gcc/fortran/decl.c > > === > > --- gcc/fortran/decl.c (revision 280157) > > +++ gcc/fortran/decl.c (working copy) > > @@ -696,6 +696,10 @@ gfc_match_data (void) > > /* F2008:C567 (R536) A data-i-do-object or a variable that appears > > as a data-stmt-object shall not be an object designator in > > which > > a pointer appears other than as the entire rightmost part-ref. > > */ > > + if (!e->ref && e->ts.type == BT_DERIVED > > + && e->symtree->n.sym->attr.pointer) > > + goto partref; > > + > > ref = e->ref; > > if (e->symtree->n.sym->ts.type == BT_DERIVED > > && e->symtree->n.sym->attr.pointer > > > > -- > > Steve > > LGTM, thanks for the patch. I will commit along with the testcases from the > PR. > Thanks. For reference, my original code did not check the left-most partref for the pointer attribute. This patch implements this check. BTW, if you haven't committed the degree trig functions, then I think you should to get the fixes in for 10.1. -- Steve
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Ping 2: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html Thanks, - Eddy B. On Fri, Mar 13, 2020, at 10:28 PM, Eduard-Mihai Burtescu wrote: > This is the libiberty (mainly for binutils/gdb) counterpart of > https://github.com/alexcrichton/rustc-demangle/pull/23. > > Relevant links for the new Rust mangling scheme (aka "v0"): > * Rust RFC: https://github.com/rust-lang/rfcs/pull/2603 > * tracking issue: https://github.com/rust-lang/rust/issues/60705 > * implementation: https://github.com/rust-lang/rust/pull/57967 > > This implementation includes full support for UTF-8 identifiers > via punycode, so I've included a testcase for that as well. > (Let me know if it causes any issues and I'll take it out) > > Last year I've submitted several small patches to rust-demangle > in preparation for upstreaming the entire new demangler, and > feedback from that has proven useful. > For example, I started with error-handling macros, but instead > the code now has "rdm->errored = 1;" before several returns/gotos. > > The patch is attached instead of inline, as it's over 1000 lines long. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Also, I have no commit access, so I'd be thankful if > someone would commit this for me if/once approved. > Attachments: > * 0001-Support-the-new-v0-mangling-scheme-in-rust-demangle.patch
[PATCH] sra: Fix sra_modify_expr handling of partial writes (PR 94482)
Hi,
when sra_modify_expr is invoked on an expression that modifies only
part of the underlying replacement, such as a BIT_FIELD_REF on a LHS
of an assignment and the SRA replacement's type is not compatible with
what is being replaced (0th operand of the B_F_R in the above
example), it does not work properly, basically throwing away the partd
of the expr that should have stayed intact.
This is fixed in two ways. For BIT_FIELD_REFs, which operate on the
binary image of the replacement (and so in a way serve as a
VIEW_CONVERT_EXPR) we just do not bother with convertsing. For
REALPART_EXPRs and IMAGPART_EXPRs, we insert a VIEW_CONVERT_EXPR under
the complex partial access expression, which is OK even in a LHS of an
assignment (and other write contexts) because in those cases the
replacement is not going to be a giple register anyway.
The testcase for handling REALPART_EXPR and IMAGPART_EXPR is a bit
fragile because SRA prefers complex and vector types over anything else
(and in between the two it decides based on TYPE_UID which in my testing
today always preferred complex types) and even when GIMPLE is wrong I
often still did not get failing runs, so I only run it at -O1 (which is
the only level where the the test fails for me).
Bootstrapped and tested on x86_64-linux, bootstrap and testing on
i686-linux and aarch64-linux underway.
OK for trunk (and subsequently for release branches) if it passes?
Thanks,
Martin
2020-04-06 Martin Jambor
PR tree-optimization/94482
* tree-sra.c (create_access_replacement): Dump new replacement with
TDF_UID.
(sra_modify_expr): Fix handling of cases when the original EXPR writes
to only part of the replacement.
testsuite/
* gcc.dg/torture/pr94482.c: New test.
* gcc.dg/tree-ssa/pr94482-2.c: Likewise.
---
gcc/ChangeLog | 8
gcc/testsuite/ChangeLog | 6 +++
gcc/testsuite/gcc.dg/torture/pr94482.c| 34 +++
gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c | 50 +++
gcc/tree-sra.c| 17 ++--
5 files changed, 111 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr94482.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e10fb251c16..36ddef64afd 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-04-06 Martin Jambor
+
+ PR tree-optimization/94482
+ * tree-sra.c (create_access_replacement): Dump new replacement with
+ TDF_UID.
+ (sra_modify_expr): Fix handling of cases when the original EXPR writes
+ to only part of the replacement.
+
2020-04-05 Zachary Spytz
* extend.texi: Add free to list of ISO C90 functions that
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 88bab5d3d19..8b85e55afe8 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-04-06 Martin Jambor
+
+ PR tree-optimization/94482
+ * gcc.dg/torture/pr94482.c: New test.
+ * gcc.dg/tree-ssa/pr94482-2.c: Likewise.
+
2020-04-05 Iain Sandoe
* g++.dg/coroutines/torture/co-await-14-template-traits.C: Rename...
diff --git a/gcc/testsuite/gcc.dg/torture/pr94482.c
b/gcc/testsuite/gcc.dg/torture/pr94482.c
new file mode 100644
index 000..5bb19e745c2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr94482.c
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+
+typedef unsigned V __attribute__ ((__vector_size__ (16)));
+union U
+{
+ V j;
+ unsigned long long i __attribute__ ((__vector_size__ (16)));
+};
+
+static inline __attribute__((always_inline)) V
+foo (unsigned long long a)
+{
+ union U z = { .j = (V) {} };
+ for (unsigned long i = 0; i < 1; i++)
+z.i[i] = a;
+ return z.j;
+}
+
+static inline __attribute__((always_inline)) V
+bar (V a, unsigned long long i, int q)
+{
+ union U z = { .j = a };
+ z.i[q] = i;
+ return z.j;
+}
+
+int
+main ()
+{
+ union U z = { .j = bar (foo (1729), 2, 1) };
+ if (z.i[0] != 1729)
+__builtin_abort ();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c
new file mode 100644
index 000..fcac9d5e439
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-O1" } */
+
+typedef unsigned long V __attribute__ ((__vector_size__ (8)));
+typedef _Complex int Ci;
+typedef _Complex float Cf;
+
+union U
+{
+ Ci ci;
+ Cf cf;
+};
+
+volatile Ci vgi;
+
+Cf foo (Cf c)
+{
+ __real c = 0x1ffp10;
+ return c;
+}
+
+Ci ioo (Ci c)
+{
+ __real c = 50;
+ return c;
+}
+
+
+int main (int argc, char *argv[])
+{
+ union U u;
+
+ __real u.ci = 500;
+ __imag u.ci = 1000;
+ vgi = u.ci;
+
+ u.ci = ioo (u.ci);
+ __imag u.ci = 100;
+
+ if (__real u.ci != 50 || __imag u.ci != 100)
+__builtin_abort();
+
+ u.cf = foo (u.cf);
+ __imag u.cf = 0x1p3;
+
+
Re: [PATCH, V4] PowerPC Turn on -mpcrel by default for -mcpu=future
On Mon, 2020-04-06 at 12:52 -0400, Michael Meissner via Gcc-patches wrote:
Hi,
Just a single extra blank line below.
I'm still not a fan of the "Do not set..." comment, but will assume there
is history that necessitates the comment.
Other sections looked OK to me.
Over to Segher. :-)
Thanks,
-Will
> Commit message:
> Enable -mpcrel for -mcpu=future if it is allowed by the ABI.
>
> 2020-04-06 Michael Meissner
>
> * config/rs6000/linux64.h (PCREL_SUPPORTED_BY_OS): Enable
> prefixed PC-relative addressing if the ABI supports it.
> * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do not
> set OPTION_MASK_PREFIXED here.
> * config/rs6000/rs6000.c (rs6000_option_override_internal): Enable
> OPTION_MASK_PREFIXED and OPTION_MASK_PCREL on -mcpu=future by
> default if the current ABI allows the options.
>
> I tested this on a little endian PowerPC power8 system, doing bootstrap and
> make check. There were no regressions. I tested by hand the various
> conditions where -mpcrel is not enabled, and they all used the normal power9
> TOC references.
>
> -mcpu=power9 generates TOC;
> -mcpu=future -mcmodel=large generates TOC;
> -mcpu=future -mcmode=smallgenerates TOC;
> -mcpu=future -mno-prefixedgenerates TOC;
> -mcpu=future -mno-pcrel generates TOC;
> -mcpu=future generates PC-relative.
>
ok,
> --- /tmp/apbaWN_linux64.h 2020-04-03 17:15:05.059677000 -0400
> +++ gcc/config/rs6000/linux64.h 2020-04-03 17:01:05.580426937 -0400
> @@ -640,3 +640,10 @@ extern int dot_symbols;
> enabling the __float128 keyword. */
> #undef TARGET_FLOAT128_ENABLE_TYPE
> #define TARGET_FLOAT128_ENABLE_TYPE 1
> +
> +/* Enable using prefixed PC-relative addressing on the 'future' machine if
> the
> + ABI supports it. The ELF v2 ABI only supports PC-relative relocations for
> + the medium code model. */
> +#define PCREL_SUPPORTED_BY_OS(TARGET_FUTURE && TARGET_PREFIXED
> \
> + && ELFv2_ABI_CHECK \
> + && (TARGET_CMODEL == CMODEL_MEDIUM))
ok
> --- /tmp/XzRKno_rs6000-cpus.def 2020-04-03 17:15:05.068676928 -0400
> +++ gcc/config/rs6000/rs6000-cpus.def 2020-04-03 17:00:50.115550614 -0400
> @@ -75,10 +75,14 @@
>| OPTION_MASK_P8_VECTOR\
>| OPTION_MASK_P9_VECTOR)
>
> -/* Support for a future processor's features. Do not enable -mpcrel until it
> - is fully functional. */
> +/* Support for a future processor's features. Do not set
> OPTION_MASK_PREFIXED
> + or OPTION_MASK_PCREL here. Those options are enabled in the function
> + rs6000_option_override if the ABI supports them. */
> #define ISA_FUTURE_MASKS_SERVER (ISA_3_0_MASKS_SERVER
> \
> - | OPTION_MASK_FUTURE \
> + | OPTION_MASK_FUTURE)
> +
> +/* Flags that need to be turned off if -mno-future. */
> +#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \
>| OPTION_MASK_PREFIXED)
>
ok.
> /* Flags that need to be turned off if -mno-future. */
> --- /tmp/nyxSRY_rs6000.c 2020-04-03 17:15:05.081676823 -0400
> +++ gcc/config/rs6000/rs6000.c2020-04-03 17:03:19.846353197 -0400
> @@ -4020,6 +4020,12 @@ rs6000_option_override_internal (bool gl
>rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
> }
>
> + /* Enable -mprefixed by default on 64-bit 'future' systems. */
> + if (TARGET_FUTURE && TARGET_POWERPC64
> + && (rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED) == 0)
> +rs6000_isa_flags |= OPTION_MASK_PREFIXED;
> +
> +
extra blank line.
>/* -mprefixed (and hence -mpcrel) requires -mcpu=future. */
>if (TARGET_PREFIXED && !TARGET_FUTURE)
> {
> @@ -4171,6 +4177,14 @@ rs6000_option_override_internal (bool gl
>SUB3TARGET_OVERRIDE_OPTIONS;
> #endif
>
> +#ifdef PCREL_SUPPORTED_BY_OS
> + /* If the ABI has support for PC-relative relocations, enable it by
> + default. */
> + if (PCREL_SUPPORTED_BY_OS
> + && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
> +rs6000_isa_flags |= OPTION_MASK_PCREL;
> +#endif
> +
ok.
>/* -mpcrel requires -mcmodel=medium, but we can't check TARGET_CMODEL until
>after the subtarget override options are done. */
>if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
>
Re: [PATCH] c++: Fix usage of CONSTRUCTOR_PLACEHOLDER_BOUNDARY inside array initializers [90996]
On Mon, 6 Apr 2020, Jason Merrill wrote:
> On 4/6/20 3:07 PM, Patrick Palka wrote:
> > This PR reports that since the introduction of the
> > CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag, we are sometimes failing to resolve
> > PLACEHOLDER_EXPRs inside array initializers that refer to some inner
> > constructor. In the testcase in the PR, we have as the initializer for "S
> > c[];"
> > the following
> >
> >{{.a=(int &) &_ZGR1c_, .b={*(&)->a}}}
> >
> > where CONSTRUCTOR_PLACEHOLDER_BOUNDARY is set on the second outermost
> > constructor. However, we pass the whole initializer to replace_placeholders
> > in
> > store_init_value, and so due to the flag being set on the second outermost
> > ctor
> > it avoids recursing into the innermost constructor and we fail to resolve
> > the
> > PLACEHOLDER_EXPR within.
> >
> > To fix this, we could perhaps either call replace_placeholders in more
> > places,
> > or we could change where we set CONSTRUCTOR_PLACEHOLDER_BOUNDARY. This
> > patch
> > takes the latter approach -- when building up an array initializer, it
> > bubbles
> > any CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set on the element initializers up
> > to
> > the array initializer. Doing so shouldn't create any new PLACEHOLDER_EXPR
> > resolution ambiguities because we don't deal with PLACEHOLDER_EXPRs of array
> > type in the frontend, as far as I can tell.
>
> Interesting. Yes, that sounds like it should work.
>
> > Does this look OK to comit after testing?
>
> Yes.
>
> Though I'm seeing "after testing" a lot; what testing are you doing before
> sending patches?
Sorry for the sloppiness -- I should be writing "after a full
bootstrap/regtest" instead of "after testing" because I do indeed do
some testing before sending a patch. In particular, I usually run and
inspect the outputs of
make check RUNTESTFLAGS="dg.exp=*.C"
make check RUNTESTFLAGS="old-deja.exp=*.C"
make check RUNTESTFLAGS="conformance.exp=*ranges*"
in a build tree that is configured with --disable-bootstrap, as a quick
smoke test for the patch. Is this a sufficient amount of testing before
sending a patch for review, or would you prefer that I do a full
bootstrap/regtest beforehand?
In any case, I'll make sure to clearly convey the amount of testing that
was done and is remaining in future patch submissions.
>
> > gcc/cp/ChangeLog:
> >
> > PR c++/90996
> > * tree.c (replace_placeholders): Look through all handled components,
> > not just COMPONENT_REFs.
> > * typeck2.c (process_init_constructor_array): Propagate
> > CONSTRUCTOR_PLACEHOLDER_BOUNDARY up from each element initializer to
> > the array initializer.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/90996
> > * g++.dg/cpp1y/pr90996.C: New test.
> > ---
> > gcc/cp/tree.c| 2 +-
> > gcc/cp/typeck2.c | 18 ++
> > gcc/testsuite/g++.dg/cpp1y/pr90996.C | 17 +
> > 3 files changed, 36 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr90996.C
> >
> > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> > index 5eb0dcd717a..d1192b7e094 100644
> > --- a/gcc/cp/tree.c
> > +++ b/gcc/cp/tree.c
> > @@ -3247,7 +3247,7 @@ replace_placeholders (tree exp, tree obj, bool *seen_p
> > /*= NULL*/)
> > /* If the object isn't a (member of a) class, do nothing. */
> > tree op0 = obj;
> > - while (TREE_CODE (op0) == COMPONENT_REF)
> > + while (handled_component_p (op0))
> > op0 = TREE_OPERAND (op0, 0);
> > if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0
> > return exp;
> > diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
> > index cf1cb5ace66..fe844bc08bb 100644
> > --- a/gcc/cp/typeck2.c
> > +++ b/gcc/cp/typeck2.c
> > @@ -1488,6 +1488,17 @@ process_init_constructor_array (tree type, tree init,
> > int nested, int flags,
> > = massage_init_elt (TREE_TYPE (type), ce->value, nested, flags,
> > complain);
> > + if (TREE_CODE (ce->value) == CONSTRUCTOR
> > + && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value))
> > + {
> > + /* Shift CONSTRUCTOR_PLACEHOLDER_BOUNDARY from the element
> > initializer
> > +up to the array initializer, so that the call to
> > +replace_placeholders from store_init_value can resolve any
> > +PLACEHOLDER_EXPRs within this element initializer. */
> > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value) = 0;
> > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1;
> > + }
> > +
> > gcc_checking_assert
> > (ce->value == error_mark_node
> > || (same_type_ignoring_top_level_qualifiers_p
> > @@ -1516,6 +1527,13 @@ process_init_constructor_array (tree type, tree init,
> > int nested, int flags,
> > /* The default zero-initialization is fine for us; don't
> > add anything to the CONSTRUCTOR. */
> > next = NULL_TREE;
> > + else if (TREE_CODE (next) == CONSTRUCTOR
> > +
[committed] cselib: Fix endless cselib loop on (plus:P (reg) (const_int 0))
Hi!
getopt.c hangs the compiler on h8300-elf with -O2 -g, because the
IL contains addition of constant 0, the first PLUS operand is determined
to have the SP_DERIVED_VALUE_P and the new code in cselib recurses
indefinitely on seeing SP_DERIVED_VALUE_P with locs of
(plus:P SP_DERIVED_VALUE_P (const_int 0)).
Fixed by making sure cselib_subst_to_values canonicalizes it, hashing
already hashes it the same too.
Bootstrapped/regtested on x86_64-linux and i686-linux, tested on the
getopt.i testcase using cross to h8300-elf, approved off-list by Jeff,
committed to trunk.
2020-04-06 Jakub Jelinek
* cselib.c (cselib_subst_to_values): For SP_DERIVED_VALUE_P
+ const0_rtx return the SP_DERIVED_VALUE_P.
--- gcc/cselib.c.jj 2020-04-04 10:31:15.409783959 +0200
+++ gcc/cselib.c2020-04-06 12:05:46.055767835 +0200
@@ -2090,13 +2090,17 @@ cselib_subst_to_values (rtx x, machine_m
{
rtx t = cselib_subst_to_values (XEXP (x, 0), memmode);
if (GET_CODE (t) == VALUE)
- for (struct elt_loc_list *l = CSELIB_VAL_PTR (t)->locs;
-l; l = l->next)
- if (GET_CODE (l->loc) == PLUS
- && GET_CODE (XEXP (l->loc, 0)) == VALUE
- && SP_DERIVED_VALUE_P (XEXP (l->loc, 0))
- && CONST_INT_P (XEXP (l->loc, 1)))
- return plus_constant (Pmode, l->loc, INTVAL (XEXP (x, 1)));
+ {
+ if (SP_DERIVED_VALUE_P (t) && XEXP (x, 1) == const0_rtx)
+ return t;
+ for (struct elt_loc_list *l = CSELIB_VAL_PTR (t)->locs;
+ l; l = l->next)
+ if (GET_CODE (l->loc) == PLUS
+ && GET_CODE (XEXP (l->loc, 0)) == VALUE
+ && SP_DERIVED_VALUE_P (XEXP (l->loc, 0))
+ && CONST_INT_P (XEXP (l->loc, 1)))
+ return plus_constant (Pmode, l->loc, INTVAL (XEXP (x, 1)));
+ }
if (t != XEXP (x, 0))
{
copy = shallow_copy_rtx (x);
Jakub
Re: [RS6000] Don't pass -many to the assembler
Hi! On Mon, Apr 06, 2020 at 10:35:34PM +0200, Sebastian Huber wrote: > What do you think about the attached patch? (Please use a correct MIME type for attachments (x-* never is correct on mailing lists. Just text/plain will do fine.) > libgcc/ > > * config/rs6000/crtresfpr.S: Disable all functions if > _SOFT_DOUBLE is defined. Can't you just use an appropriate .machine? I'd prefer that, that's how AltiVec stuff is done, for example. > * config/rs6000/crtresxfpr.S: Likewise. > * config/rs6000/crtsavfpr.S: Likewise. (Broken indentation). > +/* On PowerPC64 Linux, these functions are provided by the linker. > Soft-double > + * configurations do not need these functions. */ > +#if !defined(__powerpc64__) && !defined(_SOFT_DOUBLE) We use __NO_FPRS__ more often. All of these are the same now, but this name makes more sense as well ;-) Segher
Re: [PATCH v4 5/5] libgomp/test: Remove a build sysroot fix regression
On Mon, 6 Apr 2020, Jeff Law wrote: > > libgomp/ > > * configure.ac: Add testsuite/libgomp-site-extra.exp to output > > files. > > * configure: Regenerate. > > * testsuite/libgomp-site-extra.exp.in: New file. > > * testsuite/libgomp-test-support.exp.in (GCC_UNDER_TEST): Remove > > variable. > > * testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New > > variable. > > * testsuite/Makefile.in: Regenerate. > OK > jeff Committed now, thanks for your review. Maciej
Re: [PATCH v4 1/5] libatomic/test: Fix compilation for build sysroot
On Mon, 6 Apr 2020, Jeff Law wrote: > > libatomic/ > > * configure.ac: Add testsuite/libatomic-site-extra.exp to output > > files. > > * configure: Regenerate. > > * libatomic/testsuite/libatomic-site-extra.exp.in: New file. > > * testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New > > variable. > > * testsuite/Makefile.in: Regenerate. > OK > jeff Committed now, thanks for your review. Maciej
[PATCH] c++: Further fix for -fsanitize=vptr [PR94325]
Hi!
For -fsanitize=vptr, we insert a NULL store into the vptr instead of just
adding a CLOBBER of this. build_clobber_this makes the CLOBBER conditional
on in_charge (implicit) parameter whenever CLASSTYPE_VBASECLASSES, but when
adding this conditionalization to the -fsanitize=vptr code in PR87095,
I wanted it to catch some more cases when the class has CLASSTYPE_VBASECLASSES,
but the vptr is still not shared with something else, otherwise the
sanitization would be less effective.
The following testcase shows that the chosen test that CLASSTYPE_PRIMARY_BINFO
is non-NULL and has BINFO_VIRTUAL_P set wasn't sufficient,
the D class has still sizeof(D) == sizeof(void*) and thus contains just
a single vptr, but while in B::~B() this results in the vptr not being
cleared, in C::~C() this condition isn't true, as CLASSTYPE_PRIMARY_BINFO
in that case is B and is not BINFO_VIRTUAL_P, so it clears the vptr, but the
D::~D() dtor after invoking C::~C() invokes A::~A() with an already cleared
vptr, which is then reported.
The following patch is just a shot in the dark, keep looking through
CLASSTYPE_PRIMARY_BINFO until we find BINFO_VIRTUAL_P, but it works on the
existing testcase as well as this new one.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or do we want some other test?
2020-04-06 Jakub Jelinek
PR c++/94325
* decl.c (begin_destructor_body): For CLASSTYPE_VBASECLASSES class
dtors, if CLASSTYPE_PRIMARY_BINFO is non-NULL, but not BINFO_VIRTUAL_P,
look at CLASSTYPE_PRIMARY_BINFO of its BINFO_TYPE if it is not
BINFO_VIRTUAL_P, and so on.
* g++.dg/ubsan/vptr-15.C: New test.
--- gcc/cp/decl.c.jj2020-03-27 09:59:26.407083563 +0100
+++ gcc/cp/decl.c 2020-04-06 13:25:03.321511554 +0200
@@ -16662,14 +16662,20 @@ begin_destructor_body (void)
/* If the vptr is shared with some virtual nearly empty base,
don't clear it if not in charge, the dtor of the virtual
nearly empty base will do that later. */
- if (CLASSTYPE_VBASECLASSES (current_class_type)
- && CLASSTYPE_PRIMARY_BINFO (current_class_type)
- && BINFO_VIRTUAL_P
- (CLASSTYPE_PRIMARY_BINFO (current_class_type)))
+ if (CLASSTYPE_VBASECLASSES (current_class_type))
{
- stmt = convert_to_void (stmt, ICV_STATEMENT,
- tf_warning_or_error);
- stmt = build_if_in_charge (stmt);
+ tree c = current_class_type;
+ while (CLASSTYPE_PRIMARY_BINFO (c))
+ {
+ if (BINFO_VIRTUAL_P (CLASSTYPE_PRIMARY_BINFO (c)))
+ {
+ stmt = convert_to_void (stmt, ICV_STATEMENT,
+ tf_warning_or_error);
+ stmt = build_if_in_charge (stmt);
+ break;
+ }
+ c = BINFO_TYPE (CLASSTYPE_PRIMARY_BINFO (c));
+ }
}
finish_decl_cleanup (NULL_TREE, stmt);
}
--- gcc/testsuite/g++.dg/ubsan/vptr-15.C.jj 2020-04-06 13:32:43.501627756
+0200
+++ gcc/testsuite/g++.dg/ubsan/vptr-15.C2020-04-06 13:37:52.642001353
+0200
@@ -0,0 +1,14 @@
+// PR c++/94325
+// { dg-do run { target c++11 } }
+// { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" }
+
+struct A { virtual ~A () = default; };
+struct B : public virtual A {};
+struct C : public B {};
+struct D : public C {};
+
+int
+main ()
+{
+ D a;
+}
Jakub
Re: [PATCH 1/2] RISC-V: Update march parser
On Tue, Mar 31, 2020 at 2:06 AM Kito Cheng wrote: > - The arch string rule has changed in latest spec, it introduced new >multi-letter extension prefix with 'h' and 'z', and drop `sx`. also >adjust parsing order for 's' and 'x'. This looks OK to me. Jim
Re: [PATCH 2/2] RISC-V: Handle implied extension for -march parser.
On Tue, Mar 31, 2020 at 2:07 AM Kito Cheng wrote: > - Implied rule are introduced into latest RISC-V isa spec. > > - Only implemented D implied F-extension. Zicsr and Zifence are not > implement yet, so the rule not included in this patch. When I try this patch, I see an error: rohan:2132$ ./xgcc -B./ -O -march=rv64imafdc -mabi=lp64d tmp.c /tmp/ccULN36f.s: Assembler messages: /tmp/ccULN36f.s:3: Fatal error: -march=rv64i2p0_m2p0_a2p0_f2p0_f2p0_d2p0_c2p0: ISA string is not in canonical order. `f' rohan:2133$ Looks like you need to make sure that we don't add the implied F if it is already there. Otherwise, this looks OK to me. We don't have support for these implied extensions in binutils yet. If we are adding it to gcc, we should probably add it to binutils too. Maybe you can ask Nelson to work on that. > + /* TODO: Implied extension might use differet version. */ differet -> different Jim
[PATCH] i386: Fix emit_reduc_half on V{64Q,32H}Imode [PR94500]
Hi!
The following testcase is miscompiled in 8.x, because emit_reduc_half is
prepared to handle for 512-bit modes only i equal to 512, 256, 128 and 64.
V32HImode also needs i equal to 32 and V64QImode i equal to 32 and 16,
but emit_reduc_half in that case performs a redundant permutation exactly
like i == 32. In 9+ the testcase works because Richard in r9-3393
changed the reduc_* expanders so that they actually don't call
ix86_expand_reduc on 512-bit modes, but only 128-bit ones.
The patch fixes emit_reduc_half to handle also i of 32 and 16 similarly to
how V32QImode/V16HImode are handled for AVX2. I think it shouldn't hurt
to fix the function even on the trunk and 9 branch even when nothing uses
it ATM.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/9 and
primarily for 8.5 (obviously in that case s/i386-expand/i386/)?
2020-04-06 Jakub Jelinek
PR target/94500
* config/i386/i386-expand.c (emit_reduc_half): For V{64QI,32HI}mode
handle i < 64 using avx512bw_lshrv4ti3. Formatting fixes.
* gcc.target/i386/avx512bw-pr94500.c: New test.
--- gcc/config/i386/i386-expand.c.jj2020-03-29 19:26:31.748561262 +0200
+++ gcc/config/i386/i386-expand.c 2020-04-06 17:18:44.906242980 +0200
@@ -14891,43 +14891,51 @@ emit_reduc_half (rtx dest, rtx src, int
break;
case E_V64QImode:
case E_V32HImode:
+ if (i < 64)
+ {
+ d = gen_reg_rtx (V4TImode);
+ tem = gen_avx512bw_lshrv4ti3 (d, gen_lowpart (V4TImode, src),
+ GEN_INT (i / 2));
+ break;
+ }
+ /* FALLTHRU */
case E_V16SImode:
case E_V16SFmode:
case E_V8DImode:
case E_V8DFmode:
if (i > 128)
tem = gen_avx512f_shuf_i32x4_1 (gen_lowpart (V16SImode, dest),
- gen_lowpart (V16SImode, src),
- gen_lowpart (V16SImode, src),
- GEN_INT (0x4 + (i == 512 ? 4 : 0)),
- GEN_INT (0x5 + (i == 512 ? 4 : 0)),
- GEN_INT (0x6 + (i == 512 ? 4 : 0)),
- GEN_INT (0x7 + (i == 512 ? 4 : 0)),
- GEN_INT (0xC), GEN_INT (0xD),
- GEN_INT (0xE), GEN_INT (0xF),
- GEN_INT (0x10), GEN_INT (0x11),
- GEN_INT (0x12), GEN_INT (0x13),
- GEN_INT (0x14), GEN_INT (0x15),
- GEN_INT (0x16), GEN_INT (0x17));
+ gen_lowpart (V16SImode, src),
+ gen_lowpart (V16SImode, src),
+ GEN_INT (0x4 + (i == 512 ? 4 : 0)),
+ GEN_INT (0x5 + (i == 512 ? 4 : 0)),
+ GEN_INT (0x6 + (i == 512 ? 4 : 0)),
+ GEN_INT (0x7 + (i == 512 ? 4 : 0)),
+ GEN_INT (0xC), GEN_INT (0xD),
+ GEN_INT (0xE), GEN_INT (0xF),
+ GEN_INT (0x10), GEN_INT (0x11),
+ GEN_INT (0x12), GEN_INT (0x13),
+ GEN_INT (0x14), GEN_INT (0x15),
+ GEN_INT (0x16), GEN_INT (0x17));
else
tem = gen_avx512f_pshufd_1 (gen_lowpart (V16SImode, dest),
- gen_lowpart (V16SImode, src),
- GEN_INT (i == 128 ? 0x2 : 0x1),
- GEN_INT (0x3),
- GEN_INT (0x3),
- GEN_INT (0x3),
- GEN_INT (i == 128 ? 0x6 : 0x5),
- GEN_INT (0x7),
- GEN_INT (0x7),
- GEN_INT (0x7),
- GEN_INT (i == 128 ? 0xA : 0x9),
- GEN_INT (0xB),
- GEN_INT (0xB),
- GEN_INT (0xB),
- GEN_INT (i == 128 ? 0xE : 0xD),
- GEN_INT (0xF),
- GEN_INT (0xF),
- GEN_INT (0xF));
+ gen_lowpart (V16SImode, src),
+ GEN_INT (i == 128 ? 0x2 : 0x1),
+ GEN_INT (0x3),
+ GEN_INT (0x3),
+ GEN_INT (0x3),
+ GEN_INT (i == 128 ? 0x6 : 0x5),
+ GEN_INT (0x7),
+ GEN_I
Re: [committed] cselib: Fix endless cselib loop on (plus:P (reg) (const_int 0))
On Tue, 2020-04-07 at 00:33 +0200, Jakub Jelinek wrote: > Hi! > > getopt.c hangs the compiler on h8300-elf with -O2 -g, because the > IL contains addition of constant 0, the first PLUS operand is determined > to have the SP_DERIVED_VALUE_P and the new code in cselib recurses > indefinitely on seeing SP_DERIVED_VALUE_P with locs of > (plus:P SP_DERIVED_VALUE_P (const_int 0)). > > Fixed by making sure cselib_subst_to_values canonicalizes it, hashing > already hashes it the same too. > > Bootstrapped/regtested on x86_64-linux and i686-linux, tested on the > getopt.i testcase using cross to h8300-elf, approved off-list by Jeff, > committed to trunk. > > 2020-04-06 Jakub Jelinek > > * cselib.c (cselib_subst_to_values): For SP_DERIVED_VALUE_P > + const0_rtx return the SP_DERIVED_VALUE_P. Thanks. And just FTR, the scenario where this happens could (in theory) happen on other ports, particularly those still using reload. jeff >
Re: [AArch64][SVE][IPA] ICE caused by incompatibility of SRA and svst builtin-function
Hi,
On Thu, Apr 02 2020, Richard Biener wrote:
> On Thu, Apr 2, 2020 at 5:36 AM bule wrote:
>>
>> Hello,
>>
>> An Internal Compiler Error(ICE) is found in ipa-sra optimization pass when
>> it handle the argument of internal call svst3 for SVE.
>>
>> The problem comes from
>> gcc/testsuite/gcc.target/aarch64/sve/acle/asm/st2_bf16.c in the test suit,
>> which can be reduced to flowing code:
>>
>> #include
>> #include
>> void st2_bf16_base (svbfloat16x3_t z1, svbool_t p0, bfloat16_t *x0, intptr_t
>> x1) {
>> svst3 (p0, x0, z1);
>> }
>>
>> Compiled with -march=armv8.2-a+sve -msve-vector-bits=256 -O2, it will result
>> in a segment fault in IPA-SRA:
>>
>> > [bule@localhost gcc10_fail]$ gcc st2_bf16.i -o st2_bf16.s -S
>> > -march=armv8.2-a+sve -msve-vector-bits=256 -O2
>> > during IPA pass: sra
>> > st2_bf16.c: In function ‘st2_bf16_base’:
>> > st2_bf16.c:10:1: internal compiler error: Segmentation fault
>> > .. /* omit some stack info here. */ ..
>> > 0xa34f68 call_summary::get_create(cgraph_edge*)
>> > ../.././gcc/symbol-summary.h:642
>> > 0xa34f68 record_nonregister_call_use
>> > ../.././gcc/ipa-sra.c:1613
>> > 0xa34f68 scan_expr_access
>> > ../.././gcc/ipa-sra.c:1781
>> > .. /* omit some stack info here. */ ..
>> > Please submit a full bug report,
>> > with preprocessed source if appropriate.
>> > Please include the complete backtrace with any bug report.
>>
>> Details can be found in PR 94398.
>> Similar problem can be found in svst2、svst4 and other functions of this kind.
>>
>> This problem is cause by "record_nonregister_call_use" function trying to
>> access the call graph edge of an internal call, .MASK_STORE_LANE, which is a
>> NULL pointer.
>>
>> The reason of stepping into "record_nonregister_call_use" function is that
>> the upper level function "scan_expr_access" considered the "svbfloat16x3_t
>> z1"
>> argument as a valid candidate for further optimization.
>>
>> A simple solution here is to disqualify the candidate at "scan_expr_access"
>> level when the call graph edge is null, which indicates the call is either
>> an internal call or a call with no references. For both case, the further
>> optimization process should stop before it reference a NULL pointer.
>>
>> A proposed patch is attached.
>>
>> Any suggestions?
>
> I think internal calls should be handled like asms which means, lookig
> at the source a bit,
> instead of ISRA_CTX_ARG pass ISRA_CTX_LOAD to scan_expr_access.
>
indeed, in this situation it would be best if we simply treated such
arguments as loads (from the aggregates). Would the following (only
very mildly tested) patch work for you?
Thanks,
Martin
diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
index f0ebaec708d..b225af61427 100644
--- a/gcc/ipa-sra.c
+++ b/gcc/ipa-sra.c
@@ -1870,15 +1870,22 @@ scan_function (cgraph_node *node, struct function *fun)
case GIMPLE_CALL:
{
unsigned argument_count = gimple_call_num_args (stmt);
- scan_call_info call_info;
+ isra_scan_context ctx = ISRA_CTX_ARG;
+ scan_call_info call_info, *call_info_p = &call_info;
call_info.cs = node->get_edge (stmt);
- call_info.argument_count = argument_count;
+ if (!call_info.cs)
+ {
+ call_info_p = NULL;
+ ctx = ISRA_CTX_LOAD;
+ }
+ else
+ call_info.argument_count = argument_count;
for (unsigned i = 0; i < argument_count; i++)
{
call_info.arg_idx = i;
scan_expr_access (gimple_call_arg (stmt, i), stmt,
- ISRA_CTX_ARG, bb, &call_info);
+ ctx, bb, call_info_p);
}
tree lhs = gimple_call_lhs (stmt);
libgo patch committed: Update to close to 1.14.2 release
I've committed a libgo patch to update to the current Go 1.14 release
branch, which is close to the 1.14.2 release. Bootstrapped and tested
on x86_64-pc-linux-gnu. Committed to mainline.
Ian
2b280af10aa96cb5a6f20a695636b83a187a4e9b
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 6b4c21fabf5..c5e8b29879e 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-7a62a49e62c090118fa003d9265c5f5e2090c4f9
+4a31d064fd6996f64b620104e849292af8f25e12
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
diff --git a/libgo/MERGE b/libgo/MERGE
index 233492ada69..4398cd7dc28 100644
--- a/libgo/MERGE
+++ b/libgo/MERGE
@@ -1,4 +1,4 @@
-20a838ab94178c55bc4dc23ddc332fce8545a493
+edea4a79e8d7dea2456b688f492c8af33d381dc2
The first line of this file holds the git revision number of the
last merge done from the master library sources.
diff --git a/libgo/VERSION b/libgo/VERSION
index f000507a7fe..864916ef499 100644
--- a/libgo/VERSION
+++ b/libgo/VERSION
@@ -1 +1 @@
-go1.14
+go1.14.2
diff --git a/libgo/go/cmd/cgo/gcc.go b/libgo/go/cmd/cgo/gcc.go
index 678b348c89b..310316bdda4 100644
--- a/libgo/go/cmd/cgo/gcc.go
+++ b/libgo/go/cmd/cgo/gcc.go
@@ -2265,7 +2265,7 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos
token.Pos, parent string) *Typ
// Translate to zero-length array instead.
count = 0
}
- sub := c.loadType(dt.Type, pos, key)
+ sub := c.Type(dt.Type, pos)
t.Align = sub.Align
t.Go = &ast.ArrayType{
Len: c.intExpr(count),
@@ -2410,7 +2410,7 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos
token.Pos, parent string) *Typ
c.ptrs[key] = append(c.ptrs[key], t)
case *dwarf.QualType:
- t1 := c.loadType(dt.Type, pos, key)
+ t1 := c.Type(dt.Type, pos)
t.Size = t1.Size
t.Align = t1.Align
t.Go = t1.Go
@@ -2494,7 +2494,13 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos
token.Pos, parent string) *Typ
}
name := c.Ident("_Ctype_" + dt.Name)
goIdent[name.Name] = name
- sub := c.loadType(dt.Type, pos, key)
+ akey := ""
+ if c.anonymousStructTypedef(dt) {
+ // only load type recursively for typedefs of anonymous
+ // structs, see issues 37479 and 37621.
+ akey = key
+ }
+ sub := c.loadType(dt.Type, pos, akey)
if c.badPointerTypedef(dt) {
// Treat this typedef as a uintptr.
s := *sub
@@ -3015,6 +3021,13 @@ func fieldPrefix(fld []*ast.Field) string {
return prefix
}
+// anonymousStructTypedef reports whether dt is a C typedef for an anonymous
+// struct.
+func (c *typeConv) anonymousStructTypedef(dt *dwarf.TypedefType) bool {
+ st, ok := dt.Type.(*dwarf.StructType)
+ return ok && st.StructName == ""
+}
+
// badPointerTypedef reports whether t is a C typedef that should not be
considered a pointer in Go.
// A typedef is bad if C code sometimes stores non-pointers in this type.
// TODO: Currently our best solution is to find these manually and list them as
diff --git a/libgo/go/cmd/go/internal/generate/generate.go
b/libgo/go/cmd/go/internal/generate/generate.go
index 198ca1c1b94..315db69de8a 100644
--- a/libgo/go/cmd/go/internal/generate/generate.go
+++ b/libgo/go/cmd/go/internal/generate/generate.go
@@ -22,6 +22,7 @@ import (
"cmd/go/internal/cfg"
"cmd/go/internal/load"
"cmd/go/internal/modload"
+ "cmd/go/internal/str"
"cmd/go/internal/work"
)
@@ -438,7 +439,7 @@ func (g *Generator) exec(words []string) {
cmd.Stderr = os.Stderr
// Run the command in the package directory.
cmd.Dir = g.dir
- cmd.Env = append(cfg.OrigEnv, g.env...)
+ cmd.Env = str.StringList(cfg.OrigEnv, g.env)
err := cmd.Run()
if err != nil {
g.errorf("running %q: %s", words[0], err)
diff --git a/libgo/go/cmd/go/internal/test/test.go
b/libgo/go/cmd/go/internal/test/test.go
index fb011d4c03f..4ad142ccd87 100644
--- a/libgo/go/cmd/go/internal/test/test.go
+++ b/libgo/go/cmd/go/internal/test/test.go
@@ -1142,7 +1142,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a
*work.Action) error {
cmd := exec.Command(args[0], args[1:]...)
cmd.Dir = a.Package.Dir
- cmd.Env = base.EnvForDir(cmd.Dir, cfg.OrigEnv)
+ cmd.Env = base.EnvForDir(cmd.Dir,
cfg.OrigEnv[:len(cfg.OrigEnv):len(cfg.OrigEnv)])
cmd.Stdout = stdout
cmd.Stderr = stdout
@@ -1224,6 +1224,14 @@ func (c *runCache) builderRunTest(b *work.Builder, a
*work.Action) error {
if len(out) == 0 {
[PATCH] On PPC32 GCC9 or later can throw an ICE when built with older GCCs
When GCC9 is built with older GCC (4.7) on FreeBSD 32-bit on PowerPC an ICE is generated on stage 1 when selftests are performed. After an investigation the root cause was traced to an unnecessary and harmful instantiation of a new dump_context singleton class in order to simply get the current activated instance. I was only able to reproduce it on FreeBSD 32-bit but it can affect other 32-bit OSs in my understanding, and even 64-bit on certain cases I believe, although I was not able to reproduce it on 64-bit FreeBSD or Linux. On FreeBSD 12 32-bit the bug reproduces 100% when GCC9 is built with base GCC 4.7. For additional details, please see: * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242506 and * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=243666 Gustavo Romero (1): Fix use of singleton in optinfo framework gcc/dumpfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4
[PATCH] Fix use of singleton in optinfo framework
Currently an use of get() method of dump_context singleton in optinfo
framework causes a new class to be instantiated, which calls the singleton
dtor when the class is destroyed, freeing memory that is referenced after
free() is called, generating an ICE error.
This commit fixes the issue by using singleton's static member get()
directly to get the singleton's active instance, which doesn't instantiate
a new class, so no dtor is called.
gcc/Changelog:
2020-04-06 Gustavo Romero
* dumpfile.c:
(selftest::temp_dump_context::temp_dump_context): Fix ctor.
---
gcc/dumpfile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 468ffab..d0d8aa7 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -2076,7 +2076,7 @@ temp_dump_context::temp_dump_context (bool
forcibly_enable_optinfo,
bool forcibly_enable_dumping,
dump_flags_t test_pp_flags)
: m_context (),
- m_saved (&dump_context ().get ())
+ m_saved(&dump_context::get())
{
dump_context::s_current = &m_context;
if (forcibly_enable_optinfo)
--
2.7.4
Merge from master to gccgo branch
I merged master revision 52fa80f853c0b0f623ea9e4c7198e324ce44ff30 to the gccgo branch. Ian
[PATCH] c++: Fix ICE-on-invalid with lambda template [PR94507]
While reducing something else I noticed that we ICE on the following
invalid code. In tsubst_lambda_expr, tsubst_template_decl has already
reported an error and returned the error_mark_node, so make sure we
don't ICE on that. I'm using a goto here because we still have to
do finish_struct because it does popclass ().
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/94507 - ICE-on-invalid with lambda template.
* pt.c (tsubst_lambda_expr): Cope when tsubst_template_decl returns
error_mark_node.
* g++.dg/cpp2a/lambda-generic7.C: New test.
---
gcc/cp/pt.c | 6 ++
gcc/testsuite/g++.dg/cpp2a/lambda-generic7.C | 10 ++
2 files changed, 16 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-generic7.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 617c22f..f804ba18692 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18876,6 +18876,11 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
if (oldtmpl)
{
tmpl = tsubst_template_decl (oldtmpl, args, complain, fntype);
+ if (tmpl == error_mark_node)
+ {
+ r = error_mark_node;
+ goto out;
+ }
fn = DECL_TEMPLATE_RESULT (tmpl);
finish_member_declaration (tmpl);
}
@@ -18948,6 +18953,7 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
maybe_add_lambda_conv_op (type);
}
+out:
finish_struct (type, /*attr*/NULL_TREE);
insert_pending_capture_proxies ();
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-generic7.C
b/gcc/testsuite/g++.dg/cpp2a/lambda-generic7.C
new file mode 100644
index 000..bedba683671
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-generic7.C
@@ -0,0 +1,10 @@
+// PR c++/94507 - ICE-on-invalid with lambda template.
+// { dg-do compile { target c++2a } }
+
+struct S { };
+
+template
+auto foo(T, U)
+{
+ [] <> () { foo (S{}, S{}); }; // { dg-error "expected" }
+}
base-commit: 130f703da0c0d7b785d394b17df884379b4aadd9
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Re: [PATCH] Fix use of singleton in optinfo framework
On Mon, 2020-04-06 at 19:44 -0400, Gustavo Romero wrote:
Thanks for this patch.
The patch looks correct, but I'm not sure that the description of the
problem is exact in every detail. I think you've run into a bug in
code I wrote; sorry.
> Currently an use of get() method of dump_context singleton in optinfo
> framework causes a new class to be instantiated,
I agree, and that's a bug.
> which calls the singleton
> dtor when the class is destroyed, freeing memory that is referenced
> after
> free() is called, generating an ICE error.
I introduced dump_context when adding the opt_info machinery in order
to make it more amenable to unit-testing via the selftest framework.
If I'm reading the code correctly, dump_context has no ctor, but
instead relies on the fields being zero-initialized in the BSS segment.
It has a dtor, which deletes the m_pending field.
It looks like the initializer of m_context in temp_dump_context's ctor
uses the implicitly-defined default constructor for dump_context, and
this leaves the fields uninitialized; in particular m_pending.
I *think* what's going on is that the temporary dump_context that gets
created in the "m_saved" initializer is uninitialized, and when its
dtor runs it deletes the m_pending, thus calling delete on
uninitialized memory - whatever happens to be in the stack. I don't
see a complaint about this under valgrind though.
Sorry about this. IIRC I was trying to avoid having a ctor run before
main. If I'm reading things right there's still a dormant bug here
even with your patch.
>
> This commit fixes the issue by using singleton's static member get()
> directly to get the singleton's active instance, which doesn't
> instantiate
> a new class, so no dtor is called.
I agree.
> gcc/Changelog:
> 2020-04-06 Gustavo Romero
>
> * dumpfile.c:
> (selftest::temp_dump_context::temp_dump_context): Fix ctor.
> ---
> gcc/dumpfile.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 468ffab..d0d8aa7 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -2076,7 +2076,7 @@ temp_dump_context::temp_dump_context (bool
> forcibly_enable_optinfo,
> bool forcibly_enable_dumping,
> dump_flags_t test_pp_flags)
> : m_context (),
> - m_saved (&dump_context ().get ())
> + m_saved(&dump_context::get())
Whitespace nit: keep the space between m_saved and the open-paren.
> {
>dump_context::s_current = &m_context;
>if (forcibly_enable_optinfo)
Re: [PATCH] Fix use of singleton in optinfo framework
On Mon, 2020-04-06 at 21:42 -0400, David Malcolm via Gcc-patches wrote:
> On Mon, 2020-04-06 at 19:44 -0400, Gustavo Romero wrote:
>
> Thanks for this patch.
>
> The patch looks correct, but I'm not sure that the description of the
> problem is exact in every detail. I think you've run into a bug in
> code I wrote; sorry.
>
> > Currently an use of get() method of dump_context singleton in
> > optinfo
> > framework causes a new class to be instantiated,
>
> I agree, and that's a bug.
>
> > which calls the singleton
> > dtor when the class is destroyed, freeing memory that is referenced
> > after
> > free() is called, generating an ICE error.
>
> I introduced dump_context when adding the opt_info machinery in order
> to make it more amenable to unit-testing via the selftest framework.
>
> If I'm reading the code correctly, dump_context has no ctor, but
> instead relies on the fields being zero-initialized in the BSS
> segment.
> It has a dtor, which deletes the m_pending field.
>
> It looks like the initializer of m_context in temp_dump_context's
> ctor
> uses the implicitly-defined default constructor for dump_context, and
> this leaves the fields uninitialized; in particular m_pending.
>
> I *think* what's going on is that the temporary dump_context that
> gets
> created in the "m_saved" initializer is uninitialized, and when its
> dtor runs it deletes the m_pending, thus calling delete on
> uninitialized memory - whatever happens to be in the stack.
or rather: "using delete with an uninitialized optinfo *"
> I don't
> see a complaint about this under valgrind though.
>
> Sorry about this. IIRC I was trying to avoid having a ctor run
> before
> main. If I'm reading things right there's still a dormant bug here
> even with your patch.
>
> > This commit fixes the issue by using singleton's static member
> > get()
> > directly to get the singleton's active instance, which doesn't
> > instantiate
> > a new class, so no dtor is called.
>
> I agree.
>
> > gcc/Changelog:
> > 2020-04-06 Gustavo Romero
> >
> > * dumpfile.c:
> > (selftest::temp_dump_context::temp_dump_context): Fix ctor.
> > ---
> > gcc/dumpfile.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> > index 468ffab..d0d8aa7 100644
> > --- a/gcc/dumpfile.c
> > +++ b/gcc/dumpfile.c
> > @@ -2076,7 +2076,7 @@ temp_dump_context::temp_dump_context (bool
> > forcibly_enable_optinfo,
> > bool forcibly_enable_dumping,
> > dump_flags_t test_pp_flags)
> > : m_context (),
> > - m_saved (&dump_context ().get ())
> > + m_saved(&dump_context::get())
>
> Whitespace nit: keep the space between m_saved and the open-paren.
>
> > {
> >dump_context::s_current = &m_context;
> >if (forcibly_enable_optinfo)
答复: [AArch64][SVE][IPA] ICE caused by incompatibility of SRA and svst builtin-function
Hi,
The patch is tested and works fine. It is more appropriate to handle the
context by considering it as a section of assemble code.
A minor question is that I think svst functions are for store operations. Why
pass ISRA_CTX_LOAD to scan_expr_access rather than ISRA_CTX_STORE?
Thanks,
Bu Le
-邮件原件-
发件人: Martin Jambor [mailto:mjam...@suse.cz]
发送时间: 2020年4月7日 7:21
收件人: bule
抄送: Richard Biener ; gcc-patches@gcc.gnu.org
主题: Re: [AArch64][SVE][IPA] ICE caused by incompatibility of SRA and svst
builtin-function
Hi,
On Thu, Apr 02 2020, Richard Biener wrote:
> On Thu, Apr 2, 2020 at 5:36 AM bule wrote:
>>
>> Hello,
>>
>> An Internal Compiler Error(ICE) is found in ipa-sra optimization pass when
>> it handle the argument of internal call svst3 for SVE.
>>
>> The problem comes from
>> gcc/testsuite/gcc.target/aarch64/sve/acle/asm/st2_bf16.c in the test suit,
>> which can be reduced to flowing code:
>>
>> #include
>> #include
>> void st2_bf16_base (svbfloat16x3_t z1, svbool_t p0, bfloat16_t *x0, intptr_t
>> x1) {
>> svst3 (p0, x0, z1);
>> }
>>
>> Compiled with -march=armv8.2-a+sve -msve-vector-bits=256 -O2, it will result
>> in a segment fault in IPA-SRA:
>>
>> > [bule@localhost gcc10_fail]$ gcc st2_bf16.i -o st2_bf16.s -S
>> > -march=armv8.2-a+sve -msve-vector-bits=256 -O2 during IPA pass: sra
>> > st2_bf16.c: In function ‘st2_bf16_base’:
>> > st2_bf16.c:10:1: internal compiler error: Segmentation fault
>> > .. /* omit some stack info here. */ ..
>> > 0xa34f68 call_summary::get_create(cgraph_edge*)
>> > ../.././gcc/symbol-summary.h:642
>> > 0xa34f68 record_nonregister_call_use
>> > ../.././gcc/ipa-sra.c:1613
>> > 0xa34f68 scan_expr_access
>> > ../.././gcc/ipa-sra.c:1781
>> > .. /* omit some stack info here. */ ..
>> > Please submit a full bug report,
>> > with preprocessed source if appropriate.
>> > Please include the complete backtrace with any bug report.
>>
>> Details can be found in PR 94398.
>> Similar problem can be found in svst2、svst4 and other functions of this kind.
>>
>> This problem is cause by "record_nonregister_call_use" function trying to
>> access the call graph edge of an internal call, .MASK_STORE_LANE, which is a
>> NULL pointer.
>>
>> The reason of stepping into "record_nonregister_call_use" function is that
>> the upper level function "scan_expr_access" considered the "svbfloat16x3_t
>> z1"
>> argument as a valid candidate for further optimization.
>>
>> A simple solution here is to disqualify the candidate at "scan_expr_access"
>> level when the call graph edge is null, which indicates the call is either
>> an internal call or a call with no references. For both case, the further
>> optimization process should stop before it reference a NULL pointer.
>>
>> A proposed patch is attached.
>>
>> Any suggestions?
>
> I think internal calls should be handled like asms which means, lookig
> at the source a bit, instead of ISRA_CTX_ARG pass ISRA_CTX_LOAD to
> scan_expr_access.
>
indeed, in this situation it would be best if we simply treated such arguments
as loads (from the aggregates). Would the following (only very mildly tested)
patch work for you?
Thanks,
Martin
diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index f0ebaec708d..b225af61427 100644
--- a/gcc/ipa-sra.c
+++ b/gcc/ipa-sra.c
@@ -1870,15 +1870,22 @@ scan_function (cgraph_node *node, struct function *fun)
case GIMPLE_CALL:
{
unsigned argument_count = gimple_call_num_args (stmt);
- scan_call_info call_info;
+ isra_scan_context ctx = ISRA_CTX_ARG;
+ scan_call_info call_info, *call_info_p = &call_info;
call_info.cs = node->get_edge (stmt);
- call_info.argument_count = argument_count;
+ if (!call_info.cs)
+ {
+ call_info_p = NULL;
+ ctx = ISRA_CTX_LOAD;
+ }
+ else
+ call_info.argument_count = argument_count;
for (unsigned i = 0; i < argument_count; i++)
{
call_info.arg_idx = i;
scan_expr_access (gimple_call_arg (stmt, i), stmt,
- ISRA_CTX_ARG, bb, &call_info);
+ ctx, bb, call_info_p);
}
tree lhs = gimple_call_lhs (stmt);
[pushed] c++: Fix ICE with implicit operator== [PR94462]
duplicate_decls assumed that any TREE_ARTIFICIAL function at namespace scope
was a built-in function, but now in C++20 it's possible to have an
implicitly declared hidden friend operator==. We just need to move the
assert into the if condition.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/cp/ChangeLog
2020-04-06 Jason Merrill
PR c++/94462
* decl.c (duplicate_decls): Fix handling of DECL_HIDDEN_FRIEND_P.
---
gcc/cp/decl.c | 5 +++--
gcc/testsuite/g++.dg/cpp2a/spaceship-eq9.C | 17 +
2 files changed, 20 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-eq9.C
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 69a238997b4..a127734af69 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1451,9 +1451,10 @@ duplicate_decls (tree newdecl, tree olddecl, bool
newdecl_is_friend)
/* Check for redeclaration and other discrepancies. */
if (TREE_CODE (olddecl) == FUNCTION_DECL
- && DECL_ARTIFICIAL (olddecl))
+ && DECL_ARTIFICIAL (olddecl)
+ /* A C++20 implicit friend operator== uses the normal path (94462). */
+ && !DECL_HIDDEN_FRIEND_P (olddecl))
{
- gcc_assert (!DECL_HIDDEN_FRIEND_P (olddecl));
if (TREE_CODE (newdecl) != FUNCTION_DECL)
{
/* Avoid warnings redeclaring built-ins which have not been
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-eq9.C
b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq9.C
new file mode 100644
index 000..4f5df226410
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq9.C
@@ -0,0 +1,17 @@
+// PR c++/94462
+// { dg-do compile { target c++2a } }
+
+namespace std {
+ struct strong_ordering { };
+}
+
+namespace Synth {
+ struct B {
+friend std::strong_ordering operator<=>(B, B) = default;
+ };
+
+ struct C {
+friend bool operator==(C, C);
+ };
+}
+
base-commit: c72a1b6f8b26de37d1a922a8af143af009747498
--
2.18.1
[PATCH] RS6000: Use .machine ppc for some CRT files
Since commit e154242724b084380e3221df7c08fcdbd8460674 the flag -many is sometimes not passed to the assembler. Use .machine ppc to prevent errors if these files are assembled for an ISA which does not support FPRs. libgcc/ * config/rs6000/crtresfpr.S: Use .machine ppc. * config/rs6000/crtresxfpr.S: Likewise. * config/rs6000/crtsavfpr.S: Likewise. --- libgcc/config/rs6000/crtresfpr.S | 1 + libgcc/config/rs6000/crtresxfpr.S | 1 + libgcc/config/rs6000/crtsavfpr.S | 1 + 3 files changed, 3 insertions(+) diff --git a/libgcc/config/rs6000/crtresfpr.S b/libgcc/config/rs6000/crtresfpr.S index 6c0d01bf9c1..99e6f086026 100644 --- a/libgcc/config/rs6000/crtresfpr.S +++ b/libgcc/config/rs6000/crtresfpr.S @@ -27,6 +27,7 @@ /* Do any initializations needed for the eabi environment */ + .machine ppc .section ".text" #include "ppc-asm.h" diff --git a/libgcc/config/rs6000/crtresxfpr.S b/libgcc/config/rs6000/crtresxfpr.S index 9f01fa5094b..9931db244f7 100644 --- a/libgcc/config/rs6000/crtresxfpr.S +++ b/libgcc/config/rs6000/crtresxfpr.S @@ -27,6 +27,7 @@ /* Do any initializations needed for the eabi environment */ + .machine ppc .section ".text" #include "ppc-asm.h" diff --git a/libgcc/config/rs6000/crtsavfpr.S b/libgcc/config/rs6000/crtsavfpr.S index fa043ddd078..dd8743ae058 100644 --- a/libgcc/config/rs6000/crtsavfpr.S +++ b/libgcc/config/rs6000/crtsavfpr.S @@ -27,6 +27,7 @@ /* Do any initializations needed for the eabi environment */ + .machine ppc .section ".text" #include "ppc-asm.h" -- 2.16.4
Re: [PATCH] i386: Fix emit_reduc_half on V{64Q,32H}Imode [PR94500]
On Tue, Apr 7, 2020 at 12:51 AM Jakub Jelinek wrote:
>
> Hi!
>
> The following testcase is miscompiled in 8.x, because emit_reduc_half is
> prepared to handle for 512-bit modes only i equal to 512, 256, 128 and 64.
> V32HImode also needs i equal to 32 and V64QImode i equal to 32 and 16,
> but emit_reduc_half in that case performs a redundant permutation exactly
> like i == 32. In 9+ the testcase works because Richard in r9-3393
> changed the reduc_* expanders so that they actually don't call
> ix86_expand_reduc on 512-bit modes, but only 128-bit ones.
>
> The patch fixes emit_reduc_half to handle also i of 32 and 16 similarly to
> how V32QImode/V16HImode are handled for AVX2. I think it shouldn't hurt
> to fix the function even on the trunk and 9 branch even when nothing uses
> it ATM.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/9 and
> primarily for 8.5 (obviously in that case s/i386-expand/i386/)?
>
> 2020-04-06 Jakub Jelinek
>
> PR target/94500
> * config/i386/i386-expand.c (emit_reduc_half): For V{64QI,32HI}mode
> handle i < 64 using avx512bw_lshrv4ti3. Formatting fixes.
>
> * gcc.target/i386/avx512bw-pr94500.c: New test.
OK everywhere.
Thanks,
Uros.
> --- gcc/config/i386/i386-expand.c.jj2020-03-29 19:26:31.748561262 +0200
> +++ gcc/config/i386/i386-expand.c 2020-04-06 17:18:44.906242980 +0200
> @@ -14891,43 +14891,51 @@ emit_reduc_half (rtx dest, rtx src, int
>break;
> case E_V64QImode:
> case E_V32HImode:
> + if (i < 64)
> + {
> + d = gen_reg_rtx (V4TImode);
> + tem = gen_avx512bw_lshrv4ti3 (d, gen_lowpart (V4TImode, src),
> + GEN_INT (i / 2));
> + break;
> + }
> + /* FALLTHRU */
> case E_V16SImode:
> case E_V16SFmode:
> case E_V8DImode:
> case E_V8DFmode:
>if (i > 128)
> tem = gen_avx512f_shuf_i32x4_1 (gen_lowpart (V16SImode, dest),
> - gen_lowpart (V16SImode, src),
> - gen_lowpart (V16SImode, src),
> - GEN_INT (0x4 + (i == 512 ? 4 : 0)),
> - GEN_INT (0x5 + (i == 512 ? 4 : 0)),
> - GEN_INT (0x6 + (i == 512 ? 4 : 0)),
> - GEN_INT (0x7 + (i == 512 ? 4 : 0)),
> - GEN_INT (0xC), GEN_INT (0xD),
> - GEN_INT (0xE), GEN_INT (0xF),
> - GEN_INT (0x10), GEN_INT (0x11),
> - GEN_INT (0x12), GEN_INT (0x13),
> - GEN_INT (0x14), GEN_INT (0x15),
> - GEN_INT (0x16), GEN_INT (0x17));
> + gen_lowpart (V16SImode, src),
> + gen_lowpart (V16SImode, src),
> + GEN_INT (0x4 + (i == 512 ? 4 : 0)),
> + GEN_INT (0x5 + (i == 512 ? 4 : 0)),
> + GEN_INT (0x6 + (i == 512 ? 4 : 0)),
> + GEN_INT (0x7 + (i == 512 ? 4 : 0)),
> + GEN_INT (0xC), GEN_INT (0xD),
> + GEN_INT (0xE), GEN_INT (0xF),
> + GEN_INT (0x10), GEN_INT (0x11),
> + GEN_INT (0x12), GEN_INT (0x13),
> + GEN_INT (0x14), GEN_INT (0x15),
> + GEN_INT (0x16), GEN_INT (0x17));
>else
> tem = gen_avx512f_pshufd_1 (gen_lowpart (V16SImode, dest),
> - gen_lowpart (V16SImode, src),
> - GEN_INT (i == 128 ? 0x2 : 0x1),
> - GEN_INT (0x3),
> - GEN_INT (0x3),
> - GEN_INT (0x3),
> - GEN_INT (i == 128 ? 0x6 : 0x5),
> - GEN_INT (0x7),
> - GEN_INT (0x7),
> - GEN_INT (0x7),
> - GEN_INT (i == 128 ? 0xA : 0x9),
> - GEN_INT (0xB),
> - GEN_INT (0xB),
> - GEN_INT (0xB),
> - GEN_INT (i == 128 ? 0xE : 0xD),
> - GEN_INT (0xF),
> - GEN_INT (0xF),
> - GEN_INT (0xF));
> + gen_lowpart (V16SImode, src),
> + GEN_INT (i == 128 ? 0x2 : 0x1),
> + GEN_INT (0x3),
Re: [PATCH] Fix use of singleton in optinfo framework
On Mon, Apr 06, 2020 at 09:42:17PM -0400, David Malcolm via Gcc-patches wrote: > > 2020-04-06 Gustavo Romero > > > > * dumpfile.c: > > (selftest::temp_dump_context::temp_dump_context): Fix ctor. > > --- > > gcc/dumpfile.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c > > index 468ffab..d0d8aa7 100644 > > --- a/gcc/dumpfile.c > > +++ b/gcc/dumpfile.c > > @@ -2076,7 +2076,7 @@ temp_dump_context::temp_dump_context (bool > > forcibly_enable_optinfo, > > bool forcibly_enable_dumping, > > dump_flags_t test_pp_flags) > > : m_context (), > > - m_saved (&dump_context ().get ()) > > + m_saved(&dump_context::get()) > > Whitespace nit: keep the space between m_saved and the open-paren. And between get and open paren too. Jakub
