Re: [PATCH v2] libgcc: Expose the instruction pointer and stack pointer in SEH _Unwind_Backtrace

[Kai Tietz via Gcc-patches]

Hello,

as noone seems to be able to review this patch, I will do so, even if
this is no longer a task of mine.
The patch itself is reasonable and seems to fix a pending issue we
have on CFA support.  I had already private discussion with Martin,
and I would have loved to see a test-case illustrating this fix.  But
as Martin explained to me, there is no trivial testcase for this, so I
would approve that patch.

Regards,
Kai


Am Di., 8. Sept. 2020 um 14:24 Uhr schrieb Martin Storsjö :
>
> Previously, the SEH version of _Unwind_Backtrace did unwind
> the stack and call the provided callback function as intended,
> but there was little the caller could do within the callback to
> actually get any info about that particular level in the unwind.
>
> Set the ra and cfa pointers, which are used by _Unwind_GetIP
> and _Unwind_GetCFA, to allow using these functions from the
> callacb to inspect the state at each stack frame.
>
> 2020-09-08  Martin Storsjö  
>
> libgcc/Changelog:
> * unwind-seh.c (_Unwind_Backtrace): Set the ra and cfa pointers
> before calling the callback.
> ---
>  libgcc/unwind-seh.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c
> index 1a70180cfaa..275d782903a 100644
> --- a/libgcc/unwind-seh.c
> +++ b/libgcc/unwind-seh.c
> @@ -466,6 +466,11 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace,
> _context.disp->HandlerData,
> _context.disp->EstablisherFrame, NULL);
>
> +  /* Set values that the callback can inspect via _Unwind_GetIP
> +   * and _Unwind_GetCFA. */
> +  gcc_context.ra = ms_context.Rip;
> +  gcc_context.cfa = ms_context.Rsp;
> +
>/* Call trace function.  */
>if (trace (_context, trace_argument) != _URC_NO_REASON)
> return _URC_FATAL_PHASE1_ERROR;
> --
> 2.17.1
>

Re: [aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

Hi,

I reworked patch use a tuning flag instead of checking explicit for
CPU flavor. I will send soon an update for it, which won't use the
static variable anymore, and uses instead the SCHED-api.

I would like first to get some comments on current version.

Regards,
Kai
   Jim Wilson 
Kai Tietz 

* config/aarch64/aarch64.c (aarch64_sched_reorder): Implement
TARGET_SCHED_REORDER hook.
(aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
hook.
	(qdf24xx_): Add AARCH64_EXTRA_TUNE_SCHED_MICRO_OPS tune flag.
(TARGET_SCHED_REORDER): Define.
(TARGET_SCHED_VARIABLE_ISSUE): Likewise.
* config/aarch64/falkor.md (falkor_variable_issue): New.
	* onfig/aarch64/aarch64-tuning-flags.def (SCHED_MICRO_OPS): New flag.

Index: trunk/gcc/config/aarch64/aarch64.c
===
--- trunk.orig/gcc/config/aarch64/aarch64.c
+++ trunk/gcc/config/aarch64/aarch64.c
@@ -955,7 +955,7 @@ static const struct tune_params qdf24xx_
   _branch_cost,
   _approx_modes,
   4, /* memmov_cost  */
-  4, /* issue_rate  */
+  8, /* issue_rate  */
   (AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
| AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
   "16",	/* function_align.  */
@@ -968,7 +968,7 @@ static const struct tune_params qdf24xx_
   2,	/* min_div_recip_mul_df.  */
   0,	/* max_case_values.  */
   tune_params::AUTOPREFETCHER_WEAK,	/* autoprefetcher_model.  */
-  AARCH64_EXTRA_TUNE_RENAME_LOAD_REGS, /* tune_flags.  */
+  AARCH64_EXTRA_TUNE_RENAME_LOAD_REGS | AARCH64_EXTRA_TUNE_SCHED_MICRO_OPS, /* tune_flags.  */
   _prefetch_tune
 };
 
@@ -18037,6 +18037,109 @@ aarch64_run_selftests (void)
 
 #endif /* #if CHECKING_P */
 
+/* The number of micro ops left over after issuing the last instruction in a
+   cycle.  This is subtracted from the next cycle before we start issuing insns.
+   This is initialized to 0 at the start of every basic block.  */
+static int leftover_uops = 0;
+
+/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+aarch64_sched_reorder (FILE *file, int verbose,
+		   rtx_insn **ready ATTRIBUTE_UNUSED,
+		   int *n_readyp ATTRIBUTE_UNUSED,
+		   int clock)
+{
+  int can_issue_more = aarch64_sched_issue_rate ();
+
+  if ((aarch64_tune_params.extra_tuning_flags
+   & AARCH64_EXTRA_TUNE_SCHED_MICRO_OPS) != 0)
+{
+  /* The start of a basic block.  */
+  if (clock == 0)
+	{
+	  if (leftover_uops && file && (verbose > 3))
+	fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
+
+	  leftover_uops = 0;
+	}
+
+  /* Account for issue slots left over from previous cycle.  This value
+	 can be larger than the number of issue slots per cycle, so we need
+	 to check it here before scheduling any instructions.  */
+  else if (leftover_uops)
+	{
+	  can_issue_more -= leftover_uops;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
+		   leftover_uops);
+	  fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
+	}
+
+	  leftover_uops = 0;
+
+	  if (can_issue_more < 0)
+	{
+	  leftover_uops = 0 - can_issue_more;
+	  can_issue_more = 0;
+
+	  if (file && (verbose > 3))
+		{
+		  fprintf (file, ";;skipping issue cycle.\n");
+		  fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+		}
+	}
+	}
+}
+
+  return can_issue_more;
+}
+
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+
+static int
+aarch64_variable_issue (FILE *file, int verbose,
+			rtx_insn *insn, int more)
+{
+  if (GET_CODE (PATTERN (insn)) != USE
+  && GET_CODE (PATTERN (insn)) != CLOBBER)
+{
+  if ((aarch64_tune_params.extra_tuning_flags
+	   & AARCH64_EXTRA_TUNE_SCHED_MICRO_OPS) == 0)
+	more -= 1;
+  else
+	{
+  /* There is for now just falkor target supporting scheduling
+ 	 of micro operations. Therefore we don't need to check.  */
+	  int issue_slots = get_attr_falkor_variable_issue (insn);
+	  more -= issue_slots;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tInsn takes %d issue slots.\n", issue_slots);
+	  fprintf (file, ";;\t%d issue slots left.\n", more);
+	}
+
+	  /* We schedule an instruction first, and then subtract issue slots,
+	 which means the result can be negative.  We carry the extra over
+	 to the next cycle.  */
+
+	  if (more < 0)
+	{
+	  leftover_uops = 0 - more;
+	  more = 0;
+
+	  if (file && (verbose > 3))
+		fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+	}
+	}
+}
+
+  return more;
+}
+
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST aarch64_address_cost
 
@@ -18265,6 +18368,12 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_SCHED_ISSUE_RATE
 #define TAR

Re: [patch] [match.pd]: missing optimization on comparison

[Kai Tietz]

Sorry, I attached a patch file with an minor bug.

Kai
Hi,

this patch implements some folding patterns about integral comparison of
condition.  I noticed the lack of folding in gcc by comparing gcc with llvm
result. These optimization are interesting as they are helping to linearize
some conditional expressions.  The following patterns are implemented:

I)	(a < 0) & (b < 0) -> (a & b) < 0
II)	(a >= 0) & (b >= 0) -> (a | b) >= 0
III)	(a < 0) | (b < 0) -> (a | b) < 0
IV)	(a >= 0) | (b >= 0) -> (a & b) >= 0
V)	(a == -1) & (b == -1) -> (a & b) == -1
VI)	(a != -1) | (b != -1) -> (a & b) != -1

Additionally this patch implements some foldings of such new
combined condition, like:
	((a | b) < 0) | (a == 0) -> (a <= 0) | (b < 0)

These break-ups of combined condition  might be good candidates for
tree-ssa-reassoc pass. But for simplicity of this patch, I kept them
in match.pd.

I ran also spec 2017 and 2006 on aarch64 architecture for it. For some
tests I could messure an improvment up to 2 percent. There was no regression.

Regards,
Kai
 
ChangeLog

	Kai Tietz 

	* match.pd: New optimization for integral comparison
	of condition.
	(INTEGRAL_TYPE_P): New helper macro.
	(INTEGRAL_SAME_SIGN_P): Likewise.
	(INTEGRALS_SIGN_PREC_MATCH): Likewise.

ChangeLog testsuite

	* gcc.dg/fold-compare-9.c: New file.
	* gcc.dg/fold-compare-10.c: Likewise.
	* gcc.dg/fold-compare-11.c: Likewise.
	* gcc.dg/fold-compare-12.c: Likewise.
	* gcc.dg/fold-compare-13.c: Likewise.
	* gcc.dg/fold-compare-14.c: Likewise.

Index: trunk/gcc/match.pd
===
--- trunk.orig/gcc/match.pd
+++ trunk/gcc/match.pd
@@ -679,11 +679,124 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (if (tem)
  (rdiv { tem; } @1)
 
+#define INTEGRAL_TYPES_P(A, B) \
+  INTEGRAL_TYPE_P (TREE_TYPE (A)) && INTEGRAL_TYPE_P (TREE_TYPE (B))
+#define INTEGRAL_SAME_SIGN_P(A, B) \
+  TYPE_UNSIGNED (TREE_TYPE (A)) == TYPE_UNSIGNED (TREE_TYPE (B))
+#define INTEGRALS_SIGN_PREC_MATCH(A, B) \
+  (TYPE_PRECISION (TREE_TYPE (A)) == TYPE_PRECISION (TREE_TYPE (B)) \
+   || (TYPE_PRECISION (TREE_TYPE (A)) > TYPE_PRECISION (TREE_TYPE (B)) \
+   && !TYPE_UNSIGNED (TREE_TYPE (B
+
 /* Simplify ~X & X as zero.  */
 (simplify
  (bit_and:c (convert? @0) (convert? (bit_not @0)))
   { build_zero_cst (type); })
 
+/* These operation need to be processed without side-effect.  For diagnostic
+   messages these operation have not to be performed on AST.  */
+#if GIMPLE
+/* Combine (a == -1) & (b == -1) into (a & b) == -1 */
+(simplify (bit_and:c (eq @0 integer_minus_onep@2) (eq @1 integer_minus_onep))
+  (if (INTEGRAL_TYPES_P (@0, @1)
+   && INTEGRALS_SIGN_PREC_MATCH (@0, @1))
+(eq (bit_and @0 (convert @1)) @2)))
+/* Combine (a != -1) | (b != -1) into (a & b) != -1 */
+(simplify (bit_ior:c (ne @0 integer_minus_onep@2) (ne @1 integer_minus_onep))
+  (if (INTEGRAL_TYPES_P (@0, @1)
+   && INTEGRALS_SIGN_PREC_MATCH (@0, @1))
+(ne (bit_and @0 (convert @1)) @2)))
+/* Sink redundant compare into (a & b) == -1 for */
+/*  ... & (a <= cst) -or- ... & (a >= cst) */
+(for cmp (le ge lt gt)
+ (simplify
+  (bit_and:c (eq (bit_and:c @0 @1) integer_minus_onep@2) (cmp @0 INTEGER_CST@4))
+  (if (INTEGRAL_TYPES_P (@0, @1)
+   && !TYPE_UNSIGNED (TREE_TYPE (@0))
+   && tree_fits_shwi_p (@4))
+(with { HOST_WIDE_INT cst4 = tree_to_shwi (@4); }
+  (if ((cst4 >= -1 && cmp == LE_EXPR)
+   || (cst4 < 0 && cmp == GE_EXPR)
+   || (cst4 > -1 && cmp == LT_EXPR)
+   || (cst4 < -1 && cmp == GT_EXPR))
+(eq (bit_and @0 @1) @2)))
+(with { HOST_WIDE_INT cst4 = tree_to_shwi (@4); }
+/*   for cst < -1 into false. */
+  (if ((cst4 < -1 && cmp == LE_EXPR) || (cst4 > -1 && cmp == GE_EXPR))
+{ constant_boolean_node (false, type); })
+/* ... & (a == cst) -or- ... & (a != cst). */
+(for cmp (eq ne)
+ (simplify
+  (bit_and:c (eq (bit_and:c @0 @1) integer_minus_onep@2) (cmp @0 INTEGER_CST@4))
+  (if (INTEGRAL_TYPES_P (@0, @1)
+   && !TYPE_UNSIGNED (TREE_TYPE (@0))
+   && tree_fits_shwi_p (@4))
+(with { HOST_WIDE_INT cst4 = tree_to_shwi (@4); }
+  (if ((cst4 == -1) == (cmp == EQ_EXPR))
+(eq (bit_and @0 @1) @2)))
+(with { HOST_WIDE_INT cst4 = tree_to_shwi (@4); }
+  (if ((cst4 != -1) == (cmp == EQ_EXPR))
+{ constant_boolean_node (false, type); })
+
+/* Combine (a < 0) | (b < 0) into (a | b) < 0 */
+(simplify
+ (bit_ior:c (lt @0 integer_zerop@2) (lt @1 integer_zerop))
+ (if (INTEGRAL_TYPES_P (@0, @1)
+  && INTEGRAL_SAME_SIGN_P (@0, @1)
+  && TYPE_PRECISION (TREE_TYPE (@0)) >= TYPE_PRECISION (TREE_TYPE (@1)))
+  (lt (bit_ior @0 (convert @1)) @

[patch] [match.pd]: missing optimization on comparison

[Kai Tietz]

i,

this patch implements some folding patterns about integral comparison of
condition.  I noticed the lack of folding in gcc by comparing gcc with llvm
result. These optimization are interesting as they are helping to linearize
some conditional expressions.  The following patterns are implemented:

I)  (a < 0) & (b < 0) -> (a & b) < 0
II) (a >= 0) & (b >= 0) -> (a | b) >= 0
III)(a < 0) | (b < 0) -> (a | b) < 0
IV) (a >= 0) | (b >= 0) -> (a & b) >= 0
V)  (a == -1) & (b == -1) -> (a & b) == -1
VI) (a != -1) | (b != -1) -> (a & b) != -1

Additionally this patch implements some foldings of such new
combined condition, like:
((a | b) < 0) | (a == 0) -> (a <= 0) | (b < 0)

These break-ups of combined condition  might be good candidates for
tree-ssa-reassoc pass. But for simplicity of this patch, I kept them
in match.pd.

I ran also spec 2017 and 2006 on aarch64 architecture for it. For some
tests I could messure an improvment up to 2 percent. There was no regression.

ChangeLog

Kai Tietz 

* match.pd: New optimization for integral comparison
of condition.
(INTEGRAL_TYPE_P): New helper macro.
(INTEGRAL_SAME_SIGN_P): Likewise.
(INTEGRALS_SIGN_PREC_MATCH): Likewise.

ChangeLog testsuite

* gcc.dg/fold-compare-9.c: New file.
* gcc.dg/fold-compare-10.c: Likewise.
* gcc.dg/fold-compare-11.c: Likewise.
* gcc.dg/fold-compare-12.c: Likewise.
* gcc.dg/fold-compare-13.c: Likewise.
* gcc.dg/fold-compare-14.c: Likewise.

Regards,
Kai
Hi,

this patch implements some folding patterns about integral comparison of
condition.  I noticed the lack of folding in gcc by comparing gcc with llvm
result. These optimization are interesting as they are helping to linearize
some conditional expressions.  The following patterns are implemented:

I)	(a < 0) & (b < 0) -> (a & b) < 0
II)	(a >= 0) & (b >= 0) -> (a | b) >= 0
III)	(a < 0) | (b < 0) -> (a | b) < 0
IV)	(a >= 0) | (b >= 0) -> (a & b) >= 0
V)	(a == -1) & (b == -1) -> (a & b) == -1
VI)	(a != -1) | (b != -1) -> (a & b) != -1

Additionally this patch implements some foldings of such new
combined condition, like:
	((a | b) < 0) | (a == 0) -> (a <= 0) | (b < 0)

These break-ups of combined condition  might be good candidates for
tree-ssa-reassoc pass. But for simplicity of this patch, I kept them
in match.pd.

I ran also spec 2017 and 2006 on aarch64 architecture for it. For some
tests I could messure an improvment up to 2 percent. There was no regression.

Regards,
Kai
 
ChangeLog

	Kai Tietz 

	* match.pd: New optimization for integral comparison
	of condition.
	(INTEGRAL_TYPE_P): New helper macro.
	(INTEGRAL_SAME_SIGN_P): Likewise.
	(INTEGRALS_SIGN_PREC_MATCH): Likewise.

ChangeLog testsuite

	* gcc.dg/fold-compare-9.c: New file.
	* gcc.dg/fold-compare-10.c: Likewise.
	* gcc.dg/fold-compare-11.c: Likewise.
	* gcc.dg/fold-compare-12.c: Likewise.
	* gcc.dg/fold-compare-13.c: Likewise.
	* gcc.dg/fold-compare-14.c: Likewise.

Index: trunk/gcc/match.pd
===
--- trunk.orig/gcc/match.pd
+++ trunk/gcc/match.pd
@@ -679,11 +679,124 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (if (tem)
  (rdiv { tem; } @1)
 
+#define INTEGRAL_TYPES_P(A, B) \
+  INTEGRAL_TYPE_P (TREE_TYPE (A)) && INTEGRAL_TYPE_P (TREE_TYPE (B))
+#define INTEGRAL_SAME_SIGN_P(A, B) \
+  TYPE_UNSIGNED (TREE_TYPE (A)) == TYPE_UNSIGNED (TREE_TYPE (B))
+#define INTEGRALS_SIGN_PREC_MATCH(A, B) \
+  TYPE_PRECISION (TREE_TYPE (A)) == TYPE_PRECISION (TREE_TYPE (B)) \
+  (TYPE_PRECISION (TREE_TYPE (A)) > TYPE_PRECISION (TREE_TYPE (B)) \
+   && !TYPE_UNSIGNED (TREE_TYPE (B)))
+
 /* Simplify ~X & X as zero.  */
 (simplify
  (bit_and:c (convert? @0) (convert? (bit_not @0)))
   { build_zero_cst (type); })
 
+/* These operation need to be processed without side-effect.  For diagnostic
+   messages these operation have not to be performed on AST.  */
+#if GIMPLE
+/* Combine (a == -1) & (b == -1) into (a & b) == -1 */
+(simplify (bit_and:c (eq @0 integer_minus_onep@2) (eq @1 integer_minus_onep))
+  (if (INTEGRAL_TYPES_P (@0, @1)
+   && INTEGRALS_SIGN_PREC_MATCH (@0, @1))
+(eq (bit_and @0 (convert @1)) @2)))
+/* Combine (a != -1) | (b != -1) into (a & b) != -1 */
+(simplify (bit_ior:c (ne @0 integer_minus_onep@2) (ne @1 integer_minus_onep))
+  (if (INTEGRAL_TYPES_P (@0, @1)
+   && INTEGRALS_SIGN_PREC_MATCH (@0, @1))
+(ne (bit_and @0 (convert @1)) @2)))
+/* Sink redundant compare into (a & b) == -1 for */
+/*  ... & (a <= cst) -or- ... & (a >= cst) */
+(for cmp (le ge lt gt)
+ (simplify
+  (bit_and:c (eq (bit_and:c @0 @1) integer_minus_onep@2) (cmp @0 INTEGER_CST@4))
+  (if (INTEGRAL

Re: [aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

Hello Kyrill,

thanks for your comments.

2018-08-14 16:50 GMT+02:00 Kyrill  Tkachov :
> Hi Kai,
>
>
> On 13/08/18 17:48, Kai Tietz wrote:
>>
>> I repost updated patch containing ChangeLog entry.
>>
>> Regards,
>> Kai
>
>
> I think I understand what this patch does, please correct me if I'm wrong.
> You model the processors micro-ops and some A64 instructions use multiple
> micro-ops.
> This is what the falkor_variable_issue attribute specifies.
> In TARGET_SCHED_VARIABLE_ISSUE you count the issue slots that the micro-ops
> take and how much "space" is left over, which is stored in leftover_uops
> and you use leftover_uops in TARGET_SCHED_REORDER to tell the scheduler how
> many more micro-ops it can issue on that cycle.

Correct.

> And with that change the issue_rate is no longer the *instruction* issue
> rate, but rather the *micro-op* issue rate.
> Overall this looks very similar to the implementation of this functionality
> in the powerpc port (rs6000).
> Is this correct?

Yes, it is somewhat similar to the rs6000 variant.

> I have a few comments on the implementation inline...
>
> Jim Wilson
> Kai Tietz
>
> * config/aarch64.c (aarch64_sched_reorder): Implement
> TARGET_SCHED_REORDER hook.
> (aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
> hook.
> (TARGET_SCHED_REORDER): Define.
> (TARGET_SCHED_VARIABLE_ISSUE): Likewise.
> * config/aarch64/falkor.md (falkor_variable_issue): New.
>
> Index: aarch64/aarch64.c
> ===
> --- aarch64.orig/aarch64.c
> +++ aarch64/aarch64.c
> @@ -914,7 +914,7 @@ static const struct tune_params qdf24xx_
>_branch_cost,
>_approx_modes,
>4, /* memmov_cost  */
> -  4, /* issue_rate  */
> +  8, /* issue_rate  */
>(AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
> | AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
>"16",/* function_align.  */
> @@ -17551,6 +17551,105 @@ aarch64_run_selftests (void)
>   #endif /* #if CHECKING_P */
>  +/* The number of micro ops left over after issuing the last instruction in
> a
> +   cycle.  This is subtracted from the next cycle before we start issuing
> insns.
> +   This is initialized to 0 at the start of every basic block.  */
> +static int leftover_uops = 0;
> +
>
> I believe the scheduler provides hooks specifically for storing
> backend-specific scheduling state so we should
> avoid creating such static variables in aarch64.c. Can you use the
> TARGET_SCHED_*_SCHED_CONTEXT family of hooks here?
> Then it will be up to the scheduler midend to keep track of the state and
> between basic blocks, functions etc.

I think you refer to the ppc implementation. But if you take a closer
look to it, you will see that nevertheless such an implementation will
require global variables.
So I am not really sure it is worth to introduce the ..._SCHED_CONTEXT
API to avoid one global variable by introducing at least two others.
Neverthelss I am admit that making use of SCHED_CONTEXT could be a
general nice to have, but not necessarily a gain in that case.

>  +/* Implement TARGET_SCHED_REORDER.  */
> +
> +static int
> +aarch64_sched_reorder (FILE *file, int verbose,
> +  rtx_insn **ready ATTRIBUTE_UNUSED,
> +  int *n_readyp ATTRIBUTE_UNUSED,
> +  int clock)
> +{
> +  int can_issue_more = aarch64_sched_issue_rate ();
> +
> +  if ((enum attr_tune) aarch64_tune == TUNE_FALKOR)
> +{
> +  /* The start of a basic block.  */
> +  if (clock == 0)
> +   {
> + if (leftover_uops && file && (verbose > 3))
> +   fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
> +
> + leftover_uops = 0;
> +   }
> +
> +  /* Account for issue slots left over from previous cycle.  This value
> +can be larger than the number of issue slots per cycle, so we need
> +to check it here before scheduling any instructions.  */
> +  else if (leftover_uops)
> +   {
> + can_issue_more -= leftover_uops;
> +
> + if (file && (verbose > 3))
> +   {
> + fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
> +  leftover_uops);
> + fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
> +   }
> +
> + leftover_uops = 0;
> +
> + if (can_issue_more < 0)
> +   {
> + leftover_uops = 0 - can_issue_more;
> + can_issue_more = 0;
> +
> +   

Re: [aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

I repost updated patch containing ChangeLog entry.

Regards,
Kai
	Jim Wilson 
	Kai Tietz 

	* config/aarch64.c (aarch64_sched_reorder): Implement
	TARGET_SCHED_REORDER hook.
	(aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
	hook.
	(TARGET_SCHED_REORDER): Define.
	(TARGET_SCHED_VARIABLE_ISSUE): Likewise.
	* config/aarch64/falkor.md (falkor_variable_issue): New.

Index: aarch64/aarch64.c
===
--- aarch64.orig/aarch64.c
+++ aarch64/aarch64.c
@@ -914,7 +914,7 @@ static const struct tune_params qdf24xx_
   _branch_cost,
   _approx_modes,
   4, /* memmov_cost  */
-  4, /* issue_rate  */
+  8, /* issue_rate  */
   (AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
| AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
   "16",	/* function_align.  */
@@ -17551,6 +17551,105 @@ aarch64_run_selftests (void)
 
 #endif /* #if CHECKING_P */
 
+/* The number of micro ops left over after issuing the last instruction in a
+   cycle.  This is subtracted from the next cycle before we start issuing insns.
+   This is initialized to 0 at the start of every basic block.  */
+static int leftover_uops = 0;
+
+/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+aarch64_sched_reorder (FILE *file, int verbose,
+		   rtx_insn **ready ATTRIBUTE_UNUSED,
+		   int *n_readyp ATTRIBUTE_UNUSED,
+		   int clock)
+{
+  int can_issue_more = aarch64_sched_issue_rate ();
+
+  if ((enum attr_tune) aarch64_tune == TUNE_FALKOR)
+{
+  /* The start of a basic block.  */
+  if (clock == 0)
+	{
+	  if (leftover_uops && file && (verbose > 3))
+	fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
+
+	  leftover_uops = 0;
+	}
+
+  /* Account for issue slots left over from previous cycle.  This value
+	 can be larger than the number of issue slots per cycle, so we need
+	 to check it here before scheduling any instructions.  */
+  else if (leftover_uops)
+	{
+	  can_issue_more -= leftover_uops;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
+		   leftover_uops);
+	  fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
+	}
+
+	  leftover_uops = 0;
+
+	  if (can_issue_more < 0)
+	{
+	  leftover_uops = 0 - can_issue_more;
+	  can_issue_more = 0;
+
+	  if (file && (verbose > 3))
+		{
+		  fprintf (file, ";;skipping issue cycle.\n");
+		  fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+		}
+	}
+	}
+}
+
+  return can_issue_more;
+}
+
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+
+static int
+aarch64_variable_issue (FILE *file, int verbose,
+			rtx_insn *insn, int more)
+{
+  if (GET_CODE (PATTERN (insn)) != USE
+  && GET_CODE (PATTERN (insn)) != CLOBBER)
+{
+  if ((enum attr_tune) aarch64_tune != TUNE_FALKOR)
+	more -= 1;
+  else
+	{
+	  int issue_slots = get_attr_falkor_variable_issue (insn);
+	  more -= issue_slots;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tInsn takes %d issue slots.\n", issue_slots);
+	  fprintf (file, ";;\t%d issue slots left.\n", more);
+	}
+
+	  /* We schedule an instruction first, and then subtract issue slots,
+	 which means the result can be negative.  We carry the extra over
+	 to the next cycle.  */
+
+	  if (more < 0)
+	{
+	  leftover_uops = 0 - more;
+	  more = 0;
+
+	  if (file && (verbose > 3))
+		fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+	}
+	}
+}
+
+  return more;
+}
+
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST aarch64_address_cost
 
@@ -17779,6 +17878,12 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_SCHED_ISSUE_RATE
 #define TARGET_SCHED_ISSUE_RATE aarch64_sched_issue_rate
 
+#undef TARGET_SCHED_REORDER
+#define TARGET_SCHED_REORDER aarch64_sched_reorder
+
+#undef TARGET_SCHED_VARIABLE_ISSUE
+#define TARGET_SCHED_VARIABLE_ISSUE aarch64_variable_issue
+
 #undef TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD
 #define TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD \
   aarch64_sched_first_cycle_multipass_dfa_lookahead
Index: aarch64/falkor.md
===
--- aarch64.orig/falkor.md
+++ aarch64/falkor.md
@@ -685,3 +685,24 @@
 (define_bypass 3
   "falkor_afp_5_vxvy_mul,falkor_afp_5_vxvy_mla,falkor_afp_5_vxvy_vxvy_mul,falkor_afp_5_vxvy_vxvy_mla,falkor_afp_6_vxvy_mul,falkor_afp_6_vxvy_mla,falkor_afp_6_vxvy_vxvy_mul,falkor_afp_6_vxvy_vxvy_mla,falkor_fpdt_5_vxvy_mul,falkor_fpdt_5_vxvy_mla,falkor_fpdt_6_vxvy_mul,falkor_fpdt_6_vxvy_mla"
   "falkor_afp_5_vxvy_mla,falkor_afp_5_vxvy_vxvy_mla,falkor_afp_6_vxvy_mla,falkor_afp_6_vxvy_vxvy_mla,falkor_fpdt_5_vxvy_mla,falkor_fpdt_6_vxvy_mla")
+
+
+(define_attr "falkor_

Re: [aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

2018-08-13 18:23 GMT+02:00 Segher Boessenkool :
> I cannot okay patches for aarch64.  But I did not notice other typoes,
> if that is what you're asking.

Yes. Thanks.

So, is patch ok with updated ChangeLog:

Jim Wilson 
        Kai Tietz 

* config/aarch64.c (aarch64_sched_reorder): Implement
TARGET_SCHED_REORDER hook.
(aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
hook.
(TARGET_SCHED_REORDER): Define.
(TARGET_SCHED_VARIABLE_ISSUE): Likewise.
* config/aarch64/falkor.md (falkor_variable_issue): New.

Kai

Re: [aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

2018-08-13 17:51 GMT+02:00 Segher Boessenkool :
> Hi!
>
> On Mon, Aug 13, 2018 at 10:16:20AM +0200, Kai Tietz wrote:
>> * config/aarch64.c (aarch64_sched_reorder): Implementing
>> TARGET_SHED_REORDER hook.
>> (aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
>> hook.
>> (TARGET_SHED_REORDER): Defined.
>> (TARGET_SHED_VARIABLE_ISSUE): Likewise.
>> * config/aarch64/falor.md (falkor_variable_issue): New.
>
> SCHED, not SHED :-)  And s/falor/falkor/ .
>
>
> Segher

:) Thanks, otherwise ok?

Kai

Re: [aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

2018-08-13 15:43 GMT+02:00 Bernhard Reutner-Fischer :
> On 13 August 2018 15:12:30 CEST, Bernhard Reutner-Fischer 
>  wrote:
>>On 13 August 2018 10:16:20 CEST, Kai Tietz 
>>wrote:
>>>Hello,
>>>
>>>this patch implements variable issue rate feature for falkor cpu.
>>>Additionally this patch adjusts the issue rate for falkor 8 as this
>>>value reflects more cpu's specification.
>>>
>>>This patch was tested against SPEC 2017 & 2016 and showed in general
>>>some improvements without any regressions for thos tests.
>>>
>>>ChangeLog:
>>>
>>>Jim Wilson 
>>>Kai Tietz 
>>>
>>>* config/aarch64.c (aarch64_sched_reorder): Implementing
>>>TARGET_SHED_REORDER hook.
>>
>>Present tense in ChangeLog please.
>>
>>>   (aarch64_variable_issue): Implemented
>>TARGET_SHED_VARIABLE_ISSUE
>>>hook.
>>>(TARGET_SHED_REORDER): Defined.
>>>(TARGET_SHED_VARIABLE_ISSUE): Likewise.
>>>* config/aarch64/falor.md (falkor_variable_issue): New.
>>>
>>>Ok for apply?
>>
>>s/subtrated/subtracted/
>>
>>
>>>
>>>Regards,
>>>Kai
>>>
>>>PS: I might be in need to update my key-files for stronger bitness.
>>>Whom I can ask for doing this?
>>
>>If you still can login:
>>https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02105.html
>
> And if it's "just" you had a DSA key then add a new one with temporarily 
> accepting the old one:
> ssh -oPubkeyAcceptedKeyTypes=+ssh-dss 
>
>>HTH,
>

Thanks for you help. I corrected changelog text accordingly. Sadly I
don't have the old key anymore. So I would need an overseer to replace
my old key with an new one.

Kai

[aarch64}: added variable issue rate feature for falkor

[Kai Tietz]

Hello,

this patch implements variable issue rate feature for falkor cpu.
Additionally this patch adjusts the issue rate for falkor 8 as this
value reflects more cpu's specification.

This patch was tested against SPEC 2017 & 2016 and showed in general
some improvements without any regressions for thos tests.

ChangeLog:

Jim Wilson 
    Kai Tietz 

* config/aarch64.c (aarch64_sched_reorder): Implementing
TARGET_SHED_REORDER hook.
(aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
hook.
(TARGET_SHED_REORDER): Defined.
(TARGET_SHED_VARIABLE_ISSUE): Likewise.
* config/aarch64/falor.md (falkor_variable_issue): New.

Ok for apply?

Regards,
Kai

PS: I might be in need to update my key-files for stronger bitness.
Whom I can ask for doing this?
	Jim Wilson 
	Kai Tietz 

	* config/aarch64.c (aarch64_sched_reorder): Implementing
	TARGET_SHED_REORDER hook.
	(aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
	hook.
	(TARGET_SHED_REORDER): Defined.
	(TARGET_SHED_VARIABLE_ISSUE): Likewise.
	* config/aarch64/falor.md (falkor_variable_issue): New.

Index: aarch64/aarch64.c
===
--- aarch64.orig/aarch64.c
+++ aarch64/aarch64.c
@@ -914,7 +914,7 @@ static const struct tune_params qdf24xx_
   _branch_cost,
   _approx_modes,
   4, /* memmov_cost  */
-  4, /* issue_rate  */
+  8, /* issue_rate  */
   (AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
| AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
   "16",	/* function_align.  */
@@ -17551,6 +17551,105 @@ aarch64_run_selftests (void)
 
 #endif /* #if CHECKING_P */
 
+/* The number of micro ops left over after issuing the last instruction in a
+   cycle.  This is subtrated from the next cycle before we start issuing insns.
+   This is initialized to 0 at the start of every basic block.  */
+static int leftover_uops = 0;
+
+/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+aarch64_sched_reorder (FILE *file, int verbose,
+		   rtx_insn **ready ATTRIBUTE_UNUSED,
+		   int *n_readyp ATTRIBUTE_UNUSED,
+		   int clock)
+{
+  int can_issue_more = aarch64_sched_issue_rate ();
+
+  if ((enum attr_tune) aarch64_tune == TUNE_FALKOR)
+{
+  /* The start of a basic block.  */
+  if (clock == 0)
+	{
+	  if (leftover_uops && file && (verbose > 3))
+	fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
+
+	  leftover_uops = 0;
+	}
+
+  /* Account for issue slots left over from previous cycle.  This value
+	 can be larger than the number of issue slots per cycle, so we need
+	 to check it here before scheduling any instructions.  */
+  else if (leftover_uops)
+	{
+	  can_issue_more -= leftover_uops;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
+		   leftover_uops);
+	  fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
+	}
+
+	  leftover_uops = 0;
+
+	  if (can_issue_more < 0)
+	{
+	  leftover_uops = 0 - can_issue_more;
+	  can_issue_more = 0;
+
+	  if (file && (verbose > 3))
+		{
+		  fprintf (file, ";;skipping issue cycle.\n");
+		  fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+		}
+	}
+	}
+}
+
+  return can_issue_more;
+}
+
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+
+static int
+aarch64_variable_issue (FILE *file, int verbose,
+			rtx_insn *insn, int more)
+{
+  if (GET_CODE (PATTERN (insn)) != USE
+  && GET_CODE (PATTERN (insn)) != CLOBBER)
+{
+  if ((enum attr_tune) aarch64_tune != TUNE_FALKOR)
+	more -= 1;
+  else
+	{
+	  int issue_slots = get_attr_falkor_variable_issue (insn);
+	  more -= issue_slots;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tInsn takes %d issue slots.\n", issue_slots);
+	  fprintf (file, ";;\t%d issue slots left.\n", more);
+	}
+
+	  /* We schedule an instruction first, and then subtract issue slots,
+	 which means the result can be negative.  We carry the extra over
+	 to the next cycle.  */
+
+	  if (more < 0)
+	{
+	  leftover_uops = 0 - more;
+	  more = 0;
+
+	  if (file && (verbose > 3))
+		fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+	}
+	}
+}
+
+  return more;
+}
+
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST aarch64_address_cost
 
@@ -17779,6 +17878,12 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_SCHED_ISSUE_RATE
 #define TARGET_SCHED_ISSUE_RATE aarch64_sched_issue_rate
 
+#undef TARGET_SCHED_REORDER
+#define TARGET_SCHED_REORDER aarch64_sched_reorder
+
+#undef TARGET_SCHED_VARIABLE_ISSUE
+#define TARGET_SCHED_VARIABLE_ISSUE aarch64_variable_issue
+
 #undef TARGET_SCHED_FI

Re: [PATCH] Fix ms_struct/-mms-bitfields structure layout (PR target/52991)

[Kai Tietz]

Hello Jakub,

I can't approve this patch, but I can confirm that changes are
sensible.  I verified the structure layout - as far as possible - also
with MS' compiler, and can confirm the adjustments.

Cheers,
Ka

2018-02-28 1:26 GMT+01:00 Jakub Jelinek :
> Hi!
>
> The following patch fixes the reported ms_struct/-mms-bitfields structure
> layout issues from PR52991.
>
> There are multiple issues, two of them introduced by the
> https://gcc.gnu.org/ml/gcc-patches/2006-04/msg01064.html -mms-bitfields
> revamp from Eric and follow-up fix r114552, the rest has been introduced
> later when the known_align < desired_align case has been enabled for the ms
> bitfield layout.
>
> The first 2 hunks fix alignment of packed non-bitfield fields, we can't
> ignore all the alignment updates for them, just should use only
> desired_align which takes DECL_PACKED into account, rather than
> MAX (type_align, desired_align).  Similarly, the last hunk in stor-layout.c
> makes sure that for DECL_PACKED fields we use BITS_PER_UNIT alignment rather
> than the type alignment.
>
> The rest attempts to unbreak r184409 which enabled known_align < desired_align
> case; doing that if rli->prev_field and ms layout is wrong, we first need to
> deal with the bitfield packing and if we are within a bitfield word, we
> shouldn't do any realignment, only in between them.
>
> The patch reverts changes to bf-ms-layout{,-2}.c tests done in 2012, which
> were done just to match the r184409 changes, and adds 2 new tests.  All of
> these 4 I've tested (slightly tweaked, so that it compiles with VC) with
> the online VC compiler http://rextester.com/l/c_online_compiler_visual .
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-02-27  Jakub Jelinek  
>
> PR target/52991
> * stor-layout.c (update_alignment_for_field): For
> targetm.ms_bitfield_layout_p (rli->t), if !is_bitfield
> && !DECL_PACKED (field), do the alignment update, just use
> only desired_align instead of MAX (type_align, desired_align)
> as the alignment.
> (place_field): Don't do known_align < desired_align handling
> early if targetm.ms_bitfield_layout_p (rli->t) and rli->prev_field
> is non-NULL, instead do it after rli->prev_field handling and
> only if not within a bitfield word.  For DECL_PACKED (field)
> use type_align of BITS_PER_UNIT.
>
> * gcc.dg/bf-ms-layout.c: Revert 2012-04-26 changes.
> * gcc.dg/bf-ms-layout-2.c: Revert 2012-02-23 changes.
> * gcc.dg/bf-ms-layout-4.c: New test.
> * gcc.dg/bf-ms-layout-5.c: New test.
>
> --- gcc/stor-layout.c.jj2018-02-22 14:35:33.135216198 +0100
> +++ gcc/stor-layout.c   2018-02-27 18:56:26.906494801 +0100
> @@ -1038,7 +1038,7 @@ update_alignment_for_field (record_layou
>  the type, except that for zero-size bitfields this only
>  applies if there was an immediately prior, nonzero-size
>  bitfield.  (That's the way it is, experimentally.) */
> -  if ((!is_bitfield && !DECL_PACKED (field))
> +  if (!is_bitfield
>   || ((DECL_SIZE (field) == NULL_TREE
>|| !integer_zerop (DECL_SIZE (field)))
>   ? !DECL_PACKED (field)
> @@ -1047,7 +1047,10 @@ update_alignment_for_field (record_layou
>  && ! integer_zerop (DECL_SIZE (rli->prev_field)
> {
>   unsigned int type_align = TYPE_ALIGN (type);
> - type_align = MAX (type_align, desired_align);
> + if (!is_bitfield && DECL_PACKED (field))
> +   type_align = desired_align;
> + else
> +   type_align = MAX (type_align, desired_align);
>   if (maximum_field_alignment != 0)
> type_align = MIN (type_align, maximum_field_alignment);
>   rli->record_align = MAX (rli->record_align, type_align);
> @@ -1303,7 +1306,9 @@ place_field (record_layout_info rli, tre
>
>/* Does this field automatically have alignment it needs by virtue
>   of the fields that precede it and the record's own alignment?  */
> -  if (known_align < desired_align)
> +  if (known_align < desired_align
> +  && (! targetm.ms_bitfield_layout_p (rli->t)
> + || rli->prev_field == NULL))
>  {
>/* No, we need to skip space before this field.
>  Bump the cumulative size to multiple of field alignment.  */
> @@ -1331,8 +1336,6 @@ place_field (record_layout_info rli, tre
>
>if (! TREE_CONSTANT (rli->offset))
> rli->offset_align = desired_align;
> -  if (targetm.ms_bitfield_layout_p (rli->t))
> -   rli->prev_field = NULL;
>  }
>
>/* Handle compatibility with PCC.  Note that if the record has any
> @@ -1448,6 +1451,8 @@ place_field (record_layout_info rli, tre
>/* This is a bitfield if it exists.  */
>if (rli->prev_field)
> {
> + bool realign_p = known_align < desired_align;
> +
>   

Re: [PATCH v4 0/12] [i386] Improve 64-bit Microsoft to System V ABI pro/epilogues

[Kai Tietz]

2017-05-02 12:21 GMT+02:00 JonY <10wa...@gmail.com>:
> On 05/01/2017 11:31 AM, Uros Bizjak wrote:
>> On Thu, Apr 27, 2017 at 10:04 AM, Daniel Santos  
>> wrote:
>>> All of patches are concerned with 64-bit Microsoft ABI functions that call
>>> System V ABI function which clobbers RSI, RDI and XMM6-15 and are aimed at
>>> improving performance and .text size of Wine 64. I had previously submitted
>>> these as separate patch sets, but have combined them for simplicity. (Does
>>> this make the ChangeLogs too big? Please let me know if you want me to break
>>> these back apart.) Below are the included patchsets and a summary of changes
>>> since the previous post(s):
>>
>> Well, the ChangeLog is acceptable.
>>
>> I have comments on how new RTX patterns are generated and checked
>> (patches 9/12 and 11/12). Other patches look good to me, so after
>> issues with 9/12 and 11/12 are resolved, I think the patch set is
>> ready to go.
>>
>> After the above issue is addressed, I propose to move forward by
>> committing the patchset, and resolve any possible issues later. There
>> are just too many code paths in the stack frame construction and
>> teardown to notice all possible interactions between new and old code.
>> It looks that existing code won't be affected without activating new
>> option, so we can be a bit less cautious with the patchset. An
>> important part is thus a comprehensive added test suite, which seems
>> to pass.
>>
>> I also assume that Cygwin and MinGW people agree with the patch and
>> the functionality itself.
>>
>> Uros.
>>
>
> Cygwin and MinGW does not use SysV/MS transitions directly in their own
> code, changes should be OK.
>
>
>

Right, and Wine people will tell, if something doesn't work for them.
So ok for me too.

Kai

Re: fix crash on 64-bit mingw-w64 hosted compiler using more than 4 gb of ram

[Kai Tietz]

Hello Stanislaw.

patch is ok.  Nevertheless there is a ChangeLog entry missing for it.
It is mandatory to be provided for submissions to gcc.

Thanks,
Kai

2016-08-11 11:31 GMT+02:00 Stanislaw Halik :
> The host configuration across platforms wrongly assumes that
> sizeof(long) == sizeof(intptr_t) which is incorrect on amd64-hosted compiler
> hosting mingw-w64.
>
> Here's a patch fixing
> 
>
> cheers,
> sh

Re: [patch] Allow configuration with --disable-sjlj-exceptions on biarch x86 Windows targets

[Kai Tietz]

Ok.  I just looked into patch.  Sorry for the delay.

As it is still possible to build old behavior, the patch is ok for me.

Thanks,
Kai


2016-05-19 20:55 GMT+02:00 Sandra Loosemore <san...@codesourcery.com>:
> On 05/19/2016 12:40 PM, Kai Tietz wrote:
>>
>> Hi,
>>
>> hopefully this time gmail uses  mail-encoding elmz ask for ...
>>
>> Sorry to object here.  I would like to point out that defaulting to
>> dw2 on 32-bit if SEH is used for 64-bit is nothing good in general.
>> This is reasoned by the problems existing in dw2 in combination with
>> other compiler-generated binaries.
>
>
> The patch does not change the default behavior for any target.  It only
> makes configuring with --disable-sjlj-exceptions work in a situation where
> it previously triggered a build error.
>
> The reason why I need this is precisely because of a compatibility issue --
> I have an existing 32-bit library supplied by a third party that uses DW2 EH
> that I need to link with.
>
> -Sandra
>
>

Re: [patch] Allow configuration with --disable-sjlj-exceptions on biarch x86 Windows targets

[Kai Tietz]

Hi,

hopefully this time gmail uses  mail-encoding elmz ask for ...

Sorry to object here.  I would like to point out that defaulting to
dw2 on 32-bit if SEH is used for 64-bit is nothing good in general.
This is reasoned by the problems existing in dw2 in combination with
other compiler-generated binaries.

Regards
Kai

2016-05-19 20:05 GMT+02:00 Jeff Law :
> On 05/19/2016 11:36 AM, Sandra Loosemore wrote:
>>
>> This is a slightly revised version of the WIP patch against GCC 5.1 I
>> previously posted here:
>>
>> https://gcc.gnu.org/ml/gcc/2016-05/msg00135.html
>>
>> To recap, I needed a biarch x86_64 mingw-w64 target compiler that uses
>> DWARF-2 exception handling in 32-bit mode (for compatibility with an
>> older i686-mingw32 toolchain configured with --disable-sjlj-exceptions).
>>  But, the configuration machinery was rejecting this.  It used to be
>> that SJLJ exceptions were the only choice for 64-bit mode so it was
>> correct to reject --disable-sjlj-exceptions in a biarch toolchain, but
>> now the default for 64-bit mode is SEH instead.  With this patch,
>> configuring with --disable-sjlj-exceptions selects DWARF-2 EH in 32-bit
>> mode and SEH in 64-bit mode.
>>
>> I tested this in a cross-compiler configured to build C and C++ only for
>> x86_64 mingw-w64 target, and test results look about the same as the
>> default configuration, which uses SJLJ for 32-bit mode.  (If it's
>> relevant, I also used the compiler built with the 5.1 version of the
>> patch to build complete Windows-host toolchains for nios2-elf and
>> nios2-linux-gnu, which I tested by running the GDB testsuite.)  There
>> are no actual code changes here so I'd expect the 32-bit DWARF-2 EH
>> support to work exactly the same as in a 32-bit-only x86 configuration.
>>
>> OK to commit?
>
> OK.
>
> Thanks,
> Jeff

[patch mingw]: Fix PR/51726 - LTO and attribute 'selectany'

[Kai Tietz]

Hello,

This patch fixes the reported LTO issue, which is caused by not
implemented case of uniinitialized variables with selectany-attribute
(as common), as such variables couldn't be placed into
linkonce-section for pe-coff.

I adjusted existing testcase to reflect now supported case of
selectany-attribute on uninitialized variables, too.

ChangeLog

2015-10-01  Kai Tietz  <ktiet...@googlemail.com>

PR target/51726
* config/i386/winnt.c (ix86_handle_selectany_attribute): Handle
selectany within this function without need to keep attribute.
(i386_pe_encode_section_info): Remove selectany-code.

I regression-tested this patch for i686 and x86_64 mingw &
cygwin-targets. will apply this patch tomorrow, if there are no
objections.

Kai

Index: winnt.c
===
--- winnt.c(Revision 228280)
+++ winnt.c(Arbeitskopie)
@@ -89,17 +89,23 @@ tree
 ix86_handle_selectany_attribute (tree *node, tree name, tree, int,
  bool *no_add_attrs)
 {
+  tree decl = *node;
   /* The attribute applies only to objects that are initialized and have
  external linkage.  However, we may not know about initialization
- until the language frontend has processed the decl. We'll check for
- initialization later in encode_section_info.  */
-  if (TREE_CODE (*node) != VAR_DECL || !TREE_PUBLIC (*node))
-{
-  error ("%qE attribute applies only to initialized variables"
-" with external linkage", name);
-  *no_add_attrs = true;
+ until the language frontend has processed the decl.   Therefore
+ we make sure that variable isn't initialized as common.  */
+  if (TREE_CODE (decl) != VAR_DECL || !TREE_PUBLIC (decl))
+error ("%qE attribute applies only to initialized variables"
+  " with external linkage", name);
+  else
+{
+  make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
+  /* A variable with attribute selectany never can be common.  */
+  DECL_COMMON (decl) = 0;
 }

+  /* We don't need to keep attribute itself.  */
+  *no_add_attrs = true;
   return NULL_TREE;
 }

@@ -320,23 +326,7 @@ i386_pe_encode_section_info (tree decl, rtx rtl, i
   switch (TREE_CODE (decl))
 {
 case FUNCTION_DECL:
-  break;
-
 case VAR_DECL:
-  if (lookup_attribute ("selectany", DECL_ATTRIBUTES (decl)))
-{
-  if (DECL_INITIAL (decl)
-  /* If an object is initialized with a ctor, the static
- initialization and destruction code for it is present in
- each unit defining the object.  The code that calls the
- ctor is protected by a link-once guard variable, so that
- the object still has link-once semantics,  */
-  || TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (decl)))
-make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
-  else
-error ("%q+D:'selectany' attribute applies only to "
-   "initialized objects", decl);
-}
   break;

 default:

ChangeLog

2015-10-01  Kai Tietz  <ktiet...@googlemail.com>

PR target/51726
* g++.dg/ext/sectany2.C: Allow uninitialized variable case.

Index: g++.dg/ext/selectany2.C
===
--- g++.dg/ext/selectany2.C(Revision 228347)
+++ g++.dg/ext/selectany2.C(Arbeitskopie)
@@ -18,7 +18,7 @@ struct  f
 };
 __declspec (selectany) struct f  F= {1}; // OK

-__declspec (selectany) int boo;//{ dg-error "selectany" }
+__declspec (selectany) int boo;//  OK

 __declspec (selectany) static int bar = 1; // { dg-error "selectany" }
 int use_bar = bar;  //  Avoid defined but not used warning.

Re: [PATCH] Fix Cygwin bootstrap failing to find win32 libraries

[Kai Tietz]

2015-09-21 0:35 GMT+02:00 JonY <10wa...@gmail.com>:
> On 9/21/2015 02:05, Kai Tietz wrote:
>> 2015-09-16 13:42 GMT+02:00 JonY <10wa...@gmail.com>:
>>> libgcc is failing to find kerne32 etc during the 2nd stage when
>>> bootstraping, explicitly add w32api directory to search path.
>>>
>>> Patch OK?
>>>
>>> diff --git a/gcc/config/i386/cygwin.h b/gcc/config/i386/cygwin.h
>>> index 2a2a0bf..fd3bc0a 100644
>>> --- a/gcc/config/i386/cygwin.h
>>> +++ b/gcc/config/i386/cygwin.h
>>> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
>>>
>>>  #undef STARTFILE_SPEC
>>>  #define STARTFILE_SPEC "\
>>> +  -L%R/usr/lib/w32api \
>>>%{!shared: %{!mdll: crt0%O%s \
>>>%{pg:gcrt0%O%s}}}\
>>>%{shared:crtbeginS.o%s;:crtbegin.o%s} \
>>>
>>>
>>
>> Hello JonY,
>>
>> patch is ok with proper ChangeLog.  For sample of format see existing
>> ChangeLog files in gcc-subdirectory.
>>
>> Thanks,
>> Kai
>>
>
> 2015-09-16  Jonathan Yong  <10wa...@gmail.com>
>
> * config/i386/cygwin.h (STARTFILE_SPEC): Explicitly search
>   sysroot/usr/lib/32api for additional win32 libraries,
>   fixes failing Cygwin bootstrapping.
>
> Message OK?
>

Committed for you at rev. 227962.

Kai

Patch: Adjust my e-mail-address in MAINTAINERS file

[Kai Tietz]

Hi,

committed adjustment of my e-mail-address in MAINTAINERS file.

ChangeLog

2015-09-20  Kai Tietz

* MAINTAINERS: Update email.

Kai

Index: MAINTAINERS
===
--- MAINTAINERS (Revision 227950)
+++ MAINTAINERS (Arbeitskopie)
@@ -141,7 +141,7 @@ RTEMS Ports Sebastian Huber
<sebastian.huber@emb
 VMSDouglas Rupp<douglas.b.r...@gmail.com>
 VMSTristan Gingold <tging...@free.fr>
 VxWorks ports  Nathan Sidwell  <nat...@codesourcery.com>
-windows, cygwin, mingw Kai Tietz   <kti...@redhat.com>
+windows, cygwin, mingw Kai Tietz   <ktiet...@googlemail.com>
 windows, cygwin, mingw Dave Korn   <dave.korn.cyg...@gmail.com>

Language Front Ends Maintainers

Re: [PATCH] Fix Cygwin bootstrap failing to find win32 libraries

[Kai Tietz]

2015-09-16 13:42 GMT+02:00 JonY <10wa...@gmail.com>:
> libgcc is failing to find kerne32 etc during the 2nd stage when
> bootstraping, explicitly add w32api directory to search path.
>
> Patch OK?
>
> diff --git a/gcc/config/i386/cygwin.h b/gcc/config/i386/cygwin.h
> index 2a2a0bf..fd3bc0a 100644
> --- a/gcc/config/i386/cygwin.h
> +++ b/gcc/config/i386/cygwin.h
> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
>
>  #undef STARTFILE_SPEC
>  #define STARTFILE_SPEC "\
> +  -L%R/usr/lib/w32api \
>%{!shared: %{!mdll: crt0%O%s \
>%{pg:gcrt0%O%s}}}\
>%{shared:crtbeginS.o%s;:crtbegin.o%s} \
>
>

Hello JonY,

patch is ok with proper ChangeLog.  For sample of format see existing
ChangeLog files in gcc-subdirectory.

Thanks,
Kai

Re: [c++-delayed-folding] cp_fold_r

[Kai Tietz]

Hi Jason,

Sounds like an interesting idea.  Do have already a specific approach in mind?

My idea might be just hard to model, as we aren't sure we walked
before the complete chain.  Due cp_fold is caching, we won't try to
fold an expression a second time, but we don't cover all EXPRs in
cp_fold, which makes it hard to tell, if we ended up walking the
complete expression-tree, or ended on an unknown expression.
So we could add to cp_fold an additional return-value, which indicates
if we ended with an unknown expression (means default statement).
This we could use later on to decided if we need to walk sub-tree, or
not.  Not sure if that is best approach, but it could help to avoid
some double runs.

Kai

2015-09-17 8:10 GMT+02:00 Jason Merrill :
> I think we want to clear *walk_subtrees a lot more often in cp_fold_r; as it
> is, for most expressions we end up calling cp_fold on the full-expression,
> then uselessly on the subexpressions after we already folded the containing
> expression.
>
> Jason

Re: [c++-delayed-folding] cp_fold_r

[Kai Tietz]

2015-09-18 18:31 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> On 09/18/2015 02:19 AM, Kai Tietz wrote:
>>
>> Hi Jason,
>>
>> Sounds like an interesting idea.  Do have already a specific approach in
>> mind?
>>
>> My idea might be just hard to model, as we aren't sure we walked
>> before the complete chain.  Due cp_fold is caching, we won't try to
>> fold an expression a second time, but we don't cover all EXPRs in
>> cp_fold, which makes it hard to tell, if we ended up walking the
>> complete expression-tree, or ended on an unknown expression.
>> So we could add to cp_fold an additional return-value, which indicates
>> if we ended with an unknown expression (means default statement).
>> This we could use later on to decided if we need to walk sub-tree, or
>> not.  Not sure if that is best approach, but it could help to avoid
>> some double runs.
>
>
> That makes sense, but maybe cp_fold should handle all expressions?

This is for sure an option, but something to be checked with successor
of this task.

>
>> 2015-09-17 8:10 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>>>
>>> I think we want to clear *walk_subtrees a lot more often in cp_fold_r; as
>>> it
>>> is, for most expressions we end up calling cp_fold on the
>>> full-expression,
>>> then uselessly on the subexpressions after we already folded the
>>> containing
>>> expression.
>
>

Re: [patch match.pd c c++]: Ignore results of 'shorten_compare' and move missing patterns in match.pd

[Kai Tietz]

2015-09-17 11:12 GMT+02:00 Richard Biener <richard.guent...@gmail.com>:
> On Wed, Sep 16, 2015 at 9:51 PM, Jeff Law <l...@redhat.com> wrote:
>> On 09/15/2015 03:42 AM, Kai Tietz wrote:
>>>>>
>>>>> 2015-09-08  Kai Tietz  <kti...@redhat.com>
>>>>>
>>>>>   * gcc.dg/tree-ssa/vrp23.c: Adjust testcase to reflect that
>>>>>   pattern is matching now already within forward-propagation pass.
>>>>>   * gcc.dg/tree-ssa/vrp24.c: Likewise.
>>>>
>>>>
>>>> So for the new patterns, I would have expected testcases to ensure
>>>> they're
>>>> getting used.
>>>
>>>
>>> We were talking about that.  My approach was to disable shortening
>>> code and see what regressions we get.  For C++ our test-coverage isn't
>>> that good, as we didn't had here any regressions, but for C testcases
>>> we got some.  Eg the testcase vrp25.c is one of them
>>
>> But as I noted, I think that simply looking at testsuite regressions after
>> disabling shorten_compare is not sufficient as I don't think we have
>> significant coverage of this code.
>>
>>>
>>> By disabling "shorten_compare" one of most simple testcases, which is
>>> failing, is:
>>>
>>> int foo (short s, short t)
>>> {
>>>int a = (int) s;
>>>int b = (int) t;
>>>
>>>return a >= b;
>>> }
>>
>> And I would argue it's precisely this kind of stuff we should be building
>> tests around and resolving prior to actually disabling shorten_compare.
>>
>>
>>>
>>> Where we miss to do narrow down SSA-tree comparison to simply s >= t;
>>> If we disable fre-pass ( -fno-tree-fre) for current trunk, we can see
>>> that this pattern gets resolved in forward-propagation pass due
>>> invocation of shorten_compare.
>>>
>>> Another simple one (with disabled "shorten_compare") is:
>>>
>>> The following testcase:
>>>
>>> int foo (unsigned short a)
>>> {
>>>unsigned int b = 0;
>>>return (unsigned int) a) < b;
>>> }
>>>
>>> Here we lacked the detection of ((unsigned int) a) < b being a < 0
>>> (which is always false).
>>
>> And again, this deserves a test and resolving prior to disabling
>> shorten_compare.
>>
>>
>>
>>>
>>>> In *theory* one ought to be able to look at the dumps or .s files before
>>>> after this patch for a blob of tests and see that nothing significant has
>>>> changed.  Unfortunately, so much changes that it's hard to evaluate if
>>>> this
>>>> patch is a step forward or a step back.
>>>
>>>
>>> Hmm, actually we deal here with obvious patterns from
>>> "shorten_compare".  Of interest are here the narrowing-lines on top of
>>> this function, which we need to reflect in match.pd too, before we can
>>> deprecate it. I don't get that there are so much changes?  This are in
>>> fact just 3 basic patterns '(convert) X  (convert) Y',
>>> '((convert) X)  CST', and a more specialized variant for the last
>>> pattern for '==/!=' case.
>>>
>>>> I wonder if we'd do better to first add new match.pd patterns, one at a
>>>> time, with tests, and evaluating them along the way by looking at the
>>>> dumps
>>>> or .s files across many systems.  Then when we think we've got the right
>>>> set, then look at what happens to those dumps/.s files if we make the
>>>> changes so that shorten_compare really just emits warnings.
>>>
>>>
>>> As the shorten_compare function covers those transformations, I don't
>>> see that this is something leading to something useful.  As long as we
>>> call shorten_compare on folding in FEs, we won't see those patterns in
>>> ME that easy.  And even more important is, that patterns getting
>>> resolved if function gets invoked.
>>
>> It will tell you what will be missed from a code generation standpoint if
>> you disable shorten_compare.  And that is the best proxy we have for
>> performance regressions related to disabling shorten_compare.
>>
>> ie, in a local tree, you disable shorten_compare.  Then compare the code we
>> generate with and without shorten compare.  Reduce to a simple testcase.
>> Resolve the issue with a match.pd pattern (most likely) and repeat the
>> process.  

Re: [c++-delayed-folding] code review

[Kai Tietz]

Hello Jason,

thanks for your review.  I addressed most of your mentioned issues
already today.  To some of them I have further comments ...

2015-09-17 8:18 GMT+02:00 Jason Merrill :
>> @@ -216,6 +216,7 @@ libgcov-driver-tool.o-warn = -Wno-error
>>  libgcov-merge-tool.o-warn = -Wno-error
>>  gimple-match.o-warn = -Wno-unused
>>  generic-match.o-warn = -Wno-unused
>> +insn-modes.o-warn = -Wno-error
>
>
> This doesn't seem to be needed anymore.

Yes, removed.

>> @@ -397,11 +397,13 @@ convert_to_real (tree type, tree expr)
>> EXPR must be pointer, integer, discrete (enum, char, or bool), float,
>> fixed-point or vector; in other cases error is called.
>>
>> +   If DOFOLD is TRZE, we try to simplify newly-created patterns by
>> folding.
>
>
> "TRUE"

Fixed.  Thanks.


>> +  if (!dofold)
>> +{
>> + expr = build1 (CONVERT_EXPR,
>> +lang_hooks.types.type_for_size
>> +  (TYPE_PRECISION (intype), 0),
>> +expr);
>> + return build1 (CONVERT_EXPR, type, expr);
>> +   }
>
>
> When we're not folding, I don't think we want to do the two-step conversion,
> just the second one.  And we might want to use NOP_EXPR instead of
> CONVERT_EXPR, but I'm not sure about that.

This is indeed a point I was thinking about while writing this code.
The comments below regarding other introduced patterns are related to
the same thinking.
Sure is that we want to remove such code-modification from FEs itself,
but that means that those conversions need to be handled elsewhere
later in ME.  We have a different functional change about
shorten_compare already, which makes me headaches, as we don't deal
with all cases of it in ME right (which is a different task not
directly related to delayed-folding, but linked).  Therefore I kept
those code-modification-code also for df, which we should move out
into ME (with other patterns here in convert.c) in different task.

For above code we need to do 2 step folding, as we need to make sure
that we convert expr first to language-specific size-type, which isn't
necessarily the same as TYPE here.  Casting directly to TYPE might
cause side-effects (especially sign/unsigned-conversion issues I could
think about here)


>> @@ -818,10 +828,15 @@ convert_to_integer (tree type, tree expr)
>> if (TYPE_UNSIGNED (typex))
>>   typex = signed_type_for (typex);
>>   }
>> -   return convert (type,
>> -   fold_build2 (ex_form, typex,
>> -convert (typex, arg0),
>> -convert (typex, arg1)));
>> +   if (dofold)
>> + return convert (type,
>> + fold_build2 (ex_form, typex,
>> +  convert (typex, arg0),
>> +  convert (typex,
>> arg1)));
>> +   arg0 = build1 (CONVERT_EXPR, typex, arg0);
>> +   arg1 = build1 (CONVERT_EXPR, typex, arg1);
>> +   expr = build2 (ex_form, typex, arg0, arg1);
>> +   return build1 (CONVERT_EXPR, type, expr);
>
>
> This code path seems to be for pushing a conversion down into a binary
> expression.  We shouldn't do this at all when we aren't folding.

Yes, but see comment above.  I agree that such code-modifications are
not the thing we intend to do, but this should be part of a different
task.

>> @@ -845,9 +860,14 @@ convert_to_integer (tree type, tree expr)
>>
>> if (!TYPE_UNSIGNED (typex))
>>   typex = unsigned_type_for (typex);
>> +   if (!dofold)
>> + return build1 (CONVERT_EXPR, type,
>> +build1 (ex_form, typex,
>> +build1 (CONVERT_EXPR, typex,
>> +TREE_OPERAND (expr, 0;
>
>
> Likewise.

^^

>> @@ -867,6 +887,14 @@ convert_to_integer (tree type, tree expr)
>>  the conditional and never loses.  A COND_EXPR may have a
>> throw
>>  as one operand, which then has void type.  Just leave void
>>  operands as they are.  */
>> + if (!dofold)
>> +   return build3 (COND_EXPR, type, TREE_OPERAND (expr, 0),
>> +  VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr,
>> 1)))
>> +  ? TREE_OPERAND (expr, 1)
>> +  : build1 (CONVERT_EXPR, type, TREE_OPERAND
>> (expr, 1)),
>> +  VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr,
>> 2)))
>> +  ? TREE_OPERAND (expr, 2)
>> +  : build1 (CONVERT_EXPR, type, TREE_OPERAND
>> (expr, 2)));
>
>
> Likewise.

^^

>> @@ -903,6 +933,10 @@ convert_to_integer (tree type, tree expr)
>> 

Re: [patch match.pd c c++]: Ignore results of 'shorten_compare' and move missing patterns in match.pd

[Kai Tietz]

2015-09-15 0:45 GMT+02:00 Jeff Law <l...@redhat.com>:
> On 09/08/2015 05:17 AM, Kai Tietz wrote:
>>
>> Hi,
>>
>> This patch is the first part of obsoleting 'shorten_compare' function
>> for folding.
>> It adjusts the uses of 'shorten_compare' to ignore folding returned by
>> it, and adds
>> missing pattterns to match.pd to allow full bootstrap of C/C++ without
>> regressions.
>> Due we are using 'shorten_compare' for some diagnostic we can't simply
>> remove it.  So if this patch gets approved, the next step will be to
>> rename the function to something like 'check_compare', and adjust its
>> arguments and inner logic to reflect that we don't modify
>> arguments/expression anymore within that function.
>>
>> Bootstrap just show 2 regressions within gcc.dg testsuite due patterns
>> matched are folded more early by forward-propagation.  I adjusted
>> them, and added them to patch, too.
>>
>> I did regression-testing for x86_64-unknown-linux-gnu.
>>
>> ChangeLog
>>
>> 2015-09-08  Kai Tietz  <kti...@redhat.com>
>>
>>  * match.pd: Add missing patterns from shorten_compare.
>>  * c/c-typeck.c (build_binary_op): Discard foldings of
>> shorten_compare.
>>  * cp/typeck.c (cp_build_binary_op): Likewise.
>>
>> 2015-09-08  Kai Tietz  <kti...@redhat.com>
>>
>>  * gcc.dg/tree-ssa/vrp23.c: Adjust testcase to reflect that
>>  pattern is matching now already within forward-propagation pass.
>>  * gcc.dg/tree-ssa/vrp24.c: Likewise.
>
> So for the new patterns, I would have expected testcases to ensure they're
> getting used.

We were talking about that.  My approach was to disable shortening
code and see what regressions we get.  For C++ our test-coverage isn't
that good, as we didn't had here any regressions, but for C testcases
we got some.  Eg the testcase vrp25.c is one of them

> The fact that we're not regressing with the front-end specific shortening
> disabled like this is probably more of an artifact of lack of testing of
> this feature than anything.

This is just true for C++.  For C case we see additional fallout also
in gcc.target specific patterns in i386.  They were mainly about
"blend", and some AVX-testcases.

By disabling "shorten_compare" one of most simple testcases, which is
failing, is:

int foo (short s, short t)
{
  int a = (int) s;
  int b = (int) t;

  return a >= b;
}

Where we miss to do narrow down SSA-tree comparison to simply s >= t;
If we disable fre-pass ( -fno-tree-fre) for current trunk, we can see
that this pattern gets resolved in forward-propagation pass due
invocation of shorten_compare.

Another simple one (with disabled "shorten_compare") is:

The following testcase:

int foo (unsigned short a)
{
  unsigned int b = 0;
  return (unsigned int) a) < b;
}

Here we lacked the detection of ((unsigned int) a) < b being a < 0
(which is always false).

> In *theory* one ought to be able to look at the dumps or .s files before
> after this patch for a blob of tests and see that nothing significant has
> changed.  Unfortunately, so much changes that it's hard to evaluate if this
> patch is a step forward or a step back.

Hmm, actually we deal here with obvious patterns from
"shorten_compare".  Of interest are here the narrowing-lines on top of
this function, which we need to reflect in match.pd too, before we can
deprecate it. I don't get that there are so much changes?  This are in
fact just 3 basic patterns '(convert) X  (convert) Y',
'((convert) X)  CST', and a more specialized variant for the last
pattern for '==/!=' case.

> I wonder if we'd do better to first add new match.pd patterns, one at a
> time, with tests, and evaluating them along the way by looking at the dumps
> or .s files across many systems.  Then when we think we've got the right
> set, then look at what happens to those dumps/.s files if we make the
> changes so that shorten_compare really just emits warnings.

As the shorten_compare function covers those transformations, I don't
see that this is something leading to something useful.  As long as we
call shorten_compare on folding in FEs, we won't see those patterns in
ME that easy.  And even more important is, that patterns getting
resolved if function gets invoked.

So I would suggest here to disable shorten_compare invocation and
cleanup with fallout detectable in C-FE's testsuite.

> My worry is that we get part way through the conversion and end up with the
> match.pd patterns without actually getting shorten_compare cleaned up and
> turned into just a warning generator.

This worry I share, therefore I see it as mandatory to remove with
initial patch the use of result of shorten_compare, and better cleanup
possible detected additional fallout for other targets.  I just did
regression-testing for Intel-architecture (32-bit and 64-bit).

> jeff
>

Kai

Re: [patch match.pd c c++]: Ignore results of 'shorten_compare' and move missing patterns in match.pd

[Kai Tietz]

Hi Richi,

thanks for the review.

2015-09-15 12:06 GMT+02:00 Richard Biener <richard.guent...@gmail.com>:
> On Tue, Sep 8, 2015 at 1:17 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
>> Hi,
>>
>> This patch is the first part of obsoleting 'shorten_compare' function
>> for folding.
>> It adjusts the uses of 'shorten_compare' to ignore folding returned by
>> it, and adds
>> missing pattterns to match.pd to allow full bootstrap of C/C++ without
>> regressions.
>> Due we are using 'shorten_compare' for some diagnostic we can't simply
>> remove it.  So if this patch gets approved, the next step will be to
>> rename the function to something like 'check_compare', and adjust its
>> arguments and inner logic to reflect that we don't modify
>> arguments/expression anymore within that function.
>>
>> Bootstrap just show 2 regressions within gcc.dg testsuite due patterns
>> matched are folded more early by forward-propagation.  I adjusted
>> them, and added them to patch, too.
>
> Some comments on the patterns you added below
>
>> I did regression-testing for x86_64-unknown-linux-gnu.
>>
>> ChangeLog
>>
>> 2015-09-08  Kai Tietz  <kti...@redhat.com>
>>
>> * match.pd: Add missing patterns from shorten_compare.
>> * c/c-typeck.c (build_binary_op): Discard foldings of shorten_compare.
>> * cp/typeck.c (cp_build_binary_op): Likewise.
>>
>> 2015-09-08  Kai Tietz  <kti...@redhat.com>
>>
>> * gcc.dg/tree-ssa/vrp23.c: Adjust testcase to reflect that
>> pattern is matching now already within forward-propagation pass.
>> * gcc.dg/tree-ssa/vrp24.c: Likewise.
>>
>> Index: match.pd
>> ===
>> --- match.pd(Revision 227528)
>> +++ match.pd(Arbeitskopie)
>> @@ -1786,6 +1786,45 @@ along with GCC; see the file COPYING3.  If not see
>>(op (abs @0) zerop@1)
>>(op @0 @1)))
>>
>> +/* Simplify '((type) X) cmp ((type) Y' to shortest possible types, of X and 
>> Y,
>> +   if type's precision is wider then precision of X's and Y's type.
>> +   Logic taken from shorten_compare function.  */
>> +(for op (tcc_comparison)
>> +  (simplify
>> +(op (convert@0 @1) (convert@3 @2))
>> +(if ((TREE_CODE (TREE_TYPE (@1)) == REAL_TYPE)
>> + == (TREE_CODE (TREE_TYPE (@2)) == REAL_TYPE)
>> + && (TREE_CODE (TREE_TYPE (@1)) == REAL_TYPE)
>> +== (TREE_CODE (TREE_TYPE (@0)) == REAL_TYPE)
>
> I think these are always true, to/from REAL_TYPE is FIX_TRUNC /
> FLOAT_EXPR.  What you
> might get is conversion to/from decimal FP from/to non-decimal FP.

Right, I just wanted to handle patterns of shorten_compare here.  I
agree that - with small adjustments - we can use same logic for
FIX_TRUNC and FLOAT_EXPR, too.

>> + && single_use (@1)
>> + && single_use (@3)
>
> We have :s now, I'd like to see comments before any explicit
> single_use () uses as to why
> they were added.  Also why @1 and @3?  Either @0 and @3 or @1 and @2?

Thanks for pointing me to :s.  I removedfrom updated patch the @3.
Reason for introducing it was just for checking if expression has
single-use, as otherwise we don't want to split expression by this
transformation.

>> + && TYPE_UNSIGNED (TREE_TYPE (@1)) == TYPE_UNSIGNED (TREE_TYPE (@2))
>> + && !((TREE_CODE (TREE_TYPE (@1)) == REAL_TYPE
>> +   && DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (@1
>> +  || (TREE_CODE (TREE_TYPE (@2)) == REAL_TYPE
>> +  && DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (@2)
>
> We have DECIMAL_FLOAT_TYPE_P () for this.

DECIMAL_FLOAT_TYPE seems to check for wide kind of patterns AFAICS.
It uses internall SCALAR_FLOAT_TYPE_P,
which also checks for COMPLEX_TYPE, not just for REAL_TYPE.  I  code
in shorten_compare, which rejects DECIMAL_FLOAT_MODE_P just for
REAL_TYPEs.

But well, not sure if this is a latent bug in shorten_compare ...  so
I replaced code using DECIMAL_FLOAT_TYPE instead.

>> + && TYPE_PRECISION (TREE_TYPE (@1)) < TYPE_PRECISION (TREE_TYPE 
>> (@0))
>> + && TYPE_PRECISION (TREE_TYPE (@2)) < TYPE_PRECISION (TREE_TYPE 
>> (@0))
>
> copy-paste error, @3 (ok, it shouldn't really matter as @0 and @3
> should be the same types).

Hmm, no pasto.  Initially (as told above) I didn't used @3 as it seems
to be pretty superflous here.
You are right that it doesn't matter as convert@0 and convert@3 should
be always the same type.

>> + )
>
> 

[patch match.pd c c++]: Ignore results of 'shorten_compare' and move missing patterns in match.pd

[Kai Tietz]

Hi,

This patch is the first part of obsoleting 'shorten_compare' function
for folding.
It adjusts the uses of 'shorten_compare' to ignore folding returned by
it, and adds
missing pattterns to match.pd to allow full bootstrap of C/C++ without
regressions.
Due we are using 'shorten_compare' for some diagnostic we can't simply
remove it.  So if this patch gets approved, the next step will be to
rename the function to something like 'check_compare', and adjust its
arguments and inner logic to reflect that we don't modify
arguments/expression anymore within that function.

Bootstrap just show 2 regressions within gcc.dg testsuite due patterns
matched are folded more early by forward-propagation.  I adjusted
them, and added them to patch, too.

I did regression-testing for x86_64-unknown-linux-gnu.

ChangeLog

2015-09-08  Kai Tietz  <kti...@redhat.com>

* match.pd: Add missing patterns from shorten_compare.
* c/c-typeck.c (build_binary_op): Discard foldings of shorten_compare.
* cp/typeck.c (cp_build_binary_op): Likewise.

2015-09-08  Kai Tietz  <kti...@redhat.com>

* gcc.dg/tree-ssa/vrp23.c: Adjust testcase to reflect that
pattern is matching now already within forward-propagation pass.
* gcc.dg/tree-ssa/vrp24.c: Likewise.

Index: match.pd
===
--- match.pd(Revision 227528)
+++ match.pd(Arbeitskopie)
@@ -1786,6 +1786,45 @@ along with GCC; see the file COPYING3.  If not see
   (op (abs @0) zerop@1)
   (op @0 @1)))

+/* Simplify '((type) X) cmp ((type) Y' to shortest possible types, of X and Y,
+   if type's precision is wider then precision of X's and Y's type.
+   Logic taken from shorten_compare function.  */
+(for op (tcc_comparison)
+  (simplify
+(op (convert@0 @1) (convert@3 @2))
+(if ((TREE_CODE (TREE_TYPE (@1)) == REAL_TYPE)
+ == (TREE_CODE (TREE_TYPE (@2)) == REAL_TYPE)
+ && (TREE_CODE (TREE_TYPE (@1)) == REAL_TYPE)
+== (TREE_CODE (TREE_TYPE (@0)) == REAL_TYPE)
+ && single_use (@1)
+ && single_use (@3)
+ && TYPE_UNSIGNED (TREE_TYPE (@1)) == TYPE_UNSIGNED (TREE_TYPE (@2))
+ && !((TREE_CODE (TREE_TYPE (@1)) == REAL_TYPE
+   && DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (@1
+  || (TREE_CODE (TREE_TYPE (@2)) == REAL_TYPE
+  && DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (@2)
+ && TYPE_PRECISION (TREE_TYPE (@1)) < TYPE_PRECISION (TREE_TYPE (@0))
+ && TYPE_PRECISION (TREE_TYPE (@2)) < TYPE_PRECISION (TREE_TYPE (@0))
+ )
+   (with {
+ tree comtype = TYPE_PRECISION (TREE_TYPE (@1))
+ < TYPE_PRECISION (TREE_TYPE (@2)) ? TREE_TYPE (@2)
+   : TREE_TYPE (@1);
+ if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
+   {
+ if (TYPE_UNSIGNED (TREE_TYPE (@1)) || TYPE_UNSIGNED
(TREE_TYPE (@0)))
+   comtype = unsigned_type_for (comtype);
+ else
+   comtype = signed_type_for (comtype);
+   }
+ }
+(op (convert:comtype @1) (convert:comtype @2))
+   )
+ )
+  )
+)
+
+
 /* From fold_sign_changed_comparison and fold_widened_comparison.  */
 (for cmp (simple_comparison)
  (simplify
@@ -2046,7 +2085,43 @@ along with GCC; see the file COPYING3.  If not see
 (if (cmp == LE_EXPR)
  (ge (convert:st @0) { build_zero_cst (st); })
  (lt (convert:st @0) { build_zero_cst (st); }))
-
+
+/* Simplify '(type) X cmp CST' to 'X cmp (type-of-X) CST', if
+   CST fits into the type of X.  */
+(for cmp (simple_comparison)
+  (simplify
+(cmp (convert@2 @0) INTEGER_CST@1)
+(if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && TYPE_PRECISION (TREE_TYPE (@1)) > TYPE_PRECISION (TREE_TYPE (@0))
+ && single_use (@2)
+ && (TYPE_UNSIGNED (TREE_TYPE (@0))
+ || TYPE_UNSIGNED (TREE_TYPE (@0)) == TYPE_UNSIGNED
(TREE_TYPE (@1))
+ || cmp == NE_EXPR || cmp == EQ_EXPR)
+ && !POINTER_TYPE_P (TREE_TYPE (@0))
+ && int_fits_type_p (@1, TREE_TYPE (@0)))
+  (with { tree optype = TREE_TYPE (@0); }
+(cmp @0 (convert:optype @1))
+  )
+)
+  )
+)
+
+/* See if '(type) X ==/!= CST' represents a condition,
+   which is always true, or false due CST's value.  */
+(for cmp (ne eq)
+  (simplify
+(cmp (convert@2 @0) INTEGER_CST@1)
+(if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && TYPE_PRECISION (TREE_TYPE (@1)) >= TYPE_PRECISION (TREE_TYPE (@0))
+ && single_use (@2)
+ && !POINTER_TYPE_P (TREE_TYPE (@0))
+ && !int_fits_type_p (@1, TREE_TYPE (@0))
+ && TYPE_UNSIGNED (TREE_TYPE (@0)) == TYPE_UNSIGNED (TREE_TYPE (@1)))
+  { constant_boolean_node (cmp == NE_EXPR, 

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-09-01 10:15 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
> 2015-08-31 22:19 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>>>> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>>>>
>>>>> I will need to verify that this patch doesn't introduce regressions.
>>>>> The wacky thing here is the encapsulation of overflowed-arguments in
>>>>> maybe_constant_value function by nop-expr.
>>>>
>>>>
>>>> Do we need to worry about that?  If one of the operands is overflowed, we
>>>> don't care whether the result is overflowed.
>>>
>>> Well, we would introduce, if we don't see in condition that operand
>>> already overflowed, double overflow-warning, which seems to be
>>> something we avoided until now.  So I would say, it matters.
>>>
>>> Kai
>>
>> Similar to the binary-operation we want to do then the same for
>> unary-operations, too.
>>
>> Eg. testcase:
>>
>> #include 
>>
>> constexpr int f() { return INT_MIN; }
>>
>> int main()
>> {
>>   return -f(); // { dg-warning "overflow" }
>> }
>> With following patch we do diagnostics for it.
>>
>> Kai
>>
>> Index: semantics.c
>> ===
>> --- semantics.c (Revision 227339)
>> +++ semantics.c (Arbeitskopie)
>> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co
>>tree result = build_x_unary_op (loc, code, expr, complain);
>>tree result_ovl =  result;
>>
>> -  expr_ovl = fold_simple (expr_ovl);
>> -  result_ovl = fold_simple (result);
>> -
>> +  expr_ovl = maybe_constant_value (expr_ovl);
>> +  result_ovl = maybe_constant_value (result);
>> +  /* Strip nop-expressions added by maybe_constant_value on overflow.  */
>> +  STRIP_NOPS (expr_ovl);
>> +  STRIP_NOPS (result_ovl);
>>if ((complain & tf_warning)
>>&& TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl))
>>  overflow_warning (input_location, result_ovl);
>
> I committed patches for binary & unary operations together with
> testcases.  Regression-run still running.  There seems to be
> additional expressions needed in constexpr for this.  For now we have
> a bootstrap-issue due cast_expr in cxx_eval_constant_expression.
> There might be more of them ...

I had to add for now that cxx_eval_constant_expr ignores on CAST_EXPR,
STATIC_CAST_EXPR, OVERLOAD, and TREE_LIST.  Additionally we need to
handle for increments that offset is NULL_TREE (TREE_OPERAND (,1)) .

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-31 22:19 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>>> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>>>
>>>> I will need to verify that this patch doesn't introduce regressions.
>>>> The wacky thing here is the encapsulation of overflowed-arguments in
>>>> maybe_constant_value function by nop-expr.
>>>
>>>
>>> Do we need to worry about that?  If one of the operands is overflowed, we
>>> don't care whether the result is overflowed.
>>
>> Well, we would introduce, if we don't see in condition that operand
>> already overflowed, double overflow-warning, which seems to be
>> something we avoided until now.  So I would say, it matters.
>>
>> Kai
>
> Similar to the binary-operation we want to do then the same for
> unary-operations, too.
>
> Eg. testcase:
>
> #include 
>
> constexpr int f() { return INT_MIN; }
>
> int main()
> {
>   return -f(); // { dg-warning "overflow" }
> }
> With following patch we do diagnostics for it.
>
> Kai
>
> Index: semantics.c
> ===
> --- semantics.c (Revision 227339)
> +++ semantics.c (Arbeitskopie)
> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co
>tree result = build_x_unary_op (loc, code, expr, complain);
>tree result_ovl =  result;
>
> -  expr_ovl = fold_simple (expr_ovl);
> -  result_ovl = fold_simple (result);
> -
> +  expr_ovl = maybe_constant_value (expr_ovl);
> +  result_ovl = maybe_constant_value (result);
> +  /* Strip nop-expressions added by maybe_constant_value on overflow.  */
> +  STRIP_NOPS (expr_ovl);
> +  STRIP_NOPS (result_ovl);
>if ((complain & tf_warning)
>&& TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl))
>  overflow_warning (input_location, result_ovl);

I committed patches for binary & unary operations together with
testcases.  Regression-run still running.  There seems to be
additional expressions needed in constexpr for this.  For now we have
a bootstrap-issue due cast_expr in cxx_eval_constant_expression.
There might be more of them ...

Kai

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-09-01 10:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
> 2015-09-01 10:15 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>> 2015-08-31 22:19 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>>> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>>>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>>>>> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>>>>>
>>>>>> I will need to verify that this patch doesn't introduce regressions.
>>>>>> The wacky thing here is the encapsulation of overflowed-arguments in
>>>>>> maybe_constant_value function by nop-expr.
>>>>>
>>>>>
>>>>> Do we need to worry about that?  If one of the operands is overflowed, we
>>>>> don't care whether the result is overflowed.
>>>>
>>>> Well, we would introduce, if we don't see in condition that operand
>>>> already overflowed, double overflow-warning, which seems to be
>>>> something we avoided until now.  So I would say, it matters.
>>>>
>>>> Kai
>>>
>>> Similar to the binary-operation we want to do then the same for
>>> unary-operations, too.
>>>
>>> Eg. testcase:
>>>
>>> #include 
>>>
>>> constexpr int f() { return INT_MIN; }
>>>
>>> int main()
>>> {
>>>   return -f(); // { dg-warning "overflow" }
>>> }
>>> With following patch we do diagnostics for it.
>>>
>>> Kai
>>>
>>> Index: semantics.c
>>> ===
>>> --- semantics.c (Revision 227339)
>>> +++ semantics.c (Arbeitskopie)
>>> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co
>>>tree result = build_x_unary_op (loc, code, expr, complain);
>>>tree result_ovl =  result;
>>>
>>> -  expr_ovl = fold_simple (expr_ovl);
>>> -  result_ovl = fold_simple (result);
>>> -
>>> +  expr_ovl = maybe_constant_value (expr_ovl);
>>> +  result_ovl = maybe_constant_value (result);
>>> +  /* Strip nop-expressions added by maybe_constant_value on overflow.  */
>>> +  STRIP_NOPS (expr_ovl);
>>> +  STRIP_NOPS (result_ovl);
>>>if ((complain & tf_warning)
>>>&& TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl))
>>>  overflow_warning (input_location, result_ovl);
>>
>> I committed patches for binary & unary operations together with
>> testcases.  Regression-run still running.  There seems to be
>> additional expressions needed in constexpr for this.  For now we have
>> a bootstrap-issue due cast_expr in cxx_eval_constant_expression.
>> There might be more of them ...
>
> I had to add for now that cxx_eval_constant_expr ignores on CAST_EXPR,
> STATIC_CAST_EXPR, OVERLOAD, and TREE_LIST.  Additionally we need to
> handle for increments that offset is NULL_TREE (TREE_OPERAND (,1)) .

Issue was easier to resolve by checking in binary/unary
overflow-checking-functions that we aren't processing template
declarations.  If we aren't within template-declaration processing we
can call maybe_constant_value.  This avoids that we feed
maybe_constant_value with mentioned C++-template specific expressions.

Bootstrap ran successful, regression-testing still running for it.  I
committed additional check already to branch.

Kai

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-09-01 13:17 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
> 2015-09-01 10:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>> 2015-09-01 10:15 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>>> 2015-08-31 22:19 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>>>> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
>>>>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>>>>>> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>>>>>>
>>>>>>> I will need to verify that this patch doesn't introduce regressions.
>>>>>>> The wacky thing here is the encapsulation of overflowed-arguments in
>>>>>>> maybe_constant_value function by nop-expr.
>>>>>>
>>>>>>
>>>>>> Do we need to worry about that?  If one of the operands is overflowed, we
>>>>>> don't care whether the result is overflowed.
>>>>>
>>>>> Well, we would introduce, if we don't see in condition that operand
>>>>> already overflowed, double overflow-warning, which seems to be
>>>>> something we avoided until now.  So I would say, it matters.
>>>>>
>>>>> Kai
>>>>
>>>> Similar to the binary-operation we want to do then the same for
>>>> unary-operations, too.
>>>>
>>>> Eg. testcase:
>>>>
>>>> #include 
>>>>
>>>> constexpr int f() { return INT_MIN; }
>>>>
>>>> int main()
>>>> {
>>>>   return -f(); // { dg-warning "overflow" }
>>>> }
>>>> With following patch we do diagnostics for it.
>>>>
>>>> Kai
>>>>
>>>> Index: semantics.c
>>>> ===
>>>> --- semantics.c (Revision 227339)
>>>> +++ semantics.c (Arbeitskopie)
>>>> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co
>>>>tree result = build_x_unary_op (loc, code, expr, complain);
>>>>tree result_ovl =  result;
>>>>
>>>> -  expr_ovl = fold_simple (expr_ovl);
>>>> -  result_ovl = fold_simple (result);
>>>> -
>>>> +  expr_ovl = maybe_constant_value (expr_ovl);
>>>> +  result_ovl = maybe_constant_value (result);
>>>> +  /* Strip nop-expressions added by maybe_constant_value on overflow.  */
>>>> +  STRIP_NOPS (expr_ovl);
>>>> +  STRIP_NOPS (result_ovl);
>>>>if ((complain & tf_warning)
>>>>&& TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl))
>>>>  overflow_warning (input_location, result_ovl);
>>>
>>> I committed patches for binary & unary operations together with
>>> testcases.  Regression-run still running.  There seems to be
>>> additional expressions needed in constexpr for this.  For now we have
>>> a bootstrap-issue due cast_expr in cxx_eval_constant_expression.
>>> There might be more of them ...
>>
>> I had to add for now that cxx_eval_constant_expr ignores on CAST_EXPR,
>> STATIC_CAST_EXPR, OVERLOAD, and TREE_LIST.  Additionally we need to
>> handle for increments that offset is NULL_TREE (TREE_OPERAND (,1)) .
>
> Issue was easier to resolve by checking in binary/unary
> overflow-checking-functions that we aren't processing template
> declarations.  If we aren't within template-declaration processing we
> can call maybe_constant_value.  This avoids that we feed
> maybe_constant_value with mentioned C++-template specific expressions.
>
> Bootstrap ran successful, regression-testing still running for it.  I
> committed additional check already to branch.

All tests passed as expected.

Kai

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-09-01 16:47 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> On 08/31/2015 03:43 PM, Kai Tietz wrote:
>>
>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>>>
>>> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>>>
>>>>
>>>> I will need to verify that this patch doesn't introduce regressions.
>>>> The wacky thing here is the encapsulation of overflowed-arguments in
>>>> maybe_constant_value function by nop-expr.
>>>
>>>
>>>
>>> Do we need to worry about that?  If one of the operands is overflowed, we
>>> don't care whether the result is overflowed.
>>
>>
>> Well, we would introduce, if we don't see in condition that operand
>> already overflowed, double overflow-warning, which seems to be
>> something we avoided until now.  So I would say, it matters.
>
>
> I would rather handle this by checking whether the folded operands are
> constant before even building the folded result.

I rewrote binary/unary overflow-check logic so, that we avoid double
checking-s.  I think this address things as you intend, beside the
checking for constant value.  We would need to check for *_CST
tree-codes.  Is there a macro we could use, which is just checking for
those?

> Jason
>

Kai

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-09-01 17:31 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> On 09/01/2015 11:27 AM, Kai Tietz wrote:
>>
>> I rewrote binary/unary overflow-check logic so, that we avoid double
>> checking-s.  I think this address things as you intend, beside the
>> checking for constant value.  We would need to check for *_CST
>> tree-codes.  Is there a macro we could use, which is just checking for
>> those?
>
>
> Yes, CONSTANT_CLASS_P.
>
> Jason


Thanks, I found it too :]  I applied a patch short-cutting check for
cases that operands aren't of constant-class.

Kai

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-31 19:52 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> On 08/29/2015 10:10 AM, Kai Tietz wrote:
>>
>> Hmm, I don't think we want to call maybe_constant_value in functions
>> like cp_build_binary_op. We are interested in overflow only on
>> constant-values anyway, I don't see that we want to have here any
>> constexpr-logic, nor specific address-manipulation logic.  So I see
>> here not much advantage in using maybe_constant_value.  Maybe I simply
>> not seeing the obvious here.  Do you have a specific testcase, which
>> shows what diagnostics could be missed?
>
>
> #include 
> constexpr int f() { return INT_MAX; }
> int main()
> {
>   return f()+2; // { dg-warning "overflow" }
> }
>
> Jason
>

Ok.  Following patch should allow this missed diagnostics

I will need to verify that this patch doesn't introduce regressions.
The wacky thing here is the encapsulation of overflowed-arguments in
maybe_constant_value function by nop-expr.

Kai

Index: typeck.c
===
--- typeck.c(Revision 227339)
+++ typeck.c(Arbeitskopie)
@@ -5070,8 +5070,12 @@ cp_build_binary_op (location_t location,
   result = build2 (resultcode, build_type, op0, op1);
   if (final_type != 0)
 result = cp_convert (final_type, result, complain);
-  op0 = fold_simple (op0);
-  op1 = fold_simple (op1);
+  op0 = maybe_constant_value (op0);
+  op1 = maybe_constant_value (op1);
+  /* Strip added nop-expression for overflow-operand introduced by
+ maybe_constant_value.  */
+  STRIP_NOPS (op0);
+  STRIP_NOPS (op1);
   result_ovl = fold_build2 (resultcode, build_type, op0, op1);
   if (TREE_OVERFLOW_P (result_ovl)
   && !TREE_OVERFLOW_P (op0)

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>
>> I will need to verify that this patch doesn't introduce regressions.
>> The wacky thing here is the encapsulation of overflowed-arguments in
>> maybe_constant_value function by nop-expr.
>
>
> Do we need to worry about that?  If one of the operands is overflowed, we
> don't care whether the result is overflowed.

Well, we would introduce, if we don't see in condition that operand
already overflowed, double overflow-warning, which seems to be
something we avoided until now.  So I would say, it matters.

Kai

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-31 21:43 GMT+02:00 Kai Tietz <ktiet...@googlemail.com>:
> 2015-08-31 21:29 GMT+02:00 Jason Merrill <ja...@redhat.com>:
>> On 08/31/2015 03:08 PM, Kai Tietz wrote:
>>>
>>> I will need to verify that this patch doesn't introduce regressions.
>>> The wacky thing here is the encapsulation of overflowed-arguments in
>>> maybe_constant_value function by nop-expr.
>>
>>
>> Do we need to worry about that?  If one of the operands is overflowed, we
>> don't care whether the result is overflowed.
>
> Well, we would introduce, if we don't see in condition that operand
> already overflowed, double overflow-warning, which seems to be
> something we avoided until now.  So I would say, it matters.
>
> Kai

Similar to the binary-operation we want to do then the same for
unary-operations, too.

Eg. testcase:

#include 

constexpr int f() { return INT_MIN; }

int main()
{
  return -f(); // { dg-warning "overflow" }
}
With following patch we do diagnostics for it.

Kai

Index: semantics.c
===
--- semantics.c (Revision 227339)
+++ semantics.c (Arbeitskopie)
@@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co
   tree result = build_x_unary_op (loc, code, expr, complain);
   tree result_ovl =  result;

-  expr_ovl = fold_simple (expr_ovl);
-  result_ovl = fold_simple (result);
-
+  expr_ovl = maybe_constant_value (expr_ovl);
+  result_ovl = maybe_constant_value (result);
+  /* Strip nop-expressions added by maybe_constant_value on overflow.  */
+  STRIP_NOPS (expr_ovl);
+  STRIP_NOPS (result_ovl);
   if ((complain & tf_warning)
   && TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl))
 overflow_warning (input_location, result_ovl);

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-29 6:45 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 08/27/2015 05:21 AM, Kai Tietz wrote:

 2015-08-27 4:57 GMT+02:00 Jason Merrill ja...@redhat.com:

 Why does fold_simple fold so many patterns?  I thought we wanted
 something
 that would just fold conversions and negations of constant values.


 Yes, initial variant was handling much less patterns.  But actually we
 need for functions (eg. like build_vec_init in init.c) a simple
 routine to perform basic constant-value arithmetics (sizeof * / + -
 trunc, etc) to avoid call of maybe_constant_value.  Also for
 overflow-diagnostics we want at least to resolve such simple patterns
 for constant-values only.  We could change those calls to use
 maybe_constant_value instead, but the overhead (and some of its
 folding) leads much further then working on constant-values only (as
 fold_simple does).


 For build_vec_init, since whether maxindex is constant has semantic meaning,
 I think we want maybe_constant_value.

So you mean that handling of constexprs within vector-init is
something we need.  Ok. I will test a replace.

 I think we also want it for overflow warnings, to get better diagnostics.

Hmm, I don't think we want to call maybe_constant_value in functions
like cp_build_binary_op. We are interested in overflow only on
constant-values anyway, I don't see that we want to have here any
constexpr-logic, nor specific address-manipulation logic.  So I see
here not much advantage in using maybe_constant_value.  Maybe I simply
not seeing the obvious here.  Do you have a specific testcase, which
shows what diagnostics could be missed?

 Jason


Kai

Re: [PATCH] clone_function_name_1: Retain any stdcall suffix

[Kai Tietz]

2015-08-18 22:23 GMT+02:00 Ray Donnelly mingw.andr...@gmail.com:
 I'm not familiar with setting up GCC testcases yet so I expect to have
 to do that at least. To aid discussion, the commit message contains a
 testcase.

No problem.  Patch looks fine to me.

Thanks,
Kai

 --
 Best regards,

 Ray.

Re: [PATCH] Remove pointless -fPIC warning on Windows platforms

[Kai Tietz]

Hi Pedro,

2015-08-15 0:24 GMT+02:00 Paolo Bonzini bonz...@gnu.org:
 There are plenty of targets that do not require -fPIC because they always
 generate position independent code, but none of them feels the need to
 complain with the user about an unnecessary but perfectly valid option,
 on each and every .c-.o rule, and without a way to disable it.

 Adding insult to injury, the warning is also wrong in 3 cases out of four.
 For 32-bit, warning may make a small amount of sense because there is
 simply no such thing as PIC code (Windows just uses a preferred base
 address for DLLs); but then the text is wrong.

 For 64-bit, -fpic does not warn, but all code is position independent
 would apply just as well!

Agreed.

 As a summary, googling for the warning text finds exactly two categories
 of people: those that are annoyed by the message because they want to
 use -Werror (this includes me), and those that suggest replacing -fPIC
 with -fpic.  Just zap it.

 Paolo

Patch is ok.  Please apply.

Thanks,
Kai

 * config/i386/cygming.h (SUBTARGET_OVERRIDE_OPTIONS): Do
 not warn.

 diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h
 index fda59fb..71506ec 100644
 --- a/gcc/config/i386/cygming.h
 +++ b/gcc/config/i386/cygming.h
 @@ -198,20 +198,7 @@ along with GCC; see the file COPYING3.  If not see
  #undef  SUBTARGET_OVERRIDE_OPTIONS
  #define SUBTARGET_OVERRIDE_OPTIONS \
  do {   \
 -  if (TARGET_64BIT  flag_pic != 1)   \
 -{  \
 -  if (flag_pic  1)  
   \
 -warning (0,\
 --fPIC ignored for target (all code is position 
 independent)\
 - );\
 -  flag_pic = 1;\
 -}  \
 -  else if (!TARGET_64BIT  flag_pic)  \
 -{  \
 -  warning (0, -f%s ignored for target (all code is position 
 independent),\
 -  (flag_pic  1) ? PIC : pic); \
 -  flag_pic = 0;\
 -}  \
 +  flag_pic = TARGET_64BIT ? 1 : 0;  \
  } while (0)

  /* Define this macro if references to a symbol must be treated

Re: C++ delayed folding branch review

[Kai Tietz]

2015-08-28 4:11 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 08/27/2015 02:12 PM, Kai Tietz wrote:

 +  else if (TREE_CODE (type) == VECTOR_TYPE)
 +{
 +  if (TREE_CODE (arg1) == VECTOR_CST
 +  code == NOP_EXPR
 +  TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
 +   {
 + tree r = copy_node (arg1);
 + TREE_TYPE (arg1) = type;
 + return r;
 +   }
 +}


 I would drop the check on 'code' and add a check that

  TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE (arg1))

 Does that still pass?

Yes, is still passes.  To check here for main-variant seems to be more
robust.  I commit it to branch, and will do complete
regression-testing for it.

 Jason

Kai

Re: C++ delayed folding branch review

[Kai Tietz]

2015-08-28 9:19 GMT+02:00 Kai Tietz ktiet...@googlemail.com:
 2015-08-28 4:11 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 08/27/2015 02:12 PM, Kai Tietz wrote:

 +  else if (TREE_CODE (type) == VECTOR_TYPE)
 +{
 +  if (TREE_CODE (arg1) == VECTOR_CST
 +  code == NOP_EXPR
 +  TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
 +   {
 + tree r = copy_node (arg1);
 + TREE_TYPE (arg1) = type;
 + return r;
 +   }
 +}


 I would drop the check on 'code' and add a check that

  TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE (arg1))

 Does that still pass?

 Yes, is still passes.  To check here for main-variant seems to be more
 robust.  I commit it to branch, and will do complete
 regression-testing for it.

Completed regression-testing.  No new regressions.

Kai

Re: C++ delayed folding branch review

[Kai Tietz]

2015-08-27 15:27 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 08/27/2015 06:39 AM, Kai Tietz wrote:

 2015-08-27 4:56 GMT+02:00 Jason Merrill ja...@redhat.com:

 On 08/24/2015 03:15 AM, Kai Tietz wrote:

 2015-08-03 17:39 GMT+02:00 Jason Merrill ja...@redhat.com:

 On 08/03/2015 05:42 AM, Kai Tietz wrote:

 2015-08-03 5:49 GMT+02:00 Jason Merrill ja...@redhat.com:

 On 07/31/2015 05:54 PM, Kai Tietz wrote:



 The STRIP_NOPS-requirement in 'reduced_constant_expression_p' I
 could
 remove, but for one case in constexpr.  Without folding we don't do
 type-sinking/raising.



 Right.

 So binary/unary operations might be containing cast, which were in
 the
 past unexpected.



 Why aren't the casts folded away?



 On such cast constructs, as for this vector-sample, we can't fold away



 Which testcase is this?



 It is the g++.dg/ext/vector20.C testcase.  IIRC I mentioned this
 testcase already earlier as reference, but I might be wrong here.



 I don't see any casts in that testcase.  So the compiler is introducing
 introducing conversions back and forth between const and non-const, then?
 I
 suppose it doesn't so much matter where they come from, they should be
 folded away regardless.


 The cast gets introduced in convert.c about line 836 in function
 convert_to_integer_1 AFAIK.  There should be the alternative solution
 for this issue by disallowing for PLUS/MINUS/... expressions the
 sinking of the cast into the expression, if dofold is false, and type
 has same width as inner_type, and is of vector-kind.


 Why would we be calling convert_to_integer for conversions between vector
 types?

 the cast chain.  The difference here to none-delayed-folding branch is
 that the cast isn't moved out of the plus-expr.  What we see now is
 (plus ((vec) (const vector ...) {  }), ...).  Before we had (vec)
 (plus (const vector ...) { ... }).



 How could a PLUS_EXPR be considered a reduced constant, regardless of
 where
 the cast is?



 Of course it is just possible to sink out a cast from PLUS_EXPR, in
 pretty few circumstance (eg. on constants if both types just differ in
 const-attribute, if conversion is no view-convert).



 I don't understand how this is an answer to my question.


 (vec) (const vector) { ... } expression can't be folded.


 It currently isn't folded, but why can't we change that?

 This cast to
 none-const variant happens due the 'constexpr v = v +
 constant-value' pattern in testcase.  v is still of type vec, even
 if function itself is constexpr.


 I don't see that pattern in the testcase:

 typedef long vec __attribute__((vector_size (2 * sizeof (long;
 constexpr vec v = { 3, 4 };
 constexpr vec s = v + v;
 constexpr vec w = __builtin_shuffle (v, v);

 If we have v + constant-value, that's because we pulled out the constant
 value of one of the v's, which we ought to be doing for both of them.

 On verify_constant we check by reduced_constant_expression_p, if
 value
 is
 a constant.  We don't handle here, that NOP_EXPRs are something we
 want to
 look through here, as it doesn't change anything if this is a
 constant, or
 not.



 NOPs around constants should have been folded away by the time we get
 there.



 Not in this cases, as the we actually have here a switch from const to
 none-const.  So there is an attribute-change, which we can't ignore in
 general.



 I wasn't suggesting we ignore it, we should be able to change the type
 of
 the vector_cst.



 Well, the vector_cst we can change type, but this wouldn't help
 AFAICS.  As there is still one cast surviving within PLUS_EXPR for the
 other operand.



 Isn't the other operand also constant?  In constexpr evaluation, either
 we're dealing with a bunch of constants, in which case we should be
 folding
 things fully, including conversions between const and non-const, or we
 don't
 care.


 No other operand isn't a constant-value.  See code-pattern in
 testcase.  It is of type 'vec', which isn't constant (well, 'v' is,
 but constexpr doesn't know about it).


 What do you mean, constexpr doesn't know about it?

 So the way to solve it would be to move such conversion out of the
 expression.  For integer-scalars we do this, and for some
 floating-points too.  So it might be something we don't handle for
 operations with vector-type.



 We don't need to worry about that in constexpr evaluation, since we only
 care about constant operands.


 Sure, but the variable 'v' is the problem, not a constant-value itself.


 But I agree that for constexpr's we could special case cast
 from const to none-const (as required in expressions like const vec v
 = v + 1).



 Right.  But really this should happen in convert.c, it shouldn't be
 specific
 to C++.



 Hmm, maybe.  But isn't one of our different goals to move such
 implicit code-modification to match.pd instead?


 Folding const into a constant is hardly code modification.  But perhaps
 it
 should go into fold_unary_loc:VIEW_CONVERT_EXPR rather than into
 convert.c.


 Hmm

Re: C++ delayed folding branch review

[Kai Tietz]

With following patch testcase passes:

Index: fold-const.c
===
--- fold-const.c(Revision 227111)
+++ fold-const.c(Arbeitskopie)
@@ -2110,6 +2110,17 @@ fold_convert_const (enum tree_code code, tree type
   else if (TREE_CODE (arg1) == REAL_CST)
return fold_convert_const_fixed_from_real (type, arg1);
 }
+  else if (TREE_CODE (type) == VECTOR_TYPE)
+{
+  if (TREE_CODE (arg1) == VECTOR_CST
+  code == NOP_EXPR
+  TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
+   {
+ tree r = copy_node (arg1);
+ TREE_TYPE (arg1) = type;
+ return r;
+   }
+}
   return NULL_TREE;
 }
Index: cp/constexpr.c
===
--- constexpr.c (Revision 227111)
+++ constexpr.c (Arbeitskopie)
@@ -1441,8 +1441,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx
 bool
 reduced_constant_expression_p (tree t)
 {
-  /* Make sure we remove useless initial NOP_EXPRs.  */
-  STRIP_NOPS (t);
   switch (TREE_CODE (t))
 {
 case PTRMEM_CST:

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-27 12:34 GMT+02:00 Richard Biener richard.guent...@gmail.com:
 On Thu, Aug 27, 2015 at 11:21 AM, Kai Tietz ktiet...@googlemail.com wrote:
 2015-08-27 4:57 GMT+02:00 Jason Merrill ja...@redhat.com:
 Why does fold_simple fold so many patterns?  I thought we wanted something
 that would just fold conversions and negations of constant values.

 Yes, initial variant was handling much less patterns.  But actually we
 need for functions (eg. like build_vec_init in init.c) a simple
 routine to perform basic constant-value arithmetics (sizeof * / + -
 trunc, etc) to avoid call of maybe_constant_value.  Also for
 overflow-diagnostics we want at least to resolve such simple patterns
 for constant-values only.  We could change those calls to use
 maybe_constant_value instead, but the overhead (and some of its
 folding) leads much further then working on constant-values only (as
 fold_simple does).

 It might be that we can remove the ternary vector-cond expression from
 this routine, The cond-expr itself seems to be necessary to resolve
 patterns like (1 == 1 ? 32 : 64), which can appear pretty often via
 macro-code.  I will check if I what patterns I can remove here.

 Note that fold-const.c has constant-only folding routines (handling only
 constant operands).  const_unop, const_binop (no const_ternop split
 out yet).

 Richard.

Thanks for the point.  I will take a look into cons_unop/binop.  I
just expect that this routines would fail if they get c++-expressions,
aren't they?

 Jason

Kai

Re: C++ delayed folding branch review

[Kai Tietz]

2015-08-27 4:56 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 08/24/2015 03:15 AM, Kai Tietz wrote:

 2015-08-03 17:39 GMT+02:00 Jason Merrill ja...@redhat.com:

 On 08/03/2015 05:42 AM, Kai Tietz wrote:

 2015-08-03 5:49 GMT+02:00 Jason Merrill ja...@redhat.com:

 On 07/31/2015 05:54 PM, Kai Tietz wrote:


 The STRIP_NOPS-requirement in 'reduced_constant_expression_p' I
 could
 remove, but for one case in constexpr.  Without folding we don't do
 type-sinking/raising.


 Right.

 So binary/unary operations might be containing cast, which were in the
 past unexpected.


 Why aren't the casts folded away?


 On such cast constructs, as for this vector-sample, we can't fold away


 Which testcase is this?


 It is the g++.dg/ext/vector20.C testcase.  IIRC I mentioned this
 testcase already earlier as reference, but I might be wrong here.


 I don't see any casts in that testcase.  So the compiler is introducing
 introducing conversions back and forth between const and non-const, then?  I
 suppose it doesn't so much matter where they come from, they should be
 folded away regardless.

The cast gets introduced in convert.c about line 836 in function
convert_to_integer_1 AFAIK.  There should be the alternative solution
for this issue by disallowing for PLUS/MINUS/... expressions the
sinking of the cast into the expression, if dofold is false, and type
has same width as inner_type, and is of vector-kind.

 the cast chain.  The difference here to none-delayed-folding branch is
 that the cast isn't moved out of the plus-expr.  What we see now is
 (plus ((vec) (const vector ...) {  }), ...).  Before we had (vec)
 (plus (const vector ...) { ... }).


 How could a PLUS_EXPR be considered a reduced constant, regardless of
 where
 the cast is?


 Of course it is just possible to sink out a cast from PLUS_EXPR, in
 pretty few circumstance (eg. on constants if both types just differ in
 const-attribute, if conversion is no view-convert).


 I don't understand how this is an answer to my question.

(vec) (const vector) { ... } expression can't be folded.  This cast to
none-const variant happens due the 'constexpr v = v +
constant-value' pattern in testcase.  v is still of type vec, even
if function itself is constexpr.

 On verify_constant we check by reduced_constant_expression_p, if value
 is
 a constant.  We don't handle here, that NOP_EXPRs are something we
 want to
 look through here, as it doesn't change anything if this is a
 constant, or
 not.


 NOPs around constants should have been folded away by the time we get
 there.


 Not in this cases, as the we actually have here a switch from const to
 none-const.  So there is an attribute-change, which we can't ignore in
 general.


 I wasn't suggesting we ignore it, we should be able to change the type of
 the vector_cst.


 Well, the vector_cst we can change type, but this wouldn't help
 AFAICS.  As there is still one cast surviving within PLUS_EXPR for the
 other operand.


 Isn't the other operand also constant?  In constexpr evaluation, either
 we're dealing with a bunch of constants, in which case we should be folding
 things fully, including conversions between const and non-const, or we don't
 care.

No other operand isn't a constant-value.  See code-pattern in
testcase.  It is of type 'vec', which isn't constant (well, 'v' is,
but constexpr doesn't know about it).

The bogus error-message happens in:

#1  0x00668c20 in verify_constant (t=t@entry=0xffd3cbe8,
allow_non_constant=optimized out,
non_constant_p=non_constant_p@entry=0xe5fa6fa,
overflow_p=overflow_p@entry=0xe5fa6fb)
at ../../src/gcc/cp/constexpr.c:1480
#2  0x0066c710 in cxx_eval_binary_expression (overflow_p=0xe5fa6fb,
non_constant_p=0xe5fa6fa, t=0xffd3cba0, ctx=0xe5fa6fc)
at ../../src/gcc/cp/constexpr.c:1620
#3  cxx_eval_constant_expression (ctx=ctx@entry=0xe5fa6fc,
t=t@entry=0xffd3cba0, lval=lval@entry=false,
non_constant_p=non_constant_p@entry=0xe5fa6fa,
overflow_p=overflow_p@entry=0xe5fa6fb, jump_target=jump_target@entry=0x0)
at ../../src/gcc/cp/constexpr.c:3491

#2  0x0066c710 in cxx_eval_binary_expression (overflow_p=0xe5fa6fb,
non_constant_p=0xe5fa6fa, t=0xffd3cba0, ctx=0xe5fa6fc)
at ../../src/gcc/cp/constexpr.c:1620
1620VERIFY_CONSTANT (lhs);

 So the way to solve it would be to move such conversion out of the
 expression.  For integer-scalars we do this, and for some
 floating-points too.  So it might be something we don't handle for
 operations with vector-type.


 We don't need to worry about that in constexpr evaluation, since we only
 care about constant operands.

Sure, but the variable 'v' is the problem, not a constant-value itself.

 But I agree that for constexpr's we could special case cast
 from const to none-const (as required in expressions like const vec v
 = v + 1).


 Right.  But really this should happen in convert.c, it shouldn't be
 specific
 to C++.


 Hmm, maybe.  But isn't one of our different goals to move

Re: [c++-delayed-folding] fold_simple

[Kai Tietz]

2015-08-27 4:57 GMT+02:00 Jason Merrill ja...@redhat.com:
 Why does fold_simple fold so many patterns?  I thought we wanted something
 that would just fold conversions and negations of constant values.

Yes, initial variant was handling much less patterns.  But actually we
need for functions (eg. like build_vec_init in init.c) a simple
routine to perform basic constant-value arithmetics (sizeof * / + -
trunc, etc) to avoid call of maybe_constant_value.  Also for
overflow-diagnostics we want at least to resolve such simple patterns
for constant-values only.  We could change those calls to use
maybe_constant_value instead, but the overhead (and some of its
folding) leads much further then working on constant-values only (as
fold_simple does).

It might be that we can remove the ternary vector-cond expression from
this routine, The cond-expr itself seems to be necessary to resolve
patterns like (1 == 1 ? 32 : 64), which can appear pretty often via
macro-code.  I will check if I what patterns I can remove here.

 Jason


Kai

Re: C++ delayed folding branch review

[Kai Tietz]

Hello Jason,

after a longer delay the answer to your question.

2015-08-03 17:39 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 08/03/2015 05:42 AM, Kai Tietz wrote:

 2015-08-03 5:49 GMT+02:00 Jason Merrill ja...@redhat.com:

 On 07/31/2015 05:54 PM, Kai Tietz wrote:


 The STRIP_NOPS-requirement in 'reduced_constant_expression_p' I could
 remove, but for one case in constexpr.  Without folding we don't do
 type-sinking/raising.



 Right.

 So binary/unary operations might be containing cast, which were in the
 past unexpected.


 Why aren't the casts folded away?


 On such cast constructs, as for this vector-sample, we can't fold away


 Which testcase is this?

It is the g++.dg/ext/vector20.C testcase.  IIRC I mentioned this
testcase already earlier as reference, but I might be wrong here.

 the cast chain.  The difference here to none-delayed-folding branch is
 that the cast isn't moved out of the plus-expr.  What we see now is
 (plus ((vec) (const vector ...) {  }), ...).  Before we had (vec)
 (plus (const vector ...) { ... }).


 How could a PLUS_EXPR be considered a reduced constant, regardless of where
 the cast is?

Of course it is just possible to sink out a cast from PLUS_EXPR, in
pretty few circumstance (eg. on constants if both types just differ in
const-attribute, if conversion is no view-convert).

 On verify_constant we check by reduced_constant_expression_p, if value
 is
 a constant.  We don't handle here, that NOP_EXPRs are something we want
 to
 look through here, as it doesn't change anything if this is a constant,
 or
 not.


 NOPs around constants should have been folded away by the time we get
 there.


 Not in this cases, as the we actually have here a switch from const to
 none-const.  So there is an attribute-change, which we can't ignore in
 general.


 I wasn't suggesting we ignore it, we should be able to change the type of
 the vector_cst.

Well, the vector_cst we can change type, but this wouldn't help
AFAICS.  As there is still one cast surviving within PLUS_EXPR for the
other operand.
So the way to solve it would be to move such conversion out of the
expression.  For integer-scalars we do this, and for some
floating-points too.  So it might be something we don't handle for
operations with vector-type.

 But I agree that for constexpr's we could special case cast
 from const to none-const (as required in expressions like const vec v
 = v + 1).


 Right.  But really this should happen in convert.c, it shouldn't be specific
 to C++.

Hmm, maybe.  But isn't one of our different goals to move such
implicit code-modification to match.pd instead?

 Jason



Kai

Re: C++ delayed folding branch review

[Kai Tietz]

2015-08-03 5:49 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 07/31/2015 05:54 PM, Kai Tietz wrote:

 The STRIP_NOPS-requirement in 'reduced_constant_expression_p' I could
 remove, but for one case in constexpr.  Without folding we don't do
 type-sinking/raising.


 Right.

 So binary/unary operations might be containing cast, which were in the
 past unexpected.


 Why aren't the casts folded away?

On such cast constructs, as for this vector-sample, we can't fold away
the cast chain.  The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr.  What we see now is
(plus ((vec) (const vector ...) {  }), ...).  Before we had (vec)
(plus (const vector ...) { ... }).

 On verify_constant we check by reduced_constant_expression_p, if value is
 a constant.  We don't handle here, that NOP_EXPRs are something we want to
 look through here, as it doesn't change anything if this is a constant, or
 not.


 NOPs around constants should have been folded away by the time we get there.

Not in this cases, as the we actually have here a switch from const to
none-const.  So there is an attribute-change, which we can't ignore in
general.  But I agree that for constexpr's we could special case cast
from const to none-const (as required in expressions like const vec v
= v + 1).

 Jason


Kai

Re: C++ delayed folding branch review

[Kai Tietz]

2015-07-31 18:14 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 07/30/2015 10:48 PM, Jeff Law wrote:

 Note, anything outside of the C/C++ front-ends depending on that
 canonicalization done by shorten_compare is, IMHO, broken.


 I think the OMP code isn't relying on it being done by shorten_compare; it's
 trying to do it itself in c_finish_omp_for but is somehow thwarted by
 delayed folding.

 Jason


Well, this seems to be reasoned by finish_omp_for (), which gets
invoked in parser.c cp_parser_omp_for_loop, and/or pt.c: tsubst_expr.
In all those cases arguments aren't folded anymore.  So
c_finish_omp_for's patterns don't match anymore.  I guess we might
want to do this cannonical form in genericize-pass?

Kai

Re: C++ delayed folding branch review

[Kai Tietz]

- Ursprüngliche Mail -
 On 07/31/2015 12:46 PM, Jakub Jelinek wrote:
  On Fri, Jul 31, 2015 at 06:25:57PM +0200, Kai Tietz wrote:
  2015-07-31 18:14 GMT+02:00 Jason Merrill ja...@redhat.com:
  On 07/30/2015 10:48 PM, Jeff Law wrote:
 
  Note, anything outside of the C/C++ front-ends depending on that
  canonicalization done by shorten_compare is, IMHO, broken.
 
  I think the OMP code isn't relying on it being done by shorten_compare;
  it's
  trying to do it itself in c_finish_omp_for but is somehow thwarted by
  delayed folding.
 
  Well, this seems to be reasoned by finish_omp_for (), which gets
  invoked in parser.c cp_parser_omp_for_loop, and/or pt.c: tsubst_expr.
  In all those cases arguments aren't folded anymore.  So
  c_finish_omp_for's patterns don't match anymore.  I guess we might
  want to do this cannonical form in genericize-pass?
 
  Or just fold in finish_omp_for before calling c_finish_omp_for, so that it
  is in the expected form?
 
 That certainly sounds simpler.
 
 Jason


Well, it sounds easier. We need to do here this special COND-folding (means 
operands only), and pass this into c_finish_omp.  Of course we should just fold 
OMP_FOR's COND part.  I will try a patch for this-

Kai

Re: C++ delayed folding branch review

[Kai Tietz]

- Ursprüngliche Mail -
 On 07/30/2015 05:00 PM, Kai Tietz wrote:
  2015-07-30 18:52 GMT+02:00 Jason Merrill ja...@redhat.com:
  On 07/29/2015 06:56 PM, Kai Tietz wrote:
  @@ -13078,6 +13042,8 @@ build_enumerator (tree name, tree value,
  tree
  enumtype, tree attributes,
 if (value)
   STRIP_TYPE_NOPS (value);
 
  +  if (value)
  +value = cp_try_fold_to_constant (value);
 
 
  Again, this is unnecessary because we call cxx_constant_value
  below.
 
 
  See nops, and other unary-operations we want to reduce here to real
  constant value ...
 
 
  The cxx_constant_value call below will deal with them.
 
 
  Likewise for grokbitfield.
 
 
  Hmm, AFAIR we don't call cxx_constant_value in all code-paths.  But I
  will look into it, and come back to you on it.
 
 
  I am still on it ...  first did the other points
 
 
  Looks like this hasn't changed.
 
 
  Yes, for grokbitfield current version uses fold_simple for witdth.  So
  just expressions based on constants getting reduced to short form.  In
  grokbitfield I don't see invocation of cxx_constant_value.  So how can
  we be sure that width is reduced to integer-cst?
 
 
  We call cxx_constant_value on bit-field widths in check_bitfield_decl.
 
  Hmm, ok.  But I don't see that this function gets called in context of
  grokbitfield, after we set DECL_INITIAL.
 
 Nope, it's called later on as part of finish_struct.
 

Ok, adjusted.

  By removing this folding here, we get new failures in
  g++.dg/warn/overflow-warn-1.C testcase:
  New errors are at lin 32 that 'bif-foeld 's::anonymous' width not an
  integer constant'
  and at same line ''(1 / 0) is not a constant expression'.  Those
  message don't look wrong.
 
  The testcase next to this 'overflow-warn-3.C and overflow-warn-4.C'
  failing in the same manner for (1 / 0) case.  But there are no other
  regressions in g++.dg  libstdc++
 
  Shall I extend the testcases for this message?
 
 Please.

Done.
 
  @@ -1947,6 +1947,8 @@ build_complex (tree type, tree real, tree
  imag)
   {
 tree t = make_node (COMPLEX_CST);
 
  +  real = fold (real);
  +  imag = fold (imag);
 
 
  I still think this is wrong.  The arguments should be sufficiently
  folded.
 
 
  As we don't fold unary-operators on constants, we need to fold it
  at
  some place.  AFAICS is the C++ FE not calling directly
  build_complex.
  So this place was the easiest way to avoid issues with things like
  '-'
  '1' etc.
 
 
  Is this because of the
 
 
   value = build_complex (NULL_TREE, convert (const_type,
 
  integer_zero_node),
  value);
 
 
  Might be.  This should be indeed a 'fold_convert', isn't it?
 
 
  Yes.
 
 
  Applied modification to it.
 
 
  So can we remove the fold in build_complex now?
 

Yes. Done.

 
  in interpret_float?  I think convert definitely needs to do some
  folding, since it's called from middle-end code that expects that.
 
 
  I remember talking about convert doing some folding (and cp_convert
  not) in our 1:1 last week.
 
 
  Can't remember that.  I know that we were talking about the difference
  of convert and fold_convert.  convert can be used on C++ specifics,
  but fold_convert is something shared with ME.
 
 
  convert is called from the ME, which sometimes expects folding.
 
  So first 'fold_convert'
  isn't the same as 'fold (convert ())'.
  I don't find places we invoke convert () in ME.  We have some calls in
  convert.c (see convert_to_integer, convert_to_integer_nofold, and
  convert_to_real), which all used in AST only AFAICS.
 
 
  I was thinking of convert.c and fold-const.c to be part of the ME, since
  they are language-independent.  But I guess other people think of the ME
  starting with gimple.
 
  And it looks like the only language-independent uses of convert are in
  c-family; I guess many of them should change to fold_convert.
 
 
  Hmm, in context of this work? Or is this more a general point about
  future
  work?
 
 
  In the context of this work, if they are introducing problematic NOPs.
 
  Ok, I will take a closer look to convert () usage in c-family/.  By
  quick looking this seems to be the only place for now we needed to
  change.
 
  @@ -5080,6 +5081,7 @@ output_constructor_bitfield (oc_local_state
  *local,
  unsigned int bit_offset)
 while (TREE_CODE (local-val) == VIEW_CONVERT_EXPR
   || TREE_CODE (local-val) == NON_LVALUE_EXPR)
   local-val = TREE_OPERAND (local-val, 0);
  +  local-val = fold (local-val);
 
 
  Likewise.
 
 
  As soon as we can be sure that values getting fully_folded, or at
  least folded for constants, we should be able to remove this.
 
 
  Yep, they need to be folded before we get here.
 
 
  I didn't come to remove this line for testing.  As we fold now for
  initializers more early, and cp_fold supports constructors, it could
  be that we don't need this anymore.  It is on my pile.
 
 
  That fold is still required.  By removing it, I saw boostrap issue due
  'invalid

Re: C++ delayed folding branch review

[Kai Tietz]

2015-07-30 0:56 GMT+02:00 Kai Tietz ktiet...@googlemail.com:
 2015-07-29 19:48 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 07/28/2015 04:10 PM, Kai Tietz wrote:
 The change to adjust_temp_type seems to be no more necessary (just
 doing tests on it).

Yes, committed it.


 @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
 constexpr_ctx
 *ctx, tree t,
case CONVERT_EXPR:
case VIEW_CONVERT_EXPR:
case NOP_EXPR:
 +case UNARY_PLUS_EXPR:
  {
 +   enum tree_code tcode = TREE_CODE (t);
   tree oldop = TREE_OPERAND (t, 0);
 +
 +   if (tcode == NOP_EXPR  TREE_TYPE (t) == TREE_TYPE (oldop)
 
 TREE_OVERFLOW_P (oldop))
 + {
 +   if (!ctx-quiet)
 + permerror (input_location, overflow in constant
 expression);
 +   /* If we're being permissive (and are in an enforcing
 +   context), ignore the overflow.  */
 +   if (!flag_permissive)
 + *overflow_p = true;
 +   *non_constant_p = true;
 +
 +   return t;
 + }
   tree op = cxx_eval_constant_expression (ctx, oldop,



 Why doesn't the call to cxx_eval_constant_expression at the bottom
 here
 handle oldop having TREE_OVERFLOW set?



 I just handled the case that we see here a wrapping NOP_EXPR around an
 overflow.  As this isn't handled by cxx_eval_constant_expression.



 How does it need to be handled?  A NOP_EXPR wrapped around an overflow
 is there to indicated that the expression is non-constant, and it can't
 be simplified any farther.

 Please give an example of what was going wrong.

 ^

 I did some regression-testing on it.  This looks to me like something
 I missed to cleanup.  Most changes within constexpr-code aren't
 necessary anymore.  But looking on that, I think I papered over some
 issues I had about double-reporting of non-constant expression on
 overflows.

Committed change to branch for removing this.

 @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq
 *pre_p,
 gimple_seq *post_p)

  switch (code)
{
 +case SIZEOF_EXPR:
 +  if (SIZEOF_EXPR_TYPE_P (*expr_p))
 +   *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE
 (TREE_OPERAND
 (*expr_p,
 +
 0)),
 + SIZEOF_EXPR, false);
 +  else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
 +   *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
 0),
 + SIZEOF_EXPR, false);
 +  else
 +   *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
 0),
 + SIZEOF_EXPR, false);
 +  if (*expr_p == error_mark_node)
 +   *expr_p = size_one_node;
 +
 +  *expr_p = maybe_constant_value (*expr_p);
 +  ret = GS_OK;
 +  break;



 Why are these surviving until gimplification time?


 This might be still necessary. I will retest, when bootstrap works.
 As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
 expressions a sizeof can occure, this shouldn't be necessary anymore.
 AFAIR I saw here some issues about initialzation for global-variables,
 which weren't caught.



 Hmm, I wonder why you would see issues with global initializers that
 aren't seen on trunk?  In any case, if the issue is with global
 initializers, they should be handled sooner, not here.


 They don't survice in function-context, but outside they might.  On
 trunk we never will see an sizeof-expression in such case as they got
 folded-away much earlier.

 I will try an bootstrap with disabling it.  In ME we don't produce
 sizeof-expressions anymore, so we don't need to think about
 re-gimplifiying some AST AFAICS.

Tested.  We seems not to need the handle of SIZEOF_EXPR in gimplifier
anymore.  so removed hunk.


 @@ -1529,8 +1532,11 @@ build_expr_type_conversion (int desires, tree
 expr,
 bool complain)
  tree basetype = TREE_TYPE (expr);
  tree conv = NULL_TREE;
  tree winner = NULL_TREE;
 +  /* Want to see if EXPR is a constant.  See below checks for
 null_node.
 */
 +  tree expr_folded = cp_try_fold_to_constant (expr);

 -  if (expr == null_node
 +  STRIP_NOPS (expr_folded);
 +  if (expr_folded == null_node



 Again, we shouldn't need to fold to check for null_node, it only
 occurs
 when explicitly written.  Folding should never produce null_node
 unless
 the argument was already null_node.



 Well, we need to do this for diagnostic messages AFAIR.  We want to
 see if expression folded gets a constant, so that diagnostics getting
 displayed right.



 Again, null_node is special.  It indicates that the user typed
 __null.
 That's what we're checking for here.  Folding is both unnecessary and
 undesirable.


 So, let us remove it ...  I expect issues about casts on integers,
 which are reasoned due implicit assignments, expr won't be null_node.

I was wrong about this.  I removed the folding from
build_expr_type_conversion routine.


 @@ -1548,7 +1554,7

Re: C++ delayed folding branch review

[Kai Tietz]

2015-07-30 18:52 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 07/29/2015 06:56 PM, Kai Tietz wrote:

 @@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const
 constexpr_ctx
 *ctx,
 tree t,
 bool
 reduced_constant_expression_p (tree t)
 {
 +  /* Make sure we remove useless initial NOP_EXPRs.  */
 +  STRIP_NOPS (t);


 Checked, and removing those STRIP_NOPS cause regressions about
 vector-casts.  At least the STRIP_NOPS in
 reduced_constant_expression_p seems to be required.  See as example
 g++.dg/ext/vector20.C as testcase.
 It sees that '(vec)(const __vector(2) long int){3l, 4l}' is not a
 constant expression.


 But when was that NOP_EXPR added?  It should have been folded away before we
 get here.

See below for this.  This might be related to the store_init_value issue.


 case SIZEOF_EXPR:
 +  if (processing_template_decl
 +  (!COMPLETE_TYPE_P (TREE_TYPE (t))
 + || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) !=
 INTEGER_CST))
 +   return t;


 Why is this necessary?


 The issue is that by delayed-folding we don't fold sizeof-expressions
 until we do the folding after genericize-pass.  So those expressions
 remain, and we can run in template on sizeof-operators on incomplete
 types, if we invoke here variants of the constexpr-code.  So this
 pattern simply verifies that the sizeof-operand can be determined.  We
 could simply avoid resolving sizeof-operators in template-decl at all.
 But my idea here was to try to resolve them, if the type of the
 operand is already complete (and has an constant size).


 But this condition will never be true, as TREE_TYPE (t) is always size_t.
 So this code isn't actually addressing the situation you describe.

Hmm, right.  sizeof's type is always size_t.  Its operand would make
here a difference, but this I don't check.  I will remove it and test.

 We don't want to resolve SIZEOF_EXPR within template-declarations
 for
 incomplete types, of if its size isn't fixed.  Issue is that we
 otherwise get issues about expressions without existing type (as
 usual
 within template-declarations for some expressions).


 Yes, but we shouldn't have gotten this far with a dependent sizeof;
 maybe_constant_value just returns if
 instantiation_dependent_expression_p is true.


 Well, but we could come here by other routine then
 maybe_constant_value. For example cxx_constnat_value doesn't do checks
 here.


 Calling cxx_constant_value on a dependent expression will tend to ICE, so we
 don't need to worry about that.

 @@ -8496,16 +8467,18 @@ compute_array_index_type (tree name, tree
 size,
 tsubst_flags_t complain)
   SET_TYPE_STRUCTURAL_EQUALITY (itype);
   return itype;
 }
 -
 +
 +  /* We need to do fully folding to determine if we have VLA, or
 not.  */
 +  tree size_constant = cp_try_fold_to_constant (size);


 Again, we already called maybe_constant_value.


 Sure, but maybe_constant_value still produces nops ...


 If someone tries to create an array with a size that involves
 arithmetic
 overflow, that's undefined behavior and we should probably give an
 error rather than fold it away.


 If we need to do some reduction to constant value here, as expr might
 be actually a constant, which isn't folded here.  Eg something like:
 struct {
char abc[sizeof (int) * 8];
 };
 Due delayed folding array index isn't necessarily reduced here.  So we
 need to perform at least constant value folding for diagnostics, as we
 do right now.


 Yes, we need to do some folding, that's why we call maybe_constant_value!

 @@ -13078,6 +13042,8 @@ build_enumerator (tree name, tree value,
 tree
 enumtype, tree attributes,
   if (value)
 STRIP_TYPE_NOPS (value);

 +  if (value)
 +value = cp_try_fold_to_constant (value);


 Again, this is unnecessary because we call cxx_constant_value
 below.


 See nops, and other unary-operations we want to reduce here to real
 constant value ...


 The cxx_constant_value call below will deal with them.


 Likewise for grokbitfield.


 Hmm, AFAIR we don't call cxx_constant_value in all code-paths.  But I
 will look into it, and come back to you on it.


 I am still on it ...  first did the other points


 Looks like this hasn't changed.


 Yes, for grokbitfield current version uses fold_simple for witdth.  So
 just expressions based on constants getting reduced to short form.  In
 grokbitfield I don't see invocation of cxx_constant_value.  So how can
 we be sure that width is reduced to integer-cst?


 We call cxx_constant_value on bit-field widths in check_bitfield_decl.

Hmm, ok.  But I don't see that this function gets called in context of
grokbitfield, after we set DECL_INITIAL.
By removing this folding here, we get new failures in
g++.dg/warn/overflow-warn-1.C testcase:
New errors are at lin 32 that 'bif-foeld 's::anonymous' width not an
integer constant'
and at same line ''(1 / 0) is not a constant expression'.  Those
message don't look wrong.

The testcase next to this 'overflow

Re: C++ delayed folding branch review

[Kai Tietz]

2015-07-29 19:48 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 07/28/2015 04:10 PM, Kai Tietz wrote:

 2015-07-28 1:14 GMT+02:00 Kai Tietz ktiet...@googlemail.com:

 2015-07-27 18:51 GMT+02:00 Jason Merrill ja...@redhat.com:

 I've trimmed this to the previously mentioned issues that still need to
 be
 addressed; I'll do another full review after these are dealt with.


 Thanks for doing this summary of missing parts of prior review.

 On 06/13/2015 12:15 AM, Jason Merrill wrote:


 On 06/12/2015 12:11 PM, Kai Tietz wrote:


 @@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
{
  if (TREE_TYPE (temp) == type)
return temp;
 +  STRIP_NOPS (temp);
 +  if (TREE_TYPE (temp) == type)
 +return temp;
 @@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const constexpr_ctx
 *ctx,
 tree t,
bool
reduced_constant_expression_p (tree t)
{
 +  /* Make sure we remove useless initial NOP_EXPRs.  */
 +  STRIP_NOPS (t);

 ^


Checked, and removing those STRIP_NOPS cause regressions about
vector-casts.  At least the STRIP_NOPS in
reduced_constant_expression_p seems to be required.  See as example
g++.dg/ext/vector20.C as testcase.
It sees that '(vec)(const __vector(2) long int){3l, 4l}' is not a
constant expression.

The change to adjust_temp_type seems to be no more necessary (just
doing tests on it).

 @@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const
 constexpr_ctx
 *ctx, tree t,
  is_dummy_object (x))
   {
 x = ctx-object;
 - x = cp_build_addr_expr (x, tf_warning_or_error);
 + if (x)
 +   x = cp_build_addr_expr (x, tf_warning_or_error);
 + else
 +   x = get_nth_callarg (t, i);



 This still should not be necessary.

Replaced the x = get_nth_callarg (t,i);  by a gcc_unreachable ();,
just to be sure we hit issue, if occures.



 Yeah, most likely.  But I got initially here some issues, so I don't
 see that this code would worsen things.



 If this code path is hit, that means something has broken my design,
 and
 I don't want to just paper over that.  Please revert this change.


 ^

case SIZEOF_EXPR:
 +  if (processing_template_decl
 +  (!COMPLETE_TYPE_P (TREE_TYPE (t))
 + || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST))
 +   return t;



 Why is this necessary?

The issue is that by delayed-folding we don't fold sizeof-expressions
until we do the folding after genericize-pass.  So those expressions
remain, and we can run in template on sizeof-operators on incomplete
types, if we invoke here variants of the constexpr-code.  So this
pattern simply verifies that the sizeof-operand can be determined.  We
could simply avoid resolving sizeof-operators in template-decl at all.
But my idea here was to try to resolve them, if the type of the
operand is already complete (and has an constant size).



 We don't want to resolve SIZEOF_EXPR within template-declarations for
 incomplete types, of if its size isn't fixed.  Issue is that we
 otherwise get issues about expressions without existing type (as usual
 within template-declarations for some expressions).



 Yes, but we shouldn't have gotten this far with a dependent sizeof;
 maybe_constant_value just returns if
 instantiation_dependent_expression_p is true.

 ^

Well, but we could come here by other routine then
maybe_constant_value. For example cxx_constnat_value doesn't do checks
here.


 @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
 constexpr_ctx
 *ctx, tree t,
case CONVERT_EXPR:
case VIEW_CONVERT_EXPR:
case NOP_EXPR:
 +case UNARY_PLUS_EXPR:
  {
 +   enum tree_code tcode = TREE_CODE (t);
   tree oldop = TREE_OPERAND (t, 0);
 +
 +   if (tcode == NOP_EXPR  TREE_TYPE (t) == TREE_TYPE (oldop)
 
 TREE_OVERFLOW_P (oldop))
 + {
 +   if (!ctx-quiet)
 + permerror (input_location, overflow in constant
 expression);
 +   /* If we're being permissive (and are in an enforcing
 +   context), ignore the overflow.  */
 +   if (!flag_permissive)
 + *overflow_p = true;
 +   *non_constant_p = true;
 +
 +   return t;
 + }
   tree op = cxx_eval_constant_expression (ctx, oldop,



 Why doesn't the call to cxx_eval_constant_expression at the bottom
 here
 handle oldop having TREE_OVERFLOW set?



 I just handled the case that we see here a wrapping NOP_EXPR around an
 overflow.  As this isn't handled by cxx_eval_constant_expression.



 How does it need to be handled?  A NOP_EXPR wrapped around an overflow
 is there to indicated that the expression is non-constant, and it can't
 be simplified any farther.

 Please give an example of what was going wrong.

 ^

I did some regression-testing on it.  This looks to me like something
I missed to cleanup.  Most changes within constexpr-code aren't
necessary anymore.  But looking on that, I think I papered over some
issues I had

Re: C++ delayed folding branch review

[Kai Tietz]

2015-07-28 1:14 GMT+02:00 Kai Tietz ktiet...@googlemail.com:
 2015-07-27 18:51 GMT+02:00 Jason Merrill ja...@redhat.com:
 I've trimmed this to the previously mentioned issues that still need to be
 addressed; I'll do another full review after these are dealt with.

 Thanks for doing this summary of missing parts of prior review.

 On 06/13/2015 12:15 AM, Jason Merrill wrote:

 On 06/12/2015 12:11 PM, Kai Tietz wrote:

 @@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
   {
 if (TREE_TYPE (temp) == type)
   return temp;
 +  STRIP_NOPS (temp);
 +  if (TREE_TYPE (temp) == type)
 +return temp;
 @@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const constexpr_ctx
 *ctx,
 tree t,
   bool
   reduced_constant_expression_p (tree t)
   {
 +  /* Make sure we remove useless initial NOP_EXPRs.  */
 +  STRIP_NOPS (t);


 Within the constexpr code we should be folding away NOPs as they are
 generated, they shouldn't live this long.


 Well, we might see them on overflows ...


 We shouldn't within the constexpr code.  NOPs for expressions that are
 non-constant due to overflow are added in
 cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
 of constexpr evaluation.

 @@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
 *ctx, tree t,
 is_dummy_object (x))
  {
x = ctx-object;
 - x = cp_build_addr_expr (x, tf_warning_or_error);
 + if (x)
 +   x = cp_build_addr_expr (x, tf_warning_or_error);
 + else
 +   x = get_nth_callarg (t, i);


 This still should not be necessary.


 Yeah, most likely.  But I got initially here some issues, so I don't
 see that this code would worsen things.


 If this code path is hit, that means something has broken my design, and
 I don't want to just paper over that.  Please revert this change.

   case SIZEOF_EXPR:
 +  if (processing_template_decl
 +  (!COMPLETE_TYPE_P (TREE_TYPE (t))
 + || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST))
 +   return t;


 Why is this necessary?


 We don't want to resolve SIZEOF_EXPR within template-declarations for
 incomplete types, of if its size isn't fixed.  Issue is that we
 otherwise get issues about expressions without existing type (as usual
 within template-declarations for some expressions).


 Yes, but we shouldn't have gotten this far with a dependent sizeof;
 maybe_constant_value just returns if
 instantiation_dependent_expression_p is true.

 @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
 constexpr_ctx
 *ctx, tree t,
   case CONVERT_EXPR:
   case VIEW_CONVERT_EXPR:
   case NOP_EXPR:
 +case UNARY_PLUS_EXPR:
 {
 +   enum tree_code tcode = TREE_CODE (t);
  tree oldop = TREE_OPERAND (t, 0);
 +
 +   if (tcode == NOP_EXPR  TREE_TYPE (t) == TREE_TYPE (oldop) 
 TREE_OVERFLOW_P (oldop))
 + {
 +   if (!ctx-quiet)
 + permerror (input_location, overflow in constant
 expression);
 +   /* If we're being permissive (and are in an enforcing
 +   context), ignore the overflow.  */
 +   if (!flag_permissive)
 + *overflow_p = true;
 +   *non_constant_p = true;
 +
 +   return t;
 + }
  tree op = cxx_eval_constant_expression (ctx, oldop,


 Why doesn't the call to cxx_eval_constant_expression at the bottom here
 handle oldop having TREE_OVERFLOW set?


 I just handled the case that we see here a wrapping NOP_EXPR around an
 overflow.  As this isn't handled by cxx_eval_constant_expression.


 How does it need to be handled?  A NOP_EXPR wrapped around an overflow
 is there to indicated that the expression is non-constant, and it can't
 be simplified any farther.

 Please give an example of what was going wrong.

 @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
 gimple_seq *post_p)

 switch (code)
   {
 +case SIZEOF_EXPR:
 +  if (SIZEOF_EXPR_TYPE_P (*expr_p))
 +   *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
 (*expr_p,
 +
 0)),
 + SIZEOF_EXPR, false);
 +  else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
 +   *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
 0),
 + SIZEOF_EXPR, false);
 +  else
 +   *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
 0),
 + SIZEOF_EXPR, false);
 +  if (*expr_p == error_mark_node)
 +   *expr_p = size_one_node;
 +
 +  *expr_p = maybe_constant_value (*expr_p);
 +  ret = GS_OK;
 +  break;


 Why are these surviving until gimplification time?


 This might be still necessary. I will retest, when bootstrap works.
 As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
 expressions a sizeof can occure, this shouldn't be necessary anymore.
 AFAIR I saw here some issues

Re: C++ delayed folding branch review

[Kai Tietz]

2015-07-27 18:51 GMT+02:00 Jason Merrill ja...@redhat.com:
 I've trimmed this to the previously mentioned issues that still need to be
 addressed; I'll do another full review after these are dealt with.

Thanks for doing this summary of missing parts of prior review.

 On 06/13/2015 12:15 AM, Jason Merrill wrote:

 On 06/12/2015 12:11 PM, Kai Tietz wrote:

 @@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
   {
 if (TREE_TYPE (temp) == type)
   return temp;
 +  STRIP_NOPS (temp);
 +  if (TREE_TYPE (temp) == type)
 +return temp;
 @@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const constexpr_ctx
 *ctx,
 tree t,
   bool
   reduced_constant_expression_p (tree t)
   {
 +  /* Make sure we remove useless initial NOP_EXPRs.  */
 +  STRIP_NOPS (t);


 Within the constexpr code we should be folding away NOPs as they are
 generated, they shouldn't live this long.


 Well, we might see them on overflows ...


 We shouldn't within the constexpr code.  NOPs for expressions that are
 non-constant due to overflow are added in
 cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
 of constexpr evaluation.

 @@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
 *ctx, tree t,
 is_dummy_object (x))
  {
x = ctx-object;
 - x = cp_build_addr_expr (x, tf_warning_or_error);
 + if (x)
 +   x = cp_build_addr_expr (x, tf_warning_or_error);
 + else
 +   x = get_nth_callarg (t, i);


 This still should not be necessary.


 Yeah, most likely.  But I got initially here some issues, so I don't
 see that this code would worsen things.


 If this code path is hit, that means something has broken my design, and
 I don't want to just paper over that.  Please revert this change.

   case SIZEOF_EXPR:
 +  if (processing_template_decl
 +  (!COMPLETE_TYPE_P (TREE_TYPE (t))
 + || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST))
 +   return t;


 Why is this necessary?


 We don't want to resolve SIZEOF_EXPR within template-declarations for
 incomplete types, of if its size isn't fixed.  Issue is that we
 otherwise get issues about expressions without existing type (as usual
 within template-declarations for some expressions).


 Yes, but we shouldn't have gotten this far with a dependent sizeof;
 maybe_constant_value just returns if
 instantiation_dependent_expression_p is true.

 @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
 constexpr_ctx
 *ctx, tree t,
   case CONVERT_EXPR:
   case VIEW_CONVERT_EXPR:
   case NOP_EXPR:
 +case UNARY_PLUS_EXPR:
 {
 +   enum tree_code tcode = TREE_CODE (t);
  tree oldop = TREE_OPERAND (t, 0);
 +
 +   if (tcode == NOP_EXPR  TREE_TYPE (t) == TREE_TYPE (oldop) 
 TREE_OVERFLOW_P (oldop))
 + {
 +   if (!ctx-quiet)
 + permerror (input_location, overflow in constant
 expression);
 +   /* If we're being permissive (and are in an enforcing
 +   context), ignore the overflow.  */
 +   if (!flag_permissive)
 + *overflow_p = true;
 +   *non_constant_p = true;
 +
 +   return t;
 + }
  tree op = cxx_eval_constant_expression (ctx, oldop,


 Why doesn't the call to cxx_eval_constant_expression at the bottom here
 handle oldop having TREE_OVERFLOW set?


 I just handled the case that we see here a wrapping NOP_EXPR around an
 overflow.  As this isn't handled by cxx_eval_constant_expression.


 How does it need to be handled?  A NOP_EXPR wrapped around an overflow
 is there to indicated that the expression is non-constant, and it can't
 be simplified any farther.

 Please give an example of what was going wrong.

 @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
 gimple_seq *post_p)

 switch (code)
   {
 +case SIZEOF_EXPR:
 +  if (SIZEOF_EXPR_TYPE_P (*expr_p))
 +   *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
 (*expr_p,
 +
 0)),
 + SIZEOF_EXPR, false);
 +  else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
 +   *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
 0),
 + SIZEOF_EXPR, false);
 +  else
 +   *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
 0),
 + SIZEOF_EXPR, false);
 +  if (*expr_p == error_mark_node)
 +   *expr_p = size_one_node;
 +
 +  *expr_p = maybe_constant_value (*expr_p);
 +  ret = GS_OK;
 +  break;


 Why are these surviving until gimplification time?


 This might be still necessary. I will retest, when bootstrap works.
 As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
 expressions a sizeof can occure, this shouldn't be necessary anymore.
 AFAIR I saw here some issues about initialzation for global-variables,
 which weren't caught

Re: Fold some equal to and not equal to patterns in match.pd

[Kai Tietz]

2015-07-24 7:54 GMT+02:00 Jeff Law l...@redhat.com:
 On 07/23/2015 10:33 AM, Segher Boessenkool wrote:

 On Thu, Jul 23, 2015 at 10:09:49AM -0600, Jeff Law wrote:

 It seems to me in these kind of cases that selection of the canonical
 form should be driven by factors outside of which is better for a
 particular target.  ie, which is simpler


 I agree.  But neither form is simpler here, and we need to have both
 forms in other contexts, so there is no real benefit to canonicalising.



 a  N ==/!= 0

 Looks like two operations.  A shift and a comparison against zero regardless
 of whether or not N is constant.


 a(-1N) ==/!= 0

 For a varying N, this has a shift, logical and and comparison against zero.

 For a constant N obviously the shift collapses to a constant and we're left
 with two operations again.

 So for gimple, I'd prefer to see us using the a  N form.

 If we need both forms in other contexts, we ought to be looking to eliminate
 that need :-)

 If we go to the RTL level, then it's more complex -- it might depend on the
 constant produced by the -1  N operation, whether or not the target can
 shift by more than one bit at a time (H8/300 series is limited here for
 example), whether or not one operation sets condition codes in a useful way,
 potentially allowing the comparison to be removed, etc etc.  rtx_costs, even
 with its limitations is probably the way to drive selection of form for the
 RTL optimizers.


 Jeff

I fully agree.  But there are case there is not necessarily a 'better'
representation.  For example for sinking converts into shift-operation
can be tricky.

for 'typea a;'

(typeb) (a  N) - ((typeb) a)  N

'bitsizeof (typeb) = N' result is always zero.
'bitsizeof (typeb)  N' result needs to be calculated.

But it isn't necessarily easy to say if form '(typeb) (a  N)', or
'((typeb) a)  N' form is to be prefered.  It strongly depends on
expression this pattern is used in.

For RTL this pattern has another issue, as we need to take natural
mode into account here too,

We should define for such forms our 'normal' representation.

Kai

Re: C++ delayed folding branch review

[Kai Tietz]

Hello Jason,

Thanks for the review.  I addressed a lot of your comments directly on 
svn-branch.  See revision r224439.

- Ursprüngliche Mail -
 Generally, it seems like most of my comments from April haven't been
 addressed yet.

Yes, most of them.
 
  @@ -3023,13 +3023,14 @@ conversion_warning (location_t loc, tree type, tree
  expr
 
 Instead of adding folds here, let's make sure that the argument we pass
 (e.g. from cp_convert_and_check to warnings_for_convert_and_check) is
 fully folded.

I looked for dependencies, and address this.
 
  @@ -589,9 +589,9 @@ null_member_pointer_value_p (tree t)
   return false;
 else if (TYPE_PTRMEMFUNC_P (type))
   return (TREE_CODE (t) == CONSTRUCTOR
  -integer_zerop (CONSTRUCTOR_ELT (t, 0)-value));
  +integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)-value)));
 else if (TYPE_PTRDATAMEM_P (type))
  -return integer_all_onesp (t);
  +return integer_all_onesp (fold (t));
 
 Again, calling fold here is wrong; it doesn't handle constexpr, and we
 should have folded before we got here.

Agreed.  I will commit change for this.

Nevertheless CONSTRUCTOR_ELT's value might still be prefixed by nops due 
possible overflows, or by negative sign/invert/etc.  This is caused by 
non-constant-folding of values.  I removed for now those folds, as I agree we 
should address this at place of value-assignment.  Nevertheless we still have 
this issue
 
  @@ -5090,9 +5090,9 @@ build_conditional_expr_1 (location_t loc, tree arg1,
  tree arg2, tree arg3,
 
valid_operands:
 result = build3 (COND_EXPR, result_type, arg1, arg2, arg3);
  -  if (!cp_unevaluated_operand)
  +  if (!cp_unevaluated_operand  !processing_template_decl)
   /* Avoid folding within decltype (c++/42013) and noexcept.  */
  -result = fold_if_not_in_template (result);
  +result = fold (result);
 
 This seems related to your status report note:
 
  Additionally addressed issue about cond_expr, as we want to fold cases with
  a constant-condition.  Here we need to use fold_to_constant so that we
  just fold things to constant-value, if possible and otherwise don't modify
  anything.
 
 But why do you say we want to fold cases with a constant condition?  We
 certainly want to avoid warning about the dead branch in that case, but
 it would be better if we can do that folding only in the warning code.

Issue is that we otherwise detect in conditions that expressions aren't 
constant due never-executed-code-path.  The diagnostics we can deal 
differently, but this was actually the reason for doing this.  I can remove 
this here, but we still need a place to avoid ill detection of constexpr (or 
invalid code) on dead code-branch.  Eg.  (1 ? 0/0 : 1) etc
 
  @@ -5628,8 +5628,8 @@ build_new_op_1 (location_t loc, enum tree_code code,
  int f
  lags, tree arg1,
   decaying an enumerator to its value.  */
if (complain  tf_warning)
  warn_logical_operator (loc, code, boolean_type_node,
  -  code_orig_arg1, arg1,
  -  code_orig_arg2, arg2);
  +  code_orig_arg1, fold (arg1),
  +  code_orig_arg2, fold (arg2));
 
arg2 = convert_like (conv, arg2, complain);
  }
  @@ -5666,7 +5666,8 @@ build_new_op_1 (location_t loc, enum tree_code code,
  int f
  lags, tree arg1,
   case TRUTH_AND_EXPR:
   case TRUTH_OR_EXPR:
 warn_logical_operator (loc, code, boolean_type_node,
  -code_orig_arg1, arg1, code_orig_arg2, arg2);
  +code_orig_arg1, fold (arg1),
  +code_orig_arg2, fold (arg2));
 /* Fall through.  */
   case GT_EXPR:
   case LT_EXPR:
  @@ -5676,7 +5677,7 @@ build_new_op_1 (location_t loc, enum tree_code code,
  int f
  lags, tree arg1,
   case NE_EXPR:
 if ((code_orig_arg1 == BOOLEAN_TYPE)
^ (code_orig_arg2 == BOOLEAN_TYPE))
  -   maybe_warn_bool_compare (loc, code, arg1, arg2);
  +   maybe_warn_bool_compare (loc, code, fold (arg1), fold (arg2));
 /* Fall through.  */
   case PLUS_EXPR:
   case MINUS_EXPR:
 
 I still think these fold calls should be cp_fully_fold.

Ok, modified here use of fold to cp_fully_fold.

 
  @@ -7382,8 +7383,13 @@ build_over_call (struct z_candidate *cand, int
  flags, tsu
  bst_flags_t complain)
 
 gcc_assert (j = nargs);
 nargs = j;
  +  {
  +tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof
  (tree)))
  ;
  +for (j = 0; j  nargs; j++)
  +  fargs[j] = fold_non_dependent_expr (argarray[j]);
 
 No change needed here, but I notice that fold_non_dependent_expr is
 still using maybe_constant_value; it should probably use cp_fully_fold
 instead.

Hmm, maybe we should limit this folding on constants.  So cp_fold_to_constant 
()?
 
  @@ -1052,6 

[patch c++]: Fix PR 59759

[Kai Tietz]

Hi,

I am not sure if suggested fix is the thing we want here.  But infact
having here a VAR_DECL can be caused by prior errors introducing them.
So we simply might want to treat VAR_DECL in unify via
unify_template_argument_mismatch.

Tested for 5.x branch, and trunk.  Ok for apply?

Kai

ChangeLog

2015-05-26  Kai Tietz  kti...@redhat.com

PR c++/59759
* pt.c (unify): Don't ICE on VAR_DECL.


Regression tested on x86_64-w64-mingw32.  Ok for apply?

Regards,
Kai

Index: pt.c
===
--- pt.c(Revision 219014)
+++ pt.c(Arbeitskopie)
@@ -18490,11 +18490,12 @@ unify (tree tparms, tree targs, tree parm, tree ar

 case VAR_DECL:
   /* A non-type template parameter that is a variable should be a
-an integral constant, in which case, it whould have been
+an integral constant, in which case, it should have been
 folded into its (constant) value. So we should not be getting
-a variable here.  */
-  gcc_unreachable ();
+a variable here out-side of an error-case.  */

+  return unify_template_argument_mismatch (explain_p, parm, arg);
+
 case TYPE_ARGUMENT_PACK:
 case NONTYPE_ARGUMENT_PACK:
   return unify (tparms, targs, ARGUMENT_PACK_ARGS (parm),

Re: C++ delayed folding branch review

[Kai Tietz]

2015-04-24 20:25 GMT+02:00 Jason Merrill ja...@redhat.com:
 On 04/24/2015 09:46 AM, Kai Tietz wrote:

 Sure, we can use here instead *_fully_fold, but for what costs?  In
 general we need to deal here a simple one-level fold for simplifying
 constant-values, and/or removing useless type-conversions.


 Well, here you're doing a two-level fold.  And in general fold relies on
 subexpressions being appropriately folded.  So for warning folding, I think
 the caching approach we were talking about in the call today is the way to
 go.

Ok.  Just a question about where to hook up the hash-variable.  What
would be a good place to create (well this we could do lazy too) it,
and of more importance where to destroy the hash? We could destroy it
after genericize-pass?

 @@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
   return false;
 else if (TYPE_PTRMEMFUNC_P (type))
   return (TREE_CODE (t) == CONSTRUCTOR
 -integer_zerop (CONSTRUCTOR_ELT (t, 0)-value));
 +integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)-value)));
 else if (TYPE_PTRDATAMEM_P (type))
 -return integer_all_onesp (t);
 +return integer_all_onesp (fold (t));



 Calling fold here is wrong; it doesn't handle constexpr, and we should
 have
 folded before we got here.


 s.o. we need to make sure constant-values get rid of useless
 types-conversions/negates/etc ...


 Certainly a constant value from maybe_constant_value should not have a
 useless type conversion wrapped around it, as that makes it appear
 non-constant.

Well, beside an overflow was seen.  The interesting point is here to
find the proper place to do for constructor-elements proper folding,
or at least constant-value folding.  I made here already some tries to
find a proper place for this, but ran by this into troubles that not
in all cases maybe_constant_value was callable (endless recursion).
Additionally it happens in some cases for constructor-elements that
nop-casts are added on assign.  At least I ran into that.
In general there is the question if we shouldn't do for constructors
call a ...fully_fold?

 @@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
  expr = build3 (COMPONENT_REF,
 cp_build_qualified_type (type, type_quals),
 expr, field, NULL_TREE);
 -   expr = fold_if_not_in_template (expr);


 I don't think we need to remove this fold, since it is part of compiler
 internals rather than something the user wrote.  Really, we should
 represent
 the base conversion with something like a CONVERT_EXPR and only call this
 function when we want to fold it.  But that can wait for a later patch.


 Ok.  I remove this fold-case due simply removing
 fold_if_not_in_template function.  So well, we could re-add a call for
 fold, if not in template.


 Let's try not checking for being in a template, see if it breaks.

I tested to use here just a fold (), and I ddn't noticed any fallout
by this.  So I committed it to branch.

 +static tree
 +cp_fold (tree x, hash_maptree, tree *fold_hash)
 +{


 

 I still think we need a hybrid of this and the constexpr code: it isn't
 full
 folding if we aren't doing constexpr evaluation.  But we can't just use
 maybe_constant_value because that only folds C++ constant-expressions,
 and
 we want to fold more things than that.  I suppose one simple approach for
 now would be to call maybe_constant_value from cp_fold.


 Well, the functionality of cp_fold and maybe_constant_value (well,
 actually how constexpr.c works) are different in cases of
 non-constant results.


 I think that's exactly what I was saying above: we can't just use
 maybe_constant_value because that only folds C++ constant-expressions, and
 we want to fold more things than that.

maybe_constant_value folds constants, too.  That was actually the
reason to use it.  Nevertheless I admit that we could call instead a
..fully_fold here too.
One point I see here about creating a function using
maybe_constant_value + cp_fold is that maybe_constant_value is
something we can call safely in all cases.

 My point is that cp_fold should be a superset of maybe_constant_value, to
 fix bugs like 53792.  And the easiest way to get that would seem to be by
 calling maybe_constant_value from cp_fold.

Agreed, that bugs like PR 53792 could be solved that way.
Nevertheless they aren't directly linked to the delayed-folding
problem, are they?  We could deal with those cases also in
genericize-pass lately, as we want to catch here just a missed
optimization, aren't we?

 @@ -614,9 +614,13 @@ cp_fold_convert (tree type, tree expr)
   }
 else
   {
 -  conv = fold_convert (type, expr);
 +  if (TREE_CODE (expr) == INTEGER_CST)
 +conv = fold_convert (type, expr);
 +  else
 +conv = convert (type, expr);


 Why?  If we're calling cp_fold_convert in a place where we don't want to
 fold, we should stop calling it rather than change it.


 See, that we

Re: C++ delayed folding branch review

[Kai Tietz]

2015-04-24 6:22 GMT+02:00 Jason Merrill ja...@redhat.com:
 +  expr = fold (expr);
/* This may happen, because for LHS op= RHS we preevaluate
   RHS and create C_MAYBE_CONST_EXPR SAVE_EXPR RHS, which
   means we could no longer see the code of the EXPR.  */
if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
  expr = C_MAYBE_CONST_EXPR_EXPR (expr);
if (TREE_CODE (expr) == SAVE_EXPR)
 -expr = TREE_OPERAND (expr, 0);
 +expr = fold (TREE_OPERAND (expr, 0));


 How about moving the first fold after the SAVE_EXPR block, so that we only
 need to call fold once?

I will try.  It might be required to fold in front to strip of useless
type-conversions ...

 +case NEGATE_EXPR:
 +case BIT_NOT_EXPR:
 +case CONVERT_EXPR:
 +case VIEW_CONVERT_EXPR:
 +case NOP_EXPR:
 +case FIX_TRUNC_EXPR:
 +  {
 +   tree op1 = TREE_OPERAND (expr, 0);
 +   tree fop1 = fold (op1);
 +   if (fop1  op1 != fop1)
 + fop1 = fold_build1_loc (loc, TREE_CODE (expr), TREE_TYPE (expr),
 + fop1);

well, AFAIR was the idea here to make sure we do constant-value folding ...


 Isn't this redundant with the call to fold above?  If not, it seems that the
 above call should be to *_fully_fold.  I suppose we want an entry point
 defined by both front ends that c-common code can call which does full
 folding of an expression.

Sure, we can use here instead *_fully_fold, but for what costs?  In
general we need to deal here a simple one-level fold for simplifying
constant-values, and/or removing useless type-conversions.  As convert
doesn't fold them away anymore, it can stay with such NOP_EXPR before
constant-values, which cause us to fail on later value-analyzis.  So
sure, we can use *_fully_fold, but actually we don't need a full-fold.
(this applies to other places below too, where you mentioned the same
issue, too).

 @@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
  return false;
else if (TYPE_PTRMEMFUNC_P (type))
  return (TREE_CODE (t) == CONSTRUCTOR
 -integer_zerop (CONSTRUCTOR_ELT (t, 0)-value));
 +integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)-value)));
else if (TYPE_PTRDATAMEM_P (type))
 -return integer_all_onesp (t);
 +return integer_all_onesp (fold (t));


 Calling fold here is wrong; it doesn't handle constexpr, and we should have
 folded before we got here.

s.o. we need to make sure constant-values get rid of useless
types-conversions/negates/etc ...

 warn_logical_operator (loc, code, boolean_type_node,
 -  code_orig_arg1, arg1,
 -  code_orig_arg2, arg2);
 +  code_orig_arg1, fold (arg1),
 +  code_orig_arg2, fold (arg2));


 I think warn_logical_operator should call back into *_fully_fold. Likewise
 for most similar added calls to fold.

Same as above.  It isn't necessary for further analysis to perform
fully_fold on arg1/arg2.  But of course we can introduce this load
here.

 @@ -7356,8 +7354,13 @@ build_over_call (struct z_candidate *cand, int
 flags, tsu
 bst_flags_t complain)

gcc_assert (j = nargs);
nargs = j;
 +  {
 +tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof
 (tree)))
 ;
 +for (j = 0; j  nargs; j++)
 +  fargs[j] = fold_non_dependent_expr (argarray[j]);


 Similarly, this and build_cxx_call should use cp_fully_fold.

Well, cp_fully_fold wouldn't resolve constexpr ... at least not in completeness.

 @@ -7602,7 +7614,6 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
 current_function_decl
 DECL_DECLARED_CONSTEXPR_P (current_function_decl))
  optimize = 1;
 -  fn = fold_if_not_in_template (fn);
optimize = optimize_sav;


 Since we're removing the fold, we can also remove the changes to optimize.

Yeah, I missed that.  I removed superfluous code-path and committed it
on branch.

 @@ -443,7 +443,7 @@ build_base_path (enum tree_code code,

   t = TREE_TYPE (TYPE_VFIELD (current_class_type));
   t = build_pointer_type (t);
 - v_offset = convert (t, current_vtt_parm);
 + v_offset = fold (convert (t, current_vtt_parm));

 fold_convert should work here.

Well, fold_const isn't necessarily the same as fold (convert ())
within C++, due convert handles special cases fold_const doesn't.  At
least I ran here into issues for templates within templates AFAIR.


 @@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
 expr = build3 (COMPONENT_REF,
cp_build_qualified_type (type, type_quals),
expr, field, NULL_TREE);
 -   expr = fold_if_not_in_template (expr);


 I don't think we need to remove this fold, since it is part of compiler
 internals rather than something the user wrote.  Really, we should represent
 the base conversion with something like a CONVERT_EXPR and 

[patch emutls]:Fix PR 65566

[Kai Tietz]

Hi,

This patch avoids that we try to operate on function-decl's cfun equal
to NULL within lower_emutls_function_body.

ChangeLog

2015-03-31  Kai Tietz  kti...@redhat.com

PR target/65566
* tree-emutls.c (lower_emutls_function_body): Don't try to
operate on node's decl function is NULL.

Ok for apply?

Regards,
Kai

Index: tree-emutls.c
===
--- tree-emutls.c   (Revision 221789)
+++ tree-emutls.c   (Arbeitskopie)
@@ -635,6 +635,12 @@ lower_emutls_function_body (struct cgraph_node *no

   push_cfun (DECL_STRUCT_FUNCTION (node-decl));

+  if (!cfun)
+{
+  pop_cfun ();
+  return;
+}
+
   d.cfun_node = node;
   d.builtin_decl = builtin_decl_explicit (BUILT_IN_EMUTLS_GET_ADDRESS);
   /* This is where we introduce the declaration to the IL and so we have to

Re: [patch emutls]:Fix PR 65566

[Kai Tietz]

2015-03-31 13:42 GMT+02:00 Jakub Jelinek ja...@redhat.com:
 On Tue, Mar 31, 2015 at 01:35:56PM +0200, Kai Tietz wrote:
 Hi,

 This patch avoids that we try to operate on function-decl's cfun equal
 to NULL within lower_emutls_function_body.

 If DECL_STRUCT_FUNCTION (node-decl) is already NULL (for which functions?),
 then what is the point doing push_cfun/pop_cfun in that case at all?
 Shouldn't you just early return if DECL_STRUCT_FUNCTION (node-decl) is
 NULL?

In testcase for the function-decl mpx_test function-body is NULL.
...
addressable used static QI file thread-local-var-1-lbv.c line 28
col 5 align  8 context translation_unit_decl 0xffd19ab0 D.3888
attributes tree_list 0xffcd9030 initial block 0xfff806c8
result result_decl 0xffd19a60 D.3886 type integer_type 0xffdd0360 int
ignored SI file thread-local-var-1-lbv.c line 28 col 5 size
integer_cst  0xffef0558 32 unit size integer_cst 0xffef0570 4
align 32 context function_decl 0xffce4900 mpx_test chain
type_decl 0xffd05880 D.3315

Fair point, so patch is:

Index: tree-emutls.c
===
--- tree-emutls.c   (Revision 221789)
+++ tree-emutls.c   (Arbeitskopie)
@@ -633,6 +633,9 @@ lower_emutls_function_body (struct cgraph_node *no
   struct lower_emutls_data d;
   bool any_edge_inserts = false;

+  if (!DECL_STRUCT_FUNCTION (node-decl))
+return;
+
   push_cfun (DECL_STRUCT_FUNCTION (node-decl));

   d.cfun_node = node;

Re: [patch c++]: Fix for PR/65390

[Kai Tietz]

2015-03-31 14:34 GMT+02:00 Marek Polacek pola...@redhat.com:
 On Tue, Mar 31, 2015 at 02:32:32PM +0200, Marek Polacek wrote:
 On Tue, Mar 31, 2015 at 02:25:14PM +0200, Kai Tietz wrote:
  Hi,
 
  I had tried same approach as Marek.  For me it solved the PR, but
  caused other regressions on boostrap.  So I dropped the way via
  dependent_type_p.
 
  Well, this bootstrap-issue might be caused by some local changes I had
  forgot to remove, but I doubt it.
  Marek, have you tried to do a boostrap with your patch?

 Of course, with --enable-languages=all.  I'll re-run the bootstrap with more
 languages enabled, though.

 BTW, are you saying that your fix was exactly the same?  Did you as well check
 that index_type is non-null?

Sure, I checked for index_type.  But by looking closer I used
instantiation_dependent_expression_p - as mentioned by Jason - instead
of dependent_type_p, which seems to make here the difference.

 Marek

Kai

Re: [patch c++]: Fix for PR/65390

[Kai Tietz]

Hi,

I had tried same approach as Marek.  For me it solved the PR, but
caused other regressions on boostrap.  So I dropped the way via
dependent_type_p.

Well, this bootstrap-issue might be caused by some local changes I had
forgot to remove, but I doubt it.
Marek, have you tried to do a boostrap with your patch?

Kai

2015-03-31 13:50 GMT+02:00 Marek Polacek pola...@redhat.com:
 On Tue, Mar 24, 2015 at 01:14:50AM -0400, Jason Merrill wrote:

 Here's my shot at this.

 The problem is that the type is considered dependent in a template but is
 not actually dependent, so we can see the exact same type outside a template

 Yeah, I think this is true...

 and it's not dependent.  So, this code is creating the difference:

   /* We can only call value_dependent_expression_p on integral constant
  expressions; treat non-constant expressions as dependent, too.  */
   if (processing_template_decl
(type_dependent_expression_p (size)
   || !TREE_CONSTANT (size) || value_dependent_expression_p (size)))

 Now that we have instantiation_dependent_expression_p, we should be able to
 use that instead of checking type/value dependency separately.

 ...but I think there's another place where things go wrong.  ISTM that in
 build_cplus_array_type we consider all arrays with non-constant index as
 dependent (when processing_template_decl) -- but as the testcase shows, this
 is not always true.  The fix then could look like the following, though I
 wouldn't be surprised if this was a wrong way how to go about this.

 Bootstrapped/regtested on x86_64-linux.  Not a regression, so we might want to
 defer this patch to the next stage1.

 2015-03-31  Marek Polacek  pola...@redhat.com

 PR c++/65390
 * tree.c (build_cplus_array_type): Use dependent_type_p rather than
 checking for constness.

 * g++.dg/template/pr65390.C: New test.

 diff --git gcc/cp/tree.c gcc/cp/tree.c
 index ef53aff..97bccc0 100644
 --- gcc/cp/tree.c
 +++ gcc/cp/tree.c
 @@ -822,10 +822,9 @@ build_cplus_array_type (tree elt_type, tree index_type)
if (elt_type == error_mark_node || index_type == error_mark_node)
  return error_mark_node;

 -  bool dependent
 -= (processing_template_decl
 -(dependent_type_p (elt_type)
 -  || (index_type  !TREE_CONSTANT (TYPE_MAX_VALUE (index_type);
 +  bool dependent = (processing_template_decl
 +(dependent_type_p (elt_type)
 +   || (index_type  dependent_type_p (index_type;

if (elt_type != TYPE_MAIN_VARIANT (elt_type))
  /* Start with an array of the TYPE_MAIN_VARIANT.  */
 diff --git gcc/testsuite/g++.dg/template/pr65390.C 
 gcc/testsuite/g++.dg/template/pr65390.C
 index e69de29..299d22a 100644
 --- gcc/testsuite/g++.dg/template/pr65390.C
 +++ gcc/testsuite/g++.dg/template/pr65390.C
 @@ -0,0 +1,12 @@
 +// PR c++/65390
 +// { dg-do compile }
 +// { dg-options  }
 +
 +templatetypename T struct shared_ptr { };
 +
 +templatetypename T, typename Arg
 +shared_ptrT make_shared(Arg) { return shared_ptrT(); } // { dg-error 
 variably modified type|trying to instantiate }
 +
 +void f(int n){
 +  make_sharedint[n](1); // { dg-error no matching function }
 +}

 Marek

[patch libgomp]: Fix PR 64972

[Kai Tietz]

Hi,

ChangeLog

2015-03-25  Kai Tietz  kti...@redhat.com

PR libgomp/64972
* oacc-parallel.c (GOACC_parallel): Use PRIu64 if available.
(GOACC_data_start): Likewise.
* target.c (gomp_map_vars): Likewise.

Tested for i686-w64-mingw32.  Fix got preapproved by Jakub, so I will
commit this soon, if there are no objections.

Regards,
Kai

Index: oacc-parallel.c
===
--- oacc-parallel.c(Revision 221640)
+++ oacc-parallel.c(Arbeitskopie)
@@ -31,6 +31,9 @@
 #include libgomp_g.h
 #include gomp-constants.h
 #include oacc-int.h
+#ifdef HAVE_INTTYPES_H
+# include inttypes.h  /* For PRIu64.  */
+#endif
 #include string.h
 #include stdarg.h
 #include assert.h
@@ -99,9 +102,15 @@ GOACC_parallel (int device, void (*fn) (void *),
 gomp_fatal (num_workers (%d) different from one is not yet supported,
 num_workers);

-  gomp_debug (0, %s: mapnum=%zd, hostaddrs=%p, sizes=%p, kinds=%p,
async=%d\n,
-  __FUNCTION__, mapnum, hostaddrs, sizes, kinds, async);
-
+#ifdef HAVE_INTTYPES_H
+  gomp_debug (0, %s: mapnum=%PRIu64, hostaddrs=%p, size=%p, kinds=%p, 
+ async = %d\n,
+  __FUNCTION__, (uint64_t) mapnum, hostaddrs, sizes, kinds, async);
+#else
+  gomp_debug (0, %s: mapnum=%lu, hostaddrs=%p, sizes=%p, kinds=%p,
async=%d\n,
+  __FUNCTION__, (unsigned long) mapnum, hostaddrs, sizes, kinds,
+  async);
+#endif
   select_acc_device (device);

   thr = goacc_thread ();
@@ -178,8 +187,13 @@ GOACC_data_start (int device, size_t mapnum,
   bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
   struct target_mem_desc *tgt;

-  gomp_debug (0, %s: mapnum=%zd, hostaddrs=%p, sizes=%p, kinds=%p\n,
-  __FUNCTION__, mapnum, hostaddrs, sizes, kinds);
+#ifdef HAVE_INTTYPES_H
+  gomp_debug (0, %s: mapnum=%PRIu64, hostaddrs=%p, size=%p, kinds=%p\n,
+  __FUNCTION__, (uint64_t) mapnum, hostaddrs, sizes, kinds);
+#else
+  gomp_debug (0, %s: mapnum=%lu, hostaddrs=%p, sizes=%p, kinds=%p\n,
+  __FUNCTION__, (unsigned long) mapnum, hostaddrs, sizes, kinds);
+#endif

   select_acc_device (device);

Index: target.c
===
--- target.c(Revision 221640)
+++ target.c(Arbeitskopie)
@@ -33,6 +33,9 @@
 #include limits.h
 #include stdbool.h
 #include stdlib.h
+#ifdef HAVE_INTTYPES_H
+# include inttypes.h  /* For PRIu64.  */
+#endif
 #include string.h
 #include assert.h

@@ -438,9 +441,16 @@ gomp_map_vars (struct gomp_device_descr *devicep,
   /* We already looked up the memory region above and it
  was missing.  */
   size_t size = k-host_end - k-host_start;
+#ifdef HAVE_INTTYPES_H
   gomp_fatal (present clause: !acc_is_present (%p, 
-  %zd (0x%zx)), (void *) k-host_start,
-  size, size);
+  %PRIu64 (0x%PRIx64)),
+  (void *) k-host_start,
+  (uint64_t) size, (uint64_t) size);
+#else
+  gomp_fatal (present clause: !acc_is_present (%p, 
+  %lu (0x%lx)), (void *) k-host_start,
+  (unsigned long) size, (unsigned long) size);
+#endif
 }
 break;
   case GOMP_MAP_FORCE_DEVICEPTR:

Re: [patch c++]: Fix for PR/65390

[Kai Tietz]

Hello,

the problem here is that for cases of vla-array-types, the types don't
get finally layouted in build_cplus_array_type.  So the type-alignment
isn't set in such cases for the resulting type.

ChangeLog

2015-03-20  Kai Tietz  kti...@redhat.com

PR c++/65390
* tree.c (strip_typedefs): Ignore alignment
difference during processing template.

2015-03-20  Kai Tietz  kti...@redhat.com

PR c++/65390
* g++.dg/template/pr65390.C: New file.

Tested on x86_64-unknown-linux-gnu.  Ok for apply?

Regards,
Kai

Index: gcc/gcc/cp/tree.c
===
--- gcc.orig/gcc/cp/tree.c
+++ gcc/gcc/cp/tree.c
@@ -1356,7 +1356,8 @@ strip_typedefs (tree t)
   if (!result)
   result = TYPE_MAIN_VARIANT (t);
   if (TYPE_USER_ALIGN (t) != TYPE_USER_ALIGN (result)
-  || TYPE_ALIGN (t) != TYPE_ALIGN (result))
+  || (processing_template_decl
+   TYPE_ALIGN (t) != TYPE_ALIGN (result)))
 {
   gcc_assert (TYPE_USER_ALIGN (t));
   if (TYPE_ALIGN (t) == TYPE_ALIGN (result))
Index: gcc/gcc/testsuite/g++.dg/template/pr65390.C
===
--- /dev/null
+++ gcc/gcc/testsuite/g++.dg/template/pr65390.C
@@ -0,0 +1,12 @@
+// { dg-do compile }
+// { dg-options -Wno-vla }
+templatetypename T struct shared_ptr { };
+
+templatetypename T, typename Arg
+shared_ptrT make_shared(Arg) { return shared_ptrT(); } // {
dg-message note }
+// { dg-warning ignoring attributes template { target *-*-* } 6 }
+
+void f(int n){
+  make_sharedint[n](1); // { dg-error no matching }
+}
+// { dg-error variably modified type|trying to instantiate type {
target *-*-* } 10 }

Re: [patch c++]: Fix for PR/65390

[Kai Tietz]

2015-03-17 13:36 GMT+01:00 Jason Merrill ja...@redhat.com:
 On 03/16/2015 03:22 PM, Kai Tietz wrote:

 2015-03-16 19:07 GMT+01:00 Jason Merrill ja...@redhat.com:

 If there is an alignment mismatch without user intervention, there is a
 problem, we can't just ignore it.

 Where we run into trouble is with array types where the version built
 earlier has not been laid out yet but the new one has been.  I've been
 trying to deal with that by making sure that we lay out the original type
 as
 well, but obviously that isn't working for this case.  Why not?


 Well, TYPE_ALIGN (t) is set to 32, and it differs to TYPE_ALIGN
 (result) (value 8), and TYPE_USER_ALIGN isn't set.

 I suppose we could avoid checking TYPE_ALIGN if neither TYPE_USER_ALIGN
 nor
 TYPE_SIZE are set on 't', but checking TYPE_USER_ALIGN isn't enough.


 For t TYPE_SIZE is set, but it isn't a constant (as it is an variably
 modified type).


 TYPE_ALIGN should still be correct in that case.  So we need to figure out
 why result is getting the wrong alignment.

 Jason


By debugging in build_cplus_array_type I see that type is marked as
dependent.  This is caused by type-max being an expression
non-constant.  So we later on don't layout this type.
So result isn't a comlete layout type.  by callling layout_type on
result, alignment fits.

Kai

[patch c++]: Fix for PR/65390

[Kai Tietz]

Hi,

this patch avoids the attempt to create user-aligned-type for variants
original and main-variants type-alignment differ and original type
isn't user-aligned.
Not sure if this is the preferred variant, we could create for such
cases an aligned-type without setting user-align.
But as this should just happen for invalid types, I am not sure if we
actual need to handle later at all.
So I suggest the following fix for it:

ChangeLog

PR c++/65390
* tree.c (strip_typedefs): Don't attempt
to build user-aligned type if original doesn't
set it and main-variant differs to it.

Jonathan is just about to reduce the testcase, so final patch should
include reduced testcase for it.

Regression-tested for x86_64-unknown-linux-gnu.  Ok for apply?

Regards,
Kai

Index: tree.c
===
--- tree.c  (Revision 221277)
+++ tree.c  (Arbeitskopie)
@@ -1356,7 +1356,7 @@ strip_typedefs (tree t)
   if (!result)
   result = TYPE_MAIN_VARIANT (t);
   if (TYPE_USER_ALIGN (t) != TYPE_USER_ALIGN (result)
-  || TYPE_ALIGN (t) != TYPE_ALIGN (result))
+  || (TYPE_USER_ALIGN (t)  TYPE_ALIGN (t) != TYPE_ALIGN (result)))
 {
   gcc_assert (TYPE_USER_ALIGN (t));
   if (TYPE_ALIGN (t) == TYPE_ALIGN (result))

Re: [patch c++]: Fix for PR/65390

[Kai Tietz]

2015-03-16 19:07 GMT+01:00 Jason Merrill ja...@redhat.com:
 If there is an alignment mismatch without user intervention, there is a
 problem, we can't just ignore it.

 Where we run into trouble is with array types where the version built
 earlier has not been laid out yet but the new one has been.  I've been
 trying to deal with that by making sure that we lay out the original type as
 well, but obviously that isn't working for this case.  Why not?

Well, TYPE_ALIGN (t) is set to 32, and it differs to TYPE_ALIGN
(result) (value 8), and TYPE_USER_ALIGN isn't set.

 I suppose we could avoid checking TYPE_ALIGN if neither TYPE_USER_ALIGN nor
 TYPE_SIZE are set on 't', but checking TYPE_USER_ALIGN isn't enough.

For t TYPE_SIZE is set, but it isn't a constant (as it is an variably
modified type).  So we could add here additional check if TYPE_SIZE is
a integer-constant?

Something like this condition you mean?

...
if (TYPE_USER_ALIGN (t) != TYPE_USER_ALIGN (result)
|| ((TYPE_USER_ALIGN (t) || TREE_CODE (TYPE_SIZE (t)) == INTEGER_CST)
 TYPE_ALIGN (t) != TYPE_ALIGN (result)))
  {
...
 Jason

Kai

Re: [patch]: Fix Bug 65038 - [regression 5] Unable to find ftw.h for libgcov-util.c

[Kai Tietz]

2015-02-27 14:48 GMT+01:00 H.J. Lu hjl.to...@gmail.com:
 On Thu, Feb 26, 2015 at 6:49 AM, Kai Tietz ktiet...@googlemail.com wrote:
 Hi,

 This is the remaining fix for re-enabling native boostrap for
 Windows-variant of gcc without disabling -Werror for libgcc.

 ChangeLog

 2015-02-26  Kai Tietz  kti...@redhat.com

 PR target/65038
 * config.in: Regenerated.
 * configure: Likewise.
 * configure.ac (AC_HEADER_STDC): Add explicit.
 (AC_CHECK_HEADERS): Check for default headers
 plus for ftw.h one.
 * libgcov-util.c (gcov_read_profile_dir): Disable use
 of ftw-function, if header not found.
 (ftw_read_file): Don't translate if ftw header isn't
 present.

 Regression-tested for x86_64-unknown-linux-gnu.  Bootstrapped for
 i686-pc-mingw32.
 I will apply soon, if there are no objections.

 I believe it breaks bootstrap on Linux/x86:

 https://gcc.gnu.org/ml/gcc-regression/2015-02/msg00580.html

 --
 H.J.

Already fixed.  See bug

--
Kai

Re: [patch]: Fix Bug 65038 - [regression 5] Unable to find ftw.h for libgcov-util.c

[Kai Tietz]

Applied at rev.revision 221055.

--
Kai

[patch]: Fix Bug 65038 - [regression 5] Unable to find ftw.h for libgcov-util.c

[Kai Tietz]

Hi,

This is the remaining fix for re-enabling native boostrap for
Windows-variant of gcc without disabling -Werror for libgcc.

ChangeLog

2015-02-26  Kai Tietz  kti...@redhat.com

PR target/65038
* config.in: Regenerated.
* configure: Likewise.
* configure.ac (AC_HEADER_STDC): Add explicit.
(AC_CHECK_HEADERS): Check for default headers
plus for ftw.h one.
* libgcov-util.c (gcov_read_profile_dir): Disable use
of ftw-function, if header not found.
(ftw_read_file): Don't translate if ftw header isn't
present.

Regression-tested for x86_64-unknown-linux-gnu.  Bootstrapped for
i686-pc-mingw32.
I will apply soon, if there are no objections.

Regards.
Kai

Index: configure.ac
===
--- configure.ac(Revision 220969)
+++ configure.ac(Arbeitskopie)
@@ -47,6 +47,12 @@ AC_SUBST(libgcc_topdir)
 AC_CONFIG_AUX_DIR($libgcc_topdir)
 AC_CONFIG_HEADER(auto-target.h:config.in)

+AC_CHECK_HEADERS(inttypes.h \
+ stdint.h stdlib.h \
+ ftw.h unistd.h sys/stat.h sys/types.h \
+ string.h strings.h memory.h)
+AC_HEADER_STDC
+
 AC_ARG_ENABLE(shared,
 [  --disable-shareddon't provide a shared libgcc],
 [
Index: libgcov-util.c
===
--- libgcov-util.c(Revision 220969)
+++ libgcov-util.c(Arbeitskopie)
@@ -52,7 +52,9 @@ void gcov_set_verbose (void)

 #include obstack.h
 #include unistd.h
+#ifdef HAVE_FTW_H
 #include ftw.h
+#endif

 static void tag_function (unsigned, unsigned);
 static void tag_blocks (unsigned, unsigned);
@@ -380,6 +382,7 @@ read_gcda_file (const char *filename)
   return obj_info;
 }

+#ifdef HAVE_FTW_H
 /* This will be called by ftw(). It opens and read a gcda file FILENAME.
Return a non-zero value to stop the tree walk.  */

@@ -417,6 +420,7 @@ ftw_read_file (const char *filename,

   return 0;
 }
+#endif

 /* Initializer for reading a profile dir.  */

@@ -451,7 +455,9 @@ gcov_read_profile_dir (const char* dir_name, int r
   fnotice (stderr, %s is not a directory\n, dir_name);
   return NULL;
 }
+#ifdef HAVE_FTW_H
   ftw (., ftw_read_file, 50);
+#endif
   ret = chdir (pwd);
   free (pwd);

Re: [patch c-family]: Fix Bug 35330 - [4.8/4.9/5 regression] ICE with invalid pragma weak

[Kai Tietz]

2015-02-26 19:53 GMT+01:00 Marek Polacek pola...@redhat.com:
 On Thu, Feb 26, 2015 at 07:28:02PM +0100, Kai Tietz wrote:
 Hi,

 This patch addresses the reported ICE about #pragma weak used on
 declarations not var or function.

 ChangeLog

 2015-02-26  Kai Tietz  kti...@redhat.com

 * c-pragma.c (handle_pragma_weak): Do not try to creat

 create

 weak/alias of declarations
 not being function, or variable declarations.

 functions

 --- c-pragma.c  (Revision 221019)
 +++ c-pragma.c  (Arbeitskopie)
 @@ -392,6 +392,11 @@ handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy
decl = identifier_global_value (name);
if (decl  DECL_P (decl))
  {
 +  if (TREE_CODE (decl) != FUNCTION_DECL  TREE_CODE (decl) != VAR_DECL)

 Use !VAR_OR_FUNCTION_DECL_P.

ah, wasn't aware about this macro.  thanks for point me to it.

 +   {
 + error (weak declaration of %q+D not allowed, decl);
 + return;
 +   }

 I think that shouldn't be an error, merely a warning.  Thus, use

   GCC_BAD2 (..., ignored, decl);

 instead?

Ok, I am fine to make it a warning.

 Also please add a testcase.

 Marek

Well, testcase for the pragma ...

ChangeLog testsuite/

2015-02-26  Kai Tietz  kti...@redhat.com

* gcc.dg/weak/weak-17.c: New file

weak-17.c:
/* { dg-do compile } */
/* { dg-require-weak  } */
#pragma weak int = foo

/* { dg-warning weak declaration weak { target *-*-* } 0 } */
--- end of weak-17.c

Updated patch (regression-tested):
Index: c-pragma.c
===
--- c-pragma.c  (Revision 221019)
+++ c-pragma.c  (Arbeitskopie)
@@ -392,6 +392,8 @@ handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy
   decl = identifier_global_value (name);
   if (decl  DECL_P (decl))
 {
+  if (!VAR_OR_FUNCTION_DECL_P (decl))
+   GCC_BAD2 (weak declaration of %q+D not allowed, ignored, decl);
   apply_pragma_weak (decl, value);
   if (value)
{

[patch c-family]: Fix Bug 35330 - [4.8/4.9/5 regression] ICE with invalid pragma weak

[Kai Tietz]

Hi,

This patch addresses the reported ICE about #pragma weak used on
declarations not var or function.

ChangeLog

2015-02-26  Kai Tietz  kti...@redhat.com

* c-pragma.c (handle_pragma_weak): Do not try to creat
weak/alias of declarations
not being function, or variable declarations.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.  Ok for apply?

Regards,
Kai

Index: c-pragma.c
===
--- c-pragma.c  (Revision 221019)
+++ c-pragma.c  (Arbeitskopie)
@@ -392,6 +392,11 @@ handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy
   decl = identifier_global_value (name);
   if (decl  DECL_P (decl))
 {
+  if (TREE_CODE (decl) != FUNCTION_DECL  TREE_CODE (decl) != VAR_DECL)
+   {
+ error (weak declaration of %q+D not allowed, decl);
+ return;
+   }
   apply_pragma_weak (decl, value);
   if (value)
{

Re: [C++ PATCH] Fix ICE in C++14 with null RETURN_EXPR (PR c++/65202)

[Kai Tietz]

Hi Marek,

I have similiar change on the c++-delayed-fold branch.

2015-02-26 13:22 GMT+01:00 Marek Polacek pola...@redhat.com:
 We ICE on this invalid testcase in C++14 because in C++14 a function returning
 void is a valid constexpr function, so adl_swap is registered as one, while in
 C++11 it is not registered.  Then later on, we weren't able to properly handle
 a RETURN_EXPR with null operand when trying to evaluate the expression.

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

 2015-02-26  Marek Polacek  pola...@redhat.com

 PR c++/65202
 * constexpr.c (cxx_eval_constant_expression): Handle null tree.

 * g++.dg/cpp1y/pr65202.C: New test.

 diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
 index 32a23ff7..1d2bb2f 100644
 --- gcc/cp/constexpr.c
 +++ gcc/cp/constexpr.c
 @@ -2935,7 +2935,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
 tree t,
constexpr_ctx new_ctx;
tree r = t;

 -  if (t == error_mark_node)
 +  if (t == NULL_TREE || t == error_mark_node)
  {
*non_constant_p = true;
return t;

Just one nit.  Shouldn't we return here always error_mark_node instead?

---
Kai

Re: [patch]: [Bug tree-optimization/61917] [4.9/5 Regression] ICE on valid code at -O3 on x86_64-linux-gnu in vectorizable_reduction, at tree-vect-loop.c:4913

[Kai Tietz]

2015-02-25 12:35 GMT+01:00 Richard Biener richard.guent...@gmail.com:
 On Wed, Feb 25, 2015 at 12:05 PM, Kai Tietz ktiet...@googlemail.com wrote:
 2015-02-25 11:57 GMT+01:00 Richard Biener richard.guent...@gmail.com:
 On Wed, Feb 25, 2015 at 11:06 AM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 ChangeLog

 2015-02-25  Kai Tietz  kti...@redhat.com

 PR tree-optimization/61917
 * tree-vect-loop.c (vectorizable_reduction): Allow
 vect_internal_def without reduction to exit graceful.

 ChagneLog testsuite/

 2015-02-25  Kai Tietz  kti...@redhat.com

 PR tree-optimization/61917
 * gcc.dg/vect/vect-pr61917.c: New file.

 Tested for x86_64-unkown-linux.  Ok for apply?

 It doesn't make much sense to fail here as said in the comment
 because of patterns if the actual case isn't a pattern.  Also
 the patch causing this made vect_external_def possible, so
 why does this affect vect_internal_def here?

 It may paper over the issue - but clearly the fix is bogus.

 Well, actually I don't see any good reason for failing here at all,
 but I assumed that author wanted to get by it some missed cases.
 AFAIU the code we actually don't need/want to assert here either on
 internal (where it is pretty likely no pattern present), and also on
 externals, too.
 I would be fine to remove this assert here completely, but I thought
 it might be of interest to see that orig_stmt isn't NULL for nested
 case

 I agree that the code is overusing asserts but the path you bail out
 on is bogus.  Fact is that we somehow detected a reduction here
 (via special-casing of minus I guess) but fail to properly handle it
 later.  In fact we assert that we end up with a PHI node in the definition
 but would ICE there as well.  Thus a better patch would be

 Index: gcc/tree-vect-loop.c
 ===
 --- gcc/tree-vect-loop.c(revision 220958)
 +++ gcc/tree-vect-loop.c(working copy)
 @@ -4981,6 +4981,13 @@ vectorizable_reduction (gimple stmt, gim
if (!vectype_in)
  vectype_in = tem;
gcc_assert (is_simple_use);
 +
 +  /* If the defining stmt isn't a PHI node then this isn't a reduction.  */
 +  if (!found_nested_cycle_def)
 +reduc_def_stmt = def_stmt;
 +  if (gimple_code (reduc_def_stmt) != GIMPLE_PHI)
 +return false;
 +
if (!(dt == vect_reduction_def
 || dt == vect_nested_cycle
 || ((dt == vect_internal_def || dt == vect_external_def
 @@ -4993,10 +5000,7 @@ vectorizable_reduction (gimple stmt, gim
gcc_assert (orig_stmt);
return false;
  }
 -  if (!found_nested_cycle_def)
 -reduc_def_stmt = def_stmt;

 -  gcc_assert (gimple_code (reduc_def_stmt) == GIMPLE_PHI);
if (orig_stmt)
  gcc_assert (orig_stmt == vect_is_simple_reduction (loop_vinfo,
 reduc_def_stmt,

 Richard.

Yes, your version is better for reader and shows the intention better.
I will give it a test.

Kai

 Richard.

 Regards,
 Kai

 Index: tree-vect-loop.c
 ===
 --- tree-vect-loop.c(Revision 220958)
 +++ tree-vect-loop.c(Arbeitskopie)
 @@ -4990,7 +4990,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
/* For pattern recognized stmts, orig_stmt might be a reduction,
   but some helper statements for the pattern might not, or
   might be COND_EXPRs with reduction uses in the condition.  */
 -  gcc_assert (orig_stmt);
 +  gcc_assert (orig_stmt || dt == vect_internal_def);
return false;
  }
if (!found_nested_cycle_def)
 Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
 ===
 --- /dev/null
 +++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
 @@ -0,0 +1,13 @@
 +/* { dg-do compile } */
 +/* { dg-additional-options -O3 } */
 +
 +int a, b, c, d;
 +
 +int
 +fn1 ()
 +{
 +  for (; c; c++)
 +for (b = 0; b  2; b++)
 +  d = a - d;
 +  return d;
 +}

Re: [patch]: [Bug tree-optimization/61917] [4.9/5 Regression] ICE on valid code at -O3 on x86_64-linux-gnu in vectorizable_reduction, at tree-vect-loop.c:4913

[Kai Tietz]

Hello,

So, I did full regression-test for following patch:

ChangeLog

2015-02-25  Richard Biener  rguent...@suse.de
Kai Tietz  kti...@redhat.com

PR tree-optimization/61917
* tree-vect-loop.c (vectorizable_reduction): Allow
vect_internal_def without reduction to exit graceful.

ChagneLog testsuite/

2015-02-25  Kai Tietz  kti...@redhat.com

PR tree-optimization/61917
* gcc.dg/vect/vect-pr61917.c: New file.

Tested for x86_64-unkown-linux.  Ok for apply?

Regards,
Kai

Index: tree-vect-loop.c
===
--- tree-vect-loop.c(Revision 220958)
+++ tree-vect-loop.c(Arbeitskopie)
@@ -4981,6 +4981,12 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
   if (!vectype_in)
 vectype_in = tem;
   gcc_assert (is_simple_use);
+  if (!found_nested_cycle_def)
+reduc_def_stmt = def_stmt;
+
+  if (gimple_code (reduc_def_stmt) != GIMPLE_PHI)
+return false;
+
   if (!(dt == vect_reduction_def
 || dt == vect_nested_cycle
 || ((dt == vect_internal_def || dt == vect_external_def
@@ -4993,10 +4999,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
   gcc_assert (orig_stmt);
   return false;
 }
-  if (!found_nested_cycle_def)
-reduc_def_stmt = def_stmt;

-  gcc_assert (gimple_code (reduc_def_stmt) == GIMPLE_PHI);
   if (orig_stmt)
 gcc_assert (orig_stmt == vect_is_simple_reduction (loop_vinfo,
reduc_def_stmt,
Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
===
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-additional-options -O3 } */
+
+int a, b, c, d;
+
+int
+fn1 ()
+{
+  for (; c; c++)
+for (b = 0; b  2; b++)
+  d = a - d;
+  return d;
+}

[patch]: Fix PR target/64212

[Kai Tietz]

Hi,

The issue here is a pe-coff target specific thing that
dllimported-symbols have an noninterposable, and an interposable part.
The dllimport address itself is not interposable, but its stubbing
function/var is.

So the hook binds_to_local has to return false for dllimport,
nevertheless for clones we want that they getting interposable.
Therefore - as suggested by Honza - we need to make explicit sure that
we set DECL_DLLIMPORT_P() explicit to 0 in symtab.

ChangeLog

2015-02-25  Kai Tietz  kti...@redhat.com

PR target/64212
* symtab.c (symtab::make_decl_local): Set DECL_IMPORT_P explicit to 0.
(symtab::noninterposable_alias): Likewise.

Tested for x86_64-w64-mingw32, i686-w64-mingw32, and
x86_64-unknown-linux-gnu.  Ok for apply?

Regards,
Kai

Index: symtab.c
===
--- symtab.c(Revision 220969)
+++ symtab.c(Arbeitskopie)
@@ -1165,6 +1165,7 @@ symtab_node::make_decl_local (void)
   DECL_VISIBILITY_SPECIFIED (decl) = 0;
   DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
   TREE_PUBLIC (decl) = 0;
+  DECL_DLLIMPORT_P (decl) = 0;
   if (!DECL_RTL_SET_P (decl))
 return;

@@ -1534,7 +1535,6 @@ symtab_node::noninterposable_alias (symtab_node *n
  != flags_from_decl_or_type (fn-decl))
   || DECL_ATTRIBUTES (node-decl) != DECL_ATTRIBUTES (fn-decl))
 return false;
-
   *(symtab_node **)data = node;
   return true;
 }
@@ -1566,6 +1566,7 @@ symtab_node::noninterposable_alias (void)

   /* Otherwise create a new one.  */
   new_decl = copy_node (node-decl);
+  DECL_DLLIMPORT_P (new_decl) = 0;
   DECL_NAME (new_decl) = clone_function_name (node-decl, localalias);
   if (TREE_CODE (new_decl) == FUNCTION_DECL)
 DECL_STRUCT_FUNCTION (new_decl) = NULL;

Re: [patch]: Fix PR target/64212

[Kai Tietz]

Applied at revision 22098 to trunk.  Jan approved patch on IRC.

Regards,
Kai

[patch]: Fix regression caused by fix for 61917

[Kai Tietz]

Hi,

The patch didn't handled the case for dt being vect_constant_def,
where of course the reduc_def_stmt is NULL.
By checking for NULL before testing for PHI, we now fallback for such
cases to old behavior and return in the next if-statment.

2015-02-25  Richard Biener  rguent...@suse.de
Kai Tietz  kti...@redhat.com

PR tree-optimization/61917
* tree-vect-loop.c (vectorizable_reduction): Handle obvious case
that reduc_def_stmt is null.

Tested and will apply as obvious to trunk and 4.9 if there are no objections.

Sorry for the noise.
Kai

Index: tree-vect-loop.c
===
--- tree-vect-loop.c(Revision 220968)
+++ tree-vect-loop.c(Arbeitskopie)
@@ -4912,7 +4912,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
   if (!found_nested_cycle_def)
 reduc_def_stmt = def_stmt;

-  if (gimple_code (reduc_def_stmt) != GIMPLE_PHI)
+  if (reduc_def_stmt  gimple_code (reduc_def_stmt) != GIMPLE_PHI)
 return false;

   if (!(dt == vect_reduction_def

[patch]: [Bug tree-optimization/61917] [4.9/5 Regression] ICE on valid code at -O3 on x86_64-linux-gnu in vectorizable_reduction, at tree-vect-loop.c:4913

[Kai Tietz]

Hello,

ChangeLog

2015-02-25  Kai Tietz  kti...@redhat.com

PR tree-optimization/61917
* tree-vect-loop.c (vectorizable_reduction): Allow
vect_internal_def without reduction to exit graceful.

ChagneLog testsuite/

2015-02-25  Kai Tietz  kti...@redhat.com

PR tree-optimization/61917
* gcc.dg/vect/vect-pr61917.c: New file.

Tested for x86_64-unkown-linux.  Ok for apply?
Regards,
Kai

Index: tree-vect-loop.c
===
--- tree-vect-loop.c(Revision 220958)
+++ tree-vect-loop.c(Arbeitskopie)
@@ -4990,7 +4990,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
   /* For pattern recognized stmts, orig_stmt might be a reduction,
  but some helper statements for the pattern might not, or
  might be COND_EXPRs with reduction uses in the condition.  */
-  gcc_assert (orig_stmt);
+  gcc_assert (orig_stmt || dt == vect_internal_def);
   return false;
 }
   if (!found_nested_cycle_def)
Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
===
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-additional-options -O3 } */
+
+int a, b, c, d;
+
+int
+fn1 ()
+{
+  for (; c; c++)
+for (b = 0; b  2; b++)
+  d = a - d;
+  return d;
+}

Re: [patch]: [Bug tree-optimization/61917] [4.9/5 Regression] ICE on valid code at -O3 on x86_64-linux-gnu in vectorizable_reduction, at tree-vect-loop.c:4913

[Kai Tietz]

2015-02-25 11:57 GMT+01:00 Richard Biener richard.guent...@gmail.com:
 On Wed, Feb 25, 2015 at 11:06 AM, Kai Tietz ktiet...@googlemail.com wrote:
 Hello,

 ChangeLog

 2015-02-25  Kai Tietz  kti...@redhat.com

 PR tree-optimization/61917
 * tree-vect-loop.c (vectorizable_reduction): Allow
 vect_internal_def without reduction to exit graceful.

 ChagneLog testsuite/

 2015-02-25  Kai Tietz  kti...@redhat.com

 PR tree-optimization/61917
 * gcc.dg/vect/vect-pr61917.c: New file.

 Tested for x86_64-unkown-linux.  Ok for apply?

 It doesn't make much sense to fail here as said in the comment
 because of patterns if the actual case isn't a pattern.  Also
 the patch causing this made vect_external_def possible, so
 why does this affect vect_internal_def here?

 It may paper over the issue - but clearly the fix is bogus.

Well, actually I don't see any good reason for failing here at all,
but I assumed that author wanted to get by it some missed cases.
AFAIU the code we actually don't need/want to assert here either on
internal (where it is pretty likely no pattern present), and also on
externals, too.
I would be fine to remove this assert here completely, but I thought
it might be of interest to see that orig_stmt isn't NULL for nested
case

Kai

 Richard.

 Regards,
 Kai

 Index: tree-vect-loop.c
 ===
 --- tree-vect-loop.c(Revision 220958)
 +++ tree-vect-loop.c(Arbeitskopie)
 @@ -4990,7 +4990,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
/* For pattern recognized stmts, orig_stmt might be a reduction,
   but some helper statements for the pattern might not, or
   might be COND_EXPRs with reduction uses in the condition.  */
 -  gcc_assert (orig_stmt);
 +  gcc_assert (orig_stmt || dt == vect_internal_def);
return false;
  }
if (!found_nested_cycle_def)
 Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
 ===
 --- /dev/null
 +++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
 @@ -0,0 +1,13 @@
 +/* { dg-do compile } */
 +/* { dg-additional-options -O3 } */
 +
 +int a, b, c, d;
 +
 +int
 +fn1 ()
 +{
 +  for (; c; c++)
 +for (b = 0; b  2; b++)
 +  d = a - d;
 +  return d;
 +}

Re: Fix a MinGW warning in libiberty/strerror.c

[Kai Tietz]

Hi Eli,

patch is reasonable and ok for me.

Thanks
Kai

2015-01-16 12:18 GMT+01:00 Eli Zaretskii e...@gnu.org:
 Ping!

 Date: Fri, 02 Jan 2015 12:54:47 +0200
 From: Eli Zaretskii e...@gnu.org

 When compiling GDB 7.8.1, I get this warning in libiberty:

  gcc -c -DHAVE_CONFIG_H -O0 -g3 -D__USE_MINGW_ACCESS  -I. -I./../include 
  -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic  
 ./strerror.c -o strerror.o
  ./strerror.c:472:12: warning: '_sys_nerr' redeclared without dllimport 
 attribute: previous dllimport ignored [-Wattributes]
  ./strerror.c:473:14: warning: '_sys_errlist' redeclared without 
 dllimport attribute: previous dllimport ignored [-Wattributes]

 This happens because the MinGW system headers have some special magic
 for these variables, which are imported from a system shared library.

 The solution I propose is to refrain from declaring variables that are
 actually macros, because this should be a sign that something tricky
 is going on:

 --- libiberty/strerror.c~02014-06-11 18:34:41 +0300
 +++ libiberty/strerror.c  2014-12-30 08:12:00 +0200
 @@ -469,8 +469,13 @@

  #else

 +
 +#ifndef sys_nerr
  extern int sys_nerr;
 +#endif
 +#ifndef sys_errlist
  extern char *sys_errlist[];
 +#endif

  #endif


 OK to commit this (with a suitable ChangeLog entry)?

Re: Fix a MinGW warning in libiberty/strerror.c

[Kai Tietz]

2015-01-16 12:50 GMT+01:00 Eli Zaretskii e...@gnu.org:
 Date: Fri, 16 Jan 2015 12:34:25 +0100
 From: Kai Tietz ktiet...@googlemail.com
 Cc: GCC Patches gcc-patches@gcc.gnu.org,
   gdb-patc...@sourceware.org gdb-patc...@sourceware.org

 Hi Eli,

 patch is reasonable and ok for me.

 Thanks.  Do I need to hear from someone else approving this, or can I
 go ahead and commit?

Well, from POV of the Windows-maintainer in gcc, the patch is ok.  As
libiberty is shared with other ventures, you might want to get an ok
by other ventures, too.

I would say, as long as there are no objections - and there weren't
any now for some time this patch waiting for comments - I would go
ahead and say patch is ok.

Regards,
Kai

Re: [patch c++]: Fix PR/61198: Crash when selecting specializations through aliases.

[Kai Tietz]

2014-12-19 0:14 GMT+01:00 Jason Merrill ja...@redhat.com:
 On 12/18/2014 01:16 PM, Kai Tietz wrote:

 Well, in general I would have assumed to be able to get alias decl of
 tmpl. Wasn't able to find a simple way to get it. So, by looking into
 source I found that most cases handling args  tmpl-args by using
 inner_most_template_args instead. Defaulting to
 innermost_template_args looks indeed a bit wrong, but seemed to work.


 It's wrong, it just papers over the bug.  There shouldn't be a mismatch at
 this point.

 The problem seems to be that most_general_template isn't actually returning
 the most general template, so we're trying to instantiate the
 partially-instantiated alias template with a full set of arguments: thus the
 mismatch.

 Jason


Yes, in most_general_template we don't loop for TEMPLATE_DECL_ALIAS.

ChangeLog

2014-12-19 Kai Tietz  kti...@redhat.com

  * pt.c (most_general_template): Don't break for template-alias.

Ok for apply (with testcase as posted before)?

Regards,
Kai

Index: pt.c
==
--- pt.c(Revision 218897)
+++ pt.c(Arbeitskopie)
@@ -19207,6 +19207,7 @@ most_general_template (tree decl)
break;

   if (CLASS_TYPE_P (TREE_TYPE (decl))
+  !TYPE_DECL_ALIAS_P (TYPE_NAME (TREE_TYPE (decl)))
   CLASSTYPE_TEMPLATE_SPECIALIZATION (TREE_TYPE (decl)))
break;

Re: [patch c++]: Fix PR/61198: Crash when selecting specializations through aliases.

[Kai Tietz]

2014-12-19 17:46 GMT+01:00 H.J. Lu hjl.to...@gmail.com:

 Did you forget to check in the testcase?

No, see rev 218956.

 --
 H.J.

Re: [patch c++]: Fix PR/61198: Crash when selecting specializations through aliases.

[Kai Tietz]

2014-12-19 18:22 GMT+01:00 Paolo Carlini paolo.carl...@oracle.com:
 Hi,


 On 12/19/2014 05:48 PM, Kai Tietz wrote:

 2014-12-19 17:46 GMT+01:00 H.J. Lu hjl.to...@gmail.com:

 Did you forget to check in the testcase?

 No, see rev 218956.

 But you want to move it to the cpp0x directory, and likely name it
 alias-decl-45.C or something similar. In the template directory is also
 compiled in c++98 mode and is failing for everybody.

 Paolo.

Ok, renamed and moved.

Kai

[patch c++]: Fix PR/61198: Crash when selecting specializations through aliases.

[Kai Tietz]

Hi,

this patch adds handling of aliases within templates and tries to
resolve specialization for them.

ChangeLog

2014-12-18  Kai Tietz  kti...@redhat.com

PR c++/61198
* pt.c (retrieve_specialization): Handle using.

Tested on x86_64-w64-mingw32.  Ok for apply?

Regards,
Kai

ChangeLog testsuite

2014-12-18  Kai Tietz  kti...@redhat.com

PR c++/61198
* g++.dg/template/using29.C: New file.

// { dg-do compile }

templateint herp, typename derp_t
struct broken
{
templatetypename target_t
using rebind = brokenherp, target_
};

templatetypename derp_t
struct broken2, derp_t
{
templatetypename target_t
using rebind = broken2, target_t;
};

int main(int argc, char **argv)
{
broken2, float::rebinddouble u;

return 0;
}

Index: pt.c
===
--- pt.c(Revision 218832)
+++ pt.c(Arbeitskopie)
@@ -1033,6 +1033,8 @@ optimize_specialization_lookup_p (tree tmpl)
 static tree
 retrieve_specialization (tree tmpl, tree args, hashval_t hash)
 {
+  tree tmpl_parms;
+
   if (tmpl == NULL_TREE)
 return NULL_TREE;

@@ -1044,10 +1046,20 @@ retrieve_specialization (tree tmpl, tree args, has

   /* There should be as many levels of arguments as there are
  levels of parameters.  */
-  gcc_assert (TMPL_ARGS_DEPTH (args)
-  == (TREE_CODE (tmpl) == TEMPLATE_DECL
-  ? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
-  : template_class_depth (DECL_CONTEXT (tmpl;
+  if (TREE_CODE (tmpl) == TEMPLATE_DECL)
+{
+  tmpl_parms = DECL_TEMPLATE_PARMS (tmpl);
+  if (TMPL_ARGS_DEPTH (args)  TMPL_PARMS_DEPTH (tmpl_parms))
+args = get_innermost_template_args
+(args, TMPL_PARMS_DEPTH (tmpl_parms));
+  gcc_assert (TMPL_ARGS_DEPTH (args)
+  == TMPL_PARMS_DEPTH (tmpl_parms));
+}
+  else
+{
+  tmpl_parms = DECL_CONTEXT (tmpl);
+  gcc_assert (TMPL_ARGS_DEPTH (args) == template_class_depth (tmpl_parms));
+}

   if (optimize_specialization_lookup_p (tmpl))
 {

Re: [patch c++]: Fix PR/61198: Crash when selecting specializations through aliases.

[Kai Tietz]

2014-12-18 18:26 GMT+01:00 Jason Merrill ja...@redhat.com:
 On 12/18/2014 10:10 AM, Kai Tietz wrote:

 +  if (TMPL_ARGS_DEPTH (args)  TMPL_PARMS_DEPTH (tmpl_parms))
 +args = get_innermost_template_args
 +(args, TMPL_PARMS_DEPTH (tmpl_parms));


 It seems unlikely to be correct to arbitrarily strip off outer template
 args.  Where did the mismatch come from?

 Jason


Hmm, we are comming from #2 tsubst_decl (cp/pt.c:11278)
#3 instantiate_template_1 (cp/pt.c:15945)
#4 instantiate_template (cp/pt.c:15995)
#5 instantiate_alias_template (cp/pt.c:11827)
#6: lookup_template_class_1
#7: lookup_template_class
#8: finish_template_type
#9: cp_parser_template_id


Well, in general I would have assumed to be able to get alias decl of
tmpl. Wasn't able to find a simple way to get it. So, by looking into
source I found that most cases handling args  tmpl-args by using
inner_most_template_args instead. Defaulting to
innermost_template_args looks indeed a bit wrong, but seemed to work.

Kai

Re: [patch c++]: Fix PR/63996

[Kai Tietz]

2014-12-15 11:48 GMT+01:00 Paolo Carlini paolo.carl...@oracle.com:
 ... committed as obvious the below.

 Paolo.

 /

Thanks
Kai

[patch c++]: Fix PR/63996

[Kai Tietz]

Hi,

The loop-expression loops endless in c++14's case for cases the
statement-list isn't constant.

Bug 63996 - Infinite loop in invalid C++14 constexpr fn

ChangeLog

2014-12-12  Kai Tietz  kti...@redhat.com

PR c++/63996
* constexpr.c (cxx_eval_loop_expr): Don't loop
endless on none-constant expression.


2014-12-12  Kai Tietz  kti...@redhat.com

PR c++/63996
* g++.dg/cpp1y/pr63996.C: New file.

Tested for x86_64-w64-mingw32.  Ok for apply?

Regards,
Kai

New testcase in g++.dg/cpp1y as pr63996.C

// { dg-do compile { target c++14 } }

constexpr int
foo (int i)
{
  int a[i] = { };
}

constexpr int j = foo (1); // { dg-error is not a constant expression }


Index: constexpr.c
===
--- constexpr.c (Revision 218570)
+++ constexpr.c (Arbeitskopie)
@@ -2841,7 +2870,7 @@ cxx_eval_loop_expr (const constexpr_ctx *ctx, tree
 {
   cxx_eval_statement_list (ctx, body,
   non_constant_p, overflow_p, jump_target);
-  if (returns (jump_target) || breaks (jump_target))
+  if (returns (jump_target) || breaks (jump_target) || *non_constant_p)
break;
 }
   if (breaks (jump_target))

[patch c++]: Fix 61228 - noexcept(expression) causes internal compiler error

[Kai Tietz]

Hi,

following patch fixes reported issue.
Tested for x86_64-w64-mingw32.  Ok for apply?

Regards,
Kai

ChangeLog

2014-12-12  Kai Tietz  kti...@redhat.com

PR c++/61228
* call.c (set_flags_from_callee): Assume no throw
by deferred noexcept.

2014-12-12  Kai Tietz  kti...@redhat.com

PR c++/61228
* g++.dg/cpp0x/pr61228.C: New file.

ChangeLog testcase/g++.dg/cpp0x as pr61228.C:
// { dg-do run { target c++11 } }

#include cctype
#include algorithm

templateint ( F)(int)
constexpr int safeCtype(unsigned char c) noexcept(noexcept(F(c)))
{ return F(c); }

int main()
{
  const char t[] = a;
  std::find_if(t, t + 1, safeCtypestd::isspace);
  return 0;
}

Index: call.c
===
--- call.c  (Revision 218681)
+++ call.c  (Arbeitskopie)
@@ -335,11 +335,17 @@ set_flags_from_callee (tree call)
 {
   int nothrow;
   tree decl = get_callee_fndecl (call);
+  tree spec;

   /* We check both the decl and the type; a function may be known not to
  throw without being declared throw().  */
-  nothrow = ((decl  TREE_NOTHROW (decl))
-|| TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call);
+  nothrow = (decl  TREE_NOTHROW (decl));
+  if (!nothrow)
+{
+  spec = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)));
+  nothrow = (!DEFERRED_NOEXCEPT_SPEC_P (TYPE_RAISES_EXCEPTIONS (spec))
+  TYPE_NOTHROW_P (spec));
+}

   if (!nothrow  at_function_scope_p ()  cfun  cp_function_chain)
 cp_function_chain-can_throw = 1;

Re: [patch c++]: Fix 61228 - noexcept(expression) causes internal compiler error

[Kai Tietz]

2014-12-12 21:15 GMT+01:00 Jason Merrill ja...@redhat.com:
 I think it would be better to call maybe_instantiate_noexcept so that we can
 have a definite answer.

 Jason

Hmm, for case that decl != NULL_TREE, this is ok.  But what if decl is
NULL_TREE?

Kai

[patch c++]: Fix PR/64100 'A static assert using the the current class in a noexcept test leads to a segfault'

[Kai Tietz]

Hi,

 The issue is that lookup_destructor calls
adjust_result_of_qualified_name_lookup
with an NULL_TREE decl (returned by lookup_member).  So error-message
is missing.

As already discussed in bug-tracker:

ChangeLog

2014-12-04  Kai Tietz  kti...@redhat.com

PR c++/64100
* typeck.c (lookup_destructor): Handle incomplete
type.

Tested on x86_64-unknown-linux-gnu.

Ok for apply?

Kai
Index: typeck.c
===
--- typeck.c(Revision 218309)
+++ typeck.c(Arbeitskopie)
@@ -2536,6 +2535,12 @@ lookup_destructor (tree object, tree scope, tree d
   expr = lookup_member (dtor_type, complete_dtor_identifier,
 /*protect=*/1, /*want_type=*/false,
 tf_warning_or_error);
+  if (!expr)
+{
+  if (complain  tf_error)
+cxx_incomplete_type_error (dtor_name, dtor_type);
+  return error_mark_node;
+}
   expr = (adjust_result_of_qualified_name_lookup
   (expr, dtor_type, object_type));
   if (scope == NULL_TREE)

[patch c++]: Fix PR/64127 ICE on invalid: tree check: exprected identifier_node, have template_id_expr in cp_parser_diagnose_invalid_type_name

[Kai Tietz]

Hi,

this patch fixes an ICE happening on invalid code for  c++11.  It is
reasoned by
accessing blindly identifier without checking that it is a declaration.

ChangeLog

2014-12-04  Kai Tietz  kti...@redhat.com

PR c++/64127
* parser.c (cp_parser_diagnose_invalid_type_name): Check
id for being a declaration before accessing identifier.

Tested on x86_64-unknown-linux-gnu.

Ok for apply?

Regards,
Kai

Index: parser.c
===
--- parser.c(Revision 218309)
+++ parser.c(Arbeitskopie)
@@ -2977,6 +2977,7 @@ cp_parser_diagnose_invalid_type_name (cp_parser *p
 inform (location, C++11 %noexcept% only available with 
 -std=c++11 or -std=gnu++11);
   else if (cxx_dialect  cxx11
+DECL_P (id)
 !strcmp (IDENTIFIER_POINTER (id), thread_local))
 inform (location, C++11 %thread_local% only available with 
 -std=c++11 or -std=gnu++11);

[patch c++]: Fix PR c++/64106 ICE on valid code

[Kai Tietz]

Hi,

this patch adds INDIRECT_REF support to cxx_eval_store_expression handling.
There is a different variant suggested by Marek, which adds additional
operand-0 to ref, which looks to me wrong.

ChangeLog gcc/cp

2014-12-04  Kai Tietz  kti...@redhat.com

PR c++/64106
* constexpr.c (cxx_eval_store_expression): Handle INDIRECT_REF.

ChangeLog testsuite

2014-12-04  Kai Tietz  kti...@redhat.com

PR c++/64106
* g++.dg/pr64106.C: New file.

Tested on x86_64-unknown-linux-gnu.

Ok for apply?

Regards,
Kai

Index: constexpr.c
===
--- constexpr.c (Revision 218142)
+++ constexpr.c (Arbeitskopie)
@@ -2486,7 +2550,9 @@ cxx_eval_store_expression (const constexpr_ctx *ct
  vec_safe_push (refs, TREE_TYPE (probe));
  probe = TREE_OPERAND (probe, 0);
  break;
-
+   case INDIRECT_REF:
+ probe = TREE_OPERAND (probe, 0);
+ break;
default:
  object = probe;
  gcc_assert (DECL_P (object));
Index: gcc/gcc/testsuite/g++.dg/pr64106.C
===
--- /dev/null
+++ gcc/gcc/testsuite/g++.dg/pr64106.C
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+
+void
+f (long c, int lc, char *out)
+{
+  while (lc = 8) *out++ = (c  (lc -= 8));
+}

Re: [patch c++]: Fix PR/64100 'A static assert using the the current class in a noexcept test leads to a segfault'

[Kai Tietz]

2014-12-04 16:46 GMT+01:00 Marek Polacek pola...@redhat.com:
 On Thu, Dec 04, 2014 at 04:00:34PM +0100, Kai Tietz wrote:
 Hi,

  The issue is that lookup_destructor calls
 adjust_result_of_qualified_name_lookup
 with an NULL_TREE decl (returned by lookup_member).  So error-message
 is missing.

 As already discussed in bug-tracker:

 ChangeLog

 2014-12-04  Kai Tietz  kti...@redhat.com

 PR c++/64100
 * typeck.c (lookup_destructor): Handle incomplete
 type.

 Tested on x86_64-unknown-linux-gnu.

 Ok for apply?

 Testcase?

 Marek

Well, I spared that as this is an invalid code bug with a lot of
side-errors, which might change over time.

If wanted I can add testcase to g++/template/ collection

Kai

Re: [patch c++]: Fix PR/64127 ICE on invalid: tree check: exprected identifier_node, have template_id_expr in cp_parser_diagnose_invalid_type_name

[Kai Tietz]

2014-12-04 16:47 GMT+01:00 Marek Polacek pola...@redhat.com:
 On Thu, Dec 04, 2014 at 04:12:02PM +0100, Kai Tietz wrote:
 Hi,

 this patch fixes an ICE happening on invalid code for  c++11.  It is
 reasoned by
 accessing blindly identifier without checking that it is a declaration.

 ChangeLog

 2014-12-04  Kai Tietz  kti...@redhat.com

 PR c++/64127
 * parser.c (cp_parser_diagnose_invalid_type_name): Check
 id for being a declaration before accessing identifier.

 Tested on x86_64-unknown-linux-gnu.

 Ok for apply?

 Testcase?

 Marek

Same as said before.  Issue is a invalid-code bug with ICE, and
error-messages are pretty meaningless.  It would be helpful to have in
testsuite just the opportunity to test for no ICE.

Anyway, if testcase is requested, I can add it to g++.dg/ collection

Kai

Re: [patch c++]: Fix PR/64127 ICE on invalid: tree check: exprected identifier_node, have template_id_expr in cp_parser_diagnose_invalid_type_name

[Kai Tietz]

So added testcase for this pr (its c++98 only)

So:

ChangeLog testsuite

2014-12-04  Kai Tietz  kti...@redhat.com

  PR c++/64127
  * g++.dg/cpp/pr64127.C: New file.

Tested on x86_64-unknown-linux-gnu.

Ok to apply prior posted patch plus this new testcase?

Regards,
Kai

Index: gcc/gcc/testsuite/g++.dg/cpp/pr64127.C
===
--- /dev/null
+++ gcc/gcc/testsuite/g++.dg/cpp/pr64127.C
@@ -0,0 +1,9 @@
+/* { dg-do compile { target c++98_only } } */
+
+template 0 int __copy_streambufs_eof; // { dg-error  }
+// { dg-error numeric constant  { target *-*-* } 3 }
+// { dg-warning variable templates  { target *-*-* } 3 }
+__copy_streambufs_eof  // { dg-error  }
+// { dg-error parse error  { target *-*-* } 6 }
+// { dg-error not name a type  { target *-*-* } 6 }
+

Re: [patch c++]: Fix PR/64127 ICE on invalid: tree check: exprected identifier_node, have template_id_expr in cp_parser_diagnose_invalid_type_name

[Kai Tietz]

2014-12-04 20:35 GMT+01:00 Jason Merrill ja...@redhat.com:
 On 12/04/2014 10:12 AM, Kai Tietz wrote:

 else if (cxx_dialect  cxx11
 +DECL_P (id)
   !strcmp (IDENTIFIER_POINTER (id), thread_local))


 This doesn't make any sense: If it's a decl it isn't an identifier.  Did you
 mean to check for IDENTIFIER_NODE?

 Jason


Sure ...