Re: [PATCH] Fix inline memcpy ICE (PR tree-optimization/86526)
On Mon, 16 Jul 2018, Jakub Jelinek wrote:
> Hi!
>
> builtin_memcpy_read_str is a function meant to be called just as a callback
> and verifies that we don't cross a '\0' boundary in the string. For
> inline_string_cmp, we've checked that the length returned from c_getstr
> is fine, so we can cross as many embedded NULs as there are within the
> TREE_STRING_LENGTH.
>
> The rest of the patch is just a temporary to avoid using as_a twice in
> each loop iteration, and lots of formatting fixes, mostly to avoid trailing
> whitespace.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2018-07-16 Jakub Jelinek
>
> PR tree-optimization/86526
> * builtins.c (expand_builtin_memcmp): Formatting fixes.
> (inline_expand_builtin_string_cmp): Likewise.
> (inline_string_cmp): Likewise. Use c_readstr instead of
> builtin_memcpy_read_str. Add unit_mode temporary.
>
> * gcc.c-torture/compile/pr86526.c: New test.
>
> --- gcc/builtins.c.jj 2018-07-16 09:42:25.743985945 +0200
> +++ gcc/builtins.c2018-07-16 11:47:46.535014889 +0200
> @@ -4454,19 +4454,19 @@ expand_builtin_memcmp (tree exp, rtx tar
>no_overflow = check_access (exp, /*dst=*/NULL_TREE, /*src=*/NULL_TREE,
> len, /*maxread=*/NULL_TREE, size,
> /*objsize=*/NULL_TREE);
> - if (no_overflow)
> + if (no_overflow)
> {
>size = compute_objsize (arg2, 0);
>no_overflow = check_access (exp, /*dst=*/NULL_TREE, /*src=*/NULL_TREE,
> len, /*maxread=*/NULL_TREE, size,
> /*objsize=*/NULL_TREE);
> -}
> +}
>
> - /* Due to the performance benefit, always inline the calls first
> + /* Due to the performance benefit, always inline the calls first
> when result_eq is false. */
>rtx result = NULL_RTX;
> -
> - if (!result_eq && fcode != BUILT_IN_BCMP && no_overflow)
> +
> + if (!result_eq && fcode != BUILT_IN_BCMP && no_overflow)
> {
>result = inline_expand_builtin_string_cmp (exp, target, true);
>if (result)
> @@ -6748,7 +6748,7 @@ expand_builtin_goacc_parlevel_id_size (t
>return target;
> }
>
> -/* Expand a string compare operation using a sequence of char comparison
> +/* Expand a string compare operation using a sequence of char comparison
> to get rid of the calling overhead, with result going to TARGET if
> that's convenient.
>
> @@ -6757,7 +6757,7 @@ expand_builtin_goacc_parlevel_id_size (t
> LENGTH is the number of chars to compare;
> CONST_STR_N indicates which source string is the constant string;
> IS_MEMCMP indicates whether it's a memcmp or strcmp.
> -
> +
> to: (assume const_str_n is 2, i.e., arg2 is a constant string)
>
> target = var_str[0] - const_str[0];
> @@ -6772,41 +6772,38 @@ expand_builtin_goacc_parlevel_id_size (t
>*/
>
> static rtx
> -inline_string_cmp (rtx target, tree var_str, const char* const_str,
> +inline_string_cmp (rtx target, tree var_str, const char *const_str,
> unsigned HOST_WIDE_INT length,
> int const_str_n, machine_mode mode,
> -bool is_memcmp)
> +bool is_memcmp)
> {
>HOST_WIDE_INT offset = 0;
> - rtx var_rtx_array
> + rtx var_rtx_array
> = get_memory_rtx (var_str, build_int_cst (unsigned_type_node,length));
>rtx var_rtx = NULL_RTX;
> - rtx const_rtx = NULL_RTX;
> - rtx result = target ? target : gen_reg_rtx (mode);
> - rtx_code_label *ne_label = gen_label_rtx ();
> + rtx const_rtx = NULL_RTX;
> + rtx result = target ? target : gen_reg_rtx (mode);
> + rtx_code_label *ne_label = gen_label_rtx ();
>tree unit_type_node = is_memcmp ? unsigned_char_type_node : char_type_node;
> + scalar_int_mode unit_mode
> += as_a TYPE_MODE (unit_type_node);
>
>start_sequence ();
>
>for (unsigned HOST_WIDE_INT i = 0; i < length; i++)
> {
> - var_rtx
> + var_rtx
> = adjust_address (var_rtx_array, TYPE_MODE (unit_type_node), offset);
> - const_rtx
> - = builtin_memcpy_read_str (CONST_CAST (char *, const_str),
> -offset,
> -as_a
> -TYPE_MODE (unit_type_node));
> + const_rtx = c_readstr (const_str + offset, unit_mode);
>rtx op0 = (const_str_n == 1) ? const_rtx : var_rtx;
>rtx op1 = (const_str_n == 1) ? var_rtx : const_rtx;
> -
> - result = expand_simple_binop (mode, MINUS, op0, op1,
> - result, is_memcmp ? 1 : 0, OPTAB_WIDEN);
> - if (i < length - 1)
> -emit_cmp_and_jump_insns (result, CONST0_RTX (mode), NE, NULL_RTX,
> - mode, true, ne_label);
> - offset
> - += GET_MODE_SIZE (as_a TYPE_MODE (unit_type_node));
> +
> + result = expand_simple_binop (mode, MINUS, op0, op1,
> +
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
> 2018-07-16 21:50 GMT+02:00 Thomas Koenig : >> What I would suggest is to enable -Wfunction-eliminiation with >> -Wextra and also use that for your new warning. > > Thanks for the comments. Makes sense. Updated patch attached. Huh, after actually trying -Wfunction-elimination, I'm not so sure any more if it really makes sense to throw the new warnings in there, mainly for two reasons: a) -Wfunction-elimination is slightly different, in that it warns about removing *any* kind of function, not just impure ones. This makes it pretty verbose, and I'm not sure why one would need to know if a pure function call is removed. b) It gives warnings on places that I don't understand. Simple example: subroutine sub(r) implicit none real, intent(in) :: r if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then print *, "rrr" end if end Compiling this with "gfortran-8 -O1 -Wfunction-elimination" gives me: if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then 1 Warning: Removing call to function ‘abs’ at (1) [-Wfunction-elimination] Why can I remove the call to ABS there? If I read the dump correctly, then the two calls to ABS are optimized into a single one, which is used for both comparisons via a temporary. Is that what the warning is trying to tell me? And if yes, why should I care (if the function is pure)? The middle-end certainly does tons of optimizations that rearrange various expressions, without warning me about it ... In other words: Does it make sense to tone down -Wfunction-elimination, by only warning about impure functions? (We certainly have the diagnostic capabilites for this.) If not, I'd rather have a separate flag for the new warnings. -Wfunction-elimination is just too noisy for my taste in its current form. Cheers, Janus
[committed] Fix OpenMP taskloop handling of reference temporaries (PR middle-end/86539)
Hi!
The FE guarantees that the iterator type the middle-end sees is either some
integral type, or pointer type. For taskloop, we need to pass around
(firstprivatize) the b and lb expressions if they aren't simple constants,
but unfortunately for the middle-end reference vs. pointer type differences
are considered useless, so when we gimplify (int *) x where int &x, we can
end up with a temporary that has int & type. That matters for
firstprivate clause though, int * is firstprivatized by copying the pointer,
while int & is firstprivatized by adding an int temporary, initializing that
temporary from the orig reference and initializing the new private reference
to the address of the int temporary.
So, the following code ensures that the temporary we firstprivatize has the
right type.
Bootstrapped/regtested on x86_64-linux and i686-linux, queued for
backporting.
2018-07-17 Jakub Jelinek
PR middle-end/86539
* gimplify.c (gimplify_omp_for): Ensure taskloop firstprivatized init
and cond temporaries don't have reference type if iterator has
pointer type. For init use &for_pre_body instead of pre_p if
for_pre_body is non-empty.
* testsuite/libgomp.c++/pr86539.C: New test.
--- gcc/gimplify.c.jj 2018-07-10 09:11:24.251062149 +0200
+++ gcc/gimplify.c 2018-07-16 19:10:25.386498090 +0200
@@ -9811,9 +9811,26 @@ gimplify_omp_for (tree *expr_p, gimple_s
t = TREE_VEC_ELT (OMP_FOR_INIT (for_stmt), i);
if (!is_gimple_constant (TREE_OPERAND (t, 1)))
{
+ tree type = TREE_TYPE (TREE_OPERAND (t, 0));
TREE_OPERAND (t, 1)
= get_initialized_tmp_var (TREE_OPERAND (t, 1),
- pre_p, NULL, false);
+ gimple_seq_empty_p (for_pre_body)
+ ? pre_p : &for_pre_body, NULL,
+ false);
+ /* Reference to pointer conversion is considered useless,
+but is significant for firstprivate clause. Force it
+here. */
+ if (TREE_CODE (type) == POINTER_TYPE
+ && (TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 1)))
+ == REFERENCE_TYPE))
+ {
+ tree v = create_tmp_var (TYPE_MAIN_VARIANT (type));
+ tree m = build2 (INIT_EXPR, TREE_TYPE (v), v,
+ TREE_OPERAND (t, 1));
+ gimplify_and_add (m, gimple_seq_empty_p (for_pre_body)
+ ? pre_p : &for_pre_body);
+ TREE_OPERAND (t, 1) = v;
+ }
tree c = build_omp_clause (input_location,
OMP_CLAUSE_FIRSTPRIVATE);
OMP_CLAUSE_DECL (c) = TREE_OPERAND (t, 1);
@@ -9825,11 +9842,26 @@ gimplify_omp_for (tree *expr_p, gimple_s
t = TREE_VEC_ELT (OMP_FOR_COND (for_stmt), i);
if (!is_gimple_constant (TREE_OPERAND (t, 1)))
{
+ tree type = TREE_TYPE (TREE_OPERAND (t, 0));
TREE_OPERAND (t, 1)
= get_initialized_tmp_var (TREE_OPERAND (t, 1),
gimple_seq_empty_p (for_pre_body)
? pre_p : &for_pre_body, NULL,
false);
+ /* Reference to pointer conversion is considered useless,
+but is significant for firstprivate clause. Force it
+here. */
+ if (TREE_CODE (type) == POINTER_TYPE
+ && (TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 1)))
+ == REFERENCE_TYPE))
+ {
+ tree v = create_tmp_var (TYPE_MAIN_VARIANT (type));
+ tree m = build2 (INIT_EXPR, TREE_TYPE (v), v,
+ TREE_OPERAND (t, 1));
+ gimplify_and_add (m, gimple_seq_empty_p (for_pre_body)
+ ? pre_p : &for_pre_body);
+ TREE_OPERAND (t, 1) = v;
+ }
tree c = build_omp_clause (input_location,
OMP_CLAUSE_FIRSTPRIVATE);
OMP_CLAUSE_DECL (c) = TREE_OPERAND (t, 1);
--- libgomp/testsuite/libgomp.c++/pr86539.C.jj 2018-07-16 18:56:40.491472344
+0200
+++ libgomp/testsuite/libgomp.c++/pr86539.C 2018-07-16 18:56:34.068464199
+0200
@@ -0,0 +1,28 @@
+// PR middle-end/86539
+
+int a[384];
+
+__attribute__((noipa)) void
+foo (int &b, int &c)
+{
+ #pragma omp taskloop shared (a) collapse(3)
+ for (int i = 0; i < 1; i++)
+for (int *p = &b; p < &c; p++)
+ for (int j = 0; j < 1; j++)
+ if (p < &a[128] || p >= &a[256])
+ __builtin_abort ();
+ else
+ p[0]++;
+}
+
+int
+main ()
+{
+ #pragma omp parallel
+ #pragma omp single
+foo (a[128], a[256]);
+ f
[Ada] Fix Next_Actual when used on calls "inlined for proof"
The GNATprove backend needs to apply antialiasing checks to subprogram calls that have been rewritten into null statements while "inlining for proof". This requires the First_Actual/Next_Actual to use the Original_Node and not the N_Null_Statement that rewriting leaves as a parent. Only effective in GNATprove mode, so no frontend test provided. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Piotr Trojanek gcc/ada/ * sem_util.adb (Next_Actual): If the parent is a N_Null_Statement, which happens for inlined calls, then fetch the next actual from the original AST.--- gcc/ada/sem_util.adb +++ gcc/ada/sem_util.adb @@ -21033,7 +21033,8 @@ package body Sem_Util is - function Next_Actual (Actual_Id : Node_Id) return Node_Id is - N : Node_Id; + N : Node_Id; + Par : constant Node_Id := Parent (Actual_Id); begin -- If we are pointing at a positional parameter, it is a member of a @@ -21053,11 +21054,22 @@ package body Sem_Util is -- In case of a build-in-place call, the call will no longer be a -- call; it will have been rewritten. -if Nkind_In (Parent (Actual_Id), N_Entry_Call_Statement, - N_Function_Call, - N_Procedure_Call_Statement) +if Nkind_In (Par, N_Entry_Call_Statement, + N_Function_Call, + N_Procedure_Call_Statement) then - return First_Named_Actual (Parent (Actual_Id)); + return First_Named_Actual (Par); + +-- In case of a call rewritten in GNATprove mode while "inlining +-- for proof" go to the original call. + +elsif Nkind (Par) = N_Null_Statement then + pragma Assert + (GNATprove_Mode +and then + Nkind (Original_Node (Par)) in N_Subprogram_Call); + + return First_Named_Actual (Original_Node (Par)); else return Empty; end if;
[Ada] Add elaboration-related switches to GNAT UGN
This patch adds compiler switches -gnatH and -gnatJ to section "Alphabetical
list of all switches" of the GNAT User Guide for Native.
Tested on x86_64-pc-linux-gnu, committed on trunk
2018-07-17 Hristian Kirtchev
gcc/ada/
* doc/gnat_ugn/building_executable_programs_with_gnat.rst: Add missing
sections on -gnatH and -gnatJ compiler switches.
* gnat_ugn.texi: Regenerate.--- gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
+++ gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
@@ -720,9 +720,9 @@ is passed to ``gcc`` (e.g., :switch:`-O`, :switch:`-gnato,` etc.)
.. index:: --RTS (gnatmake)
:switch:`--RTS={rts-path}`
- Specifies the default location of the runtime library. GNAT looks for the
- runtime
- in the following directories, and stops as soon as a valid runtime is found
+ Specifies the default location of the run-time library. GNAT looks for the
+ run-time
+ in the following directories, and stops as soon as a valid run-time is found
(:file:`adainclude` or :file:`ada_source_path`, and :file:`adalib` or
:file:`ada_object_path` present):
@@ -1505,7 +1505,7 @@ Alphabetical List of All Switches
In the example above, the first call to ``Detect_Aliasing`` fails with a
- ``Program_Error`` at runtime because the actuals for ``Val_1`` and
+ ``Program_Error`` at run time because the actuals for ``Val_1`` and
``Val_2`` denote the same object. The second call executes without raising
an exception because ``Self(Obj)`` produces an anonymous object which does
not share the memory location of ``Obj``.
@@ -1817,14 +1817,12 @@ Alphabetical List of All Switches
.. index:: -gnatg (gcc)
:switch:`-gnatg`
- Internal GNAT implementation mode. This should not be used for
- applications programs, it is intended only for use by the compiler
- and its run-time library. For documentation, see the GNAT sources.
- Note that :switch:`-gnatg` implies
- :switch:`-gnatw.ge` and
- :switch:`-gnatyg`
- so that all standard warnings and all standard style options are turned on.
- All warnings and style messages are treated as errors.
+ Internal GNAT implementation mode. This should not be used for applications
+ programs, it is intended only for use by the compiler and its run-time
+ library. For documentation, see the GNAT sources. Note that :switch:`-gnatg`
+ implies :switch:`-gnatw.ge` and :switch:`-gnatyg` so that all standard
+ warnings and all standard style options are turned on. All warnings and style
+ messages are treated as errors.
.. index:: -gnatG[nn] (gcc)
@@ -1839,6 +1837,13 @@ Alphabetical List of All Switches
Output usage information. The output is written to :file:`stdout`.
+.. index:: -gnatH (gcc)
+
+:switch:`-gnatH`
+ Legacy elaboration-checking mode enabled. When this switch is in effect, the
+ pre-18.x access-before-elaboration model becomes the de facto model.
+
+
.. index:: -gnati (gcc)
:switch:`-gnati{c}`
@@ -1874,6 +1879,27 @@ Alphabetical List of All Switches
Reformat error messages to fit on ``nn`` character lines
+.. index:: -gnatJ (gcc)
+
+:switch:`-gnatJ`
+ Permissive elaboration-checking mode enabled. When this switch is in effect,
+ the post-18.x access-before-elaboration model ignores potential issues with:
+
+ - Accept statements
+ - Activations of tasks defined in instances
+ - Assertion pragmas
+ - Calls from within an instance to its enclosing context
+ - Calls through generic formal parameters
+ - Calls to subprograms defined in instances
+ - Entry calls
+ - Indirect calls using 'Access
+ - Requeue statements
+ - Select statements
+ - Synchronous task suspension
+
+ and does not emit compile-time diagnostics or run-time checks.
+
+
.. index:: -gnatk (gcc)
:switch:`-gnatk={n}`
@@ -2195,7 +2221,7 @@ Alphabetical List of All Switches
.. index:: --RTS (gcc)
:switch:`--RTS={rts-path}`
- Specifies the default location of the runtime library. Same meaning as the
+ Specifies the default location of the run-time library. Same meaning as the
equivalent ``gnatmake`` flag (:ref:`Switches_for_gnatmake`).
@@ -5062,7 +5088,7 @@ switches refine this default behavior.
that a certain check will necessarily fail, it will generate code to
do an unconditional 'raise', even if checks are suppressed. The
compiler warns in this case. Another case in which checks may not be
- eliminated is when they are embedded in certain run time routines such
+ eliminated is when they are embedded in certain run-time routines such
as math library routines.
Of course, run-time checks are omitted whenever the compiler can prove
@@ -5858,7 +5884,7 @@ Debugging Control
Exception Handling Control
--
-GNAT uses two methods for handling exceptions at run-time. The
+GNAT uses two methods for handling exceptions at run time. The
``setjmp/longjmp`` method saves the context when entering
a frame with an exception handler. Then whe
[Ada] Secondary stack leak in loop iterator
When the evaluation of the loop iterator invokes a function whose result relies on the secondary stack the compiler does not generate code to release the consumed memory as soon as the loop terminates. After this patch the following test works fine. with Text_IO; use Text_IO; pragma Warnings (Off); with System.Secondary_Stack; pragma Warnings (On); procedure Sec_Stack_Leak is function F (X : String) return Integer is begin return 10; end F; function G (X : Integer) return String is begin return (1 .. X => 'x'); end G; procedure Info is new System.Secondary_Stack.Ss_Info (Put_Line); procedure Nest is begin for I in Integer range 1 .. 100 loop for J in Integer range 1 .. F (G (10_000)) loop null; end loop; Info; end loop; Info; end Nest; begin Info; Nest; Info; end Sec_Stack_Leak; Commands: gnatmake -q sec_stack_leak.adb sec_stack_leak | grep "Current allocated space :" | uniq Output: Current allocated space : 0 bytes Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Javier Miranda gcc/ada/ * sem_ch5.adb (Has_Call_Using_Secondary_Stack): Moved to library level to reuse it. (Analyze_Loop_Statement): Wrap the loop in a block when the evaluation of the loop iterator relies on the secondary stack.--- gcc/ada/sem_ch5.adb +++ gcc/ada/sem_ch5.adb @@ -83,6 +83,12 @@ package body Sem_Ch5 is -- messages. This variable is recursively saved on entry to processing the -- construct, and restored on exit. + function Has_Call_Using_Secondary_Stack (N : Node_Id) return Boolean; + -- N is the node for an arbitrary construct. This function searches the + -- construct N to see if any expressions within it contain function + -- calls that use the secondary stack, returning True if any such call + -- is found, and False otherwise. + procedure Preanalyze_Range (R_Copy : Node_Id); -- Determine expected type of range or domain of iteration of Ada 2012 -- loop by analyzing separate copy. Do the analysis and resolution of the @@ -2692,12 +2698,6 @@ package body Sem_Ch5 is -- forms. In this case it is not sufficent to check the static predicate -- function only, look for a dynamic predicate aspect as well. - function Has_Call_Using_Secondary_Stack (N : Node_Id) return Boolean; - -- N is the node for an arbitrary construct. This function searches the - -- construct N to see if any expressions within it contain function - -- calls that use the secondary stack, returning True if any such call - -- is found, and False otherwise. - procedure Process_Bounds (R : Node_Id); -- If the iteration is given by a range, create temporaries and -- assignment statements block to capture the bounds and perform @@ -2782,65 +2782,6 @@ package body Sem_Ch5 is end if; end Check_Predicate_Use; - - -- Has_Call_Using_Secondary_Stack -- - - - function Has_Call_Using_Secondary_Stack (N : Node_Id) return Boolean is - function Check_Call (N : Node_Id) return Traverse_Result; - -- Check if N is a function call which uses the secondary stack - - - -- Check_Call -- - - - function Check_Call (N : Node_Id) return Traverse_Result is -Nam : Node_Id; -Subp : Entity_Id; -Typ : Entity_Id; - - begin -if Nkind (N) = N_Function_Call then - Nam := Name (N); - - -- Obtain the subprogram being invoked - - loop - if Nkind (Nam) = N_Explicit_Dereference then - Nam := Prefix (Nam); - - elsif Nkind (Nam) = N_Selected_Component then - Nam := Selector_Name (Nam); - - else - exit; - end if; - end loop; - - Subp := Entity (Nam); - Typ := Etype (Subp); - - if Requires_Transient_Scope (Typ) then - return Abandon; - - elsif Sec_Stack_Needed_For_Return (Subp) then - return Abandon; - end if; -end if; - --- Continue traversing the tree - -return OK; - end Check_Call; - - function Check_Calls is new Traverse_Func (Check_Call); - - -- Start of processing for Has_Call_Using_Secondary_Stack - - begin - return Check_Calls (N) = Abandon; - end Has_Call_Using_Secondary_Stack; - -- Process_Bounds -- @@ -3644,6 +3585,56 @@ package body Sem_Ch5 is end; end if; + -- Wrap the loop in a block when the evaluation of the loop iterator
[Ada] Attach the special GNATprove HEAP entity to the Standard package
In GNATprove mode we use frontend cross-references to synthesize the Global contract of subprograms with SPARK_Mode => Off and represent a read/write via a pointer as a read/write of a special entity called HEAP. This entity is now attached to the Standard package, so that we can safely check the Ekind of its Scope, which now happens in Scope_Within. This only affects GNATprove, so no frontend test provided. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Piotr Trojanek gcc/ada/ * lib-xref-spark_specific.adb (Create_Heap): Attach the HEAP entity to the Standard package.--- gcc/ada/lib-xref-spark_specific.adb +++ gcc/ada/lib-xref-spark_specific.adb @@ -287,6 +287,7 @@ package body SPARK_Specific is Set_Ekind (Heap, E_Variable); Set_Is_Internal (Heap, True); + Set_Scope (Heap, Standard_Standard); Set_Has_Fully_Qualified_Name (Heap); end Create_Heap;
[Ada] Crash on case expression in build-in-place function
This patch modifies the recursive tree replication routine New_Copy_Tree to create new entities and remap old entities to the new ones for constructs in N_Expression_With_Actions nodes when requested by a caller. This in turn allows the build-in-place mechanism to avoid sharing entities between the 4 variants of returns it generates. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Hristian Kirtchev gcc/ada/ * exp_ch6.adb (Build_Heap_Or_Pool_Allocator): Ensure that scoping constructs and entities within receive new entities when replicating a tree. (Expand_N_Extended_Return_Statement): Ensure that scoping constructs and entities within receive new entities when replicating a tree. * sem_util.adb (New_Copy_Tree): Add new formal Scopes_In_EWA_OK. (Visit_Entity): Visit entities within scoping constructs inside expression with actions nodes when requested by the caller. Add blocks, labels, and procedures to the list of entities which need replication. * sem_util.ads (New_Copy_Tree): Add new formal Scopes_In_EWA_OK. Update the comment on usage. gcc/testsuite/ * gnat.dg/bip_case_expr.adb, gnat.dg/bip_case_expr_pkg.ads: New testcase.--- gcc/ada/exp_ch6.adb +++ gcc/ada/exp_ch6.adb @@ -4562,7 +4562,10 @@ package body Exp_Ch6 is Fin_Mas_Id : constant Entity_Id := Build_In_Place_Formal (Func_Id, BIP_Finalization_Master); - Orig_Expr : constant Node_Id := New_Copy_Tree (Alloc_Expr); + Orig_Expr : constant Node_Id := + New_Copy_Tree +(Source => Alloc_Expr, + Scopes_In_EWA_OK => True); Stmts : constant List_Id := New_List; Desig_Typ : Entity_Id; Local_Id : Entity_Id; @@ -5022,7 +5025,10 @@ package body Exp_Ch6 is Init_Assignment := Make_Assignment_Statement (Loc, Name => New_Occurrence_Of (Ret_Obj_Id, Loc), - Expression => New_Copy_Tree (Ret_Obj_Expr)); + Expression => +New_Copy_Tree + (Source => Ret_Obj_Expr, + Scopes_In_EWA_OK => True)); Set_Etype (Name (Init_Assignment), Etype (Ret_Obj_Id)); Set_Assignment_OK (Name (Init_Assignment)); @@ -5153,7 +5159,10 @@ package body Exp_Ch6 is Subtype_Mark => New_Occurrence_Of (Etype (Ret_Obj_Expr), Loc), -Expression => New_Copy_Tree (Ret_Obj_Expr))); +Expression => + New_Copy_Tree +(Source => Ret_Obj_Expr, + Scopes_In_EWA_OK => True))); else -- If the function returns a class-wide type we cannot @@ -5193,7 +5202,11 @@ package body Exp_Ch6 is -- except we set Storage_Pool and Procedure_To_Call so -- it will use the user-defined storage pool. - Pool_Allocator := New_Copy_Tree (Heap_Allocator); + Pool_Allocator := + New_Copy_Tree + (Source => Heap_Allocator, + Scopes_In_EWA_OK => True); + pragma Assert (Alloc_For_BIP_Return (Pool_Allocator)); -- Do not generate the renaming of the build-in-place @@ -5235,7 +5248,11 @@ package body Exp_Ch6 is -- allocation. else -SS_Allocator := New_Copy_Tree (Heap_Allocator); +SS_Allocator := + New_Copy_Tree +(Source => Heap_Allocator, + Scopes_In_EWA_OK => True); + pragma Assert (Alloc_For_BIP_Return (SS_Allocator)); -- The heap and pool allocators are marked as --- gcc/ada/sem_util.adb +++ gcc/ada/sem_util.adb @@ -19505,10 +19505,11 @@ package body Sem_Util is --- function New_Copy_Tree - (Source: Node_Id; - Map : Elist_Id := No_Elist; - New_Sloc : Source_Ptr := No_Location; - New_Scope : Entity_Id := Empty) return Node_Id + (Source : Node_Id; + Map : Elist_Id := No_Elist; + New_Sloc : Source_Ptr := No_Location; + New_Scope: Entity_Id := Empty; + Scopes_In_EWA_OK : Boolean:= False) return Node_Id is -- This routine per
[Ada] New ignored Ghost code removal mechanism
This patch reimplements the mechanism which removes ignored Ghost code from the
tree.
The previous mechanism proved to be unreliable because it assumed that no new
scoping constructs would be created after some ignored Ghost code had already
notified its enclosing scoping constructs that they contain such code. The
assumption can be broken by having a call to an ignored Ghost procedure within
the extended return statement of a function. The procedure call would signal
the enclosing function that it contains ignored Ghost code, however the return
statement would introduce an extra block, effectively hiding the procedure call
from the ignored Ghost code elimination pass.
The new mechanism implemented in this patch forgoes directed tree pruning in
favor of storing the actual ignored Ghost code, and later directly eliminating
it from the tree.
For this approach to operate efficiently, only "top level" ignored Ghost
constructs are stored. The top level constructs are essentially nodes which can
appear within a declarative or statement list and be safely rewritten into null
statements. This ensures that only "root" ignored Ghost construct need to be
processed, as opposed to all ignored Ghost nodes within a subtree.
The approach has one drawback however. Due to the generation and analysis of
ignored Ghost code, a construct may be recorded multiple times (usually twice).
The mechanism simply deals with this artefact instead of employing expensive
solutions such as hash tables or a common flag shared by all nodes to eliminate
the duplicates.
-- Source --
-- main.adb
with Ada.Text_IO; use Ada.Text_IO;
procedure Main is
procedure Ghost_Proc with Ghost;
procedure Ghost_Proc is
begin
Put_Line ("ERROR: Ghost_Proc called");
end Ghost_Proc;
function Func return Integer is
begin
return Res : Integer := 123 do
Ghost_Proc;
end return;
end Func;
Val : Integer with Ghost;
begin
Val := Func;
end Main;
-- Compilation and output --
$ gcc -c -gnatDG main.adb
$ grep -c "ghost" main.adb.dg
0
Tested on x86_64-pc-linux-gnu, committed on trunk
2018-07-17 Hristian Kirtchev
gcc/ada/
* alloc.ads: Update the allocation metrics of the ignored Ghost nodes
table.
* atree.adb: Add a soft link for a procedure which is invoked whenever
an ignored Ghost node or entity is created.
(Change_Node): Preserve relevant attributes which come from the Flags
table.
(Mark_New_Ghost_Node): Record a newly created ignored Ghost node or
entity.
(Rewrite): Preserve relevant attributes which come from the Flags
table.
(Set_Ignored_Ghost_Recording_Proc): New routine.
* atree.ads: Define an access-to-suprogram type for a soft link which
records a newly created ignored Ghost node or entity.
(Set_Ignored_Ghost_Recording_Proc): New routine.
* ghost.adb: Remove with and use clause for Lib. Remove table
Ignored_Ghost_Units. Add new table Ignored_Ghost_Nodes.
(Add_Ignored_Ghost_Unit): Removed.
(Initialize): Initialize the table which stores ignored Ghost nodes.
Set the soft link which allows Atree.Mark_New_Ghost_Node to record an
ignored Ghost node.
(Is_Ignored_Ghost_Unit): Use the ultimate original node when checking
an eliminated ignored Ghost unit.
(Lock): Release and lock the table which stores ignored Ghost nodes.
(Mark_And_Set_Ghost_Assignment): Record rather than propagate ignored
Ghost nodes.
(Mark_And_Set_Ghost_Procedure_Call): Record rather than propagate
ignored Ghost nodes.
(Mark_Ghost_Clause): Record rather than propagate ignored Ghost nodes.
(Mark_Ghost_Declaration_Or_Body): Record rather than propagate ignored
Ghost nodes.
(Mark_Ghost_Pragma): Record rather than propagate ignored Ghost nodes.
(Propagate_Ignored_Ghost_Code): Removed.
(Record_Ignored_Ghost_Node): New routine.
(Remove_Ignored_Ghost_Code): Reimplemented.
(Remove_Ignored_Ghost_Node): New routine.
(Ultimate_Original_Node): New routine.
* ghost.ads (Check_Ghost_Completion): Removed.
* sem_ch8.adb (Analyze_Use_Package): Remove obsolete code. Mark a use
package clause as ignored Ghost if applicable.
* sem_util.adb (Is_Body_Or_Package_Declaration): Reimplemented.--- gcc/ada/alloc.ads
+++ gcc/ada/alloc.ads
@@ -67,8 +67,8 @@ package Alloc is
In_Out_Warnings_Initial : constant := 100;-- Sem_Warn
In_Out_Warnings_Increment: constant := 100;
- Ignored_Ghost_Units_Initial : constant := 20; -- Sem_Util
- Ignored_Ghost_Units_Increment: constant := 50;
+ Ignored_Ghost_Nodes_Initial : constant := 100;-- Ghost
+ Ignored_Ghost_Nodes_Increment: constant := 100;
[Ada] Spurious error on unused Part_Of constituent
This patch updates the analysis of indicator Part_Of (or the lack thereof), to ignore generic formal parameters for purposes of determining the visible state space because they are not visible outside the generic and related instances. -- Source -- -- gen_pack.ads generic In_Formal : in Integer := 0; In_Out_Formal : in out Integer; package Gen_Pack is Exported_In_Formal : Integer renames In_Formal; Exported_In_Out_Formal : Integer renames In_Out_Formal; end Gen_Pack; -- pack.ads with Gen_Pack; package Pack with Abstract_State => State is procedure Force_Body; Val : Integer; private package OK_1 is new Gen_Pack (In_Out_Formal => Val) with Part_Of => State;-- OK package OK_2 is new Gen_Pack (In_Formal => 1, In_Out_Formal => Val) with Part_Of => State;-- OK package Error_1 is-- Error new Gen_Pack (In_Out_Formal => Val); package Error_2 is-- Error new Gen_Pack (In_Formal => 2, In_Out_Formal => Val); end Pack; -- pack.adb package body Pack with Refined_State => -- Error (State => (OK_1.Exported_In_Formal, OK_1.Exported_In_Out_Formal)) is procedure Force_Body is null; end Pack; -- gen_pack.ads generic In_Formal : in Integer := 0; In_Out_Formal : in out Integer; package Gen_Pack is Exported_In_Formal : Integer renames In_Formal; Exported_In_Out_Formal : Integer renames In_Out_Formal; end Gen_Pack; -- pack.ads with Gen_Pack; package Pack with Abstract_State => State is procedure Force_Body; Val : Integer; private package OK_1 is new Gen_Pack (In_Out_Formal => Val) with Part_Of => State;-- OK package OK_2 is new Gen_Pack (In_Formal => 1, In_Out_Formal => Val) with Part_Of => State;-- OK package Error_1 is-- Error new Gen_Pack (In_Out_Formal => Val); package Error_2 is-- Error new Gen_Pack (In_Formal => 2, In_Out_Formal => Val); end Pack; -- pack.adb package body Pack with Refined_State => -- Error (State => (OK_1.Exported_In_Formal, OK_1.Exported_In_Out_Formal)) is procedure Force_Body is null; end Pack; -- Compilation and output -- $ gcc -c pack.adb pack.adb:3:11: state "State" has unused Part_Of constituents pack.adb:3:11: constant "Exported_In_Formal" defined at gen_pack.ads:6, instance at pack.ads:15 pack.adb:3:11: variable "Exported_In_Out_Formal" defined at gen_pack.ads:7, instance at pack.ads:15 pack.ads:19:12: indicator Part_Of is required in this context (SPARK RM 7.2.6(2)) pack.ads:19:12: "Error_1" is declared in the private part of package "Pack" pack.ads:21:12: indicator Part_Of is required in this context (SPARK RM 7.2.6(2)) pack.ads:21:12: "Error_2" is declared in the private part of package "Pack" Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Hristian Kirtchev gcc/ada/ * sem_prag.adb (Has_Visible_State): Do not consider generic formals because they are not part of the visible state space. Add constants to the list of acceptable visible states. (Propagate_Part_Of): Do not consider generic formals when propagating the Part_Of indicator. * sem_util.adb (Entity_Of): Do not follow renaming chains which go through a generic formal because they are not visible for SPARK purposes. * sem_util.ads (Entity_Of): Update the comment on usage.--- gcc/ada/sem_prag.adb +++ gcc/ada/sem_prag.adb @@ -19982,6 +19982,13 @@ package body Sem_Prag is if not Comes_From_Source (Item_Id) then null; + -- Do not consider generic formals or their corresponding + -- actuals because they are not part of a visible state. + -- Note that both entities are marked as hidden. + + elsif Is_Hidden (Item_Id) then +null; + -- The Part_Of indicator turns an abstract state or an -- object into a constituent of the encapsulating state. @@ -28775,9 +28782,19 @@ package body Sem_Prag is if not Comes_From_Source (Item_Id) then null; +-- Do not consider generic formals or their corresponding actuals +-- because they are not part of a visible state. Note that both +-- entities are marked as hidden. + +
[PATCH] Fix PR86456
This cures the ICEs and wrong assembler that currently happen when
using -flto with -gdwarf-5. gdb is also happy with small test programs
but readelf is still complaining (see PR, I have no clue what goes
wrong).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap with -gdwarf-5 running ontop of the GCC 8 branch.
Jakub, do you know of any issues with dwarf5 supports in tools
like readelf? I've just checked that from binutils 2.31 but even
that isn't happy.
Anyhow, since this solves ICEs and gdb is happy I'm going to commit
it unless I hear objections (also to the branch after LTO bootstrap
and testing completet there).
Thanks,
Richard.
2018-07-17 Richard Biener
PR lto/86456
* dwarf2out.c (init_sections_and_labels): Always generate
a debug_line_str_section for early LTO debug.
(dwarf2out_finish): Reset debug_line_str_hash output early.
Bump counter for extra dwarf5 .debug_loc labels to not conflict
with fat LTO part.
(dwarf2out_early_finish): Output debug_line_str.
* g++.dg/debug/dwarf2/pr86456.C: New testcase.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 1e33cf07f09..bd45e0b0685 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -28488,7 +28488,7 @@ init_sections_and_labels (bool early_lto_debug)
debug_str_section = get_section (DEBUG_LTO_STR_SECTION,
DEBUG_STR_SECTION_FLAGS
| SECTION_EXCLUDE, NULL);
- if (!dwarf_split_debug_info && !output_asm_line_debug_info ())
+ if (!dwarf_split_debug_info)
debug_line_str_section
= get_section (DEBUG_LTO_LINE_STR_SECTION,
DEBUG_STR_SECTION_FLAGS | SECTION_EXCLUDE, NULL);
@@ -31149,6 +31149,11 @@ dwarf2out_finish (const char *)
/* Remove indirect string decisions. */
debug_str_hash->traverse (NULL);
+ if (debug_line_str_hash)
+ {
+ debug_line_str_hash->traverse (NULL);
+ debug_line_str_hash = NULL;
+ }
}
#if ENABLE_ASSERT_CHECKING
@@ -31433,8 +31438,8 @@ dwarf2out_finish (const char *)
switch_to_section (debug_loc_section);
if (dwarf_version >= 5)
{
- ASM_GENERATE_INTERNAL_LABEL (l1, DEBUG_LOC_SECTION_LABEL, 1);
- ASM_GENERATE_INTERNAL_LABEL (l2, DEBUG_LOC_SECTION_LABEL, 2);
+ ASM_GENERATE_INTERNAL_LABEL (l1, DEBUG_LOC_SECTION_LABEL, 2);
+ ASM_GENERATE_INTERNAL_LABEL (l2, DEBUG_LOC_SECTION_LABEL, 3);
if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
dw2_asm_output_data (4, 0x,
"Initial length escape value indicating "
@@ -32053,6 +32058,13 @@ dwarf2out_early_finish (const char *filename)
/* If we emitted any indirect strings, output the string table too. */
if (debug_str_hash || skeleton_debug_str_hash)
output_indirect_strings ();
+ if (debug_line_str_hash)
+{
+ switch_to_section (debug_line_str_section);
+ const enum dwarf_form form = DW_FORM_line_strp;
+ debug_line_str_hash->traverse (form);
+}
/* Switch back to the text section. */
switch_to_section (text_section);
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/pr86456.C
b/gcc/testsuite/g++.dg/debug/dwarf2/pr86456.C
new file mode 100644
index 000..56e08f39e64
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/pr86456.C
@@ -0,0 +1,5 @@
+// { dg-do compile }
+// { dg-require-effective-target lto }
+// { dg-options "-g -gdwarf-5 -flto -ffat-lto-objects" }
+
+int a;
[Ada] Secondary stack leak in statements block located in a loop
When a loop iterator has a block declaration containing statements that invoke functions whose result is returned on the secondary stack (such as a string-returning function), the compiler fails to generate code to release the allocated memory when the loop terminates. After this patch the following test works fine. with Ada.Text_IO; use Ada.Text_IO; with Ada.Strings.Unbounded; use Ada.Strings.Unbounded; pragma Warnings (Off); with System.Secondary_Stack; pragma Warnings (On); procedure Small is procedure Info is new System.Secondary_Stack.Ss_Info (Put_Line); US : Unbounded_String; begin Info; for J in 1 .. 100_000 loop Leaky_Block : declare begin if (J mod 2) = 0 then Info; end if; Ada.Text_IO.Put_Line (To_String (US)); -- Test if (J mod 2) = 0 then Info; end if; end Leaky_Block; end loop; Info; end; Command: gnatmake small.adb; small | grep "Current allocated space :" | uniq Output: Current allocated space : 0 bytes Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Javier Miranda gcc/ada/ * exp_ch7.adb (Make_Transient_Block): When determining whether an enclosing scope already handles the secondary stack, take into account transient blocks nested in a block that do not manage the secondary stack and are located within a loop.--- gcc/ada/exp_ch7.adb +++ gcc/ada/exp_ch7.adb @@ -8695,9 +8695,33 @@ package body Exp_Ch7 is Action : Node_Id; Par: Node_Id) return Node_Id is + function Within_Loop_Statement (N : Node_Id) return Boolean; + -- Return True when N appears within a loop and no block is containing N + function Manages_Sec_Stack (Id : Entity_Id) return Boolean; -- Determine whether scoping entity Id manages the secondary stack + --- + -- Within_Loop_Statement -- + --- + + function Within_Loop_Statement (N : Node_Id) return Boolean is + Par : Node_Id := Parent (N); + + begin + while not (Nkind_In (Par, + N_Loop_Statement, + N_Handled_Sequence_Of_Statements, + N_Package_Specification) + or else Nkind (Par) in N_Proper_Body) + loop +pragma Assert (Present (Par)); +Par := Parent (Par); + end loop; + + return Nkind (Par) = N_Loop_Statement; + end Within_Loop_Statement; + --- -- Manages_Sec_Stack -- --- @@ -8780,6 +8804,16 @@ package body Exp_Ch7 is elsif Ekind (Scop) = E_Loop then exit; +-- Ditto when the block appears without a block that does not +-- manage the secondary stack and is located within a loop. + +elsif Ekind (Scop) = E_Block + and then not Manages_Sec_Stack (Scop) + and then Present (Block_Node (Scop)) + and then Within_Loop_Statement (Block_Node (Scop)) +then + exit; + -- The transient block does not need to manage the secondary stack -- when there is an enclosing construct which already does that. -- This optimization saves on SS_Mark and SS_Release calls but may
[Ada] Spurious error on Part_Of indicator
This patch modifies the verification of a missing Part_Of indicator to avoid considering constants as visible state of a package instantiation because the compiler cannot determine whether their values depend on variable input. This diagnostic is left to GNATprove. -- Source -- -- gnat.adc pragma SPARK_Mode; -- gen_pack.ads generic package Gen_Pack is Val : constant Integer := 123; end Gen_Pack; -- pack.ads with Gen_Pack; package Pack with Abstract_State => Pack_State is procedure Force_Body; private package Inst_1 is new Gen_Pack; -- OK package Inst_2 is new Gen_Pack with Part_Of => Pack_State;-- OK end Pack; -- pack.adb package body Pack with Refined_State => (Pack_State => Inst_2.Val) is procedure Force_Body is null; end Pack; - -- Compilation -- - $ gcc -c pack.adb Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Hristian Kirtchev gcc/ada/ * sem_prag.adb (Has_Visible_State): Do not consider constants as visible state because it is not possible to determine whether a constant depends on variable input. (Propagate_Part_Of): Add comment clarifying the behavior with respect to constant.--- gcc/ada/sem_prag.adb +++ gcc/ada/sem_prag.adb @@ -19991,6 +19991,9 @@ package body Sem_Prag is -- The Part_Of indicator turns an abstract state or an -- object into a constituent of the encapsulating state. + -- Note that constants are considered here even though + -- they may not depend on variable input. This check is + -- left to the SPARK prover. elsif Ekind_In (Item_Id, E_Abstract_State, E_Constant, @@ -28789,12 +28792,12 @@ package body Sem_Prag is elsif Is_Hidden (Item_Id) then null; --- A visible state has been found +-- A visible state has been found. Note that constants are not +-- considered here because it is not possible to determine whether +-- they depend on variable input. This check is left to the SPARK +-- prover. -elsif Ekind_In (Item_Id, E_Abstract_State, - E_Constant, - E_Variable) -then +elsif Ekind_In (Item_Id, E_Abstract_State, E_Variable) then return True; -- Recursively peek into nested packages and instantiations
[Ada] Avoid confusing warning on exception propagation in GNATprove mode
When compiling with the restriction No_Exception_Propagation, GNAT compiler may issue a warning about exceptions not being propagated. This warning is useless and confusing to users for GNATprove analysis, as GNATprove precisely detects possible exceptions, so disable the warning in that mode. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Yannick Moy gcc/ada/ * gnat1drv.adb (Gnat1drv): Do not issue warning about exception not being propagated in GNATprove mode.--- gcc/ada/gnat1drv.adb +++ gcc/ada/gnat1drv.adb @@ -467,6 +467,12 @@ procedure Gnat1drv is Ineffective_Inline_Warnings := True; + -- Do not issue warnings for possible propagation of exception. + -- GNATprove already issues messages about possible exceptions. + + No_Warn_On_Non_Local_Exception := True; + Warn_On_Non_Local_Exception := False; + -- Disable front-end optimizations, to keep the tree as close to the -- source code as possible, and also to avoid inconsistencies between -- trees when using different optimization switches.
[Ada] Crash processing abstract state aspect of a package
The compiler may crash processing an aspect Part_Of used in a package spec which has also an Initial_Condition aspect. After this patch the following test compiles fine. package P with SPARK_Mode => On, Abstract_State => (Count_State), Initial_Condition => (Get_Count = 0) -- Test is type Count_Type is range 0 .. 16; function Get_Count return Count_Type; procedure Dummy; private C: Count_Type := 0 with Part_Of => Count_State; -- Test function Get_Count return Count_Type is (C); end P; package body P with SPARK_Mode => On, Refined_State => (Count_State => C) is procedure Dummy is null; end P; Command: gcc -c p.adb Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Javier Miranda gcc/ada/ * exp_ch13.adb (Expand_N_Freeze_Entity): Handle subtype declared for an iterator. * freeze.adb (Freeze_Expression): Handle freeze of an entity defined outside of a subprogram body. This case was previously handled during preanalysis; the frozen entities were remembered and left pending until we continued freezeing entities outside of the subprogram. Now, when climbing the parents chain to locate the correct placement for the freezeing node, we check if the entity can be frozen and only when no enclosing node is marked as Must_Not_Freeze the entity is frozen. * sem_ch3.ads (Preanalyze_Default_Expression): Declaration moved to the package body. * sem_ch3.adb (Preanalyze_Default_Expression): Code adjusted to invoke the new subprogram Preanalyze_With_Freezing_And_Resolve. * sem_ch6.adb (Preanalyze_Formal_Expression): New subprogram. (Analyze_Expression_Function, Process_Formals): Invoke Preanalyze_Formal_Expression instead of Preanalyze_Spec_Expression since the analysis of the formals may freeze entities. (Analyze_Subprogram_Body_Helper): Skip building the body of the class-wide clone for eliminated subprograms. * sem_res.ads, sem_res.adb (Preanalyze_And_Resolve): New subprogram. Its code is basically the previous version of this routine but extended with an additional parameter which is used to specify if during preanalysis we are allowed to freeze entities. If the new parameter is True then the subtree root node is marked as Must_Not_Freeze and no entities are frozen during preanalysis. (Preanalyze_And_Resolve): Invokes the internal version of Preanalyze_And_Resolve without entity freezing. (Preanalyze_With_Freezing_And_Resolve): Invokes the internal version of Prenalyze_And_Resolve with freezing enabled.--- gcc/ada/exp_ch13.adb +++ gcc/ada/exp_ch13.adb @@ -470,6 +470,11 @@ package body Exp_Ch13 is and then Ekind (E_Scope) not in Concurrent_Kind then E_Scope := Scope (E_Scope); + + -- The entity may be a subtype declared for an iterator. + + elsif Ekind (E_Scope) = E_Loop then + E_Scope := Scope (E_Scope); end if; -- Remember that we are processing a freezing entity and its freezing --- gcc/ada/freeze.adb +++ gcc/ada/freeze.adb @@ -6936,20 +6936,6 @@ package body Freeze is --- procedure Freeze_Expression (N : Node_Id) is - In_Spec_Exp : constant Boolean := In_Spec_Expression; - Typ : Entity_Id; - Nam : Entity_Id; - Desig_Typ : Entity_Id; - P : Node_Id; - Parent_P: Node_Id; - - Freeze_Outside : Boolean := False; - -- This flag is set true if the entity must be frozen outside the - -- current subprogram. This happens in the case of expander generated - -- subprograms (_Init_Proc, _Input, _Output, _Read, _Write) which do - -- not freeze all entities like other bodies, but which nevertheless - -- may reference entities that have to be frozen before the body and - -- obviously cannot be frozen inside the body. function Find_Aggregate_Component_Desig_Type return Entity_Id; -- If the expression is an array aggregate, the type of the component @@ -7038,6 +7024,29 @@ package body Freeze is end if; end In_Expanded_Body; + -- Local variables + + In_Spec_Exp : constant Boolean := In_Spec_Expression; + Typ : Entity_Id; + Nam : Entity_Id; + Desig_Typ : Entity_Id; + P : Node_Id; + Parent_P: Node_Id; + + Freeze_Outside : Boolean := False; + -- This flag is set true if the entity must be frozen outside the + -- current subprogram. This happens in the case of expander generated + -- subprograms (_Init_Proc, _Input, _Output, _Read, _Write) which do + -- not freeze all entities like other bodies, but which nevertheless + -- may reference entities that have to be frozen before the body and + -- obviously cannot be
[Ada] Missing check on illegal equality operation in subprogram
In Ada2012 it is illegal to declare an equality operation on an untagged
type when the operation is primitive and the type is already frozem (see
RM 4.5.2 (9.8)). previously the test to detect this illegality only examined
declarations within a package. This patch covers the case where type and
operation are both declared within a subprogram body.
Tested on x86_64-pc-linux-gnu, committed on trunk
2018-07-17 Ed Schonberg
gcc/ada/
* sem_ch6.adb (Check_Untagged_Equality): Extend check to operations
declared in the same scope as the operand type, when that scope is a
procedure.
gcc/testsuite/
* gnat.dg/equal3.adb: New testcase.--- gcc/ada/sem_ch6.adb
+++ gcc/ada/sem_ch6.adb
@@ -8581,14 +8581,10 @@ package body Sem_Ch6 is
if Is_Frozen (Typ) then
- -- If the type is not declared in a package, or if we are in the body
- -- of the package or in some other scope, the new operation is not
- -- primitive, and therefore legal, though suspicious. Should we
- -- generate a warning in this case ???
+ -- The check applies to a primitive operation, so check that type
+ -- and equality operation are in the same scope.
- if Ekind (Scope (Typ)) /= E_Package
- or else Scope (Typ) /= Current_Scope
- then
+ if Scope (Typ) /= Current_Scope then
return;
-- If the type is a generic actual (sub)type, the operation is not
@@ -8631,7 +8627,7 @@ package body Sem_Ch6 is
("\move declaration to package spec (Ada 2012)?y?", Eq_Op);
end if;
--- Otherwise try to find the freezing point
+-- Otherwise try to find the freezing point for better message.
else
Obj_Decl := Next (Parent (Typ));
@@ -8659,6 +8655,13 @@ package body Sem_Ch6 is
end if;
exit;
+
+ -- If we reach generated code for subprogram declaration
+ -- or body, it is the body that froze the type and the
+ -- declaration is legal.
+
+ elsif Sloc (Obj_Decl) = Sloc (Decl) then
+ return;
end if;
Next (Obj_Decl);
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/equal3.adb
@@ -0,0 +1,22 @@
+-- { dg-do compile }
+
+procedure Equal3 is
+type R is record
+ A, B : Integer;
+end record;
+
+package Pack is
+ type RR is record
+ C : R;
+ end record;
+
+ X : RR := (C => (A => 1, B => 1));
+ Y : RR := (C => (A => 1, B => 2));
+ pragma Assert (X /= Y); --@ASSERT:PASS
+
+end Pack;
+use Pack;
+function "=" (X, Y : R) return Boolean is (X.A = Y.A); -- { dg-error "equality operator must be declared before type \"R\" is frozen \\(RM 4.5.2 \\(9.8\\)\\) \\(Ada 2012\\)" }
+begin
+pragma Assert (X /= Y); --@ASSERT:FAIL
+end Equal3;
[Ada] Argument_String_To_List creates empty items from whitespace
This patch corrects an issue whereby leading whitespace in a non-quoted argument list passed to Argument_String_To_List caused extraneous empty arguments to be returned. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Justin Squirek gcc/ada/ * libgnat/s-os_lib.adb (Argument_String_To_List): Fix trimming of whitespace. gcc/testsuite/ * gnat.dg/split_args.adb: New testcase.--- gcc/ada/libgnat/s-os_lib.adb +++ gcc/ada/libgnat/s-os_lib.adb @@ -178,7 +178,6 @@ package body System.OS_Lib is return Len; end Args_Length; - - -- Argument_String_To_List -- - @@ -191,6 +190,9 @@ package body System.OS_Lib is Idx : Integer; New_Argc : Natural := 0; + Backqd : Boolean := False; + Quoted : Boolean := False; + Cleaned : String (1 .. Arg_String'Length); Cleaned_Idx : Natural; -- A cleaned up version of the argument. This function is taking @@ -205,75 +207,71 @@ package body System.OS_Lib is Idx := Arg_String'First; loop - exit when Idx > Arg_String'Last; + -- Skip extraneous spaces - declare -Backqd : Boolean := False; -Quoted : Boolean := False; - - begin -Cleaned_Idx := Cleaned'First; + while Idx <= Arg_String'Last and then Arg_String (Idx) = ' ' loop +Idx := Idx + 1; + end loop; -loop - -- An unquoted space is the end of an argument + exit when Idx > Arg_String'Last; - if not (Backqd or Quoted) - and then Arg_String (Idx) = ' ' - then - exit; + Cleaned_Idx := Cleaned'First; + Backqd := False; + Quoted := False; - -- Start of a quoted string + loop +-- An unquoted space is the end of an argument - elsif not (Backqd or Quoted) - and then Arg_String (Idx) = '"' - then - Quoted := True; - Cleaned (Cleaned_Idx) := Arg_String (Idx); - Cleaned_Idx := Cleaned_Idx + 1; +if not (Backqd or Quoted) + and then Arg_String (Idx) = ' ' +then + exit; - -- End of a quoted string and end of an argument +-- Start of a quoted string - elsif (Quoted and not Backqd) - and then Arg_String (Idx) = '"' - then - Cleaned (Cleaned_Idx) := Arg_String (Idx); - Cleaned_Idx := Cleaned_Idx + 1; - Idx := Idx + 1; - exit; +elsif not (Backqd or Quoted) + and then Arg_String (Idx) = '"' +then + Quoted := True; + Cleaned (Cleaned_Idx) := Arg_String (Idx); + Cleaned_Idx := Cleaned_Idx + 1; - -- Turn off backquoting after advancing one character +-- End of a quoted string and end of an argument - elsif Backqd then - Backqd := False; - Cleaned (Cleaned_Idx) := Arg_String (Idx); - Cleaned_Idx := Cleaned_Idx + 1; +elsif (Quoted and not Backqd) + and then Arg_String (Idx) = '"' +then + Cleaned (Cleaned_Idx) := Arg_String (Idx); + Cleaned_Idx := Cleaned_Idx + 1; + Idx := Idx + 1; + exit; - -- Following character is backquoted +-- Turn off backquoting after advancing one character - elsif not Backslash_Is_Sep and then Arg_String (Idx) = '\' then - Backqd := True; +elsif Backqd then + Backqd := False; + Cleaned (Cleaned_Idx) := Arg_String (Idx); + Cleaned_Idx := Cleaned_Idx + 1; - else - Cleaned (Cleaned_Idx) := Arg_String (Idx); - Cleaned_Idx := Cleaned_Idx + 1; - end if; +-- Following character is backquoted - Idx := Idx + 1; - exit when Idx > Arg_String'Last; -end loop; +elsif not Backslash_Is_Sep and then Arg_String (Idx) = '\' then + Backqd := True; --- Found an argument +else + Cleaned (Cleaned_Idx) := Arg_String (Idx); + Cleaned_Idx := Cleaned_Idx + 1; +end if; -New_Argc := New_Argc + 1; -New_Argv (New_Argc) := - new String'(Cleaned (Cleaned'First .. Cleaned_Idx - 1)); +Idx := Idx + 1; +exit when Idx > Arg_String'Last; + end loop; --- Skip extraneous spaces + -- Found an argument -while Idx <= Arg_String'Last and th
[Ada] Minor fix for imported C++ constructors
C++ constructors are imported as functions and then internally rewritten into procedures taking the "this" pointer as first parameter. Now this parameter is not of an access type but of the type directly, so it must be In/Out and not just In. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Eric Botcazou gcc/ada/ * exp_disp.adb (Gen_Parameters_Profile): Make the _Init parameter an In/Out parameter. (Set_CPP_Constructors): Adjust comment accordingly.--- gcc/ada/exp_disp.adb +++ gcc/ada/exp_disp.adb @@ -8181,7 +8181,8 @@ package body Exp_Disp is function Gen_Parameters_Profile (E : Entity_Id) return List_Id; -- Duplicate the parameters profile of the imported C++ constructor - -- adding an access to the object as an additional parameter. + -- adding the "this" pointer to the object as the additional first + -- parameter under the usual form _Init : in out Typ. -- Gen_Parameters_Profile -- @@ -8198,6 +8199,8 @@ package body Exp_Disp is Make_Parameter_Specification (Loc, Defining_Identifier => Make_Defining_Identifier (Loc, Name_uInit), + In_Present => True, + Out_Present => True, Parameter_Type => New_Occurrence_Of (Typ, Loc))); if Present (Parameter_Specifications (Parent (E))) then @@ -8244,9 +8247,7 @@ package body Exp_Disp is Found := True; Loc := Sloc (E); Parms := Gen_Parameters_Profile (E); -IP:= - Make_Defining_Identifier (Loc, -Chars => Make_Init_Proc_Name (Typ)); +IP:= Make_Defining_Identifier (Loc, Make_Init_Proc_Name (Typ)); -- Case 1: Constructor of untagged type @@ -8273,14 +8274,14 @@ package body Exp_Disp is -- Case 2: Constructor of a tagged type --- In this case we generate the IP as a wrapper of the the --- C++ constructor because IP must also save copy of the _tag +-- In this case we generate the IP routine as a wrapper of the +-- C++ constructor because IP must also save a copy of the _tag -- generated in the C++ side. The copy of the _tag is used by -- Build_CPP_Init_Procedure to elaborate derivations of C++ types. -- Generate: --- procedure IP (_init : Typ; ...) is ---procedure ConstructorP (_init : Typ; ...); +-- procedure IP (_init : in out Typ; ...) is +--procedure ConstructorP (_init : in out Typ; ...); --pragma Import (ConstructorP); -- begin --ConstructorP (_init, ...); @@ -8352,7 +8353,7 @@ package body Exp_Disp is loop -- Skip the following assertion with primary tags -- because Related_Type is not set on primary tag --- components +-- components. pragma Assert (Tag_Comp = First_Tag_Component (Typ)
[Ada] Assertion_Policy for class-wide precondition
This patch fixes the compiler to that class-wide preconditions on primitive operations of interfaces are not checked at run time when the Assertion_Policy indicates that they should be ignored. This is required by the RM. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Bob Duff gcc/ada/ * exp_disp.adb (Build_Class_Wide_Check): Return early if the precondition is supposed to be ignored.--- gcc/ada/exp_disp.adb +++ gcc/ada/exp_disp.adb @@ -809,7 +809,7 @@ package body Exp_Disp is Prec := Next_Pragma (Prec); end loop; -if No (Prec) then +if No (Prec) or else Is_Ignored (Prec) then return; end if;
[Ada] Configuration state not observed for instance bodies
This patch ensures that the processing of instantiated and inlined bodies uses the proper configuration context available at the point of the instantiation or inlining. Previously configuration pragmas which appear prior to the context items of a unit would lose their effect when a body is instantiated or inlined. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Hristian Kirtchev gcc/ada/ * frontend.adb (Frontend): Update the call to Register_Config_Switches. * inline.ads: Add new component Config_Switches to record Pending_Body_Info which captures the configuration state of the pending body. Remove components Version, Version_Pragma, SPARK_Mode, and SPARK_Mode_Pragma from record Pending_Body_Info because they are already captured in component Config_Switches. * opt.adb (Register_Opt_Config_Switches): Rename to Register_Config_Switches. (Restore_Opt_Config_Switches): Rename to Restore_Config_Switches. (Save_Opt_Config_Switches): Rename to Save_Config_Switches. This routine is now a function, and returns the saved configuration state as an aggregate to avoid missing an attribute. (Set_Opt_Config_Switches): Rename to Set_Config_Switches. * opt.ads (Register_Opt_Config_Switches): Rename to Register_Config_Switches. (Restore_Opt_Config_Switches): Rename to Restore_Config_Switches. (Save_Opt_Config_Switches): Rename to Save_Config_Switches. This routine is now a function. (Set_Opt_Config_Switches): Rename to Set_Config_Switches. * par.adb (Par): Update the calls to configuration switch-related subprograms. * sem.adb (Semantics): Update the calls to configuration switch-related subprograms. * sem_ch10.adb (Analyze_Package_Body_Stub): Update the calls to configuration switch-related subprograms. (Analyze_Protected_Body_Stub): Update the calls to configuration switch-related subprograms. (Analyze_Subprogram_Body_Stub): Update calls to configuration switch-related subprograms. * sem_ch12.adb (Add_Pending_Instantiation): Update the capture of pending instantiation attributes. (Inline_Instance_Body): Update the capture of pending instantiation attributes. It is no longer needed to explicitly manipulate the SPARK mode. (Instantiate_Package_Body): Update the restoration of the context attributes. (Instantiate_Subprogram_Body): Update the restoration of context attributes. (Load_Parent_Of_Generic): Update the capture of pending instantiation attributes. (Set_Instance_Env): Update the way relevant configuration attributes are saved and restored. gcc/testsuite/ * gnat.dg/config_pragma1.adb, gnat.dg/config_pragma1_pkg.ads: New testcase.--- gcc/ada/frontend.adb +++ gcc/ada/frontend.adb @@ -303,7 +303,7 @@ begin -- capture the values of the configuration switches (see Opt for further -- details). - Opt.Register_Opt_Config_Switches; + Register_Config_Switches; -- Check for file which contains No_Body pragma --- gcc/ada/inline.ads +++ gcc/ada/inline.ads @@ -63,21 +63,24 @@ package Inline is -- See full description in body of Sem_Ch12 for more details type Pending_Body_Info is record - Inst_Node : Node_Id; - -- Node for instantiation that requires the body - Act_Decl : Node_Id; -- Declaration for package or subprogram spec for instantiation - Expander_Status : Boolean; - -- If the body is instantiated only for semantic checking, expansion - -- must be inhibited. + Config_Switches : Config_Switches_Type; + -- Capture the values of configuration switches Current_Sem_Unit : Unit_Number_Type; -- The semantic unit within which the instantiation is found. Must be -- restored when compiling the body, to insure that internal entities -- use the same counter and are unique over spec and body. + Expander_Status : Boolean; + -- If the body is instantiated only for semantic checking, expansion + -- must be inhibited. + + Inst_Node : Node_Id; + -- Node for instantiation that requires the body + Scope_Suppress : Suppress_Record; Local_Suppress_Stack_Top : Suppress_Stack_Entry_Ptr; -- Save suppress information at the point of instantiation. Used to @@ -93,21 +96,8 @@ package Inline is -- This means we have to capture this information from the current scope -- at the point of instantiation. - Version : Ada_Version_Type; - -- The body must be compiled with the same language version as the - -- spec. The version may be set by a configuration pragma in a separate - -- file or in the current file, and may differ from body to body. - - Versio
[Ada] Use standard version of s-memory.adb for mingw32
This patch switches mingw32 targets to use the standard version of s-memory.adb as Windows now has the capability of limiting the amount of memory used by process. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Patrick Bernardi gcc/ada/ * libgnat/s-memory__mingw.adb: Remove. * Makefile.rtl: Remove s-memory.adb target pair from the Cygwin/Mingw32 section. gcc/testsuite/ * gnat.dg/memorytest.adb: New testcase.--- gcc/ada/Makefile.rtl +++ gcc/ada/Makefile.rtl @@ -1960,19 +1960,17 @@ endif # Cygwin/Mingw32 ifeq ($(strip $(filter-out cygwin% mingw32% pe,$(target_os))),) # Cygwin provides a full Posix environment, and so we use the default - # versions of s-memory and g-socthi rather than the Windows-specific - # MinGW versions. Ideally we would use all the default versions for - # Cygwin and none of the MinGW versions, but for historical reasons - # the Cygwin port has always been a CygMing frankenhybrid and it is - # a long-term project to disentangle them. + # versions g-socthi rather than the Windows-specific MinGW version. + # Ideally we would use all the default versions for Cygwin and none + # of the MinGW versions, but for historical reasons the Cygwin port + # has always been a CygMing frankenhybrid and it is a long-term project + # to disentangle them. ifeq ($(strip $(filter-out cygwin%,$(target_os))),) LIBGNAT_TARGET_PAIRS = \ -s-memory.adbhttp://www.gnu.org/licenses/>. -- --- -- --- GNAT was originally developed by the GNAT team at New York University. -- --- Extensive contributions were provided by Ada Core Technologies Inc. -- --- -- --- - --- This version provides ways to limit the amount of used memory for systems --- that do not have OS support for that. - --- The amount of available memory available for dynamic allocation is limited --- by setting the environment variable GNAT_MEMORY_LIMIT to the number of --- kilobytes that can be used. --- --- Windows is currently using this version. - -with Ada.Exceptions; -with System.Soft_Links; - -package body System.Memory is - - use Ada.Exceptions; - use System.Soft_Links; - - function c_malloc (Size : size_t) return System.Address; - pragma Import (C, c_malloc, "malloc"); - - procedure c_free (Ptr : System.Address); - pragma Import (C, c_free, "free"); - - function c_realloc - (Ptr : System.Address; Size : size_t) return System.Address; - pragma Import (C, c_realloc, "realloc"); - - function msize (Ptr : System.Address) return size_t; - pragma Import (C, msize, "_msize"); - - function getenv (Str : String) return System.Address; - pragma Import (C, getenv); - - function atoi (Str : System.Address) return Integer; - pragma Import (C, atoi); - - Available_Memory : size_t := 0; - -- Amount of memory that is available for heap allocations. - -- A value of 0 means that the amount is not yet initialized. - - Msize_Accuracy : constant := 4096; - -- Defines the amount of memory to add to requested allocation sizes, - -- because malloc may return a bigger block than requested. As msize - -- is used when by Free, it must be used on allocation as well. To - -- prevent underflow of available_memory we need to use a reserve. - - procedure Check_Available_Memory (Size : size_t); - -- This routine must be called while holding the task lock. When the - -- memory limit is not yet initialized, it will be set to the value of - -- the GNAT_MEMORY_LIMIT environment variable or to unlimited if that - -- does not exist. If the size is larger than the amount of available - -- memory, the task lock will be freed and a storage_error exception - -- will be raised. - - --- - -- Alloc -- - --- - - function Alloc (Size : size_t) return System.Address is - Result : System.Address; - Actual_Size : size_t := Size; - - begin - if Size = size_t'Last then - Raise_Exception (Storage_Error'Identity, "object too large"); - end if; - - -- Change size from zero to non-zero. We still want a proper pointer - -- for the zero case because pointers to zero length objects have to - -- be distinct, but we can't just go ahead and allocate zero bytes, - -- since some malloc's return zero for a zero argument. - - if Size = 0 then - Actual_Size := 1; - end if; - - Lock_Task.all; - - if Actual_Size + Msize_Accuracy >= Available_Memory then - Check_Available_Memory (Size + Msize_Accuracy); - end if; - - Result := c_malloc (Actual_Size); - - if Result /= System.Null_Address then - Available_Memory := Available_Memory - msize (Result); -
[Ada] Faulty ignored Ghost code removal
This patch ensures that removal of ignored Ghost code is the absolute last operation performed on the tree. Previously the removal was performed prior to issuing delayed warnings, however the warning mechanism may see a heavily modified tree and fail. No small reproducer available. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Hristian Kirtchev gcc/ada/ * frontend.adb (Frontend): The removal of ignored Ghost code must be the last semantic operation performed on the tree.--- gcc/ada/frontend.adb +++ gcc/ada/frontend.adb @@ -451,11 +451,6 @@ begin Check_Elaboration_Scenarios; - -- Remove any ignored Ghost code as it must not appear in the - -- executable. - - Remove_Ignored_Ghost_Code; - -- Examine all top level scenarios collected during analysis and -- resolution in order to diagnose conditional ABEs, even in the -- presence of serious errors. @@ -483,6 +478,14 @@ begin Sem_Warn.Output_Unreferenced_Messages; Sem_Warn.Check_Unused_Withs; Sem_Warn.Output_Unused_Warnings_Off_Warnings; + +-- Remove any ignored Ghost code as it must not appear in the +-- executable. This action must be performed last because it +-- heavily alters the tree. + +if Operating_Mode = Generate_Code or else GNATprove_Mode then + Remove_Ignored_Ghost_Code; +end if; end if; end if; end;
[Ada] Fix incompatibility Default_Scalar_Storage_Order/tagged types
The pragma Default_Scalar_Storage_Order cannot reliably be used to set the
non-default scalar storage order for a program that declares tagged types, if
it also declares user-defined primitives.
This is fixed by making Make_Tags use the same base array type as Make_DT and
Make_Secondary_DT when accessing the array of user-defined primitives.
Tested on x86_64-pc-linux-gnu, committed on trunk
2018-07-17 Eric Botcazou
gcc/ada/
* exp_disp.adb (Make_Tags): When the type has user-defined primitives,
build the access type that is later used by Build_Get_Prim_Op_Address
as pointing to a subtype of Ada.Tags.Address_Array.
gcc/testsuite/
* gnat.dg/sso10.adb, gnat.dg/sso10_pkg.ads: New testcase.--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -7179,7 +7179,7 @@ package body Exp_Disp is
Analyze_List (Result);
-- Generate:
- -- type Typ_DT is array (1 .. Nb_Prims) of Prim_Ptr;
+ -- subtype Typ_DT is Address_Array (1 .. Nb_Prims);
-- type Typ_DT_Acc is access Typ_DT;
else
@@ -7196,20 +7196,19 @@ package body Exp_Disp is
Name_DT_Prims_Acc);
begin
Append_To (Result,
- Make_Full_Type_Declaration (Loc,
+ Make_Subtype_Declaration (Loc,
Defining_Identifier => DT_Prims,
-Type_Definition =>
- Make_Constrained_Array_Definition (Loc,
-Discrete_Subtype_Definitions => New_List (
- Make_Range (Loc,
-Low_Bound => Make_Integer_Literal (Loc, 1),
-High_Bound => Make_Integer_Literal (Loc,
- DT_Entry_Count
- (First_Tag_Component (Typ),
-Component_Definition =>
- Make_Component_Definition (Loc,
-Subtype_Indication =>
- New_Occurrence_Of (RTE (RE_Prim_Ptr), Loc);
+Subtype_Indication =>
+ Make_Subtype_Indication (Loc,
+Subtype_Mark =>
+ New_Occurrence_Of (RTE (RE_Address_Array), Loc),
+Constraint =>
+ Make_Index_Or_Discriminant_Constraint (Loc, New_List (
+Make_Range (Loc,
+ Low_Bound => Make_Integer_Literal (Loc, 1),
+ High_Bound => Make_Integer_Literal (Loc,
+ DT_Entry_Count
+ (First_Tag_Component (Typ);
Append_To (Result,
Make_Full_Type_Declaration (Loc,
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/sso10.adb
@@ -0,0 +1,16 @@
+-- { dg-do run }
+
+with SSO10_Pkg; use SSO10_Pkg;
+
+procedure SSO10 is
+
+ procedure Inner (R : Root'Class) is
+ begin
+Run (R);
+ end;
+
+ R : Root;
+
+begin
+ Inner (R);
+end;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/sso10_pkg.ads
@@ -0,0 +1,9 @@
+pragma Default_Scalar_Storage_Order (High_Order_First);
+
+package SSO10_Pkg is
+
+ type Root is tagged null record;
+
+ procedure Run (R : Root) is null;
+
+end SSO10_Pkg;
[Ada] Spurious error on prefixed call in an instantiation
This patch fixes a spurious error on a prefixed call in an instance, when the generic parameters include an interface type and an abstract operation of that type, and the actuals in the instance include an interface type and a corresponding abstract operation of it, with a different name than the corresponding generic subprogram parameter. The patch also fixes a similar error involving class-wide operations and generic private types. Tested on x86_64-pc-linux-gnu, committed on trunk 2018-07-17 Ed Schonberg gcc/ada/ * sem_ch4.adb (Try_Object_Operation): Handle properly a prefixed call in an instance, when the generic parameters include an interface type and a abstract operation of that type, and the actuals in the instance include an interface type and a corresponding abstract operation of it, with a different name than the corresponding generic subprogram parameter. gcc/testsuite/ * gnat.dg/generic_call_cw.adb, gnat.dg/generic_call_iface.adb: New testcase.--- gcc/ada/sem_ch4.adb +++ gcc/ada/sem_ch4.adb @@ -8928,11 +8928,38 @@ package body Sem_Ch4 is (Anc_Type : Entity_Id; Error: out Boolean) is +Candidate : Entity_Id; +-- If homonym is a renaming, examine the renamed program + Cls_Type: Entity_Id; Hom : Entity_Id; Hom_Ref : Node_Id; Success : Boolean; +function First_Formal_Match + (Typ : Entity_Id) return Boolean; +-- Predicate to verify that the first formal of a class-wide +-- candidate matches the type of the prefix. + + +-- First_Formal_Match -- + + +function First_Formal_Match + (Typ : Entity_Id) return Boolean +is + Ctrl : constant Entity_Id := First_Formal (Candidate); +begin + return Present (Ctrl) + and then + (Base_Type (Etype (Ctrl)) = Typ + or else + (Ekind (Etype (Ctrl)) = E_Anonymous_Access_Type + and then + Base_Type +(Designated_Type (Etype (Ctrl))) = Typ)); +end First_Formal_Match; + begin Error := False; @@ -8948,25 +8975,23 @@ package body Sem_Ch4 is while Present (Hom) loop if Ekind_In (Hom, E_Procedure, E_Function) - and then (not Is_Hidden (Hom) or else In_Instance) - and then Scope (Hom) = Scope (Base_Type (Anc_Type)) - and then Present (First_Formal (Hom)) - and then - (Base_Type (Etype (First_Formal (Hom))) = Cls_Type - or else - (Is_Access_Type (Etype (First_Formal (Hom))) - and then - Ekind (Etype (First_Formal (Hom))) = - E_Anonymous_Access_Type - and then - Base_Type - (Designated_Type (Etype (First_Formal (Hom = - Cls_Type)) + and then Present (Renamed_Entity (Hom)) + and then Is_Generic_Actual_Subprogram (Hom) + then + Candidate := Renamed_Entity (Hom); + else + Candidate := Hom; + end if; + + if Ekind_In (Candidate, E_Procedure, E_Function) + and then (not Is_Hidden (Candidate) or else In_Instance) + and then Scope (Candidate) = Scope (Base_Type (Anc_Type)) + and then First_Formal_Match (Cls_Type) then -- If the context is a procedure call, ignore functions -- in the name of the call. - if Ekind (Hom) = E_Function + if Ekind (Candidate) = E_Function and then Nkind (Parent (N)) = N_Procedure_Call_Statement and then N = Name (Parent (N)) then @@ -8975,7 +9000,7 @@ package body Sem_Ch4 is -- If the context is a function call, ignore procedures -- in the name of the call. - elsif Ekind (Hom) = E_Procedure + elsif Ekind (Candidate) = E_Procedure and then Nkind (Parent (N)) /= N_Procedure_Call_Statement then goto Next_Hom; @@ -8986,7 +9011,7 @@ package body Sem_Ch4 is Success := False; if No (Matching_Op) then - Hom_Ref := New_Occurrence_Of (Hom, Sloc (Subprog)); + Hom_Ref
Re: [PR fortran/83184] Fix matching code for clist/old-style initializers when encountering assumed-rank declarations
Did someone actually approve this patch? Apparently it was committed as r262744 and caused the following regression: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86543 Cheers, Janus 2018-06-27 23:07 GMT+02:00 Fritz Reese : > The attached patch fixes PR fortran/83184, which is actually two > distinct bugs as described in the PR. Passes regtests. > > The patch is attached. OK for trunk and 7/8-branch? > > From 238f0a0e80c93209bb4e62ba2f719f74f5da164f Mon Sep 17 00:00:00 2001 > From: Fritz Reese > Date: Wed, 27 Jun 2018 16:16:31 -0400 > Subject: [PATCH 2/3] PR fortran/83184 > > Fix handling of invalid assumed-shape/size arrays in legacy initializer > lists. > > gcc/fortran/ > > * decl.c (match_old_style_init): Initialize locus of variable expr > when > creating a data variable. > (match_clist_expr): Verify array is explicit shape/size before > attempting to allocate constant array constructor. > > gcc/testsuite/ > > * gfortran.dg/assumed_rank_14.f90: New testcase. > * gfortran.dg/assumed_rank_15.f90: New testcase. > * gfortran.dg/dec_structure_8.f90: Update error messages. > * gfortran.dg/dec_structure_23.f90: Update error messages. > --- > gcc/fortran/decl.c | 63 > +++--- > gcc/testsuite/gfortran.dg/assumed_rank_14.f90 | 11 + > gcc/testsuite/gfortran.dg/assumed_rank_15.f90 | 11 + > gcc/testsuite/gfortran.dg/dec_structure_23.f90 | 6 +-- > gcc/testsuite/gfortran.dg/dec_structure_8.f90 | 6 +-- > 5 files changed, 64 insertions(+), 33 deletions(-) > create mode 100644 gcc/testsuite/gfortran.dg/assumed_rank_14.f90 > create mode 100644 gcc/testsuite/gfortran.dg/assumed_rank_15.f90
Re: [PATCH] Fix PR86523
Hi Richard,
> The following fixes PR86523, we failed to assing DIE parents to some
> function-local entities with the idea scope vars would pick them up
> but that's not true for some of them.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, LTO bootstrapped
> on the GCC 8 branch (it's said LTO bootstrap is broken on trunk).
the new testcases FAIL on Solaris:
+FAIL: g++.dg/lto/pr86523-1 cp_lto_pr86523-1_0.o-cp_lto_pr86523-1_0.o link, -O2
-flto -g -shared
+FAIL: g++.dg/lto/pr86523-2 cp_lto_pr86523-2_0.o-cp_lto_pr86523-2_0.o link, -O2
-flto -g -shared
32-bit sparc and 64-bit x86 only
On sparc:
ld: warning: unwind table: file /var/tmp//ccRLBN4b.ltrans0.ltrans.o: section
.eh_frame: suspicious InitialLoc value 0: binary search table will be
incomplete if section requires runtime relocation
On x86:
Text relocation remains referenced
against symbol offset in file
.text (section) 0x20cp_lto_pr86523-1_0.o
ld: fatal: relocations remain against allocatable but non-writable sections
The latter happens because the Solaris linker defaults to -z text, and
the tests aren't built as PIC. The following patch fixes this,
requiring fpic and shared support along the way, too, and also fixes the
Solaris/SPARC failure.
Tested on i386-pc-solaris2.11, sparc-sun-solaris2.11, and
x86_64-pc-linux-gnu. Ok for mainline?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-07-17 Rainer Orth
* g++.dg/lto/pr86523-1_0.C: Require fpic, shared support.
(dg-lto-options): Add -fPIC.
* g++.dg/lto/pr86523-2_0.C: Likewise.
diff --git a/gcc/testsuite/g++.dg/lto/pr86523-1_0.C b/gcc/testsuite/g++.dg/lto/pr86523-1_0.C
--- a/gcc/testsuite/g++.dg/lto/pr86523-1_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr86523-1_0.C
@@ -1,5 +1,7 @@
// { dg-lto-do link }
-// { dg-lto-options {{-O2 -flto -g -shared}} }
+// { dg-require-effective-target fpic }
+// { dg-require-effective-target shared }
+// { dg-lto-options { { -O2 -fPIC -flto -g -shared } } }
namespace {
class a typedef b;
class a {};
diff --git a/gcc/testsuite/g++.dg/lto/pr86523-2_0.C b/gcc/testsuite/g++.dg/lto/pr86523-2_0.C
--- a/gcc/testsuite/g++.dg/lto/pr86523-2_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr86523-2_0.C
@@ -1,5 +1,7 @@
// { dg-lto-do link }
-// { dg-lto-options {{-O2 -flto -g -shared}} }
+// { dg-require-effective-target fpic }
+// { dg-require-effective-target shared }
+// { dg-lto-options { { -O2 -fPIC -flto -g -shared } } }
class a typedef b;
class a {};
class c {
Re: [PATCH] Fix PR86523
On Tue, 17 Jul 2018, Rainer Orth wrote: > Hi Richard, > > > The following fixes PR86523, we failed to assing DIE parents to some > > function-local entities with the idea scope vars would pick them up > > but that's not true for some of them. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, LTO bootstrapped > > on the GCC 8 branch (it's said LTO bootstrap is broken on trunk). > > the new testcases FAIL on Solaris: > > +FAIL: g++.dg/lto/pr86523-1 cp_lto_pr86523-1_0.o-cp_lto_pr86523-1_0.o link, > -O2 -flto -g -shared > +FAIL: g++.dg/lto/pr86523-2 cp_lto_pr86523-2_0.o-cp_lto_pr86523-2_0.o link, > -O2 -flto -g -shared > > 32-bit sparc and 64-bit x86 only > > On sparc: > > ld: warning: unwind table: file /var/tmp//ccRLBN4b.ltrans0.ltrans.o: section > .eh_frame: suspicious InitialLoc value 0: binary search table will be > incomplete if section requires runtime relocation > > On x86: > > Text relocation remains referenced > against symbol offset in file > .text (section) 0x20cp_lto_pr86523-1_0.o > ld: fatal: relocations remain against allocatable but non-writable sections > > The latter happens because the Solaris linker defaults to -z text, and > the tests aren't built as PIC. The following patch fixes this, > requiring fpic and shared support along the way, too, and also fixes the > Solaris/SPARC failure. > > Tested on i386-pc-solaris2.11, sparc-sun-solaris2.11, and > x86_64-pc-linux-gnu. Ok for mainline? OK. Thanks, Richard.
Re: [PATCH][debug] Fix pre_dec handling in vartrack
Hi Jakub,
> On Mon, Jul 16, 2018 at 09:24:10AM +0200, Jakub Jelinek wrote:
>> On Sun, Jul 15, 2018 at 11:21:56PM +0200, Tom de Vries wrote:
>> > 2018-07-15 Tom de Vries
>> >
>> >* var-tracking.c (vt_initialize): Fix pre_dec handling. Print adjusted
>> >insn slim if dump_flags request TDF_SLIM.
>> >
>> >* gcc.target/i386/vartrack-1.c: New test.
>> >
>> > ---
>> > --- a/gcc/var-tracking.c
>> > +++ b/gcc/var-tracking.c
>> > @@ -115,6 +115,7 @@
>> > #include "tree-pretty-print.h"
>> > #include "rtl-iter.h"
>> > #include "fibonacci_heap.h"
>> > +#include "print-rtl.h"
>> >
>> > typedef fibonacci_heap bb_heap_t;
>> > typedef fibonacci_node bb_heap_node_t;
>> > @@ -10208,12 +10209,17 @@ vt_initialize (void)
>> >log_op_type (PATTERN (insn), bb, insn,
>> > MO_ADJUST, dump_file);
>> > VTI (bb)->mos.safe_push (mo);
>> > -VTI (bb)->out.stack_adjust += pre;
>> >}
>> >}
>> >
>> > cselib_hook_called = false;
>> > adjust_insn (bb, insn);
>> > +
>> > +if (!frame_pointer_needed)
>> > + {
>> > +if (pre)
>> > + VTI (bb)->out.stack_adjust += pre;
>> > + }
>>
>> That is certainly unexpected. For the pre side-effects, they should be
>> applied before adjusting the insn, not after that.
>> I'll want to see this under the debugger.
>
> You're right, adjust_mems takes the PRE_INC/PRE_DEC/PRE_MODIFY into account
> too, so by adjusting stack_adjust before adjust_insn the effects happen
> twice:
> case PRE_INC:
> case PRE_DEC:
> size = GET_MODE_SIZE (amd->mem_mode);
> addr = plus_constant (GET_MODE (loc), XEXP (loc, 0),
> GET_CODE (loc) == PRE_INC ? size : -size);
> Unless we have instructions where we e.g. pre_dec sp on the lhs and
> use some mem based on sp on the rhs, but I'd hope that should be invalid
> RTL, because it is ambiguous what value would sp on the rhs have.
>
> Please change the above patch to do:
> adjust_insn (bb, insn);
> +
> + if (!frame_pointer_needed && pre)
> + VTI (bb)->out.stack_adjust += pre;
>
> Ok for trunk with that change.
it turns out the test FAILs on i386-pc-soalris2.11 with -m64: the dump
doesn't even contain argp: However, adding -fomit-frame-pointer makes
it work, and still PASSes on x86_64-pc-linux-gnu.
Ok for for mainline?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-07-17 Rainer Orth
* gcc.target/i386/vartrack-1.c (dg-options): Add
-fomit-frame-pointer.
diff --git a/gcc/testsuite/gcc.target/i386/vartrack-1.c b/gcc/testsuite/gcc.target/i386/vartrack-1.c
--- a/gcc/testsuite/gcc.target/i386/vartrack-1.c
+++ b/gcc/testsuite/gcc.target/i386/vartrack-1.c
@@ -1,5 +1,5 @@
/* { dg-require-effective-target lp64 } */
-/* { dg-options "-O1 -g -fdump-rtl-vartrack-details-slim" } */
+/* { dg-options "-O1 -g -fomit-frame-pointer -fdump-rtl-vartrack-details-slim" } */
static volatile int vv = 1;
Re: [PATCH][debug] Fix pre_dec handling in vartrack
On Tue, Jul 17, 2018 at 11:17:33AM +0200, Rainer Orth wrote:
> it turns out the test FAILs on i386-pc-soalris2.11 with -m64: the dump
> doesn't even contain argp: However, adding -fomit-frame-pointer makes
> it work, and still PASSes on x86_64-pc-linux-gnu.
>
> Ok for for mainline?
Ok, thanks.
> 2018-07-17 Rainer Orth
>
> * gcc.target/i386/vartrack-1.c (dg-options): Add
> -fomit-frame-pointer.
>
> diff --git a/gcc/testsuite/gcc.target/i386/vartrack-1.c
> b/gcc/testsuite/gcc.target/i386/vartrack-1.c
> --- a/gcc/testsuite/gcc.target/i386/vartrack-1.c
> +++ b/gcc/testsuite/gcc.target/i386/vartrack-1.c
> @@ -1,5 +1,5 @@
> /* { dg-require-effective-target lp64 } */
> -/* { dg-options "-O1 -g -fdump-rtl-vartrack-details-slim" } */
> +/* { dg-options "-O1 -g -fomit-frame-pointer
> -fdump-rtl-vartrack-details-slim" } */
>
> static volatile int vv = 1;
>
Jakub
Re: [PATCH 1/4] Clean up of new format of -falign-FOO.
On 07/16/2018 11:24 PM, Jeff Law wrote: > All 4 patches in this kit are fine after you fix the minor issue Pat > reported with patch #1 on ppc. > > Jeff Thank you Jeff for the trust. I fixed the minor issue and installed the patch set. Martin
Re: [PATCH] Make function clone name numbering independent.
On Tue, Jul 17, 2018 at 8:10 AM Bernhard Reutner-Fischer wrote: > > On 16 July 2018 21:38:36 CEST, Michael Ploujnikov > wrote: > >Hi, > > > > >+clone_fn_ids = hash_map::create_ggc > >(1000); > > Isn't 1000 a bit excessive? What about 64 or thereabouts? I'm not sure we should throw memory at this "problem" in this way. What specific issue does this address? Iff then I belive forgoing the automatic counter addidion is the way to go and hand control of that to the callers (for example the caller in lto/lto-partition.c doesn't even seem to need that counter. You also assume the string you key on persists - luckily the lto-partition.c caller leaks it but this makes your approach quite fragile in my eye (put the logic into clone_function_name instead, where you at least know you are dealing with a string from an indentifier which are never collected). Richard. > thanks,
[Ada] Fix spurious check failure for Character discriminant
This is a regression present on the mainline, 8 and 7 branches. The compiler
generates a spurious check failure for a Character discriminant declared in a
discriminated record type with variant part if one of the variants is selected
by a range of values which contains at least the values at position 127 & 128.
It's also indirectly responsible for failure of gnat.dg/debug11.adb on RISC-V.
Tested on x86-64/Linux, applied on the mainline, 8 and 7 branches.
2018-07-17 Eric Botcazou
* gcc-interface/decl.c (choices_to_gnu): Rename parameters. Deal with
an operand of Character type. Factor out range generation to the end.
Check that the bounds are literals and convert them to the type of the
operand before building the ranges.
* gcc-interface/utils.c (make_dummy_type): Minor tweak.
(make_packable_type): Propagate TYPE_DEBUG_TYPE.
(maybe_pad_type): Likewise.
2018-07-17 Eric Botcazou
* gnat.dg/discr55.adb: New test.
--
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c (revision 262803)
+++ gcc-interface/decl.c (working copy)
@@ -6705,65 +6705,44 @@ elaborate_reference (tree ref, Entity_Id
the value passed against the list of choices. */
static tree
-choices_to_gnu (tree operand, Node_Id choices)
+choices_to_gnu (tree gnu_operand, Node_Id gnat_choices)
{
- Node_Id choice;
- Node_Id gnat_temp;
- tree result = boolean_false_node;
- tree this_test, low = 0, high = 0, single = 0;
+ tree gnu_result = boolean_false_node, gnu_type;
- for (choice = First (choices); Present (choice); choice = Next (choice))
+ gnu_operand = maybe_character_value (gnu_operand);
+ gnu_type = TREE_TYPE (gnu_operand);
+
+ for (Node_Id gnat_choice = First (gnat_choices);
+ Present (gnat_choice);
+ gnat_choice = Next (gnat_choice))
{
- switch (Nkind (choice))
+ tree gnu_low = NULL_TREE, gnu_high = NULL_TREE;
+ tree gnu_test;
+
+ switch (Nkind (gnat_choice))
{
case N_Range:
- low = gnat_to_gnu (Low_Bound (choice));
- high = gnat_to_gnu (High_Bound (choice));
-
- this_test
- = build_binary_op (TRUTH_ANDIF_EXPR, boolean_type_node,
- build_binary_op (GE_EXPR, boolean_type_node,
- operand, low, true),
- build_binary_op (LE_EXPR, boolean_type_node,
- operand, high, true),
- true);
-
+ gnu_low = gnat_to_gnu (Low_Bound (gnat_choice));
+ gnu_high = gnat_to_gnu (High_Bound (gnat_choice));
break;
case N_Subtype_Indication:
- gnat_temp = Range_Expression (Constraint (choice));
- low = gnat_to_gnu (Low_Bound (gnat_temp));
- high = gnat_to_gnu (High_Bound (gnat_temp));
-
- this_test
- = build_binary_op (TRUTH_ANDIF_EXPR, boolean_type_node,
- build_binary_op (GE_EXPR, boolean_type_node,
- operand, low, true),
- build_binary_op (LE_EXPR, boolean_type_node,
- operand, high, true),
- true);
+ gnu_low = gnat_to_gnu (Low_Bound (Range_Expression
+ (Constraint (gnat_choice;
+ gnu_high = gnat_to_gnu (High_Bound (Range_Expression
+ (Constraint (gnat_choice;
break;
case N_Identifier:
case N_Expanded_Name:
- /* This represents either a subtype range, an enumeration
- literal, or a constant Ekind says which. If an enumeration
- literal or constant, fall through to the next case. */
- if (Ekind (Entity (choice)) != E_Enumeration_Literal
- && Ekind (Entity (choice)) != E_Constant)
+ /* This represents either a subtype range or a static value of
+ some kind; Ekind says which. */
+ if (Is_Type (Entity (gnat_choice)))
{
- tree type = gnat_to_gnu_type (Entity (choice));
+ tree gnu_type = get_unpadded_type (Entity (gnat_choice));
- low = TYPE_MIN_VALUE (type);
- high = TYPE_MAX_VALUE (type);
-
- this_test
- = build_binary_op (TRUTH_ANDIF_EXPR, boolean_type_node,
- build_binary_op (GE_EXPR, boolean_type_node,
- operand, low, true),
- build_binary_op (LE_EXPR, boolean_type_node,
- operand, high, true),
- true);
+ gnu_low = TYPE_MIN_VALUE (gnu_type);
+ gnu_high = TYPE_MAX_VALUE (gnu_type);
break;
}
@@ -6771,27 +6750,49 @@ choices_to_gnu (tree operand, Node_Id ch
case N_Character_Literal:
case N_Integer_Literal:
- single = gnat_to_gnu (choice);
- this_test = build_binary_op (EQ_EXPR, boolean_type_node, operand,
- single, true);
+ gnu_low = gnat_to_gnu (gnat_choice);
break;
case N_Others_Choice:
- this_test = boolean_true_node;
break;
default:
gcc_unreachable ();
}
- if (result == boolean_false_node)
- result = this_test;
+ /* Everything should be folded into constants at this point. */
+ gcc_assert (!gnu_low || TREE_CODE (gnu_low) == INTEGER_CST);
+ gcc_assert (!gnu_high || TREE_CODE (gnu_high) == INTEGER_CST)
[PATCH] S/390: Add CFI for mcount call sequences
Fixes unwind for mcount.
2018-07-17 Ilya Leoshkevich
* config/s390/s390.c (s390_function_profiler):
Generate CFI.
---
gcc/config/s390/s390.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index ba18cb1c39a..3627f41eb05 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -13153,7 +13153,7 @@ output_asm_nops (const char *user, int hw)
void
s390_function_profiler (FILE *file, int labelno)
{
- rtx op[7];
+ rtx op[8];
char label[128];
ASM_GENERATE_INTERNAL_LABEL (label, "LP", labelno);
@@ -13163,6 +13163,7 @@ s390_function_profiler (FILE *file, int labelno)
op[0] = gen_rtx_REG (Pmode, RETURN_REGNUM);
op[1] = gen_rtx_REG (Pmode, STACK_POINTER_REGNUM);
op[1] = gen_rtx_MEM (Pmode, plus_constant (Pmode, op[1], UNITS_PER_LONG));
+ op[7] = GEN_INT (UNITS_PER_LONG);
op[2] = gen_rtx_REG (Pmode, 1);
op[3] = gen_rtx_SYMBOL_REF (Pmode, label);
@@ -13196,9 +13197,13 @@ s390_function_profiler (FILE *file, int labelno)
else
{
output_asm_insn ("stg\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_rel_offset\t%0,%7", op);
output_asm_insn ("larl\t%2,%3", op);
output_asm_insn ("brasl\t%0,%4", op);
output_asm_insn ("lg\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_restore\t%0", op);
}
}
else if (TARGET_CPU_ZARCH)
@@ -13209,9 +13214,13 @@ s390_function_profiler (FILE *file, int labelno)
else
{
output_asm_insn ("st\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_rel_offset\t%0,%7", op);
output_asm_insn ("larl\t%2,%3", op);
output_asm_insn ("brasl\t%0,%4", op);
output_asm_insn ("l\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_restore\t%0", op);
}
}
else if (!flag_pic)
@@ -13225,6 +13234,8 @@ s390_function_profiler (FILE *file, int labelno)
else
{
output_asm_insn ("st\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_rel_offset\t%0,%7", op);
output_asm_insn ("bras\t%2,%l6", op);
output_asm_insn (".long\t%4", op);
output_asm_insn (".long\t%3", op);
@@ -13234,6 +13245,8 @@ s390_function_profiler (FILE *file, int labelno)
output_asm_insn ("l\t%2,4(%2)", op);
output_asm_insn ("basr\t%0,%0", op);
output_asm_insn ("l\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_restore\t%0", op);
}
}
else
@@ -13248,6 +13261,8 @@ s390_function_profiler (FILE *file, int labelno)
else
{
output_asm_insn ("st\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_rel_offset\t%0,%7", op);
output_asm_insn ("bras\t%2,%l6", op);
targetm.asm_out.internal_label (file, "L",
CODE_LABEL_NUMBER (op[5]));
@@ -13260,6 +13275,8 @@ s390_function_profiler (FILE *file, int labelno)
output_asm_insn ("a\t%2,4(%2)", op);
output_asm_insn ("basr\t%0,%0", op);
output_asm_insn ("l\t%0,%1", op);
+ if (flag_dwarf2_cfi_asm)
+output_asm_insn (".cfi_restore\t%0", op);
}
}
--
2.17.1
[committed] Fix OpenMP taskloop lowering (PR middle-end/86542)
Hi!
Another bug I've ran into while working on OpenMP 5.0 range for support.
For collapse(2) and higher taskloops, if there is any clause that
needs a cpyfn callback (e.g. firstprivate with non-POD, or VLA), we wouldn't
be copying over the temporaries needed for collapsed loop; without cpyfn
the default memcpy that copies the passed in data to the task arg area
handles that already.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk, queued for backports.
2018-07-17 Jakub Jelinek
PR middle-end/86542
* omp-low.c (create_task_copyfn): Copy over also fields corresponding
to _looptemp_ clauses, other than the first two.
* testsuite/libgomp.c++/pr86542.C: New test.
--- gcc/omp-low.c.jj2018-06-22 19:17:12.219437746 +0200
+++ gcc/omp-low.c 2018-07-17 10:18:02.551015945 +0200
@@ -7026,6 +7026,7 @@ create_task_copyfn (gomp_task *task_stmt
splay_tree_node n;
struct omp_taskcopy_context tcctx;
location_t loc = gimple_location (task_stmt);
+ size_t looptempno = 0;
child_fn = gimple_omp_task_copy_fn (task_stmt);
child_cfun = DECL_STRUCT_FUNCTION (child_fn);
@@ -7139,6 +7140,15 @@ create_task_copyfn (gomp_task *task_stmt
t = build2 (MODIFY_EXPR, TREE_TYPE (dst), dst, src);
append_to_statement_list (t, &list);
break;
+ case OMP_CLAUSE__LOOPTEMP_:
+ /* Fields for first two _looptemp_ clauses are initialized by
+ GOMP_taskloop*, the rest are handled like firstprivate. */
+if (looptempno < 2)
+ {
+ looptempno++;
+ break;
+ }
+ /* FALLTHRU */
case OMP_CLAUSE_FIRSTPRIVATE:
decl = OMP_CLAUSE_DECL (c);
if (is_variable_sized (decl))
@@ -7164,7 +7174,10 @@ create_task_copyfn (gomp_task *task_stmt
src = decl;
dst = build_simple_mem_ref_loc (loc, arg);
dst = omp_build_component_ref (dst, f);
- t = lang_hooks.decls.omp_clause_copy_ctor (c, dst, src);
+ if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE__LOOPTEMP_)
+ t = build2 (MODIFY_EXPR, TREE_TYPE (dst), dst, src);
+ else
+ t = lang_hooks.decls.omp_clause_copy_ctor (c, dst, src);
append_to_statement_list (t, &list);
break;
case OMP_CLAUSE_PRIVATE:
--- libgomp/testsuite/libgomp.c++/pr86542.C.jj 2018-07-17 10:20:02.214127004
+0200
+++ libgomp/testsuite/libgomp.c++/pr86542.C 2018-07-17 10:19:39.207105565
+0200
@@ -0,0 +1,37 @@
+// PR middle-end/86542
+
+struct S { int s; S (); ~S (); S (const S &); };
+S s;
+
+S::S ()
+{
+}
+
+S::~S ()
+{
+}
+
+S::S (const S &x)
+{
+ s = x.s;
+}
+
+__attribute__((noipa)) void
+foo (int i, int j, int k, S s)
+{
+ if (i != 0 || j != 0 || k != 0 || s.s != 12)
+__builtin_abort ();
+}
+
+int
+main ()
+{
+ volatile int inc = 16, jnc = 16, knc = 16;
+ s.s = 12;
+ #pragma omp taskloop collapse (3) firstprivate (s)
+ for (int i = 0; i < 16; i += inc)
+for (int j = 0; j < 16; j += jnc)
+ for (int k = 0; k < 16; k += knc)
+ foo (i, j, k, s);
+ return 0;
+}
Jakub
Re: [PATCH, ARM] PR85434: Prevent spilling of stack protector guard's address on ARM
Fixed in attached patch. ChangeLog entries are unchanged: *** gcc/ChangeLog *** 2018-07-05 Thomas Preud'homme PR target/85434 * target-insns.def (stack_protect_combined_set): Define new standard pattern name. (stack_protect_combined_test): Likewise. * cfgexpand.c (stack_protect_prologue): Try new stack_protect_combined_set pattern first. * function.c (stack_protect_epilogue): Try new stack_protect_combined_test pattern first. * config/arm/arm.c (require_pic_register): Add pic_reg and compute_now parameters to control which register to use as PIC register and force reloading PIC register respectively. (legitimize_pic_address): Expose above new parameters in prototype and adapt recursive calls accordingly. (arm_legitimize_address): Adapt to new legitimize_pic_address prototype. (thumb_legitimize_address): Likewise. (arm_emit_call_insn): Adapt to new require_pic_register prototype. * config/arm/arm-protos.h (legitimize_pic_address): Adapt to prototype change. * config/arm/arm.md (movsi expander): Adapt to legitimize_pic_address prototype change. (stack_protect_combined_set): New insn_and_split pattern. (stack_protect_set): New insn pattern. (stack_protect_combined_test): New insn_and_split pattern. (stack_protect_test): New insn pattern. * config/arm/unspecs.md (UNSPEC_SP_SET): New unspec. (UNSPEC_SP_TEST): Likewise. * doc/md.texi (stack_protect_combined_set): Document new standard pattern name. (stack_protect_set): Clarify that the operand for guard's address is legal. (stack_protect_combined_test): Document new standard pattern name. (stack_protect_test): Clarify that the operand for guard's address is legal. *** gcc/testsuite/ChangeLog *** 2018-07-05 Thomas Preud'homme PR target/85434 * gcc.target/arm/pr85434.c: New test. Best regards, Thomas On Mon, 16 Jul 2018 at 22:46, Jeff Law wrote: > > On 07/05/2018 08:48 AM, Thomas Preudhomme wrote: > > In case of high register pressure in PIC mode, address of the stack > > protector's guard can be spilled on ARM targets as shown in PR85434, > > thus allowing an attacker to control what the canary would be compared > > against. ARM does lack stack_protect_set and stack_protect_test insn > > patterns, defining them does not help as the address is expanded > > regularly and the patterns only deal with the copy and test of the > > guard with the canary. > > > > This problem does not occur for x86 targets because the PIC access and > > the test can be done in the same instruction. Aarch64 is exempt too > > because PIC access insn pattern are mov of UNSPEC which prevents it from > > the second access in the epilogue being CSEd in cse_local pass with the > > first access in the prologue. > > > > The approach followed here is to create new "combined" set and test > > standard pattern names that take the unexpanded guard and do the set or > > test. This allows the target to use an opaque pattern (eg. using UNSPEC) > > to hide the individual instructions being generated to the compiler and > > split the pattern into generic load, compare and branch instruction > > after register allocator, therefore avoiding any spilling. This is here > > implemented for the ARM targets. For targets not implementing these new > > standard pattern names, the existing stack_protect_set and > > stack_protect_test pattern names are used. > > > > To be able to split PIC access after register allocation, the functions > > had to be augmented to force a new PIC register load and to control > > which register it loads into. This is because sharing the PIC register > > between prologue and epilogue could lead to spilling due to CSE again > > which an attacker could use to control what the canary gets compared > > against. > > > > ChangeLog entries are as follows: > > > > *** gcc/ChangeLog *** > > > > 2018-07-05 Thomas Preud'homme > > > > PR target/85434 > > * target-insns.def (stack_protect_combined_set): Define new standard > > pattern name. > > (stack_protect_combined_test): Likewise. > > * cfgexpand.c (stack_protect_prologue): Try new > > stack_protect_combined_set pattern first. > > * function.c (stack_protect_epilogue): Try new > > stack_protect_combined_test pattern first. > > * config/arm/arm.c (require_pic_register): Add pic_reg and compute_now > > parameters to control which register to use as PIC register and force > > reloading PIC register respectively. > > (legitimize_pic_address): Expose above new parameters in prototype and > > adapt recursive calls accordingly. > > (arm_legitimize_address): Adapt to new legitimize_pic_address > > prototype. > > (thumb_legitimize_address): Likewise. > > (arm_emit_call_insn): Adapt to new require_pic_register prototype. > > * config/arm/arm-protos.h (legitimize_pic_address): Adapt to prototype > > change. > > * conf
Re: Avoid assembler warnings from AArch64 constructor/destructor priorities
On 02/02/18 15:14, Kyrill Tkachov wrote: On 01/02/18 17:26, Joseph Myers wrote: On Thu, 1 Feb 2018, Kyrill Tkachov wrote: Hi Joseph, aarch64 maintainers, On 28/09/17 13:31, Joseph Myers wrote: Many GCC tests fail for AArch64 with current binutils because of assembler warnings of the form "Warning: ignoring incorrect section type for .init_array.00100". The same issue was fixed for ARM in r247015 by using SECTION_NOTYPE when creating those sections; this patch applies the same fix to AArch64. Tested with no regressions with cross to aarch64-linux-gnu. OK to commit? There is a user request to backport this patch to the GCC 7 and 6 branches: PR 84168. Would that be acceptable? I don't object, but I'm not aware of this being a regression. I think it's been present since the beginning of the port so it's not a regression, just a user request for a bugfix backport. The patch applies, bootstraps and tests cleanly on both branches FWIW. Gentle ping on this (I had almost forgotten about it). Do the aarch64 maintainers mind this patch being packported to the GCC 7 and 6 branches? https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00054.html Thanks, Kyrill
[C++ Patch] Another permerror related tweak
Hi again,
here I noticed a pair of consecutive permerrors (instead of permerror +
inform) and also that the first one isn't exploiting
DECL_SOURCE_LOCATION (which makes a real difference at least in a couple
of cases which I'm explicitly testing below). Tested x86_64-linux.
Thanks, Paolo.
/
/cp
2018-07-17 Paolo Carlini
* class.c (note_name_declared_in_class): Prefer permerror + inform
to a pair of permerrors; use DECL_SOURCE_LOCATION.
/testsuite
2018-07-17 Paolo Carlini
* g++.dg/ext/uow-3.C: Adjust.
* g++.dg/ext/uow-4.C: Likewise.
* g++.dg/lookup/name-clash11.C: Likewise.
* g++.dg/lookup/name-clash7.C: Likewise.
* g++.dg/lookup/redecl1.C: Likewise.
* g++.dg/warn/changes-meaning.C: Likewise.
* g++.old-deja/g++.jason/scoping8.C: Likewise.
* g++.old-deja/g++.law/nest1.C: Likewise.
Index: cp/class.c
===
--- cp/class.c (revision 262803)
+++ cp/class.c (working copy)
@@ -8285,10 +8285,12 @@ note_name_declared_in_class (tree name, tree decl)
A name N used in a class S shall refer to the same declaration
in its context and when re-evaluated in the completed scope of
S. */
- permerror (input_location, "declaration of %q#D", decl);
- permerror (location_of ((tree) n->value),
-"changes meaning of %qD from %q#D",
-OVL_NAME (decl), (tree) n->value);
+ if (permerror (DECL_SOURCE_LOCATION (decl),
+"declaration of %q#D changes meaning of %qD",
+decl, OVL_NAME (decl)))
+ inform (location_of ((tree) n->value),
+ "%qD declared here as %q#D",
+ OVL_NAME (decl), (tree) n->value);
}
}
Index: testsuite/g++.dg/ext/uow-3.C
===
--- testsuite/g++.dg/ext/uow-3.C(revision 262803)
+++ testsuite/g++.dg/ext/uow-3.C(working copy)
@@ -1,8 +1,8 @@
/* { dg-do compile } */
/* { dg-options "-Wall" } */
-typedef int UOW; /* { dg-error "" } */
+typedef int UOW; /* { dg-message "declared here" } */
struct ABC {
- UOW UOW; /* { dg-error "" } */
+ UOW UOW; /* { dg-error "changes meaning" } */
};
Index: testsuite/g++.dg/ext/uow-4.C
===
--- testsuite/g++.dg/ext/uow-4.C(revision 262803)
+++ testsuite/g++.dg/ext/uow-4.C(working copy)
@@ -3,9 +3,9 @@
extern "C" {
-typedef int UOW; /* { dg-error "" } */
+typedef int UOW; /* { dg-message "declared here" } */
struct ABC {
- UOW UOW; /* { dg-error "" } */
+ UOW UOW; /* { dg-error "changes meaning" } */
};
}
Index: testsuite/g++.dg/lookup/name-clash11.C
===
--- testsuite/g++.dg/lookup/name-clash11.C (revision 262803)
+++ testsuite/g++.dg/lookup/name-clash11.C (working copy)
@@ -13,11 +13,11 @@
void test_bitset ()
{
- int x;// { dg-warning "changes meaning" }
+ int x;// { dg-message "declared here" }
{
struct S {
- int x: sizeof x; // { dg-warning "declaration" }
+ int x: sizeof x; // { dg-warning "changes meaning" }
};
}
}
@@ -25,11 +25,11 @@ void test_bitset ()
void test_enum ()
{
// Also exercise (not covered by c++/69023):
- int y;// { dg-warning "changes meaning" }
+ int y;// { dg-message "declared here" }
{
struct S {
enum E {
-y = sizeof y// { dg-warning "declaration" }
+y = sizeof y// { dg-warning "9:declaration of .y. changes
meaning" }
};
// Verify the enumerator has the correct value.
@@ -40,7 +40,7 @@ void test_enum ()
void test_alignas ()
{
- enum { A = 16 }; // { dg-warning "changes meaning" }
+ enum { A = 16 }; // { dg-message "declared here" }
{
struct S {
#if __cplusplus >= 201103L
@@ -48,7 +48,7 @@ void test_alignas ()
#else
__attribute__ ((aligned (A)))
#endif
- int A;// { dg-warning "declaration" }
+ int A;// { dg-warning "changes meaning" }
// Verify the member has the correct alignment.
void test () { ASSERT (__alignof__ (this->A) == 16); }
@@ -58,10 +58,10 @@ void test_alignas ()
void test_array ()
{
- enum { A = 16 }; // { dg-warning "changes meaning" }
+ enum { A = 16 }; // { dg-message "declared here" }
{
struct S {
- int A [A];// { dg-warning "declaration" }
+ int A [A];// { dg-warning "changes meaning" }
// Verify the member has the correct alignment.
void test () { ASSERT (sizeof (this->A) == 16 * sizeof (int)); }
@@ -71,10 +71,10 @@ void test_array ()
void t
Re: Avoid assembler warnings from AArch64 constructor/destructor priorities
On 28/09/17 13:31, Joseph Myers wrote: > Many GCC tests fail for AArch64 with current binutils because of > assembler warnings of the form "Warning: ignoring incorrect section > type for .init_array.00100". The same issue was fixed for ARM in > r247015 by using SECTION_NOTYPE when creating those sections; this > patch applies the same fix to AArch64. > > Tested with no regressions with cross to aarch64-linux-gnu. OK to > commit? > > 2017-09-28 Joseph Myers > > * config/aarch64/aarch64.c (aarch64_elf_asm_constructor) > (aarch64_elf_asm_destructor): Pass SECTION_NOTYPE to get_section > when creating .init_array and .fini_array sections with priority > specified. > > Index: gcc/config/aarch64/aarch64.c > === > --- gcc/config/aarch64/aarch64.c (revision 253248) > +++ gcc/config/aarch64/aarch64.c (working copy) > @@ -6095,7 +6095,7 @@ aarch64_elf_asm_constructor (rtx symbol, int prior > -Wformat-truncation false positive, use a larger size. */ >char buf[23]; >snprintf (buf, sizeof (buf), ".init_array.%.5u", priority); > - s = get_section (buf, SECTION_WRITE, NULL); > + s = get_section (buf, SECTION_WRITE | SECTION_NOTYPE, NULL); >switch_to_section (s); >assemble_align (POINTER_SIZE); >assemble_aligned_integer (POINTER_BYTES, symbol); > @@ -6115,7 +6115,7 @@ aarch64_elf_asm_destructor (rtx symbol, int priori > -Wformat-truncation false positive, use a larger size. */ >char buf[23]; >snprintf (buf, sizeof (buf), ".fini_array.%.5u", priority); > - s = get_section (buf, SECTION_WRITE, NULL); > + s = get_section (buf, SECTION_WRITE | SECTION_NOTYPE, NULL); >switch_to_section (s); >assemble_align (POINTER_SIZE); >assemble_aligned_integer (POINTER_BYTES, symbol); > Ok, sorry I missed this one. R.
Re: Avoid assembler warnings from AArch64 constructor/destructor priorities
On 17/07/18 12:09, Kyrill Tkachov wrote: > > On 02/02/18 15:14, Kyrill Tkachov wrote: >> >> On 01/02/18 17:26, Joseph Myers wrote: >>> On Thu, 1 Feb 2018, Kyrill Tkachov wrote: >>> Hi Joseph, aarch64 maintainers, On 28/09/17 13:31, Joseph Myers wrote: > Many GCC tests fail for AArch64 with current binutils because of > assembler warnings of the form "Warning: ignoring incorrect section > type for .init_array.00100". The same issue was fixed for ARM in > r247015 by using SECTION_NOTYPE when creating those sections; this > patch applies the same fix to AArch64. > > Tested with no regressions with cross to aarch64-linux-gnu. OK to > commit? There is a user request to backport this patch to the GCC 7 and 6 branches: PR 84168. Would that be acceptable? >>> I don't object, but I'm not aware of this being a regression. >> >> I think it's been present since the beginning of the port so >> it's not a regression, just a user request for a bugfix backport. >> >> The patch applies, bootstraps and tests cleanly on both branches FWIW. >> > > Gentle ping on this (I had almost forgotten about it). > Do the aarch64 maintainers mind this patch being packported to the GCC 7 > and 6 branches? > > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00054.html > > Thanks, > Kyrill > > It looks pretty safe to me. OK unless the RMs object. R.
[PATCH][COMMITTED][ARC] Don't use deprecated align_labels_log variable.
From: claziss
Use new align_lables struct instead of the deprecated align_labels_log
variable. Committed as obvious.
2018-07-17 Claudiu Zissulescu
* config/arc/arc.c (arc_label_align): Use align_labels instead of
deprecated align_labels_log.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@262820
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/ChangeLog| 5 +
gcc/config/arc/arc.c | 9 +
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b87c00a2d49..27f8a722fa5 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2018-07-17 Claudiu Zissulescu
+
+ * config/arc/arc.c (arc_label_align): Use align_labels instead of
+ deprecated align_labels_log.
+
2018-07-17 Richard Biener
PR lto/86456
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 8bb7d74c2c9..4cbf7ab2b33 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -9762,18 +9762,19 @@ arc_scheduling_not_expected (void)
return cfun->machine->arc_reorg_started;
}
+/* Code has a minimum p2 alignment of 1, which we must restore after
+ an ADDR_DIFF_VEC. */
+
int
arc_label_align (rtx_insn *label)
{
- /* Code has a minimum p2 alignment of 1, which we must restore after an
- ADDR_DIFF_VEC. */
- if (align_labels_log < 1)
+ if (align_labels.levels[0].log < 1)
{
rtx_insn *next = next_nonnote_nondebug_insn (label);
if (INSN_P (next) && recog_memoized (next) >= 0)
return 1;
}
- return align_labels_log;
+ return align_labels.levels[0].log;
}
/* Return true if LABEL is in executable code. */
--
2.17.1
[PATCH][C++] Fix PR86523
The following makes sure to generate early debug for globals that
end up being pushed to the backend via the write_out_vars call in
c_parse_final_cleanups. rest_of_decl_compilation is confused
by seeing current_function_decl set and skips the debug-hook
invocation because of that. So the easy fix is to flush out
globals before starting the init function.
I verified we now properly generate early DIEs for
e_missingvar and related structure types which otherwise with
LTO are completely missing (and we ICE). Without LTO we
may not be able to recover DIEs for those late once we start
to run free-lang-data unconditionally.
Bootstrap & regtest running on x86_64-unknown-linux-gnu.
OK for trunk?
Thanks,
Richard.
>From f73561b03a38a448115726a27749a06011507d65 Mon Sep 17 00:00:00 2001
From: Richard Guenther
Date: Tue, 17 Jul 2018 14:24:17 +0200
Subject: [PATCH] fix-pr86523-2
2018-07-17 Richard Biener
PR debug/86523
cp/
* decl2.c (c_parse_final_cleanups): Call write_out_vars before
start_static_storage_duration_function sets current_function_decl.
* g++.dg/lto/pr86523-3_0.C: New testcase.
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index f8fc20e4093..d67ced097da 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4754,14 +4754,14 @@ c_parse_final_cleanups (void)
inline, with resulting performance improvements. */
tree ssdf_body;
+ /* Make sure the back end knows about all the variables. */
+ write_out_vars (vars);
+
/* Set the line and file, so that it is obviously not from
the source file. */
input_location = locus_at_end_of_parsing;
ssdf_body = start_static_storage_duration_function (ssdf_count);
- /* Make sure the back end knows about all the variables. */
- write_out_vars (vars);
-
/* First generate code to do all the initializations. */
if (vars)
do_static_initialization_or_destruction (vars, /*initp=*/true);
diff --git a/gcc/testsuite/g++.dg/lto/pr86523-3_0.C
b/gcc/testsuite/g++.dg/lto/pr86523-3_0.C
new file mode 100644
index 000..31063b8773a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr86523-3_0.C
@@ -0,0 +1,24 @@
+// { dg-lto-do link }
+// { dg-require-effective-target fpic }
+// { dg-require-effective-target shared }
+// { dg-lto-options { { -fPIC -flto -g -shared } } }
+class a {
+int b;
+};
+int const c = 0, d = 1, f = 2, g = 3;
+struct B {
+typedef a h;
+h i;
+};
+template B j();
+template struct k_context { static B const e_missingvar; };
+template B const k_context::e_missingvar = j();
+inline B m() {
+switch (0) {
+ case c:
+ case d:
+ return k_context::e_missingvar;
+ case f:
+ case g:;
+}
+}
[PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi all,
This is my first Fortran patch, so apologies if I'm missing something.
The current expansion of the min and max intrinsics explicitly expands
the comparisons between each argument to calculate the global min/max.
Some targets, like aarch64, have instructions that can calculate the min/max
of two real (floating-point) numbers with the proper NaN-handling semantics
(if both inputs are NaN, return Nan. If one is NaN, return the other) and those
are the semantics provided by the __builtin_fmin/max family of functions that
expand
to these instructions.
This patch makes the frontend emit __builtin_fmin/max directly to compare each
pair of numbers when the numbers are floating-point, and use MIN_EXPR/MAX_EXPR
otherwise
(integral types and -ffast-math) which should hopefully be easier to recognise
in the
midend and optimise. The previous approach of generating the open-coded version
of that
is used when we don't have an appropriate __builtin_fmin/max available.
For example, for a configuration of x86_64-unknown-linux-gnu that I tested
there was no
128-bit __built_fminl available.
With this patch I'm seeing more than 7000 FMINNM/FMAXNM instructions being
generated at -O3
on aarch64 for 521.wrf from fprate SPEC2017 where none before were generated
(we were generating explicit comparisons and NaN checks). This gave a 2.4%
improvement
in performance on a Cortex-A72.
Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-unknown-linux-gnu.
Ok for trunk?
Thanks,
Kyrill
2018-07-17 Kyrylo Tkachov
* f95-lang.c (gfc_init_builtin_functions): Define __builtin_fmin,
__builtin_fminf, __builtin_fminl, __builtin_fmax, __builtin_fmaxf,
__builtin_fmaxl.
* trans-intrinsic.c: Include builtins.h.
(gfc_conv_intrinsic_minmax): Emit __builtin_fmin/max or MIN/MAX_EXPR
functions to calculate the min/max.
2018-07-17 Kyrylo Tkachov
* gfortran.dg/max_fmaxf.f90: New test.
* gfortran.dg/min_fminf.f90: Likewise.
* gfortran.dg/minmax_integer.f90: Likewise.
* gfortran.dg/max_fmaxl_aarch64.f90: Likewise.
* gfortran.dg/min_fminl_aarch64.f90: Likewise.
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 0f39f0ca788ea9e5868d4718c5f90c102958968f..5dd58f3d3d0242d77a5838ffa0395e7b941c8f85 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -724,6 +724,20 @@ gfc_init_builtin_functions (void)
gfc_define_builtin ("__builtin_roundf", mfunc_float[0],
BUILT_IN_ROUNDF, "roundf", ATTR_CONST_NOTHROW_LEAF_LIST);
+ gfc_define_builtin ("__builtin_fmin", mfunc_double[1],
+ BUILT_IN_FMIN, "fmin", ATTR_CONST_NOTHROW_LEAF_LIST);
+ gfc_define_builtin ("__builtin_fminf", mfunc_float[1],
+ BUILT_IN_FMINF, "fminf", ATTR_CONST_NOTHROW_LEAF_LIST);
+ gfc_define_builtin ("__builtin_fminl", mfunc_longdouble[1],
+ BUILT_IN_FMINL, "fminl", ATTR_CONST_NOTHROW_LEAF_LIST);
+
+ gfc_define_builtin ("__builtin_fmax", mfunc_double[1],
+ BUILT_IN_FMAX, "fmax", ATTR_CONST_NOTHROW_LEAF_LIST);
+ gfc_define_builtin ("__builtin_fmaxf", mfunc_float[1],
+ BUILT_IN_FMAXF, "fmaxf", ATTR_CONST_NOTHROW_LEAF_LIST);
+ gfc_define_builtin ("__builtin_fmaxl", mfunc_longdouble[1],
+ BUILT_IN_FMAXL, "fmaxl", ATTR_CONST_NOTHROW_LEAF_LIST);
+
gfc_define_builtin ("__builtin_truncl", mfunc_longdouble[0],
BUILT_IN_TRUNCL, "truncl", ATTR_CONST_NOTHROW_LEAF_LIST);
gfc_define_builtin ("__builtin_trunc", mfunc_double[0],
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index d306e3a5a6209c1621d91f99ffc366acecd9c3d0..5dde54a3f3c2606f987b42480b1921e6304ccda5 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see
#include "trans.h"
#include "stringpool.h"
#include "fold-const.h"
+#include "builtins.h"
#include "tree-nested.h"
#include "stor-layout.h"
#include "toplev.h" /* For rest_of_decl_compilation. */
@@ -3874,14 +3875,13 @@ gfc_conv_intrinsic_ttynam (gfc_se * se, gfc_expr * expr)
minmax (a1, a2, a3, ...)
{
mvar = a1;
- if (a2 .op. mvar || isnan (mvar))
-mvar = a2;
- if (a3 .op. mvar || isnan (mvar))
-mvar = a3;
+ __builtin_fmin/max (mvar, a2);
+ __builtin_fmin/max (mvar, a3);
...
- return mvar
+ return mvar;
}
- */
+ For integral types or when we don't care about NaNs use
+ MIN/MAX_EXPRs. */
/* TODO: Mismatching types can occur when specific names are used.
These should be handled during resolution. */
@@ -3891,7 +3891,6 @@ gfc_conv_intrinsic_minmax (gfc_se * se, gfc_expr * expr, enum tree_code op)
tree tmp;
tree mvar;
tree val;
- tree thencase;
tree *args;
tree type;
gfc_actual_arglist *argexpr;
@@ -3912,55 +3911,79 @@ gfc_conv_intrinsic_minmax (gfc_se * se, gfc_expr * expr, enum tree_code op)
mvar = gfc_create_var (type, "M");
gfc_add_modify (&se->pre, mvar, args[0]);
- for (i = 1,
[PATCH] Remove unused explicit instantiation of __bind_simple
The explicit instantiation of std::call_once used to require an
instantiation of __bind_simple, but call_once was changed by r241031 to
not use __bind_simple. The instantiation of __bind_simple (and the
definitions it uses) are not needed. They should have been removed
instead of doing the changes in r24 that kept them compiling.
The use of std::call_once by _Async_state_common::_M_join can be
simplified to use a pointer instead of reference wrapper. The call_once
symbol isn't exported so the change isn't visible outside the library.
* src/c++11/compatibility-thread-c++0x.cc [_GLIBCXX_SHARED]
(_Async_state_common::_M_join): Simplify use of std::call_once and
corresponding explicit instantiation.
(_Maybe_wrap_member_pointer, _Bind_simple, _Bind_simple_helper)
(__bind_simple): Remove definitions and explicit instantiation that
are not required by exported symbols.
Tested powerpc64le-linux, committed to trunk.
commit 8c615f0dcb66beed4cf0d4d17f4ed640664932cb
Author: Jonathan Wakely
Date: Tue Jul 17 13:37:00 2018 +0100
Remove unused explicit instantiation of __bind_simple
The explicit instantiation of std::call_once used to require an
instantiation of __bind_simple, but call_once was changed by r241031 to
not use __bind_simple. The instantiation of __bind_simple (and the
definitions it uses) are not needed. They should have been removed
instead of doing the changes in r24 that kept them compiling.
The use of std::call_once by _Async_state_common::_M_join can be
simplified to use a pointer instead of reference wrapper. The call_once
symbol isn't exported so the change isn't visible outside the library.
* src/c++11/compatibility-thread-c++0x.cc [_GLIBCXX_SHARED]
(_Async_state_common::_M_join): Simplify use of std::call_once and
corresponding explicit instantiation.
(_Maybe_wrap_member_pointer, _Bind_simple, _Bind_simple_helper)
(__bind_simple): Remove definitions and explicit instantiation that
are not required by exported symbols.
diff --git a/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
b/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
index 7abbb59877d..e60c8f9bfd6 100644
--- a/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
+++ b/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
@@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
protected:
~_Async_state_common();
virtual void _M_run_deferred() { _M_join(); }
-void _M_join() { std::call_once(_M_once, &thread::join, ref(_M_thread)); }
+void _M_join() { std::call_once(_M_once, &thread::join, &_M_thread); }
thread _M_thread;
once_flag _M_once;
};
@@ -117,84 +117,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Replaced with inline definition in gcc-4.8.0
__future_base::_Async_state_common::~_Async_state_common() { _M_join(); }
- template
-struct _Maybe_wrap_member_pointer;
-
- template
-struct _Maybe_wrap_member_pointer<_Tp _Class::*>
-{
- typedef _Mem_fn<_Tp _Class::*> type;
-
- static constexpr type
- __do_wrap(_Tp _Class::* __pm)
- { return type(__pm); }
-};
-
- template
-struct _Bind_simple;
-
- template
-struct _Bind_simple<_Callable(_Args...)>
-{
- typedef typename result_of<_Callable(_Args...)>::type result_type;
-
- template
-explicit
-_Bind_simple(_Tp&& __f, _Up&&... __args)
-: _M_bound(std::forward<_Tp>(__f), std::forward<_Up>(__args)...)
-{ }
-
- _Bind_simple(const _Bind_simple&) = default;
- _Bind_simple(_Bind_simple&&) = default;
-
- result_type
- operator()()
- {
-typedef typename _Build_index_tuple::__type _Indices;
-return _M_invoke(_Indices());
- }
-
-private:
- template
-typename result_of<_Callable(_Args...)>::type
-_M_invoke(_Index_tuple<_Indices...>)
-{
- // std::bind always forwards bound arguments as lvalues,
- // but this type can call functions which only accept rvalues.
- return std::forward<_Callable>(std::get<0>(_M_bound))(
- std::forward<_Args>(std::get<_Indices+1>(_M_bound))...);
-}
-
- std::tuple<_Callable, _Args...> _M_bound;
-};
-
- template
-struct _Bind_simple_helper
-{
- typedef _Maybe_wrap_member_pointer::type>
-__maybe_type;
- typedef typename __maybe_type::type __func_type;
- typedef _Bind_simple<__func_type(typename decay<_BoundArgs>::type...)>
- __type;
-};
-
- // Simplified version of std::bind for internal use, without support for
- // unbound arguments, placeholders or nested bind expressions.
- template
-typename _Bind_simple_helper<_Callable, _Args...>::__type
-__bind_simple(_Callable&& __callable, _Args&&... __args)
-{
- typedef _Bind_simple_helper<_Callable, _Args...> __helper_typ
[PATCH] PR libstdc++/86450 use -Wabi=2 and simplify -Werror use
Use -Wabi=2 to fix warnings about -Wabi having no effect on its own.
This requires suppressing two warnings in src/c++11/debug.cc which do
not affect the library ABI.
Previously libstdc++ defaulted to --enable-werror but the -Werror flag
was not actually added unless --enable-maintainer-mode was used. This is
not documented and not the expected behaviour. This removes any special
treatment for maintainer-mode, makes -Werror depend directly on
--enable-werror, and changes the default to --enable-werror=no.
PR libstdc++/86450
* acinclude.m4 (GLIBCXX_CHECK_COMPILER_FEATURES): Don't define WERROR.
(GLIBCXX_EXPORT_FLAGS): Use -Wabi=2 instead of -Wabi.
* configure: Regenerate.
* configure.ac: Change GLIBCXX_ENABLE_WERROR default to "no".
* doc/Makefile.in: Regenerate.
* fragment.am: Set WERROR_FLAG to -Werror instead of $(WERROR).
* include/Makefile.in: Regenerate.
* libsupc++/Makefile.in: Regenerate.
* po/Makefile.in: Regenerate.
* python/Makefile.in: Regenerate.
* src/Makefile.in: Regenerate.
* src/c++11/Makefile.in: Regenerate.
* src/c++11/debug.cc: Use diagnostic pragmas to suppress warnings
from -Wabi=2 that don't affect exported symbols.
* src/c++98/Makefile.in: Regenerate.
* src/filesystem/Makefile.in: Regenerate.
* testsuite/Makefile.in: Regenerate.
Tested powerpc64le-linux, committed to trunk.
commit 0e99fd82ce20499c4da0d0950e7197f14d45311f
Author: Jonathan Wakely
Date: Tue Jul 17 00:59:17 2018 +0100
PR libstdc++/86450 use -Wabi=2 and simplify -Werror use
Use -Wabi=2 to fix warnings about -Wabi having no effect on its own.
This requires suppressing two warnings in src/c++11/debug.cc which do
not affect the library ABI.
Previously libstdc++ defaulted to --enable-werror but the -Werror flag
was not actually added unless --enable-maintainer-mode was used. This is
not documented and not the expected behaviour. This removes any special
treatment for maintainer-mode, makes -Werror depend directly on
--enable-werror, and changes the default to --enable-werror=no.
PR libstdc++/86450
* acinclude.m4 (GLIBCXX_CHECK_COMPILER_FEATURES): Don't define
WERROR.
(GLIBCXX_EXPORT_FLAGS): Use -Wabi=2 instead of -Wabi.
* configure: Regenerate.
* configure.ac: Change GLIBCXX_ENABLE_WERROR default to "no".
* doc/Makefile.in: Regenerate.
* fragment.am: Set WERROR_FLAG to -Werror instead of $(WERROR).
* include/Makefile.in: Regenerate.
* libsupc++/Makefile.in: Regenerate.
* po/Makefile.in: Regenerate.
* python/Makefile.in: Regenerate.
* src/Makefile.in: Regenerate.
* src/c++11/Makefile.in: Regenerate.
* src/c++11/debug.cc: Use diagnostic pragmas to suppress warnings
from -Wabi=2 that don't affect exported symbols.
* src/c++98/Makefile.in: Regenerate.
* src/filesystem/Makefile.in: Regenerate.
* testsuite/Makefile.in: Regenerate.
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index cf5add167e6..bbf3c8df3e1 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -140,13 +140,6 @@ AC_DEFUN([GLIBCXX_CHECK_COMPILER_FEATURES], [
ac_test_CXXFLAGS="${CXXFLAGS+set}"
ac_save_CXXFLAGS="$CXXFLAGS"
- # Check for maintainer-mode bits.
- if test x"$USE_MAINTAINER_MODE" = xno; then
-WERROR=''
- else
-WERROR='-Werror'
- fi
-
# Check for -ffunction-sections -fdata-sections
AC_MSG_CHECKING([for g++ that supports -ffunction-sections -fdata-sections])
CXXFLAGS='-g -Werror -ffunction-sections -fdata-sections'
@@ -163,7 +156,6 @@ AC_DEFUN([GLIBCXX_CHECK_COMPILER_FEATURES], [
AC_MSG_RESULT($ac_fdsections)
AC_LANG_RESTORE
- AC_SUBST(WERROR)
AC_SUBST(SECTION_FLAGS)
])
@@ -733,7 +725,7 @@ AC_DEFUN([GLIBCXX_EXPORT_FLAGS], [
# OPTIMIZE_CXXFLAGS = -O3 -fstrict-aliasing -fvtable-gc
AC_SUBST(OPTIMIZE_CXXFLAGS)
- WARN_FLAGS='-Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi'
+ WARN_FLAGS="-Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2"
AC_SUBST(WARN_FLAGS)
])
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 7e1fd84606a..1e0a33fb3ea 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -175,7 +175,7 @@ GLIBCXX_ENABLE_CXX_FLAGS
GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING([no])
GLIBCXX_ENABLE_EXTERN_TEMPLATE([yes])
GLIBCXX_ENABLE_PYTHON
-GLIBCXX_ENABLE_WERROR([yes])
+GLIBCXX_ENABLE_WERROR([no])
GLIBCXX_ENABLE_VTABLE_VERIFY([no])
# Checks for operating systems support that doesn't require linking.
diff --git a/libstdc++-v3/fragment.am b/libstdc++-v3/fragment.am
index 898569520b9..216c572fc60 100644
--- a/libstdc++-v3/fragment.am
+++ b/libstdc++-v3/fragment.am
@@ -14,9 +14,9 @@ toolexecdir = $(glibcxx_toolexecdir
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
On Tue, Jul 17, 2018 at 2:35 PM Kyrill Tkachov wrote: > > Hi all, > > This is my first Fortran patch, so apologies if I'm missing something. > The current expansion of the min and max intrinsics explicitly expands > the comparisons between each argument to calculate the global min/max. > Some targets, like aarch64, have instructions that can calculate the min/max > of two real (floating-point) numbers with the proper NaN-handling semantics > (if both inputs are NaN, return Nan. If one is NaN, return the other) and > those > are the semantics provided by the __builtin_fmin/max family of functions that > expand > to these instructions. > > This patch makes the frontend emit __builtin_fmin/max directly to compare each > pair of numbers when the numbers are floating-point, and use > MIN_EXPR/MAX_EXPR otherwise > (integral types and -ffast-math) which should hopefully be easier to > recognise in the What is Fortrans requirement on min/max intrinsics? Doesn't it only require things that are guaranteed by MIN/MAX_EXPR anyways? The only restriction here is /* Minimum and maximum values. When used with floating point, if both operands are zeros, or if either operand is NaN, then it is unspecified which of the two operands is returned as the result. */ which means MIN/MAX_EXPR are not strictly IEEE compliant with signed zeros or NaNs. Thus the correct test would be !HONOR_SIGNED_ZEROS && !HONOR_NANS if singed zeros are significant. I'm not sure if using fmin/max calls when we cannot use MIN/MAX_EXPR is a good idea, this may both generate bigger code and be slower. Richard. > midend and optimise. The previous approach of generating the open-coded > version of that > is used when we don't have an appropriate __builtin_fmin/max available. > For example, for a configuration of x86_64-unknown-linux-gnu that I tested > there was no > 128-bit __built_fminl available. > > With this patch I'm seeing more than 7000 FMINNM/FMAXNM instructions being > generated at -O3 > on aarch64 for 521.wrf from fprate SPEC2017 where none before were generated > (we were generating explicit comparisons and NaN checks). This gave a 2.4% > improvement > in performance on a Cortex-A72. > > Bootstrapped and tested on aarch64-none-linux-gnu and > x86_64-unknown-linux-gnu. > > Ok for trunk? > Thanks, > Kyrill > > 2018-07-17 Kyrylo Tkachov > > * f95-lang.c (gfc_init_builtin_functions): Define __builtin_fmin, > __builtin_fminf, __builtin_fminl, __builtin_fmax, __builtin_fmaxf, > __builtin_fmaxl. > * trans-intrinsic.c: Include builtins.h. > (gfc_conv_intrinsic_minmax): Emit __builtin_fmin/max or MIN/MAX_EXPR > functions to calculate the min/max. > > 2018-07-17 Kyrylo Tkachov > > * gfortran.dg/max_fmaxf.f90: New test. > * gfortran.dg/min_fminf.f90: Likewise. > * gfortran.dg/minmax_integer.f90: Likewise. > * gfortran.dg/max_fmaxl_aarch64.f90: Likewise. > * gfortran.dg/min_fminl_aarch64.f90: Likewise.
New Japanese PO file for 'gcc' (version 8.1.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Japanese team of translators. The file is available at: http://translationproject.org/latest/gcc/ja.po (This file, 'gcc-8.1.0.ja.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [GCC][PATCH][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks
On 17/07/18 02:33, Richard Henderson wrote:
> On 07/16/2018 10:10 AM, Sam Tebbs wrote:
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -1439,6 +1439,14 @@ aarch64_hard_regno_caller_save_mode (unsigned regno,
>> unsigned,
>> return SImode;
>> }
>>
>> +/* Implement IS_LEFT_CONSECUTIVE. Check if an integer's bits are
>> consecutive
>> + ones from the MSB. */
>> +bool
>> +aarch64_is_left_consecutive (HOST_WIDE_INT i)
>> +{
>> + return (i | (i - 1)) == HOST_WIDE_INT_M1;
>> +}
>> +
> ...
>> +(define_insn "*aarch64_bfxil"
>> + [(set (match_operand:DI 0 "register_operand" "=r")
>> +(ior:DI (and:DI (match_operand:DI 1 "register_operand" "r")
>> +(match_operand 3 "const_int_operand"))
>> +(and:DI (match_operand:DI 2 "register_operand" "0")
>> +(match_operand 4 "const_int_operand"]
>> + "INTVAL (operands[3]) == ~INTVAL (operands[4])
>> +&& aarch64_is_left_consecutive (INTVAL (operands[4]))"
>
> Better is to use a define_predicate to merge both that second test and the
> const_int_operand.
>
> (I'm not sure about the "left_consecutive" language either.
> Isn't it more descriptive to say that op3 is a power of 2 minus 1?)
>
> (define_predicate "pow2m1_operand"
> (and (match_code "const_int")
>(match_test "exact_pow2 (INTVAL(op) + 1) > 0")))
ITYM exact_log2()
R.
>
> and use
>
> (match_operand:DI 3 "pow2m1_operand")
>
> and then just the
>
> INTVAL (operands[3]) == ~INTVAL (operands[4])
>
> test.
>
> Also, don't omit the modes for the constants.
> Also, there's no reason this applies only to DI mode;
> use the GPI iterator and % in the output template.
>
>> +HOST_WIDE_INT op3 = INTVAL (operands[3]);
>> +operands[3] = GEN_INT (ceil_log2 (op3));
>> +output_asm_insn ("bfxil\\t%0, %1, 0, %3", operands);
>> +return "";
>
> You can just return the string that you passed to output_asm_insn.
>
>> + }
>> + [(set_attr "type" "bfx")]
>
> The other aliases of the BFM insn use type "bfm";
> "bfx" appears to be aliases of UBFM and SBFM.
> Not that it appears to matter to the scheduling
> descriptions, but it is inconsistent.
>
>
> r~
>
Re: [PR fortran/83184] Fix matching code for clist/old-style initializers when encountering assumed-rank declarations
On Tue, Jul 17, 2018, 04:31 Janus Weil wrote: > Did someone actually approve this patch? Apparently it was committed > as r262744 and caused the following regression: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86543 > > Cheers, > Janus > > > > 2018-06-27 23:07 GMT+02:00 Fritz Reese : > > The attached patch fixes PR fortran/83184, which is actually two > > distinct bugs as described in the PR. Passes regtests. > > > > The patch is attached. OK for trunk and 7/8-branch? > > > > From 238f0a0e80c93209bb4e62ba2f719f74f5da164f Mon Sep 17 00:00:00 2001 > > From: Fritz Reese > > Date: Wed, 27 Jun 2018 16:16:31 -0400 > > Subject: [PATCH 2/3] PR fortran/83184 > > > > Fix handling of invalid assumed-shape/size arrays in legacy initializer > > lists. > > > > gcc/fortran/ > > > > * decl.c (match_old_style_init): Initialize locus of variable > expr when > > creating a data variable. > > (match_clist_expr): Verify array is explicit shape/size before > > attempting to allocate constant array constructor. > > > > gcc/testsuite/ > > > > * gfortran.dg/assumed_rank_14.f90: New testcase. > > * gfortran.dg/assumed_rank_15.f90: New testcase. > > * gfortran.dg/dec_structure_8.f90: Update error messages. > > * gfortran.dg/dec_structure_23.f90: Update error messages. > > --- > > gcc/fortran/decl.c | 63 > +++--- > > gcc/testsuite/gfortran.dg/assumed_rank_14.f90 | 11 + > > gcc/testsuite/gfortran.dg/assumed_rank_15.f90 | 11 + > > gcc/testsuite/gfortran.dg/dec_structure_23.f90 | 6 +-- > > gcc/testsuite/gfortran.dg/dec_structure_8.f90 | 6 +-- > > 5 files changed, 64 insertions(+), 33 deletions(-) > > create mode 100644 gcc/testsuite/gfortran.dg/assumed_rank_14.f90 > > create mode 100644 gcc/testsuite/gfortran.dg/assumed_rank_15.f90 > The patch touches only code which I introduced originally. However apparently my revision of trunk when I tested this patch was before the error message was changed for dec_structure_23.f90 by Steve in r257971 (I had trouble bootstrapping the latest at the time). Therefore the committing of this patch “reverted” the change to the testcase. The error message should match “array with nonconstant bounds”. I’ll fix the testcase later today when I am in a place where I can do so. Fritz >
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi Richard, On 17/07/18 14:27, Richard Biener wrote: On Tue, Jul 17, 2018 at 2:35 PM Kyrill Tkachov wrote: Hi all, This is my first Fortran patch, so apologies if I'm missing something. The current expansion of the min and max intrinsics explicitly expands the comparisons between each argument to calculate the global min/max. Some targets, like aarch64, have instructions that can calculate the min/max of two real (floating-point) numbers with the proper NaN-handling semantics (if both inputs are NaN, return Nan. If one is NaN, return the other) and those are the semantics provided by the __builtin_fmin/max family of functions that expand to these instructions. This patch makes the frontend emit __builtin_fmin/max directly to compare each pair of numbers when the numbers are floating-point, and use MIN_EXPR/MAX_EXPR otherwise (integral types and -ffast-math) which should hopefully be easier to recognise in the What is Fortrans requirement on min/max intrinsics? Doesn't it only require things that are guaranteed by MIN/MAX_EXPR anyways? The only restriction here is The current implementation expands to: mvar = a1; if (a2 .op. mvar || isnan (mvar)) mvar = a2; if (a3 .op. mvar || isnan (mvar)) mvar = a3; ... return mvar; That is, if one of the operands is a NaN it will return the other argument. If both (all) are NaNs, it will return NaN. This is the same as the semantics of fmin/max as far as I can tell. /* Minimum and maximum values. When used with floating point, if both operands are zeros, or if either operand is NaN, then it is unspecified which of the two operands is returned as the result. */ which means MIN/MAX_EXPR are not strictly IEEE compliant with signed zeros or NaNs. Thus the correct test would be !HONOR_SIGNED_ZEROS && !HONOR_NANS if singed zeros are significant. True, MIN/MAX_EXPR would not be appropriate in that condition. I guarded their use on !HONOR_NANS (type) only. I'll update it to !HONOR_SIGNED_ZEROS (type) && !HONOR_NANS (type). I'm not sure if using fmin/max calls when we cannot use MIN/MAX_EXPR is a good idea, this may both generate bigger code and be slower. The patch will generate fmin/fmax calls (or the fminf,fminl variants) when mathfn_built_in advertises them as available (does that mean they'll have a fast inline implementation?) If the above doesn't hold and we can't use either MIN/MAX_EXPR of fmin/fmax then the patch falls back to the existing expansion. FWIW, this patch does improve performance on 521.wrf from SPEC2017 on aarch64. Thanks, Kyrill Richard. midend and optimise. The previous approach of generating the open-coded version of that is used when we don't have an appropriate __builtin_fmin/max available. For example, for a configuration of x86_64-unknown-linux-gnu that I tested there was no 128-bit __built_fminl available. With this patch I'm seeing more than 7000 FMINNM/FMAXNM instructions being generated at -O3 on aarch64 for 521.wrf from fprate SPEC2017 where none before were generated (we were generating explicit comparisons and NaN checks). This gave a 2.4% improvement in performance on a Cortex-A72. Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-unknown-linux-gnu. Ok for trunk? Thanks, Kyrill 2018-07-17 Kyrylo Tkachov * f95-lang.c (gfc_init_builtin_functions): Define __builtin_fmin, __builtin_fminf, __builtin_fminl, __builtin_fmax, __builtin_fmaxf, __builtin_fmaxl. * trans-intrinsic.c: Include builtins.h. (gfc_conv_intrinsic_minmax): Emit __builtin_fmin/max or MIN/MAX_EXPR functions to calculate the min/max. 2018-07-17 Kyrylo Tkachov * gfortran.dg/max_fmaxf.f90: New test. * gfortran.dg/min_fminf.f90: Likewise. * gfortran.dg/minmax_integer.f90: Likewise. * gfortran.dg/max_fmaxl_aarch64.f90: Likewise. * gfortran.dg/min_fminl_aarch64.f90: Likewise.
Re: [PATCH][C++] Fix PR86523
On 07/17/2018 08:32 AM, Richard Biener wrote: The following makes sure to generate early debug for globals that end up being pushed to the backend via the write_out_vars call in c_parse_final_cleanups. rest_of_decl_compilation is confused by seeing current_function_decl set and skips the debug-hook invocation because of that. So the easy fix is to flush out globals before starting the init function. looks sensible, ok. nathan -- Nathan Sidwell
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
2018-07-17 9:52 GMT+02:00 Janus Weil :
>> 2018-07-16 21:50 GMT+02:00 Thomas Koenig :
>>> What I would suggest is to enable -Wfunction-eliminiation with
>>> -Wextra and also use that for your new warning.
>>
>> Thanks for the comments. Makes sense. Updated patch attached.
>
>
> Huh, after actually trying -Wfunction-elimination, I'm not so sure any
> more if it really makes sense to throw the new warnings in there,
> mainly for two reasons:
>
> a) -Wfunction-elimination is slightly different, in that it warns
> about removing *any* kind of function, not just impure ones. This
> makes it pretty verbose, and I'm not sure why one would need to know
> if a pure function call is removed.
> b) It gives warnings on places that I don't understand. Simple example:
>
> subroutine sub(r)
>implicit none
>real, intent(in) :: r
>if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then
> print *, "rrr"
>end if
> end
>
> Compiling this with "gfortran-8 -O1 -Wfunction-elimination" gives me:
>
> if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then
> 1
> Warning: Removing call to function ‘abs’ at (1) [-Wfunction-elimination]
>
>
> Why can I remove the call to ABS there? If I read the dump correctly,
> then the two calls to ABS are optimized into a single one, which is
> used for both comparisons via a temporary. Is that what the warning is
> trying to tell me? And if yes, why should I care (if the function is
> pure)? The middle-end certainly does tons of optimizations that
> rearrange various expressions, without warning me about it ...
>
> In other words: Does it make sense to tone down
> -Wfunction-elimination, by only warning about impure functions?
Here is an update of the patch which does that. Regtesting now ...
Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c (revision 262764)
+++ gcc/fortran/dump-parse-tree.c (working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
fputs (" ELEMENTAL", dumpfile);
if (attr->pure)
fputs (" PURE", dumpfile);
+ if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
if (attr->recursive)
fputs (" RECURSIVE", dumpfile);
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c (revision 262764)
+++ gcc/fortran/frontend-passes.c (working copy)
@@ -840,17 +840,17 @@ create_var (gfc_expr * e, const char *vname)
static void
do_warn_function_elimination (gfc_expr *e)
{
- if (e->expr_type != EXPR_FUNCTION)
-return;
- if (e->value.function.esym)
-gfc_warning (OPT_Wfunction_elimination,
- "Removing call to function %qs at %L",
- e->value.function.esym->name, &(e->where));
- else if (e->value.function.isym)
-gfc_warning (OPT_Wfunction_elimination,
- "Removing call to function %qs at %L",
- e->value.function.isym->name, &(e->where));
+ const char *name;
+ if (e->expr_type == EXPR_FUNCTION && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e))
+ {
+ if (name)
+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function %qs at %L", name, &(e->where));
+ else
+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function at %L", &(e->where));
+ }
}
+
+
/* Callback function for the code walker for doing common function
elimination. This builds up the list of functions in the expression
and goes through them to detect duplicates, which it then replaces
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h (revision 262764)
+++ gcc/fortran/gfortran.h (working copy)
@@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *)
bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
extern int gfc_do_concurrent_flag;
const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
+int gfc_pure_function (gfc_expr *e, const char **name);
+int gfc_implicit_pure_function (gfc_expr *e);
/* array.c */
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi (revision 262764)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
@menu
* KIND Type Parameters::
* Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
* Thread-safety of the runtime library::
* Data consistency and durability::
* Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expre
[PATCH] Fix error level of warning about C++ style comments
Greetings,
Below is a simple patch for fixing the error level of an additional note
that follows a warning about using C++ style comments. The ChangeLog
entry is below, followed by the patch.
Tried to figure out how to run the tests, but not sure if it's necessary for
something so simple... would appreciate some guidance here.
Let me know if everything looks good. Thanks!
2018-07-17 Jason Franklin
* libcpp/lex.c: Fix error level for note following warning about
the use of C++ style comments.
>From abd68997bdaf77d48aeb653fab7311b16791d9da Mon Sep 17 00:00:00 2001
From: Jason Franklin
Date: Tue, 17 Jul 2018 10:47:28 -0400
Subject: [PATCH] Fix error level for note on C++ comment warning
---
libcpp/lex.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcpp/lex.c b/libcpp/lex.c
index 37c365a3560..4b93691bd1e 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -2874,7 +2874,7 @@ _cpp_lex_direct (cpp_reader *pfile)
{
cpp_error (pfile, CPP_DL_PEDWARN,
"C++ style comments are not allowed in ISO C90");
- cpp_error (pfile, CPP_DL_PEDWARN,
+ cpp_error (pfile, CPP_DL_NOTE,
"(this will be reported only once per input file)");
buffer->warned_cplusplus_comments = 1;
}
@@ -2885,7 +2885,7 @@ _cpp_lex_direct (cpp_reader *pfile)
{
cpp_error (pfile, CPP_DL_WARNING,
"C++ style comments are incompatible with C90");
- cpp_error (pfile, CPP_DL_WARNING,
+ cpp_error (pfile, CPP_DL_NOTE,
"(this will be reported only once per input file)");
buffer->warned_cplusplus_comments = 1;
}
--
2.17.1
Re: [AArch64][PATCH 1/2] Fix addressing printing of LDP/STP
On Mon, Jun 25, 2018 at 03:48:13AM -0500, Andre Simoes Dias Vieira wrote: > On 18/06/18 09:08, Andre Simoes Dias Vieira wrote: > > Hi Richard, > > > > Sorry for the delay I have been on holidays. I had a look and I think you > > are right. With these changes Umq and Uml seem to have the same > > functionality though, so I would suggest using only one. Maybe use a > > different name for both, removing both Umq and Uml in favour of Umn, where > > the n indicates it narrows the addressing mode. How does that sound to you? > > > > I also had a look at Ump, but that one is used in the parallel pattern for > > STP/LDP which does not use this "narrowing". So we should leave that one as > > is. > > > > Cheers, > > Andre > > > > > > From: Richard Sandiford > > Sent: Thursday, June 14, 2018 12:28:16 PM > > To: Andre Simoes Dias Vieira > > Cc: gcc-patches@gcc.gnu.org; nd > > Subject: Re: [AArch64][PATCH 1/2] Fix addressing printing of LDP/STP > > > > Andre Simoes Dias Vieira writes: > >> @@ -5716,10 +5717,17 @@ aarch64_classify_address (struct > >> aarch64_address_info *info, > >>unsigned int vec_flags = aarch64_classify_vector_mode (mode); > >>bool advsimd_struct_p = (vec_flags == (VEC_ADVSIMD | VEC_STRUCT)); > >>bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP > >> + || type == ADDR_QUERY_LDP_STP_N > >> || mode == TImode > >> || mode == TFmode > >> || (BYTES_BIG_ENDIAN && advsimd_struct_p)); > >> > >> + /* If we are dealing with ADDR_QUERY_LDP_STP_N that means the incoming > >> mode > >> + corresponds to the actual size of the memory being loaded/stored and > >> the > >> + mode of the corresponding addressing mode is half of that. */ > >> + if (type == ADDR_QUERY_LDP_STP_N && known_eq (GET_MODE_SIZE (mode), 16)) > >> +mode = DFmode; > >> + > >>bool allow_reg_index_p = (!load_store_pair_p > >> && (known_lt (GET_MODE_SIZE (mode), 16) > >> || vec_flags == VEC_ADVSIMD > > > > I don't know whether it matters in practice, but that description also > > applies to Umq, not just Uml. It might be worth changing it too so > > that things stay consistent. > > > > Thanks, > > Richard > > > Hi all, > > This is a reworked patched, replacing Umq and Uml with Umn now. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > Is this OK for trunk? OK. Does this also need backporting to 8? Thanks, James > > gcc > 2018-06-25 Andre Vieira > > * config/aarch64/aarch64-simd.md (aarch64_simd_mov): > Replace > Umq with Umn. > (store_pair_lanes): Likewise. > * config/aarch64/aarch64-protos.h (aarch64_addr_query_type): Add new > enum value 'ADDR_QUERY_LDP_STP_N'. > * config/aarch64/aarch64.c (aarch64_addr_query_type): Likewise. > (aarch64_print_address_internal): Add declaration. > (aarch64_print_ldpstp_address): Remove. > (aarch64_classify_address): Adapt mode for 'ADDR_QUERY_LDP_STP_N'. > (aarch64_print_operand): Change printing of 'y'. > * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Use > new enum value 'ADDR_QUERY_LDP_STP_N', don't hardcode mode and use > 'true' rather than '1'. > * gcc/config/aarch64/constraints.md (Uml): Likewise. > (Uml): Rename to Umn. > (Umq): Remove.
Re: [AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes
On Mon, Jun 25, 2018 at 03:48:43AM -0500, Andre Simoes Dias Vieira wrote: > On 14/06/18 12:47, Richard Sandiford wrote: > > Kyrill Tkachov writes: > >> Hi Andre, > >> On 07/06/18 18:02, Andre Simoes Dias Vieira wrote: > >>> Hi, > >>> > >>> See below a patch to address PR 83009. > >>> > >>> Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and > >>> fortran. > >>> Ran the adjusted testcase for -mabi=ilp32. > >>> > >>> Is this OK for trunk? > >>> > >>> Cheers, > >>> Andre > >>> > >>> PR target/83009: Relax strict address checking for store pair lanes > >>> > >>> The operand constraint for the memory address of store/load pair lanes was > >>> enforcing strictly hardware registers be allowed as memory addresses. We > >>> want > >>> to relax that such that these patterns can be used by combine, prior > >>> to reload. > >>> During register allocation the register constraint will enforce the > >>> correct > >>> register is chosen. > >>> > >>> gcc > >>> 2018-06-07 Andre Vieira > >>> > >>> PR target/83009 > >>> * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): > >>> Make > >>> address check not strict prior to reload. > >>> > >>> gcc/testsuite > >>> 2018-06-07 Andre Vieira > >>> > >>> PR target/83009 > >>> * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. > >> > >> diff --git a/gcc/config/aarch64/predicates.md > >> b/gcc/config/aarch64/predicates.md > >> index > >> f0917af8b4cec945ba4e38e4dc670200f8812983..30aa88838671bf343a883077c2b606a035c030dd > >> 100644 > >> --- a/gcc/config/aarch64/predicates.md > >> +++ b/gcc/config/aarch64/predicates.md > >> @@ -227,7 +227,7 @@ > >> (define_predicate "aarch64_mem_pair_lanes_operand" > >> (and (match_code "mem") > >> (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP > >> (op, 0), > >> -true, > >> +reload_completed, > >> ADDR_QUERY_LDP_STP_N)"))) > >> > >> > >> If you want to enforce strict checking during reload and later then I > >> think you need to use reload_in_progress || reload_completed ? > > > > That was the old way, but it would be lra_in_progress now. > > However... > > > >> I guess that would be equivalent to !can_create_pseudo (). > > > > We should never see pseudos when reload_completed, so the choice > > shouldn't really matter then. And I don't think we should use > > lra_in_progress either, since that would make the checks stricter > > before RA has actually happened, which would likely lead to an > > unrecognisable insn ICE if recog is called during one of the LRA > > subpasses. > > > > So unless we know a reason otherwise, I think this should just > > be "false" (like it already is for aarch64_mem_pair_operand). > > > > Thanks, > > Richard > > > Changed it to false. > > Bootstrapped and regression testing for aarch64-none-linux-gnu. > > Is this OK for trunk? OK. Thanks, James > gcc > 2018-06-25 Andre Vieira > > PR target/83009 > * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): > Make > address check not strict. > > gcc/testsuite > 2018-06-25 Andre Vieira > > PR target/83009 > * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests.
Re: [PATCH] Fix error level of warning about C++ style comments
On Tue, Jul 17, 2018 at 07:51:42AM -0700, Jason Franklin wrote:
> Greetings,
>
> Below is a simple patch for fixing the error level of an additional note
> that follows a warning about using C++ style comments. The ChangeLog
> entry is below, followed by the patch.
>
> Tried to figure out how to run the tests, but not sure if it's necessary for
> something so simple... would appreciate some guidance here.
>
> Let me know if everything looks good. Thanks!
>
>
> 2018-07-17 Jason Franklin
>
> * libcpp/lex.c: Fix error level for note following warning about
> the use of C++ style comments.
The ChangeLog entry for libcpp/ should not include libcpp/ prefix and
should include the function name, so:
* lex.c (_cpp_lex_direct): Fix ...
> diff --git a/libcpp/lex.c b/libcpp/lex.c
> index 37c365a3560..4b93691bd1e 100644
> --- a/libcpp/lex.c
> +++ b/libcpp/lex.c
> @@ -2874,7 +2874,7 @@ _cpp_lex_direct (cpp_reader *pfile)
> {
> cpp_error (pfile, CPP_DL_PEDWARN,
> "C++ style comments are not allowed in ISO C90");
> - cpp_error (pfile, CPP_DL_PEDWARN,
> + cpp_error (pfile, CPP_DL_NOTE,
> "(this will be reported only once per input file)");
This has the undesirable effect that for say:
// foo
int i;
you get with this patch:
gcc -S -pedantic test.c -std=gnu89 -w
test.c:1:1: note: (this will be reported only once per input file)
// foo
^
while before that you'd get no diagnostics at all.
So, instead it should do:
if (cpp_error (pfile, CPP_DL_PEDWARN,
"C++ style comments are not allowed in ISO C90"))
cpp_error (pfile, CPP_DL_NOTE,
"(this will be reported only once per input file)");
which makes sure the note is emitted only if the warning/error before it is
emitted.
> buffer->warned_cplusplus_comments = 1;
> }
> @@ -2885,7 +2885,7 @@ _cpp_lex_direct (cpp_reader *pfile)
> {
> cpp_error (pfile, CPP_DL_WARNING,
> "C++ style comments are incompatible with C90");
> - cpp_error (pfile, CPP_DL_WARNING,
> + cpp_error (pfile, CPP_DL_NOTE,
> "(this will be reported only once per input file)");
> buffer->warned_cplusplus_comments = 1;
> }
Likewise. Furthermore, there is
cpp_error (pfile, CPP_DL_ERROR,
"C++ style comments are not allowed in ISO C90");
cpp_error (pfile, CPP_DL_ERROR,
"(this will be reported only once per input "
"file)");
a few lines below, this third spot needs a similar change too.
Jakub
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
On Tue, Jul 17, 2018 at 10:32 AM Janus Weil wrote:
>
> 2018-07-17 9:52 GMT+02:00 Janus Weil :
> >> 2018-07-16 21:50 GMT+02:00 Thomas Koenig :
> >>> What I would suggest is to enable -Wfunction-eliminiation with
> >>> -Wextra and also use that for your new warning.
> >>
> >> Thanks for the comments. Makes sense. Updated patch attached.
> >
> >
> > Huh, after actually trying -Wfunction-elimination, I'm not so sure any
> > more if it really makes sense to throw the new warnings in there,
> > mainly for two reasons:
[...]
> > In other words: Does it make sense to tone down
> > -Wfunction-elimination, by only warning about impure functions?
>
> Here is an update of the patch which does that. Regtesting now ...
>
> Cheers,
> Janus
Would not this break the testcase function_optimize_5.f90 :
write (unit=line, fmt='(4F7.2)') matmul(a,b) & ! { dg-warning
"Removing call to function 'matmul'" }
& + matmul(a,b)
z = sin(x) + 2.0 + sin(x) ! { dg-warning "Removing call to function 'sin'" }
print *,z
x = ext_func(a) + 23 + ext_func(a)
print *,d,x
z = element(x) + element(x) ! { dg-warning "Removing call to
function 'element'" }
print *,z
i = mypure(x) - mypure(x) ! { dg-warning "Removing call to function
'mypure'" }
The docs for -Wfunction-elimination would read:
> Warn if any calls to functions are eliminated by the optimizations
> enabled by the @option{-ffrontend-optimize} option.
> This option is implied by @option{-Wextra}.
However, with your patch, it should probably read something like "warn
if any calls to impure functions are eliminated..." Possibly with an
explicit remark indicating that pure functions are not warned.
Additionally:
$ ./contrib/check_GNU_style.sh pr85599_v6.diff
Lines should not exceed 80 characters.
33:+ if (e->expr_type == EXPR_FUNCTION && !gfc_pure_function (e,
&name) && !gfc_implicit_pure_function (e))
36:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function %qs at %L", name, &(e->where));
38:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function at %L", &(e->where));
201:+ if (f != last && !gfc_pure_function (f, &name) &&
!gfc_implicit_pure_function (f))
Blocks of 8 spaces should be replaced with tabs.
36:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function %qs at %L", name, &(e->where));
38:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function at %L", &(e->where));
230:+ with impure function as second operand. */
Dot, space, space, new sentence.
79:+expression, if they do not contribute to the final result. For logical
82:+result of the expression can be established without them. However, since not
Have you considered using levels for the flag, such that the original
behavior of -Wfunction-elimination would be retained with e.g.
-Wfunction-elimination=2 whereas one could use
-Wfunction-elimination=1 (or similar) to omit warnings about impure
functions?
- Fritz
[PATCH] fix a couple of bugs in const string folding (PR 86532)
My enhancement to extract constant strings out of complex
aggregates committed last week introduced a couple of bugs in
dealing with non-constant indices and offsets. One of the bugs
was fixed earlier today (PR 86528) but another one remains. It
causes strlen (among other functions) to incorrectly fold
expressions involving a non-constant index into an array of
strings by treating the index the same as a non-consatnt
offset into it.
The non-constant index should either prevent the folding, or it
needs to handle it differently from an offset.
The attached patch takes the conservative approach of avoiding
the folding in this case. The remaining changes deal with
the fallout from the fix.
Tested on x86_64-linux.
Martin
PR tree-optimization/86532 - Wrong code due to a wrong strlen folding starting with r262522
gcc/ChangeLog:
PR tree-optimization/86532
* builtins.c (c_strlen): Correct handling of non-constant offsets.
(check_access): Be prepared for non-constant length ranges.
* expr.c (string_constant): Only handle the minor non-constant
array index.
gcc/testsuite/ChangeLog:
PR tree-optimization/86532
* gcc.c-torture/execute/strlen-2.c: New test.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 262764)
+++ gcc/builtins.c (working copy)
@@ -517,7 +517,7 @@ get_pointer_alignment (tree exp)
return align;
}
-/* Return the number of non-zero elements in the sequence
+/* Return the number of leading non-zero elements in the sequence
[ PTR, PTR + MAXELTS ) where each element's size is ELTSIZE bytes.
ELTSIZE must be a power of 2 less than 8. Used by c_strlen. */
@@ -605,15 +605,22 @@ c_strlen (tree src, int only_value)
/* Set MAXELTS to sizeof (SRC) / sizeof (*SRC) - 1, the maximum possible
length of SRC. Prefer TYPE_SIZE() to TREE_STRING_LENGTH() if possible
- in case the latter is less than the size of the array. */
- HOST_WIDE_INT maxelts = TREE_STRING_LENGTH (src);
+ in case the latter is less than the size of the array, such as when
+ SRC refers to a short string literal used to initialize a large array.
+ In that case, the elements of the array after the terminating NUL are
+ all NUL. */
+ HOST_WIDE_INT strelts = TREE_STRING_LENGTH (src);
+ strelts = strelts / eltsize - 1;
+
+ HOST_WIDE_INT maxelts = strelts;
tree type = TREE_TYPE (src);
if (tree size = TYPE_SIZE_UNIT (type))
if (tree_fits_shwi_p (size))
- maxelts = tree_to_uhwi (size);
+ {
+ maxelts = tree_to_uhwi (size);
+ maxelts = maxelts / eltsize - 1;
+ }
- maxelts = maxelts / eltsize - 1;
-
/* PTR can point to the byte representation of any string type, including
char* and wchar_t*. */
const char *ptr = TREE_STRING_POINTER (src);
@@ -620,10 +627,12 @@ c_strlen (tree src, int only_value)
if (byteoff && TREE_CODE (byteoff) != INTEGER_CST)
{
- /* If the string has an internal zero byte (e.g., "foo\0bar"), we can't
- compute the offset to the following null if we don't know where to
+ /* If the string has an internal NUL character followed by any
+ non-NUL characters (e.g., "foo\0bar"), we can't compute
+ the offset to the following NUL if we don't know where to
start searching for it. */
- if (string_length (ptr, eltsize, maxelts) < maxelts)
+ unsigned len = string_length (ptr, eltsize, strelts);
+ if (len < strelts)
{
/* Return when an embedded null character is found. */
return NULL_TREE;
@@ -633,12 +642,18 @@ c_strlen (tree src, int only_value)
return ssize_int (0);
/* We don't know the starting offset, but we do know that the string
- has no internal zero bytes. We can assume that the offset falls
- within the bounds of the string; otherwise, the programmer deserves
- what he gets. Subtract the offset from the length of the string,
- and return that. This would perhaps not be valid if we were dealing
- with named arrays in addition to literal string constants. */
- return size_diffop_loc (loc, size_int (maxelts * eltsize), byteoff);
+ has no internal zero bytes. If the offset falls within the bounds
+ of the string subtract the offset from the length of the string,
+ and return that. Otherwise the length is zero. Take care to
+ use SAVE_EXPR in case the OFFSET has side-effects. */
+ tree offsave = fold_build1_loc (loc, SAVE_EXPR,
+ TREE_TYPE (byteoff), byteoff);
+ tree condexp = fold_build2_loc (loc, LE_EXPR, boolean_type_node, offsave,
+ build_int_cst (size_type_node,
+ len * eltsize));
+ tree lenexp = size_diffop_loc (loc, size_int (strelts * eltsize), offsave);
+ return fold_build3_loc (loc, COND_EXPR, size_type_node, condexp, lenexp,
+ build_zero_cst (size_type_node));
}
/* Offset from the beginning of the string in elements. */
@@ -3192,15 +3207,13 @@ check_access (tree exp, tree, tree, tree dstwrite,
if (dstw
Re: [PATCH, GCC, AARCH64] Add support for +profile extension
On Mon, Jul 09, 2018 at 08:20:53AM -0500, Andre Vieira (lists) wrote:
> Hi,
>
> This patch adds support for the Statistical Profiling Extension (SPE) on
> AArch64. Even though the compiler will not generate code any differently
> given this extension, it will need to pass it on to the assembler in
> order to let it correctly assemble inline asm containing accesses to the
> extension's system registers. The same applies when using the
> preprocessor on an assembly file as this first must pass through cc1.
>
> I left the hwcaps string for SPE empty as the kernel does not define a
> feature string for this extension. The current effect of this is that
> driver will disable profile feature bit in GCC. This is OK though
> because we don't, nor do we ever, enable this feature bit, as codegen is
> not affect by the SPE support and more importantly the driver will still
> pass the extension down to the assembler regardless.
Please make these conditions clear in the documentation. Something like.
> +@item profile
> +Enable the Statistical Profiling extension. This option only changes
> +the behavior of the assembler, and does not change code generation.
Maybe worded better...
>
> Boostrapped aarch64-none-linux-gnu and ran regression tests.
>
> Is it OK for trunk?
>
> gcc/ChangeLog:
> 2018-07-09 Andre Vieira
>
> * config/aarch64/aarch64-option-extensions.def: New entry for profile
> extension.
> * config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
> * doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
> extension.
>
> gcc/testsuite/ChangeLog:
> 2018-07-09 Andre Vieira
>
> * gcc.target/aarch64/profile.c: New test.
This test will fail for targets with old assemblers. That isn't ideal, we
don't normally add these assembler tests for new instructions for that
reason. Personally I'd drop the test down to a compile-only and scan the
assembler for "+profile".
OK with those changes.
Thanks,
James
> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def
> b/gcc/config/aarch64/aarch64-option-extensions.def
> index
> 5fe5e3f7dddf622a48a5b9458ef30449a886f395..69ab796a4e1a959b89ebb55b599919c442cfb088
> 100644
> --- a/gcc/config/aarch64/aarch64-option-extensions.def
> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
> @@ -105,4 +105,7 @@ AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML,
> AARCH64_FL_FP | AARCH64_FL_F
> Disabling "sve" just disables "sve". */
> AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD
> | AARCH64_FL_F16, 0, "sve")
>
> +/* Enabling/Disabling "profile" does not enable/disable any other feature.
> */
> +AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, "")
> +
> #undef AARCH64_OPT_EXTENSION
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index
> f284e74bfb8c9bab2aa22cc6c5a67750cbbba3c2..c1218503bab19323eee1cca8b7e4bea8fbfcf573
> 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -158,6 +158,9 @@ extern unsigned aarch64_architecture_version;
> #define AARCH64_FL_SHA3(1 << 18) /* Has ARMv8.4-a SHA3 and
> SHA512. */
> #define AARCH64_FL_F16FML (1 << 19) /* Has ARMv8.4-a FP16 extensions.
> */
>
> +/* Statistical Profiling extensions. */
> +#define AARCH64_FL_PROFILE(1 << 20)
> +
> /* Has FP and SIMD. */
> #define AARCH64_FL_FPSIMD (AARCH64_FL_FP | AARCH64_FL_SIMD)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index
> 56cd122b0d7b420e2b16ceb02907860879d3b9d7..4ca68a563297482afc75abed4a31c106af38caf7
> 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14813,6 +14813,8 @@ instructions. Use of this option with architectures
> prior to Armv8.2-A is not su
> @item sm4
> Enable the sm3 and sm4 crypto extension. This also enables Advanced SIMD
> instructions.
> Use of this option with architectures prior to Armv8.2-A is not supported.
> +@item profile
> +Enable the Statistical Profiling extension.
>
> @end table
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/profile.c
> b/gcc/testsuite/gcc.target/aarch64/profile.c
> new file mode 100644
> index
> ..db51b4746dd60009d784bc0b37ea99b2f120d856
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/profile.c
> @@ -0,0 +1,9 @@
> +/* { dg-do assemble } */
> +/* { dg-options "-std=gnu99 -march=armv8.2-a+profile" } */
> +
> +int foo (void)
> +{
> + int ret;
> + asm ("mrs %0, pmblimitr_el1" : "=r" (ret));
> + return ret;
> +}
C++ PATCH for c++/86480, nested variadic lambda and constexpr if
The problem here was that when looking for unexpanded packs, we looked
into IF_STMT_EXTRA_ARGS of a constexpr if, which includes the bindings
for function parameter packs, but not in a way that uses them. So we
shouldn't look there for unexpanded packs.
Trying to write a smaller testcase revealed another bug, whereby
collecting the bindings for captured variables wasn't respecting
unevaluated context.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit c63bbedf903c28225d9cc273c14d3023e6074fa5
Author: Jason Merrill
Date: Fri Jul 13 23:22:26 2018 +1000
PR c++/86480 - nested variadic lambda and constexpr if.
* pt.c (find_parameter_packs_r) [IF_STMT]: Don't walk into
IF_STMT_EXTRA_ARGS.
* tree.c (cp_walk_subtrees) [DECLTYPE_TYPE]: Set
cp_unevaluated_operand.
[ALIGNOF_EXPR] [SIZEOF_EXPR] [NOEXCEPT_EXPR]: Likewise.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7e858a19d76..2b885793d3f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3865,6 +3865,17 @@ find_parameter_packs_r (tree *tp, int *walk_subtrees, void* data)
return NULL_TREE;
}
+case IF_STMT:
+ cp_walk_tree (&IF_COND (t), &find_parameter_packs_r,
+ ppd, ppd->visited);
+ cp_walk_tree (&THEN_CLAUSE (t), &find_parameter_packs_r,
+ ppd, ppd->visited);
+ cp_walk_tree (&ELSE_CLAUSE (t), &find_parameter_packs_r,
+ ppd, ppd->visited);
+ /* Don't walk into IF_STMT_EXTRA_ARGS. */
+ *walk_subtrees = 0;
+ return NULL_TREE;
+
default:
return NULL_TREE;
}
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index b1333f55e39..7e2a77bf67b 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4865,7 +4865,19 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
break;
case DECLTYPE_TYPE:
- WALK_SUBTREE (DECLTYPE_TYPE_EXPR (*tp));
+ ++cp_unevaluated_operand;
+ /* We can't use WALK_SUBTREE here because of the goto. */
+ result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (*tp), func, data, pset);
+ --cp_unevaluated_operand;
+ *walk_subtrees_p = 0;
+ break;
+
+case ALIGNOF_EXPR:
+case SIZEOF_EXPR:
+case NOEXCEPT_EXPR:
+ ++cp_unevaluated_operand;
+ result = cp_walk_tree (&TREE_OPERAND (*tp, 0), func, data, pset);
+ --cp_unevaluated_operand;
*walk_subtrees_p = 0;
break;
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if24.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if24.C
new file mode 100644
index 000..cbdb38d95c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if24.C
@@ -0,0 +1,21 @@
+// PR c++/86480
+// { dg-additional-options -std=c++17 }
+
+template constexpr bool val = true;
+
+template
+void f()
+{
+ [](auto... p)
+{
+ []{
+ if constexpr (val) { return true; }
+ return false;
+ }();
+}(42);
+}
+
+int main()
+{
+ f();
+}
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi Kyrill, The current implementation expands to: mvar = a1; if (a2 .op. mvar || isnan (mvar)) mvar = a2; if (a3 .op. mvar || isnan (mvar)) mvar = a3; ... return mvar; That is, if one of the operands is a NaN it will return the other argument. If both (all) are NaNs, it will return NaN. This is the same as the semantics of fmin/max as far as I can tell. I've looked at the F2008 standard, and, interestingly enough, the requirement on MIN and MAX do not mention NaNs at all. 13.7.106 has, for MAX, Result Value. The value of the result is that of the largest argument. plus some stuff about character variables (not relevant here). Similar for MIN. Also, the section on IEEE_ARITHMETIC (14.9) does not mention comparisons; also, "Complete conformance with IEC 60559:1989 is not required", what is required is the correct support for +,-, and *, plus support for / if IEEE_SUPPORT_DIVIDE is covered. So, the Fortran standard does not impose many requirements. I do think that a patch such as yours should not change the current behavior unless we know what it does and do think it is a good idea. Hmm... Having said that, I think we pretty much cover all the corner cases in nan_1.f90, so if that test passes without regression, then that aspect should be fine. Question: You have found an advantage on Aarm64. Do you have access to other architectures so see if there is also a speed advantage, or maybe a disadvantage? Regards Thomas
Re: [PATCH] fix a couple of bugs in const string folding (PR 86532)
On 07/17/2018 09:19 AM, Martin Sebor wrote:
> My enhancement to extract constant strings out of complex
> aggregates committed last week introduced a couple of bugs in
> dealing with non-constant indices and offsets. One of the bugs
> was fixed earlier today (PR 86528) but another one remains. It
> causes strlen (among other functions) to incorrectly fold
> expressions involving a non-constant index into an array of
> strings by treating the index the same as a non-consatnt
> offset into it.
>
> The non-constant index should either prevent the folding, or it
> needs to handle it differently from an offset.
>
> The attached patch takes the conservative approach of avoiding
> the folding in this case. The remaining changes deal with
> the fallout from the fix.
>
> Tested on x86_64-linux.
>
> Martin
>
>
> gcc-86532.diff
>
>
> PR tree-optimization/86532 - Wrong code due to a wrong strlen folding
> starting with r262522
>
> gcc/ChangeLog:
>
> PR tree-optimization/86532
> * builtins.c (c_strlen): Correct handling of non-constant offsets.
> (check_access): Be prepared for non-constant length ranges.
> * expr.c (string_constant): Only handle the minor non-constant
> array index.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/86532
> * gcc.c-torture/execute/strlen-2.c: New test.
[ ... ]
> Index: gcc/expr.c
> ===
> --- gcc/expr.c(revision 262764)
> +++ gcc/expr.c(working copy)
> @@ -11343,16 +11356,15 @@ string_constant (tree arg, tree *ptr_offset)
> {
>if (TREE_CODE (TREE_TYPE (array)) != ARRAY_TYPE)
> return NULL_TREE;
> - if (tree eltsize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (array
> - {
> - /* Add the scaled variable index to the constant offset. */
> - tree eltoff = fold_build2 (MULT_EXPR, TREE_TYPE (offset),
> - fold_convert (sizetype, varidx),
> - eltsize);
> - offset = fold_build2 (PLUS_EXPR, TREE_TYPE (offset), offset, eltoff);
> - }
> - else
> - return NULL_TREE;
> +
> + while (TREE_CODE (chartype) != INTEGER_TYPE)
> + chartype = TREE_TYPE (chartype);
This is a bit concerning. First under what conditions is chartype not
going to be an INTEGER_TYPE? And under what conditions will extracting
its type ultimately lead to something that is an INTEGER_TYPE?
The rest looks pretty reasonable.
Jeff
Re: [PATCH] Fix error level of warning about C++ style comments
Jakub,
Thanks so much for the review! Below is an updated ChangeLog entry and
patch.
2018-07-17 Jason Franklin
* lex.c (_cpp_lex_direct): Fix error level for note following
warning about the use of C++ style comments.
>From 1380d73031d76be76019e67f203cb7d98fb5ee60 Mon Sep 17 00:00:00 2001
From: Jason Franklin
Date: Tue, 17 Jul 2018 11:36:39 -0400
Subject: [PATCH] Fix error level of warning about C++ comments
---
libcpp/lex.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/libcpp/lex.c b/libcpp/lex.c
index 37c365a3560..1bf89b8da7a 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -2872,10 +2872,10 @@ _cpp_lex_direct (cpp_reader *pfile)
&& CPP_PEDANTIC (pfile)
&& ! buffer->warned_cplusplus_comments)
{
- cpp_error (pfile, CPP_DL_PEDWARN,
-"C++ style comments are not allowed in ISO C90");
- cpp_error (pfile, CPP_DL_PEDWARN,
-"(this will be reported only once per input file)");
+ if (cpp_error (pfile, CPP_DL_PEDWARN,
+"C++ style comments are not allowed in ISO C90"))
+cpp_error (pfile, CPP_DL_NOTE,
+ "(this will be reported only once per input file)");
buffer->warned_cplusplus_comments = 1;
}
/* Or if specifically desired via -Wc90-c99-compat. */
@@ -2883,10 +2883,10 @@ _cpp_lex_direct (cpp_reader *pfile)
&& ! CPP_OPTION (pfile, cplusplus)
&& ! buffer->warned_cplusplus_comments)
{
- cpp_error (pfile, CPP_DL_WARNING,
-"C++ style comments are incompatible with C90");
- cpp_error (pfile, CPP_DL_WARNING,
-"(this will be reported only once per input file)");
+ if (cpp_error (pfile, CPP_DL_WARNING,
+"C++ style comments are incompatible with C90"))
+cpp_error (pfile, CPP_DL_NOTE,
+ "(this will be reported only once per input file)");
buffer->warned_cplusplus_comments = 1;
}
/* In C89/C94, C++ style comments are forbidden. */
@@ -2906,11 +2906,11 @@ _cpp_lex_direct (cpp_reader *pfile)
}
else if (! buffer->warned_cplusplus_comments)
{
- cpp_error (pfile, CPP_DL_ERROR,
-"C++ style comments are not allowed in ISO C90");
- cpp_error (pfile, CPP_DL_ERROR,
-"(this will be reported only once per input "
-"file)");
+ if (cpp_error (pfile, CPP_DL_ERROR,
+"C++ style comments are not allowed in ISO C90"))
+cpp_error (pfile, CPP_DL_NOTE,
+ "(this will be reported only once per input "
+ "file)");
buffer->warned_cplusplus_comments = 1;
}
}
--
2.17.1
[PATCH, testsuite] Restrict some PPC tests
Two of the powerpc testcases have additional requirements. Fixed thus.
Thanks, David
* gcc.target/powerpc/pr57150.c: Require longdouble128.
* gcc.target/powerpc/pr79916.c: Require dfp.
Index: pr79916.c
===
--- pr79916.c (revision 262824)
+++ pr79916.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target dfp } */
/* { dg-options "-fno-expensive-optimizations --param
ira-max-conflict-table-size=0 -mno-popcntd -O3" } */
#define PASTE2(A,B) A ## B
Index: pr57150.c
===
--- pr57150.c (revision 262824)
+++ pr57150.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-require-effective-target longdouble128 } */
/* { dg-require-effective-target powerpc_vsx_ok } */
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*"
} { "-mcpu=power7" } } */
/* { dg-options "-O3 -mcpu=power7 -fcaller-saves" } */
Re: [GCC][PATCH][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks
On 07/17/2018 06:33 AM, Richard Earnshaw (lists) wrote: >> (define_predicate "pow2m1_operand" >> (and (match_code "const_int") >>(match_test "exact_pow2 (INTVAL(op) + 1) > 0"))) > ITYM exact_log2() Yes of course. Thanks, r~
Re: [PATCH] fix a couple of bugs in const string folding (PR 86532)
On 07/17/2018 09:38 AM, Jeff Law wrote:
On 07/17/2018 09:19 AM, Martin Sebor wrote:
My enhancement to extract constant strings out of complex
aggregates committed last week introduced a couple of bugs in
dealing with non-constant indices and offsets. One of the bugs
was fixed earlier today (PR 86528) but another one remains. It
causes strlen (among other functions) to incorrectly fold
expressions involving a non-constant index into an array of
strings by treating the index the same as a non-consatnt
offset into it.
The non-constant index should either prevent the folding, or it
needs to handle it differently from an offset.
The attached patch takes the conservative approach of avoiding
the folding in this case. The remaining changes deal with
the fallout from the fix.
Tested on x86_64-linux.
Martin
gcc-86532.diff
PR tree-optimization/86532 - Wrong code due to a wrong strlen folding starting
with r262522
gcc/ChangeLog:
PR tree-optimization/86532
* builtins.c (c_strlen): Correct handling of non-constant offsets.
(check_access): Be prepared for non-constant length ranges.
* expr.c (string_constant): Only handle the minor non-constant
array index.
gcc/testsuite/ChangeLog:
PR tree-optimization/86532
* gcc.c-torture/execute/strlen-2.c: New test.
[ ... ]
Index: gcc/expr.c
===
--- gcc/expr.c (revision 262764)
+++ gcc/expr.c (working copy)
@@ -11343,16 +11356,15 @@ string_constant (tree arg, tree *ptr_offset)
{
if (TREE_CODE (TREE_TYPE (array)) != ARRAY_TYPE)
return NULL_TREE;
- if (tree eltsize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (array
- {
- /* Add the scaled variable index to the constant offset. */
- tree eltoff = fold_build2 (MULT_EXPR, TREE_TYPE (offset),
-fold_convert (sizetype, varidx),
-eltsize);
- offset = fold_build2 (PLUS_EXPR, TREE_TYPE (offset), offset, eltoff);
- }
- else
- return NULL_TREE;
+
+ while (TREE_CODE (chartype) != INTEGER_TYPE)
+ chartype = TREE_TYPE (chartype);
This is a bit concerning. First under what conditions is chartype not
going to be an INTEGER_TYPE? And under what conditions will extracting
its type ultimately lead to something that is an INTEGER_TYPE?
chartype is usually (maybe even always) pointer type here:
const char a[] = "123";
extern int i;
n = strlen (&a[i]);
Martin
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi Thomas, On 17/07/18 16:36, Thomas Koenig wrote: Hi Kyrill, The current implementation expands to: mvar = a1; if (a2 .op. mvar || isnan (mvar)) mvar = a2; if (a3 .op. mvar || isnan (mvar)) mvar = a3; ... return mvar; That is, if one of the operands is a NaN it will return the other argument. If both (all) are NaNs, it will return NaN. This is the same as the semantics of fmin/max as far as I can tell. I've looked at the F2008 standard, and, interestingly enough, the requirement on MIN and MAX do not mention NaNs at all. 13.7.106 has, for MAX, Result Value. The value of the result is that of the largest argument. plus some stuff about character variables (not relevant here). Similar for MIN. Also, the section on IEEE_ARITHMETIC (14.9) does not mention comparisons; also, "Complete conformance with IEC 60559:1989 is not required", what is required is the correct support for +,-, and *, plus support for / if IEEE_SUPPORT_DIVIDE is covered. Thanks for checking this. So, the Fortran standard does not impose many requirements. I do think that a patch such as yours should not change the current behavior unless we know what it does and do think it is a good idea. Hmm... Having said that, I think we pretty much cover all the corner cases in nan_1.f90, so if that test passes without regression, then that aspect should be fine. Looking at the test it looks like there is a de facto expected behaviour. For example it contains: if (max(2.d0, nan) /= 2.d0) STOP 9 So it definitely expects comparison with NaN to return the non-NaN result, which is a the behaviour what my patch preserves. On integral arguments or when we don't care about NaNs (-Ofast and such) we'll be using the MIN/MAX_EXPR, which doesn't specify what's returned on a NaN argument, thus allowing for more aggressive optimisations. Question: You have found an advantage on Aarm64. Do you have access to other architectures so see if there is also a speed advantage, or maybe a disadvantage? Because the expansion now emits straightline code rather than conditionals and branches it should be easier to optimise in general, so I'd expect this to be an improvement overall. That said, I have benchmarked it on SPEC2017 on aarch64. If you have any benchmarks of interest to you you (or somebody else) can run on a target that you care about I would be very grateful for any results. Thanks, Kyrill Regards Thomas
Re: [PATCH, PR86257, i386/debug] Fix insn prefix in tls_global_dynamic_64_
On 06/25/2018 03:02 PM, Tom de Vries wrote:
> On 06/25/2018 02:45 PM, Nathan Sidwell wrote:
>> On 06/25/2018 08:25 AM, Tom de Vries wrote:
>>
>>> If we'd implemented something like this in gas:
>>> ...
>>> .insn
>>> .byte 0x66
>>> .endinsn
>>> ...
>>> we could fix this more generically.
>>
>> Doesn't arm gas provide this functionality with some target-specific
>> pseudo? It'd be good to copy that.
>>
>> ah, inst:
>>
>> @cindex @code{.inst} directive, ARM
>> @item .inst @var{opcode} [ , @dots{} ]
>> @itemx .inst.n @var{opcode} [ , @dots{} ]
>> @itemx .inst.w @var{opcode} [ , @dots{} ]
>> Generates the instruction corresponding to the numerical value
>> @var{opcode}.
>> @code{.inst.n} and @code{.inst.w} allow the Thumb instruction size to be
>> specified explicitly, overriding the normal encoding rules.
>>
>>
>> (ARM needs to distinguish data from insns to emit the mapping symbols
>> needed for be8 endianness xforms).
>>
>
> Hmm, thanks for pointing that out. There's also something similar for
> s390 and riscv.
>
> For mips there's another form of .insn (
> https://sourceware.org/binutils/docs/as/MIPS-insn.html#index-_002einsn
> ), which is similar to what I was proposing:
> ...
> 9.27.8 Directive to mark data as an instruction
>
> The .insn directive tells as that the following data is actually
> instructions. This makes a difference in MIPS 16 and microMIPS modes:
> when loading the address of a label which precedes instructions, as
> automatically adds 1 to the value, so that jumping to the loaded address
> will do the right thing.
> ...
>
Filed as PR23423 - "generic directive to mark data as insn" (
https://sourceware.org/bugzilla/show_bug.cgi?id=23423 ).
Thanks,
- Tom
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
2018-07-17 17:18 GMT+02:00 Fritz Reese :
>> 2018-07-17 9:52 GMT+02:00 Janus Weil :
>> > In other words: Does it make sense to tone down
>> > -Wfunction-elimination, by only warning about impure functions?
>>
>> Here is an update of the patch which does that. Regtesting now ...
>
> Would not this break the testcase function_optimize_5.f90 :
My regtest says so as well ;)
> The docs for -Wfunction-elimination would read:
>
>> Warn if any calls to functions are eliminated by the optimizations
>> enabled by the @option{-ffrontend-optimize} option.
>> This option is implied by @option{-Wextra}.
>
> However, with your patch, it should probably read something like "warn
> if any calls to impure functions are eliminated..." Possibly with an
> explicit remark indicating that pure functions are not warned.
Yes.
However, the test case above seems to indicate that the
function-elimination optimization is not applied to impure functions
anyway (which is good IMHO). It that is true, then my modifications
practically disable the old -Wfunction-elimination warnings completely
:/
> Have you considered using levels for the flag, such that the original
> behavior of -Wfunction-elimination would be retained with e.g.
> -Wfunction-elimination=2 whereas one could use
> -Wfunction-elimination=1 (or similar) to omit warnings about impure
> functions?
One could do that, but IMHO it would be overkill. I don't see much
sense in warning for the elimination of pure functions. But maybe I'm
missing something?
Cheers,
Janus
Go patch committed: Connect the concrete type and the placeholder for circular types
This patch by Cherry Zhang changes the Go frontend to more clearly
connect the concrete type and the placeholder for circular types.
Previously, when creating the backend representation of a circular
type, we resolved the placeholder to a circular_pointer_type. The
backend didn't know what the concrete type would be. This patch
changes the frontend to resolve the placeholder to the concrete type
instead, so the backend can have better knowledge of the concrete
type. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 262658)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-db991403fc97854201b3f40492f4f6b9d471cabc
+d6338c94e5574b63469c740159d064e89c6718bf
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc (revision 262658)
+++ gcc/go/gofrontend/types.cc (working copy)
@@ -10786,15 +10786,10 @@ Named_type::do_get_backend(Gogo* gogo)
// Don't build a circular data structure. GENERIC can't handle
// it.
if (this->seen_in_get_backend_)
- {
- this->is_circular_ = true;
- return gogo->backend()->circular_pointer_type(bt, true);
- }
+return gogo->backend()->circular_pointer_type(bt, true);
this->seen_in_get_backend_ = true;
bt1 = Type::get_named_base_btype(gogo, base);
this->seen_in_get_backend_ = false;
- if (this->is_circular_)
- bt1 = gogo->backend()->circular_pointer_type(bt, true);
if (!gogo->backend()->set_placeholder_pointer_type(bt, bt1))
bt = gogo->backend()->error_type();
return bt;
@@ -10803,15 +10798,10 @@ Named_type::do_get_backend(Gogo* gogo)
// Don't build a circular data structure. GENERIC can't handle
// it.
if (this->seen_in_get_backend_)
- {
- this->is_circular_ = true;
- return gogo->backend()->circular_pointer_type(bt, false);
- }
+return gogo->backend()->circular_pointer_type(bt, false);
this->seen_in_get_backend_ = true;
bt1 = Type::get_named_base_btype(gogo, base);
this->seen_in_get_backend_ = false;
- if (this->is_circular_)
- bt1 = gogo->backend()->circular_pointer_type(bt, false);
if (!gogo->backend()->set_placeholder_pointer_type(bt, bt1))
bt = gogo->backend()->error_type();
return bt;
Index: gcc/go/gofrontend/types.h
===
--- gcc/go/gofrontend/types.h (revision 262658)
+++ gcc/go/gofrontend/types.h (working copy)
@@ -3243,8 +3243,8 @@ class Named_type : public Type
interface_method_tables_(NULL), pointer_interface_method_tables_(NULL),
location_(location), named_btype_(NULL), dependencies_(),
is_alias_(false), is_visible_(true), is_error_(false), in_heap_(true),
- is_placeholder_(false), is_converted_(false), is_circular_(false),
- is_verified_(false), seen_(false), seen_in_compare_is_identity_(false),
+ is_placeholder_(false), is_converted_(false), is_verified_(false),
+ seen_(false), seen_in_compare_is_identity_(false),
seen_in_get_backend_(false), seen_alias_(false)
{ }
@@ -3345,12 +3345,6 @@ class Named_type : public Type
is_valid() const
{ return !this->is_error_; }
- // Whether this is a circular type: a pointer or function type that
- // refers to itself, which is not possible in C.
- bool
- is_circular() const
- { return this->is_circular_; }
-
// Return the base type for this type.
Type*
named_base();
@@ -3557,9 +3551,6 @@ class Named_type : public Type
// Whether this type has been converted to the backend
// representation. Implies that is_placeholder_ is false.
bool is_converted_;
- // Whether this is a pointer or function type which refers to the
- // type itself.
- bool is_circular_;
// Whether this type has been verified.
bool is_verified_;
// In a recursive operation such as has_pointer, this flag is used
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
Am 17.07.2018 um 19:19 schrieb Janus Weil:
2018-07-17 17:18 GMT+02:00 Fritz Reese :
2018-07-17 9:52 GMT+02:00 Janus Weil :
In other words: Does it make sense to tone down
-Wfunction-elimination, by only warning about impure functions?
Here is an update of the patch which does that. Regtesting now ...
Would not this break the testcase function_optimize_5.f90 :
My regtest says so as well ;)
The docs for -Wfunction-elimination would read:
Warn if any calls to functions are eliminated by the optimizations
enabled by the @option{-ffrontend-optimize} option.
This option is implied by @option{-Wextra}.
However, with your patch, it should probably read something like "warn
if any calls to impure functions are eliminated..." Possibly with an
explicit remark indicating that pure functions are not warned.
Yes.
However, the test case above seems to indicate that the
function-elimination optimization is not applied to impure functions
anyway (which is good IMHO).
If you specify -faggressive-function-elimination, it is also
done for impure (and non implicitly-pure) functions.
Problem is that, in all probability, nobody uses this option at the
moment.
It that is true, then my modifications
practically disable the old -Wfunction-elimination warnings completely
:/
I do not think it would be a problem not to warn for removing
calls to pure or implicitly pure fuctions. The test cases can
easily be modified not to emit this warning, as you did.
As the author of the original test cases, I may be able to
say so with a certain amount of credibility.
The actual elimination is checked for by counting the
function names in the *.original dump file, which is done.
[PATCH, testsuite] PPC testsuite pr85456.c
And another test that needs to be restricted because it assumes 128
bit long double.
Thanks, David
* gcc.target/powerpc/pr85456.c: Require longdouble128.
Index: pr85456.c
===
--- pr85456.c (revision 262824)
+++ pr85456.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */
/* Check that the __builtin_powil generates a call to the correct function
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi Kyrill, Because the expansion now emits straightline code rather than conditionals and branches it should be easier to optimise in general, so I'd expect this to be an improvement overall. That said, I have benchmarked it on SPEC2017 on aarch64. If you have any benchmarks of interest to you you (or somebody else) can run on a target that you care about I would be very grateful for any results. Well, most people currently use x86_64 for scientific computing, so I would be concerned most about this architecture. As for the test case, min / max performance clearly has an effect on 521.wrf, so this would be ideal. If you could run 521.wrf on x86_64, and find that it does not regress measureably (or even shows an improvement), the patch is OK. I'd be interested in the timings you get. Regards Thomas
Re: [PATCH] Fix error level of warning about C++ style comments
On Tue, Jul 17, 2018 at 08:39:30AM -0700, Jason Franklin wrote:
> Thanks so much for the review! Below is an updated ChangeLog entry and
> patch.
Thanks.
I've fixed some formatting issues and testsuite (3 tests started FAILing),
plus added one more test, bootstrapped/regtested on x86_64-linux and
i686-linux and committed to trunk:
2018-07-17 Jason Franklin
Jakub Jelinek
* lex.c (_cpp_lex_direct): Use CPP_DL_NOTE instead of CPP_DL_PEDWARN,
CPP_DL_WARNING or CPP_DL_ERROR for note that diagnostics for C++ style
comments is reported only once per file and guard those calls on the
preceding cpp_error returning true.
* gcc.dg/cpp/pr61854-c90.c (foo): Expect a note, rather than error.
* gcc.dg/cpp/pr61854-c94.c (foo): Likewise.
* gcc.dg/cpp/pr61854-4.c (foo): Likewise.
* gcc.dg/cpp/pr61854-8.c: New test.
--- libcpp/lex.c.jj 2018-02-28 18:13:38.118386495 +0100
+++ libcpp/lex.c2018-07-17 17:43:55.388843136 +0200
@@ -2872,10 +2872,10 @@ _cpp_lex_direct (cpp_reader *pfile)
&& CPP_PEDANTIC (pfile)
&& ! buffer->warned_cplusplus_comments)
{
- cpp_error (pfile, CPP_DL_PEDWARN,
-"C++ style comments are not allowed in ISO C90");
- cpp_error (pfile, CPP_DL_PEDWARN,
-"(this will be reported only once per input file)");
+ if (cpp_error (pfile, CPP_DL_PEDWARN,
+"C++ style comments are not allowed in ISO C90"))
+ cpp_error (pfile, CPP_DL_NOTE,
+ "(this will be reported only once per input file)");
buffer->warned_cplusplus_comments = 1;
}
/* Or if specifically desired via -Wc90-c99-compat. */
@@ -2883,10 +2883,10 @@ _cpp_lex_direct (cpp_reader *pfile)
&& ! CPP_OPTION (pfile, cplusplus)
&& ! buffer->warned_cplusplus_comments)
{
- cpp_error (pfile, CPP_DL_WARNING,
-"C++ style comments are incompatible with C90");
- cpp_error (pfile, CPP_DL_WARNING,
-"(this will be reported only once per input file)");
+ if (cpp_error (pfile, CPP_DL_WARNING,
+"C++ style comments are incompatible with C90"))
+ cpp_error (pfile, CPP_DL_NOTE,
+ "(this will be reported only once per input file)");
buffer->warned_cplusplus_comments = 1;
}
/* In C89/C94, C++ style comments are forbidden. */
@@ -2906,11 +2906,12 @@ _cpp_lex_direct (cpp_reader *pfile)
}
else if (! buffer->warned_cplusplus_comments)
{
- cpp_error (pfile, CPP_DL_ERROR,
-"C++ style comments are not allowed in ISO C90");
- cpp_error (pfile, CPP_DL_ERROR,
-"(this will be reported only once per input "
-"file)");
+ if (cpp_error (pfile, CPP_DL_ERROR,
+"C++ style comments are not allowed in "
+"ISO C90"))
+ cpp_error (pfile, CPP_DL_NOTE,
+ "(this will be reported only once per input "
+ "file)");
buffer->warned_cplusplus_comments = 1;
}
}
--- gcc/testsuite/gcc.dg/cpp/pr61854-c90.c.jj 2017-04-19 15:46:24.412008934
+0200
+++ gcc/testsuite/gcc.dg/cpp/pr61854-c90.c 2018-07-17 17:50:05.458331441
+0200
@@ -7,7 +7,7 @@ foo (void)
{
// 1st
/* { dg-error "C\\+\\+ style comments are not allowed in ISO C90" "comments"
{ target *-*-*} .-1 } */
- /* { dg-error "reported only once" "" { target *-*-*} .-2 } */
+ /* { dg-message "note: \[^\n\r]*reported only once" "" { target *-*-*} .-2
} */
// 2nd
// 3rd
}
--- gcc/testsuite/gcc.dg/cpp/pr61854-c94.c.jj 2017-04-19 15:46:25.265997760
+0200
+++ gcc/testsuite/gcc.dg/cpp/pr61854-c94.c 2018-07-17 17:50:22.670354143
+0200
@@ -7,7 +7,7 @@ foo (void)
{
// 1st
/* { dg-error "C\\+\\+ style comments are not allowed in ISO C90" "comments"
{ target *-*-*} .-1 } */
- /* { dg-error "reported only once" "" { target *-*-*} .-2 } */
+ /* { dg-message "note: \[^\n\r]*reported only once" "" { target *-*-*} .-2
} */
// 2nd
// 3rd
}
--- gcc/testsuite/gcc.dg/cpp/pr61854-4.c.jj 2014-09-18 15:48:24.297978857
+0200
+++ gcc/testsuite/gcc.dg/cpp/pr61854-4.c2018-07-17 17:49:37.289294264
+0200
@@ -12,5 +12,5 @@ foo (void)
// But error here.
#endif
/* { dg-error "C\\+\\+ style comments are not allowed in ISO C90" "comments"
{ target *-*-*} 12 } */
- /* { dg-error "reported only once" "" { target *-*-*} 12 } */
+ /* { dg-message "note: \[^\n\r]*reported only once" "" { target *-*-*} 12 }
Go patch committed: Don't set btype_ field too early for an alias type
This patch by Cherry Zhang fixes a problem in the Go frontend
introduced by https://golang.org/cl/123362. A type's btype_ field
should not be set before named types are converted if it is a
placeholder. For alias type, it iwas set too early. That could
result in unresolved placeholders. This patch fixes the problem.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 262830)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-d6338c94e5574b63469c740159d064e89c6718bf
+38850073f25f9de4f3daa33d799def3a33c942ab
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc (revision 262830)
+++ gcc/go/gofrontend/types.cc (working copy)
@@ -992,8 +992,10 @@ Type::get_backend(Gogo* gogo)
return this->btype_;
if (this->named_type() != NULL && this->named_type()->is_alias()) {
-this->btype_ = this->unalias()->get_backend(gogo);
-return this->btype_;
+Btype* bt = this->unalias()->get_backend(gogo);
+if (gogo != NULL && gogo->named_types_are_converted())
+ this->btype_ = bt;
+return bt;
}
if (this->forward_declaration_type() != NULL
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
2018-07-17 19:34 GMT+02:00 Thomas Koenig :
> Am 17.07.2018 um 19:19 schrieb Janus Weil:
>> However, the test case above seems to indicate that the
>> function-elimination optimization is not applied to impure functions
>> anyway (which is good IMHO).
>
> If you specify -faggressive-function-elimination, it is also
> done for impure (and non implicitly-pure) functions.
Ah, ok.
> Problem is that, in all probability, nobody uses this option at the
> moment.
That's probably true, but it should get a little better once we move
it into -Wextra.
>> It that is true, then my modifications
>> practically disable the old -Wfunction-elimination warnings completely
>> :/
>
> I do not think it would be a problem not to warn for removing
> calls to pure or implicitly pure fuctions. The test cases can
> easily be modified not to emit this warning, as you did.
>
> As the author of the original test cases, I may be able to
> say so with a certain amount of credibility.
Good, so let's do this. Attached is another update of my patch, which
incorporates all of Fritz' comments and should regtest cleanly now.
In function_optimize_5.f90 I have removed the warnings for pure
functions and instead added -faggressive-function-elimination and the
corresponding warnings for impure functions, which were apparently not
covered by the testsuite before.
I do hope that things have converged by now and that this will be the
last incarnation of the patch. If there is no more feedback in the
next 24 hours, I'll commit this tomorrow.
Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c (revision 262828)
+++ gcc/fortran/dump-parse-tree.c (working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
fputs (" ELEMENTAL", dumpfile);
if (attr->pure)
fputs (" PURE", dumpfile);
+ if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
if (attr->recursive)
fputs (" RECURSIVE", dumpfile);
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c (revision 262828)
+++ gcc/fortran/frontend-passes.c (working copy)
@@ -840,17 +840,22 @@ create_var (gfc_expr * e, const char *vname)
static void
do_warn_function_elimination (gfc_expr *e)
{
- if (e->expr_type != EXPR_FUNCTION)
-return;
- if (e->value.function.esym)
-gfc_warning (OPT_Wfunction_elimination,
- "Removing call to function %qs at %L",
- e->value.function.esym->name, &(e->where));
- else if (e->value.function.isym)
-gfc_warning (OPT_Wfunction_elimination,
- "Removing call to function %qs at %L",
- e->value.function.isym->name, &(e->where));
+ const char *name;
+ if (e->expr_type == EXPR_FUNCTION
+ && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e))
+ {
+ if (name)
+ gfc_warning (OPT_Wfunction_elimination,
+ "Removing call to impure function %qs at %L", name,
+ &(e->where));
+ else
+ gfc_warning (OPT_Wfunction_elimination,
+ "Removing call to impure function at %L",
+ &(e->where));
+ }
}
+
+
/* Callback function for the code walker for doing common function
elimination. This builds up the list of functions in the expression
and goes through them to detect duplicates, which it then replaces
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h (revision 262828)
+++ gcc/fortran/gfortran.h (working copy)
@@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *)
bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
extern int gfc_do_concurrent_flag;
const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
+int gfc_pure_function (gfc_expr *e, const char **name);
+int gfc_implicit_pure_function (gfc_expr *e);
/* array.c */
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi (revision 262828)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
@menu
* KIND Type Parameters::
* Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
* Thread-safety of the runtime library::
* Data consistency and durability::
* Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expression, if they do not contribute to the final result. For logical
+expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU
+Fortran will optimize out func
Fix invalid cc_status after [const_][us]mulsi3_highpart
The mulu.l insn sets the CC according to the 64-bit result, but we are
only using the high part, so the Z flag cannot be used. The other flags
would still be valid, but we have no cc_status flag for that case.
* config/m68k/m68k.md (umulsi3_highpart+1, const_umulsi3_highpart)
(smulsi3_highpart+1, const_smulsi3_highpart): Add CC_STATUS_INIT.
testsuite/:
* gcc.target/m68k/mulsi_highpart.c: New test.
diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
index f8a674d7d9..74c366fc63 100644
--- a/gcc/config/m68k/m68k.md
+++ b/gcc/config/m68k/m68k.md
@@ -3240,7 +3240,10 @@
(const_int 32
(clobber (match_operand:SI 1 "register_operand" "=d"))]
"TARGET_68020 && !TUNE_68060 && !TARGET_COLDFIRE"
- "mulu%.l %3,%0:%1")
+{
+ CC_STATUS_INIT;
+ return "mulu%.l %3,%0:%1";
+})
(define_insn "const_umulsi3_highpart"
[(set (match_operand:SI 0 "register_operand" "=d")
@@ -3251,7 +3254,10 @@
(const_int 32
(clobber (match_operand:SI 1 "register_operand" "=d"))]
"TARGET_68020 && !TUNE_68060 && !TARGET_COLDFIRE"
- "mulu%.l %3,%0:%1")
+{
+ CC_STATUS_INIT;
+ return "mulu%.l %3,%0:%1";
+})
(define_expand "smulsi3_highpart"
[(parallel
@@ -3283,7 +3289,10 @@
(const_int 32
(clobber (match_operand:SI 1 "register_operand" "=d"))]
"TARGET_68020 && !TUNE_68060 && !TARGET_COLDFIRE"
- "muls%.l %3,%0:%1")
+{
+ CC_STATUS_INIT;
+ return "muls%.l %3,%0:%1";
+})
(define_insn "const_smulsi3_highpart"
[(set (match_operand:SI 0 "register_operand" "=d")
@@ -3294,7 +3303,10 @@
(const_int 32
(clobber (match_operand:SI 1 "register_operand" "=d"))]
"TARGET_68020 && !TUNE_68060 && !TARGET_COLDFIRE"
- "muls%.l %3,%0:%1")
+{
+ CC_STATUS_INIT;
+ return "muls%.l %3,%0:%1";
+})
(define_expand "mul3"
[(set (match_operand:FP 0 "nonimmediate_operand" "")
diff --git a/gcc/testsuite/gcc.target/m68k/mulsi_highpart.c
b/gcc/testsuite/gcc.target/m68k/mulsi_highpart.c
new file mode 100644
index 00..f17586ee52
--- /dev/null
+++ b/gcc/testsuite/gcc.target/m68k/mulsi_highpart.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -m68020" } */
+/* Don't optimize away a compare after [us]mulsi_highpart. */
+/* { dg-final { scan-assembler {tst\.?l} } } */
+int cmp (unsigned int a, unsigned int b)
+{
+ return (a > 0x / b);
+}
--
2.18.0
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
On Tue, Jul 17, 2018 at 2:36 PM Janus Weil wrote: > > 2018-07-17 19:34 GMT+02:00 Thomas Koenig : > > Am 17.07.2018 um 19:19 schrieb Janus Weil: [...] > > I do hope that things have converged by now and that this will be the > last incarnation of the patch. If there is no more feedback in the > next 24 hours, I'll commit this tomorrow. > > Cheers, > Janus I hate to be pedantic but it is still worth fixing the style discrepancies: $ ./contrib/check_GNU_style.sh pr85599_v7.diff > > Lines should not exceed 80 characters. > 209:+ if (f != last && !gfc_pure_function (f, &name) && > !gfc_implicit_pure_function (f)) > > Blocks of 8 spaces should be replaced with tabs. > 37:+ gfc_warning (OPT_Wfunction_elimination, > 41:+ gfc_warning (OPT_Wfunction_elimination, > 238:+ with impure function as second operand. */ > > Dot, space, space, new sentence. > 84:+expression, if they do not contribute to the final result. For logical > 87:+result of the expression can be established without them. However, since > not My only other comment is I am not sure why you make pure_function()/gfc_implicit_pure_function() public... but I have no real problem with it. Just means rebuilding all of f951 instead of one object. :-( Otherwise if the patch does what it appears to do and passes tests then it seems fine to me. Fritz
[C++ PATCH] Disallow type specifiers among lambda-declarator decl-specifier-seq (PR c++/86550)
Hi!
The standard says:
"In the decl-specifier-seq of the lambda-declarator, each decl-specifier shall
either be mutable or constexpr."
and the C++ FE has CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR flag for that.
But as implemented, it is actually
CP_PARSER_FLAGS_ONLY_TYPE_OR_MUTABLE_OR_CONSTEXPR
as it allows mutable, constexpr and type specifiers.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2018-07-17 Jakub Jelinek
PR c++/86550
* parser.c (cp_parser_decl_specifier_seq): Don't parse a type specifier
if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR.
* g++.dg/cpp0x/lambda/lambda-86550.C: New test.
--- gcc/cp/parser.c.jj 2018-07-16 09:42:24.534984748 +0200
+++ gcc/cp/parser.c 2018-07-17 16:22:03.208497718 +0200
@@ -13738,7 +13738,9 @@ cp_parser_decl_specifier_seq (cp_parser*
/* If we don't have a DECL_SPEC yet, then we must be looking at
a type-specifier. */
- if (!found_decl_spec && !constructor_p)
+ if (!found_decl_spec
+ && !constructor_p
+ && !(flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR))
{
int decl_spec_declares_class_or_enum;
bool is_cv_qualifier;
--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-86550.C.jj 2018-07-17
16:24:09.95292 +0200
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-86550.C2018-07-17
16:29:22.558078546 +0200
@@ -0,0 +1,9 @@
+// PR c++/86550
+// { dg-do compile { target c++11 } }
+
+void
+foo ()
+{
+ auto a = []() bool {}; // { dg-error "expected" }
+ auto b = []() bool bool bool bool int {};// { dg-error "expected" }
+}
Jakub
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
2018-07-17 20:55 GMT+02:00 Fritz Reese :
> On Tue, Jul 17, 2018 at 2:36 PM Janus Weil wrote:
>>
>> 2018-07-17 19:34 GMT+02:00 Thomas Koenig :
>> > Am 17.07.2018 um 19:19 schrieb Janus Weil:
> [...]
>>
>> I do hope that things have converged by now and that this will be the
>> last incarnation of the patch. If there is no more feedback in the
>> next 24 hours, I'll commit this tomorrow.
>
> I hate to be pedantic but it is still worth fixing the style discrepancies:
Oh, sure. Such things are non-optional in GCC, I was just a bit sloppy
with this. Thanks for catching! Should be fixed in the attached
update.
> My only other comment is I am not sure why you make
> pure_function()/gfc_implicit_pure_function() public... but I have no
> real problem with it. Just means rebuilding all of f951 instead of one
> object. :-(
Well, originally they were only used in resolve.c, but now I need them
also in frontend-passes.c, therefore they have to be public.
> Otherwise if the patch does what it appears to do and passes tests
> then it seems fine to me.
Thanks for the review!
Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c (revision 262828)
+++ gcc/fortran/dump-parse-tree.c (working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
fputs (" ELEMENTAL", dumpfile);
if (attr->pure)
fputs (" PURE", dumpfile);
+ if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
if (attr->recursive)
fputs (" RECURSIVE", dumpfile);
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c (revision 262828)
+++ gcc/fortran/frontend-passes.c (working copy)
@@ -840,17 +840,22 @@ create_var (gfc_expr * e, const char *vname)
static void
do_warn_function_elimination (gfc_expr *e)
{
- if (e->expr_type != EXPR_FUNCTION)
-return;
- if (e->value.function.esym)
-gfc_warning (OPT_Wfunction_elimination,
- "Removing call to function %qs at %L",
- e->value.function.esym->name, &(e->where));
- else if (e->value.function.isym)
-gfc_warning (OPT_Wfunction_elimination,
- "Removing call to function %qs at %L",
- e->value.function.isym->name, &(e->where));
+ const char *name;
+ if (e->expr_type == EXPR_FUNCTION
+ && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e))
+ {
+ if (name)
+ gfc_warning (OPT_Wfunction_elimination,
+ "Removing call to impure function %qs at %L", name,
+ &(e->where));
+ else
+ gfc_warning (OPT_Wfunction_elimination,
+ "Removing call to impure function at %L",
+ &(e->where));
+ }
}
+
+
/* Callback function for the code walker for doing common function
elimination. This builds up the list of functions in the expression
and goes through them to detect duplicates, which it then replaces
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h (revision 262828)
+++ gcc/fortran/gfortran.h (working copy)
@@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *)
bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
extern int gfc_do_concurrent_flag;
const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
+int gfc_pure_function (gfc_expr *e, const char **name);
+int gfc_implicit_pure_function (gfc_expr *e);
/* array.c */
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi (revision 262828)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
@menu
* KIND Type Parameters::
* Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
* Thread-safety of the runtime library::
* Data consistency and durability::
* Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expression, if they do not contribute to the final result. For logical
+expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU
+Fortran will optimize out function calls (even to impure functions) if the
+result of the expression can be established without them. However, since not
+all compilers do that, and such an optimization can potentially modify the
+program flow and subsequent results, GNU Fortran throws warnings for such
+situations with the @option{-Wfunction-elimination} flag.
+
+
@node Thread-safety of the runtime library
@section Thread-safety of the runtime library
@cindex thread-safety, thre
Re: [PATCH, testsuite] PPC testsuite pr85456.c
On Tue, Jul 17, 2018 at 01:34:18PM -0400, David Edelsohn wrote: > And another test that needs to be restricted because it assumes 128 > bit long double. Thanks :-) Segher
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
On Tue, Jul 17, 2018 at 6:36 PM, Thomas Koenig
wrote:
> Hi Kyrill,
>
> The current implementation expands to:
>> mvar = a1;
>> if (a2 .op. mvar || isnan (mvar))
>>mvar = a2;
>> if (a3 .op. mvar || isnan (mvar))
>>mvar = a3;
>> ...
>> return mvar;
>>
>> That is, if one of the operands is a NaN it will return the other
>> argument.
>> If both (all) are NaNs, it will return NaN. This is the same as the
>> semantics of fmin/max
>> as far as I can tell.
>>
>
> I've looked at the F2008 standard, and, interestingly enough, the
> requirement on MIN and MAX do not mention NaNs at all. 13.7.106
> has, for MAX,
>
> Result Value. The value of the result is that of the largest argument.
>
> plus some stuff about character variables (not relevant here). Similar
> for MIN.
>
FWIW, this has not changed in the latest(?) draft for F2018 (N2146), see
16.9.125.
Also, the section on IEEE_ARITHMETIC (14.9) does not mention
> comparisons; also, "Complete conformance with IEC 60559:1989 is not
> required", what is required is the correct support for +,-, and *,
> plus support for / if IEEE_SUPPORT_DIVIDE is covered.
>
Interestingly, here the F2018 draft has new intrinsics in the
IEEE_ARITHMETIC module, IEEE_MAX_NUM, IEEE_MAX_NUM_MAG, IEEE_MIN_NUM,
IEEE_MIN_NUM_MAG. These correspond to the {max,min}num{,_mag} operations in
IEEE 754-2008, which AFAICT has the same NaN semantics as __builtin_fmax
etc.
> So, the Fortran standard does not impose many requirements.
If so, why don't we just use {MAX,MIN}_EXPR unconditionally? Those who
worry about the behavior wrt. NaNs, infinities etc. can use the intrinsics
from IEEE_ARITHMETIC?
This thread also has some interesting discussion on the topic:
https://github.com/JuliaLang/julia/issues/7866
--
Janne Blomqvist
[committed] Restore nios2 builds
The recent alignment changes broke nios2 as it referenced align_labels_log. This patch updates it to use align_labels.levels[0].log. Installing on the trunk. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 2e724217fa4..d8057a074a1 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2018-07-17 Jeff Law + + * config/nios2/nios2.c (nios2_label_align): Update for recent + changes which dropped ALIGN_LABELS_LOG. + 2018-07-17 Andreas Schwab * config/m68k/m68k.md (umulsi3_highpart+1, const_umulsi3_highpart) diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c index 3581ad9340d..344181a496b 100644 --- a/gcc/config/nios2/nios2.c +++ b/gcc/config/nios2/nios2.c @@ -5403,8 +5403,8 @@ nios2_label_align (rtx label) int n = CODE_LABEL_NUMBER (label); if (label_align && n >= min_labelno && n <= max_labelno) -return MAX (label_align[n - min_labelno], align_labels_log); - return align_labels_log; +return MAX (label_align[n - min_labelno], align_labels.levels[0].log); + return align_labels.levels[0].log; } /* Implement ADJUST_REG_ALLOC_ORDER. We use the default ordering
[PATCH] Make function clone name numbering independent.
On 2018-07-17 06:02 AM, Richard Biener wrote:
> On Tue, Jul 17, 2018 at 8:10 AM Bernhard Reutner-Fischer
> wrote:
>>
>> On 16 July 2018 21:38:36 CEST, Michael Ploujnikov
>> wrote:
>>> Hi,
>>>
>>
>>> +clone_fn_ids = hash_map::create_ggc
>>> (1000);
>>
>> Isn't 1000 a bit excessive? What about 64 or thereabouts?
>
> I'm not sure we should throw memory at this "problem" in this way.
> What specific issue
> does this address?
This goes along with the general theme of preventing changes to one function
affecting codegen of others. What I'm seeing in this case is when a function
bar is modified codegen decides to create more clones of it (eg: during the
constprop pass). These extra clones cause the global counter to increment so
the clones of the unchanged function foo are named differently only because of
a source change to bar. I was hoping that the testcase would be a good
illustration, but perhaps not; is there anything else I can do to make this
clearer?
>
> Iff then I belive forgoing the automatic counter addidion is the way
> to go and hand
> control of that to the callers (for example the caller in
> lto/lto-partition.c doesn't
> even seem to need that counter.
How can you tell that privatize_symbol_name_1 doesn't need the counter? I'm
assuming it has a good reason to call clone_function_name_1 rather than
appending ".lto_priv" itself.
> You also assume the string you key on persists - luckily the
> lto-partition.c caller
> leaks it but this makes your approach quite fragile in my eye (put the logic
> into clone_function_name instead, where you at least know you are dealing
> with a string from an indentifier which are never collected).
>
> Richard.
>
Is this what you had in mind?:
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index 6e84a31..f000420 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -512,7 +512,7 @@ cgraph_node::create_clone (tree new_decl, profile_count
prof_count,
return new_node;
}
-static GTY(()) unsigned int clone_fn_id_num;
+static GTY(()) hash_map *clone_fn_ids;
/* Return a new assembler name for a clone with SUFFIX of a decl named
NAME. */
@@ -521,14 +521,13 @@ tree
clone_function_name_1 (const char *name, const char *suffix)
{
size_t len = strlen (name);
- char *tmp_name, *prefix;
+ char *prefix;
prefix = XALLOCAVEC (char, len + strlen (suffix) + 2);
memcpy (prefix, name, len);
strcpy (prefix + len + 1, suffix);
prefix[len] = symbol_table::symbol_suffix_separator ();
- ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, clone_fn_id_num++);
- return get_identifier (tmp_name);
+ return get_identifier (prefix);
}
/* Return a new assembler name for a clone of DECL with SUFFIX. */
@@ -537,7 +536,17 @@ tree
clone_function_name (tree decl, const char *suffix)
{
tree name = DECL_ASSEMBLER_NAME (decl);
- return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
+ char *decl_name = IDENTIFIER_POINTER (name);
+ char *numbered_name;
+ unsigned int *suffix_counter;
+ if (!clone_fn_ids) {
+/* Initialize the per-function counter hash table if this is the first
call */
+clone_fn_ids = hash_map::create_ggc (64);
+ }
+ suffix_counter = &clone_fn_ids->get_or_insert(name);
+ ASM_FORMAT_PRIVATE_NAME (numbered_name, decl_name, *suffix_counter);
+ *suffix_counter = *suffix_counter + 1;
+ return clone_function_name_1 (numbered_name, suffix);
}
- Michael
signature.asc
Description: OpenPGP digital signature
[committed] Fix FRV builds
More fallout from the alignment changes. This time in frv_align_label.
Committed to the trunk.
jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d8057a074a1..51e91221147 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,8 @@
2018-07-17 Jeff Law
+ * config/frv/frv.c (frv_label_align): Update for recent changes
+ to label_to_alignment.
+
* config/nios2/nios2.c (nios2_label_align): Update for recent
changes which dropped ALIGN_LABELS_LOG.
diff --git a/gcc/config/frv/frv.c b/gcc/config/frv/frv.c
index 78f1a80cdf9..6920e6ddace 100644
--- a/gcc/config/frv/frv.c
+++ b/gcc/config/frv/frv.c
@@ -7936,7 +7936,7 @@ frv_align_label (void)
{
if (LABEL_P (x))
{
- unsigned int subalign = 1 << label_to_alignment (x);
+ unsigned int subalign = 1 << label_to_alignment (x).levels[0].log;
alignment = MAX (alignment, subalign);
label = x;
}
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
On Tue, Jul 17, 2018 at 11:06 PM, Janne Blomqvist wrote:
> On Tue, Jul 17, 2018 at 6:36 PM, Thomas Koenig
> wrote:
>
>> Hi Kyrill,
>>
>> The current implementation expands to:
>>> mvar = a1;
>>> if (a2 .op. mvar || isnan (mvar))
>>>mvar = a2;
>>> if (a3 .op. mvar || isnan (mvar))
>>>mvar = a3;
>>> ...
>>> return mvar;
>>>
>>> That is, if one of the operands is a NaN it will return the other
>>> argument.
>>> If both (all) are NaNs, it will return NaN. This is the same as the
>>> semantics of fmin/max
>>> as far as I can tell.
>>>
>>
>> I've looked at the F2008 standard, and, interestingly enough, the
>> requirement on MIN and MAX do not mention NaNs at all. 13.7.106
>> has, for MAX,
>>
>> Result Value. The value of the result is that of the largest argument.
>>
>> plus some stuff about character variables (not relevant here). Similar
>> for MIN.
>>
>
> FWIW, this has not changed in the latest(?) draft for F2018 (N2146), see
> 16.9.125.
>
> Also, the section on IEEE_ARITHMETIC (14.9) does not mention
>> comparisons; also, "Complete conformance with IEC 60559:1989 is not
>> required", what is required is the correct support for +,-, and *,
>> plus support for / if IEEE_SUPPORT_DIVIDE is covered.
>>
>
> Interestingly, here the F2018 draft has new intrinsics in the
> IEEE_ARITHMETIC module, IEEE_MAX_NUM, IEEE_MAX_NUM_MAG, IEEE_MIN_NUM,
> IEEE_MIN_NUM_MAG. These correspond to the {max,min}num{,_mag} operations in
> IEEE 754-2008, which AFAICT has the same NaN semantics as __builtin_fmax
> etc.
>
>
>> So, the Fortran standard does not impose many requirements.
>
>
> If so, why don't we just use {MAX,MIN}_EXPR unconditionally? Those who
> worry about the behavior wrt. NaNs, infinities etc. can use the intrinsics
> from IEEE_ARITHMETIC?
>
>
> This thread also has some interesting discussion on the topic:
> https://github.com/JuliaLang/julia/issues/7866
>
Oh, and on http://754r.ucbtest.org/ there is information about the next
update after IEEE 754-2008. In particular,
http://754r.ucbtest.org/changes.html notes that the above mentioned
{max,min}num{,_mag} have been deleted, and "new
{min,max}imum{,Number,Magnitude,MagnitudeNumber} operations are
recommended; NaN and signed zero handling are changed from 754-2008 5.3.1.
".
--
Janne Blomqvist
[PATCH, rs6000] Sort Altivec/VSX built-in functions into subsubsections according to configuration requirements
The many PowerPC built-in functions (intrinsics) that are enabled by including
each have different configuration requirements. To simplify the
description of the requirements, this patch sorts these functions into
different subsubsections.
A subsequent patch will add and remove various functions from each section to
correct incompatibilities between what is implemented and what is documented.
I have built and regression tested this patch on powerpc64le-unknown-linux and
on powerpc-linux (P8 big-endian) with no regressions. I have also built and
reviewed the gcc.pdf file.
Is this ok for trunk?
gcc/ChangeLog:
2018-07-17 Kelvin Nilsen
* doc/extend.texi (PowerPC AltiVec/VSX Built-in Functions):
Corrected spelling of this subsection. Moved some material to new
subsubsections "PowerPC AltiVec Built-in Functions on ISA 2.06" and
"PowerPC AltiVec Built-in Functions on ISA 2.07".
(PowerPC Altivec Built-in Functions on ISA 2.05): New subsubsection.
(PowerPC Altivec Built-in Functions on ISA 2.06): Likewise.
(PowerPC Altivec Built-in Functions on ISA 2.07): Likewise.
(PowerPC Altivec Built-in Functions on ISA 3.0): Likewise.
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 262747)
+++ gcc/doc/extend.texi (working copy)
@@ -15941,10 +15941,8 @@ The @code{__builtin_dfp_dtstsfi_ov_dd} and
require that the type of the @code{value} argument be
@code{__Decimal64} and @code{__Decimal128} respectively.
-
-
@node PowerPC AltiVec/VSX Built-in Functions
-@subsection PowerPC AltiVec Built-in Functions
+@subsection PowerPC AltiVec/VSX Built-in Functions
GCC provides an interface for the PowerPC family of processors to access
the AltiVec operations described in Motorola's AltiVec Programming
@@ -15969,19 +15967,6 @@ vector bool int
vector float
@end smallexample
-If @option{-mvsx} is used the following additional vector types are
-implemented.
-
-@smallexample
-vector unsigned long
-vector signed long
-vector double
-@end smallexample
-
-The long types are only implemented for 64-bit code generation, and
-the long type is only used in the floating point/integer conversion
-instructions.
-
GCC's implementation of the high-level language interface available from
C and C++ code differs from Motorola's documentation in several ways.
@@ -16039,6 +16024,16 @@ the interfaces described therein. However, histor
additional interfaces for access to vector instructions. These are
briefly described below.
+@menu
+* PowerPC AltiVec Built-in Functions on ISA 2.05::
+* PowerPC AltiVec Built-in Functions Available on ISA 2.06::
+* PowerPC AltiVec Built-in Functions Available on ISA 2.07::
+* PowerPC AltiVec Built-in Functions Available on ISA 3.0::
+@end menu
+
+@node PowerPC AltiVec Built-in Functions on ISA 2.05
+@subsubsection PowerPC AltiVec Built-in Functions on ISA 2.05
+
The following interfaces are supported for the generic and specific
AltiVec operations and the AltiVec predicates. In cases where there
is a direct mapping between generic and specific operations, only the
@@ -17581,132 +17576,152 @@ vector unsigned char vec_xor (vector unsigned char
vector unsigned char vec_xor (vector unsigned char, vector unsigned char);
@end smallexample
-The following built-in functions which are currently documented in
-this section are not alphabetized with other built-in functions of
-this section because they belong in different sections.
+@node PowerPC AltiVec Built-in Functions Available on ISA 2.06
+@subsubsection PowerPC AltiVec Built-in Functions Available on ISA 2.06
+The AltiVec built-in functions described in this section are
+available on the PowerPC family of processors starting with ISA 2.06
+or later. These are normally enabled by adding @option{-mvsx} to the
+command line.
+
+When @option{-mvsx} is used, the following additional vector types are
+implemented.
+
@smallexample
-/* __int128, long long, and double arguments and results require -mvsx. */
+vector unsigned __int128
+vector signed __int128
+vector unsigned long long int
+vector signed long long int
+vector double
+@end smallexample
+
+The long long types are only implemented for 64-bit code generation.
+
+@smallexample
+
vector bool long long vec_and (vector bool long long int, vector bool long
long);
+
vector double vec_ctf (vector unsigned long, const int);
vector double vec_ctf (vector signed long, const int);
+
vector signed long vec_cts (vector double, const int);
+
vector unsigned long vec_ctu (vector double, const int);
+
void vec_dst (const unsigned long *, int, const int);
void vec_dst (const long *, int, const int);
+
void vec_dststt (const unsigned long *, int, const int);
void vec_dststt (const long *, int, const int);
+
void vec_dstt (const unsigned long *, int, const int);
void vec_dstt (const long *, int, const int);
+
vector unsigned char vec_lvsl (i
Re: [PATCH] Fix PR86456
On 07/17/2018 02:23 AM, Richard Biener wrote: > > This cures the ICEs and wrong assembler that currently happen when > using -flto with -gdwarf-5. gdb is also happy with small test programs > but readelf is still complaining (see PR, I have no clue what goes > wrong). > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. > LTO bootstrap with -gdwarf-5 running ontop of the GCC 8 branch. > > Jakub, do you know of any issues with dwarf5 supports in tools > like readelf? I've just checked that from binutils 2.31 but even > that isn't happy. Mark Weilaard has been poking at dwarf5 support in elfutils. You may have better luck with those rather than readelf from binutils. Tom Rix indicated some interest in this space as well, but I haven't seen much code yet. Jeff
[committed] Restoring MIPS builds after alignment changes
Similar to the others... Committed to the trunk. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 51e91221147..ac4abdaf1ba 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,8 @@ 2018-07-17 Jeff Law + * config/mips/mips.c (vr4130_align_insns): Update for recent + changes to label_to_alignment. + * config/frv/frv.c (frv_label_align): Update for recent changes to label_to_alignment. diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 75ee834137e..ea2fae1d6db 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -18539,7 +18539,7 @@ vr4130_align_insns (void) } /* See whether INSN is an aligned label. */ - if (LABEL_P (insn) && label_to_alignment (insn) >= 3) + if (LABEL_P (insn) && label_to_alignment (insn).levels[0].log >= 3) aligned_p = true; } dfa_finish ();
Re: [PATCH, rs6000] Sort Altivec/VSX built-in functions into subsubsections according to configuration requirements
On Tue, Jul 17, 2018 at 03:44:43PM -0500, Kelvin Nilsen wrote: > The many PowerPC built-in functions (intrinsics) that are enabled by > including each have different configuration requirements. To > simplify the description of the requirements, this patch sorts these > functions into different subsubsections. > > A subsequent patch will add and remove various functions from each section to > correct incompatibilities between what is implemented and what is documented. > > I have built and regression tested this patch on powerpc64le-unknown-linux > and on powerpc-linux (P8 big-endian) with no regressions. I have also built > and reviewed the gcc.pdf file. A changelog nit: > * doc/extend.texi (PowerPC AltiVec/VSX Built-in Functions): > Corrected spelling of this subsection. Moved some material to new > subsubsections "PowerPC AltiVec Built-in Functions on ISA 2.06" and > "PowerPC AltiVec Built-in Functions on ISA 2.07". * doc/extend.texi (PowerPC AltiVec Built-in Functions): Rename to... (PowerPC AltiVec/VSX Built-in Functions): ... this. Move some material to new subsubsections "PowerPC AltiVec Built-in Functions on ISA 2.06" and "PowerPC AltiVec Built-in Functions on ISA 2.07". (I.e., mention old name as well as new, and use imperative instead of past tense). Looks fine otherwise. Okay for trunk. Thanks! Segher
[PATCH, rs6000] Fix AIX test case failures
Segher:
I was requested to backport the patch for the AIX test case failures to
GCC 8. The trunk patch applied cleanly to GCC 8. I updated the
changelog patch, built and retested the patch on:
powerpc64le-unknown-linux-gnu (Power 8 LE)
powerpc64-unknown-linux-gnu (Power 8 BE)
AIX 7200-00-01-1543 (Power 8 BE)
With no regressions.
Please let me know if it is OK to apply the patch to the GCC 8 branch.
Thanks.
Carl Love
-
gcc/testsuite/ChangeLog:
2018-07-17 Carl Love
Backport from mainline
2018-07-16 Carl Love
PR target/86414
* gcc.target/powerpc/divkc3-2.c: Add dg-require-effective-target
longdouble128.
* gcc.target/powerpc/divkc3-3.c: Ditto.
* gcc.target/powerpc/mulkc3-2.c: Ditto.
* gcc.target/powerpc/mulkc3-3.c: Ditto.
* gcc.target/powerpc/fold-vec-mergehl-double.c: Update counts.
* gcc.target/powerpc/pr85456.c: Make check Linux and AIX specific.
---
gcc/testsuite/gcc.target/powerpc/divkc3-2.c| 1 +
gcc/testsuite/gcc.target/powerpc/divkc3-3.c| 1 +
gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-double.c | 4 +---
gcc/testsuite/gcc.target/powerpc/mulkc3-2.c| 1 +
gcc/testsuite/gcc.target/powerpc/mulkc3-3.c| 1 +
gcc/testsuite/gcc.target/powerpc/pr85456.c | 3 ++-
6 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/gcc.target/powerpc/divkc3-2.c
b/gcc/testsuite/gcc.target/powerpc/divkc3-2.c
index d3fcbed..e34ed40 100644
--- a/gcc/testsuite/gcc.target/powerpc/divkc3-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/divkc3-2.c
@@ -1,5 +1,6 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */
/* Check that complex multiply generates the right call when long double is
diff --git a/gcc/testsuite/gcc.target/powerpc/divkc3-3.c
b/gcc/testsuite/gcc.target/powerpc/divkc3-3.c
index 45695fe..c0fda8b 100644
--- a/gcc/testsuite/gcc.target/powerpc/divkc3-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/divkc3-3.c
@@ -1,5 +1,6 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */
/* Check that complex multiply generates the right call when long double is
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-double.c
b/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-double.c
index 25f4bc6..14f9448 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-double.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-double.c
@@ -19,7 +19,5 @@ testd_h (vector double vd2, vector double vd3)
return vec_mergeh (vd2, vd3);
}
-/* vec_merge with doubles tend to just use xxpermdi (3 ea for BE, 1 ea for
LE). */
-/* { dg-final { scan-assembler-times "xxpermdi" 2 { target { powerpc*le-*-* }
}} } */
-/* { dg-final { scan-assembler-times "xxpermdi" 6 { target { powerpc-*-* } }
} } */
+/* { dg-final { scan-assembler-times "xxpermdi" 2 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/mulkc3-2.c
b/gcc/testsuite/gcc.target/powerpc/mulkc3-2.c
index 9ba577a..eee6de9 100644
--- a/gcc/testsuite/gcc.target/powerpc/mulkc3-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/mulkc3-2.c
@@ -1,5 +1,6 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */
/* Check that complex multiply generates the right call when long double is
diff --git a/gcc/testsuite/gcc.target/powerpc/mulkc3-3.c
b/gcc/testsuite/gcc.target/powerpc/mulkc3-3.c
index db87301..b6d2bdf 100644
--- a/gcc/testsuite/gcc.target/powerpc/mulkc3-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/mulkc3-3.c
@@ -1,5 +1,6 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */
/* Check that complex multiply generates the right call when long double is
diff --git a/gcc/testsuite/gcc.target/powerpc/pr85456.c
b/gcc/testsuite/gcc.target/powerpc/pr85456.c
index b9df16a..b928292 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr85456.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr85456.c
@@ -11,4 +11,5 @@ do_powl (long double a, int i)
return __builtin_powil (a, i);
}
-/* { dg-final { scan-assembler "bl __powikf2" } } */
+/* { dg-final { scan-assembler "bl __powikf2" { target { powerpc*-*-linux* } }
} } */
+/* { dg-final { scan-assembl
backporting fix for 85602 to GCC 8
If there are no objections I'd like to backport the solution for PR 85602 to avoid a class of unnecessary warnings for safe uses of nonstring arrays. With the release coming up later this week I'll go ahead and commit the patch tomorrow. https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261718 Thanks Martin
[committed] Fix typo/thinko in recent tree-ssa-dse.c change
I meant to mask off low bits in the head trim and wrote... -*trim_head &= (UNITS_PER_WORD - 1); Which, of course is wrong as it's missing the bit-not. This led to a regression on at least one 32bit target for pr30375's testcase. This patch adds the missing bit-not. It fixes the regression on i686. I was briefly worried because a couple of libgomp tests failed. But after deeper investigation it turns out they can fail due to resource limitations. I re-ran those tests independently and with a much lower parallelism factor and they all passed. Installed on the trunk after bootstrapping x86_64 and regression testing x86_64 and the -m32 multilib. Jeff diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c index ebc4a1e..4cb8c0f8 100644 --- a/gcc/tree-ssa-dse.c +++ b/gcc/tree-ssa-dse.c @@ -260,7 +260,7 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail, /* If more than a word remains, then make sure to keep the starting point at least word aligned. */ if (last_live - first_live > UNITS_PER_WORD) -*trim_head &= (UNITS_PER_WORD - 1); +*trim_head &= ~(UNITS_PER_WORD - 1); if ((*trim_head || *trim_tail) && dump_file && (dump_flags & TDF_DETAILS))
Re: [C++ PATCH] Disallow type specifiers among lambda-declarator decl-specifier-seq (PR c++/86550)
On Wed, Jul 18, 2018 at 4:57 AM, Jakub Jelinek wrote: > The standard says: > "In the decl-specifier-seq of the lambda-declarator, each decl-specifier shall > either be mutable or constexpr." > and the C++ FE has CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR flag for that. > But as implemented, it is actually > CP_PARSER_FLAGS_ONLY_TYPE_OR_MUTABLE_OR_CONSTEXPR > as it allows mutable, constexpr and type specifiers. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > 2018-07-17 Jakub Jelinek > > PR c++/86550 > * parser.c (cp_parser_decl_specifier_seq): Don't parse a type > specifier > if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR. I think the diagnostic would be better if we parse the type-specifier and then give an error about it being invalid in this context, rather than not parse it and therefore give a syntax error; the constraint is semantic rather than syntactic. Jason
Re: [C++ Patch] Another permerror related tweak
OK. And I think this qualifies as obvious; you don't need to wait for approval on further patches like this. Jason On Tue, Jul 17, 2018 at 10:17 PM, Paolo Carlini wrote: > Hi again, > > here I noticed a pair of consecutive permerrors (instead of permerror + > inform) and also that the first one isn't exploiting DECL_SOURCE_LOCATION > (which makes a real difference at least in a couple of cases which I'm > explicitly testing below). Tested x86_64-linux. > > Thanks, Paolo. > > / >
Re: C++ PATCH for c++/86190, bogus -Wsign-conversion warning
OK.
On Fri, Jul 13, 2018 at 11:43 PM, Marek Polacek wrote:
> Ping.
>
> On Tue, Jul 03, 2018 at 09:35:24AM -0400, Marek Polacek wrote:
>> This PR complains about bogus -Wsign-conversion warning even with an
>> explicit static_cast. It started with this hunk from the delayed folding
>> merge:
>>
>> @@ -5028,20 +5022,12 @@ cp_build_binary_op (location_t location,
>>
>>if (short_compare)
>> {
>> - /* Don't write &op0, etc., because that would prevent op0
>> -from being kept in a register.
>> -Instead, make copies of the our local variables and
>> -pass the copies by reference, then copy them back afterward. */
>> - tree xop0 = op0, xop1 = op1, xresult_type = result_type;
>> + /* We call shorten_compare only for diagnostic-reason. */
>> + tree xop0 = fold_simple (op0), xop1 = fold_simple (op1),
>> + xresult_type = result_type;
>> enum tree_code xresultcode = resultcode;
>> - tree val
>> - = shorten_compare (location, &xop0, &xop1, &xresult_type,
>> + shorten_compare (location, &xop0, &xop1, &xresult_type,
>>&xresultcode);
>> - if (val != 0)
>> - return cp_convert (boolean_type_node, val, complain);
>> - op0 = xop0, op1 = xop1;
>> - converted = 1;
>> - resultcode = xresultcode;
>> }
>>
>>if ((short_compare || code == MIN_EXPR || code == MAX_EXPR)
>>
>> which means that converted is now unset so we go to
>>
>> 5350 if (! converted)
>> 5351 {
>> 5352 if (TREE_TYPE (op0) != result_type)
>> 5353 op0 = cp_convert_and_check (result_type, op0, complain);
>> 5354 if (TREE_TYPE (op1) != result_type)
>> 5355 op1 = cp_convert_and_check (result_type, op1, complain);
>>
>> and cp_convert_and_check gives those warning. The direct comparison
>> of types instead of same_type_p means we can try to convert same types,
>> but it still wouldn't fix this PR. What we should probably do is to
>> simply disable -Wsign-conversion conversion for comparison, because
>> -Wsign-compare will warn for those. With this patch, the C++ FE will
>> follow what the C FE and clang++ do.
>>
>> Also fix some formatting that's been bothering me, while at it.
>>
>> Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
>>
>> 2018-07-03 Marek Polacek
>>
>> PR c++/86190 - bogus -Wsign-conversion warning
>> * typeck.c (cp_build_binary_op): Fix formatting. Add a warning
>> sentinel.
>>
>> * g++.dg/warn/Wsign-conversion-3.C: New test.
>> * g++.dg/warn/Wsign-conversion-4.C: New test.
>>
>> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
>> index 3a4f1cdf479..cfd1dd8b150 100644
>> --- gcc/cp/typeck.c
>> +++ gcc/cp/typeck.c
>> @@ -5311,12 +5311,13 @@ cp_build_binary_op (location_t location,
>>
>>if (short_compare)
>> {
>> - /* We call shorten_compare only for diagnostic-reason. */
>> - tree xop0 = fold_simple (op0), xop1 = fold_simple (op1),
>> -xresult_type = result_type;
>> + /* We call shorten_compare only for diagnostics. */
>> + tree xop0 = fold_simple (op0);
>> + tree xop1 = fold_simple (op1);
>> + tree xresult_type = result_type;
>> enum tree_code xresultcode = resultcode;
>> shorten_compare (location, &xop0, &xop1, &xresult_type,
>> -&xresultcode);
>> +&xresultcode);
>> }
>>
>>if ((short_compare || code == MIN_EXPR || code == MAX_EXPR)
>> @@ -5349,6 +5350,7 @@ cp_build_binary_op (location_t location,
>> otherwise, it will be given type RESULT_TYPE. */
>>if (! converted)
>> {
>> + warning_sentinel w (warn_sign_conversion, short_compare);
>>if (TREE_TYPE (op0) != result_type)
>> op0 = cp_convert_and_check (result_type, op0, complain);
>>if (TREE_TYPE (op1) != result_type)
>> diff --git gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> index e69de29bb2d..2c3fef31475 100644
>> --- gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> +++ gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> @@ -0,0 +1,13 @@
>> +// PR c++/86190
>> +// { dg-options "-Wsign-conversion -Wsign-compare" }
>> +
>> +typedef unsigned long sz_t;
>> +sz_t s();
>> +bool f(int i) { return s() < (unsigned long) i; }
>> +bool f2(int i) { return s() < static_cast(i); }
>> +bool f3(int i) { return s() < i; } // { dg-warning "comparison of integer
>> expressions of different signedness" }
>> +bool f4(int i) { return s() < (long) i; } // { dg-warning "comparison of
>> integer expressions of different signedness" }
>> +bool f5(short int i) { return s() < (int) i; } // { dg-warning "comparison
>> of integer expressions of different signedness" }
>> +bool f6(signed char i) { return s() < (int) i; } // { dg-warning
>> "comparison of integer expressions of different signedness" }
>> +bool f7(unsigned char i) { return s() < i; }
>> +bool f8(signed char i) {
[PR86544] Fix Popcount detection generates different code on C and C++
Attached patch fixes phi-opt not optimizing c++ testcase where we have
if (b_4(D) == 0) instead of if (b_4(D) != 0) as shown in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86544
Patch bootstrapped and regression tested on x86_64-linux-gnu with no
new regressions.
Is this OK for trunk?
Thanks,
Kugan
gcc/ChangeLog:
2018-07-18 Kugan Vivekanandarajah
PR middle-end/86544
* tree-ssa-phiopt.c (cond_removal_in_popcount_pattern): Handle
comparison with EQ_EXPR
in last stmt.
gcc/testsuite/ChangeLog:
2018-07-18 Kugan Vivekanandarajah
PR middle-end/86544
* g++.dg/tree-ssa/pr86544.C: New test.
From c482a4225764e0b338abafb7ccea4553f273f5d5 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah
Date: Wed, 18 Jul 2018 08:14:16 +1000
Subject: [PATCH] fix cpp testcase
Change-Id: Icd59b31faef2ac66beb42990cb69cbbe38c238aa
---
gcc/testsuite/g++.dg/tree-ssa/pr86544.C | 15 +++
gcc/tree-ssa-phiopt.c | 26 +++---
2 files changed, 30 insertions(+), 11 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr86544.C
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr86544.C b/gcc/testsuite/g++.dg/tree-ssa/pr86544.C
new file mode 100644
index 000..8a90089
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr86544.C
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-phiopt3 -fdump-tree-optimized" } */
+
+int PopCount (long b) {
+int c = 0;
+
+while (b) {
+ b &= b - 1;
+ c++;
+}
+return c;
+}
+
+/* { dg-final { scan-tree-dump-times "__builtin_popcount" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "if" 0 "phiopt3" } } */
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 656f840..1667bad 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -1614,8 +1614,22 @@ cond_removal_in_popcount_pattern (basic_block cond_bb, basic_block middle_bb,
arg = gimple_assign_rhs1 (cast);
}
+ cond = last_stmt (cond_bb);
+
+ /* Cond_bb has a check for b_4 [!=|==] 0 before calling the popcount
+ builtin. */
+ if (gimple_code (cond) != GIMPLE_COND
+ || (gimple_cond_code (cond) != NE_EXPR
+ && gimple_cond_code (cond) != EQ_EXPR)
+ || !integer_zerop (gimple_cond_rhs (cond))
+ || arg != gimple_cond_lhs (cond))
+return false;
+
/* Canonicalize. */
- if (e2->flags & EDGE_TRUE_VALUE)
+ if ((e2->flags & EDGE_TRUE_VALUE
+ && gimple_cond_code (cond) == NE_EXPR)
+ || (e1->flags & EDGE_TRUE_VALUE
+ && gimple_cond_code (cond) == EQ_EXPR))
{
std::swap (arg0, arg1);
std::swap (e1, e2);
@@ -1625,16 +1639,6 @@ cond_removal_in_popcount_pattern (basic_block cond_bb, basic_block middle_bb,
if (lhs != arg0 || !integer_zerop (arg1))
return false;
- cond = last_stmt (cond_bb);
-
- /* Cond_bb has a check for b_4 != 0 before calling the popcount
- builtin. */
- if (gimple_code (cond) != GIMPLE_COND
- || gimple_cond_code (cond) != NE_EXPR
- || !integer_zerop (gimple_cond_rhs (cond))
- || arg != gimple_cond_lhs (cond))
-return false;
-
/* And insert the popcount builtin and cast stmt before the cond_bb. */
gsi = gsi_last_bb (cond_bb);
if (cast)
--
2.7.4
Re: [PATCH] fix a couple of bugs in const string folding (PR 86532)
The attached update takes care of a couple of problems pointed
out by Bernd Edlinger in his comments on the bug. The ICE he
mentioned in comment #20 was due mixing sizetype, ssizetype,
and size_type_node in c_strlen(). AFAICS, some of it predates
the patch but my changes made it worse and also managed trigger
it.
On 07/17/2018 09:19 AM, Martin Sebor wrote:
My enhancement to extract constant strings out of complex
aggregates committed last week introduced a couple of bugs in
dealing with non-constant indices and offsets. One of the bugs
was fixed earlier today (PR 86528) but another one remains. It
causes strlen (among other functions) to incorrectly fold
expressions involving a non-constant index into an array of
strings by treating the index the same as a non-consatnt
offset into it.
The non-constant index should either prevent the folding, or it
needs to handle it differently from an offset.
The attached patch takes the conservative approach of avoiding
the folding in this case. The remaining changes deal with
the fallout from the fix.
Tested on x86_64-linux.
Martin
PR tree-optimization/86532 - Wrong code due to a wrong strlen folding starting with r262522
gcc/ChangeLog:
PR tree-optimization/86532
* builtins.c (c_strlen): Correct handling of non-constant offsets.
(check_access): Be prepared for non-constant length ranges.
* expr.c (string_constant): Only handle the minor non-constant
array index.
gcc/testsuite/ChangeLog:
PR tree-optimization/86532
* gcc.c-torture/execute/strlen-2.c: New test.
* gcc.c-torture/execute/strlen-3.c: New test.
diff --git a/gcc/builtins.c b/gcc/builtins.c
index c069d66..03cf012 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -517,7 +517,7 @@ get_pointer_alignment (tree exp)
return align;
}
-/* Return the number of non-zero elements in the sequence
+/* Return the number of leading non-zero elements in the sequence
[ PTR, PTR + MAXELTS ) where each element's size is ELTSIZE bytes.
ELTSIZE must be a power of 2 less than 8. Used by c_strlen. */
@@ -605,14 +605,21 @@ c_strlen (tree src, int only_value)
/* Set MAXELTS to sizeof (SRC) / sizeof (*SRC) - 1, the maximum possible
length of SRC. Prefer TYPE_SIZE() to TREE_STRING_LENGTH() if possible
- in case the latter is less than the size of the array. */
- HOST_WIDE_INT maxelts = TREE_STRING_LENGTH (src);
+ in case the latter is less than the size of the array, such as when
+ SRC refers to a short string literal used to initialize a large array.
+ In that case, the elements of the array after the terminating NUL are
+ all NUL. */
+ HOST_WIDE_INT strelts = TREE_STRING_LENGTH (src);
+ strelts = strelts / eltsize - 1;
+
+ HOST_WIDE_INT maxelts = strelts;
tree type = TREE_TYPE (src);
if (tree size = TYPE_SIZE_UNIT (type))
if (tree_fits_shwi_p (size))
- maxelts = tree_to_uhwi (size);
-
- maxelts = maxelts / eltsize - 1;
+ {
+ maxelts = tree_to_uhwi (size);
+ maxelts = maxelts / eltsize - 1;
+ }
/* PTR can point to the byte representation of any string type, including
char* and wchar_t*. */
@@ -620,10 +627,12 @@ c_strlen (tree src, int only_value)
if (byteoff && TREE_CODE (byteoff) != INTEGER_CST)
{
- /* If the string has an internal zero byte (e.g., "foo\0bar"), we can't
- compute the offset to the following null if we don't know where to
+ /* If the string has an internal NUL character followed by any
+ non-NUL characters (e.g., "foo\0bar"), we can't compute
+ the offset to the following NUL if we don't know where to
start searching for it. */
- if (string_length (ptr, eltsize, maxelts) < maxelts)
+ unsigned len = string_length (ptr, eltsize, strelts);
+ if (len < strelts)
{
/* Return when an embedded null character is found. */
return NULL_TREE;
@@ -633,12 +642,17 @@ c_strlen (tree src, int only_value)
return ssize_int (0);
/* We don't know the starting offset, but we do know that the string
- has no internal zero bytes. We can assume that the offset falls
- within the bounds of the string; otherwise, the programmer deserves
- what he gets. Subtract the offset from the length of the string,
- and return that. This would perhaps not be valid if we were dealing
- with named arrays in addition to literal string constants. */
- return size_diffop_loc (loc, size_int (maxelts * eltsize), byteoff);
+ has no internal zero bytes. If the offset falls within the bounds
+ of the string subtract the offset from the length of the string,
+ and return that. Otherwise the length is zero. Take care to
+ use SAVE_EXPR in case the OFFSET has side-effects. */
+ tree offsave = TREE_SIDE_EFFECTS (byteoff) ? save_expr (byteoff) : byteoff;
+ offsave = fold_convert (ssizetype, offsave);
+ tree condexp = fold_build2_loc (loc, LE_EXPR, boolean_type_node, offsave,
+ build_int_cst (ssizetype, len * eltsize));
+ tree lenexp = siz
Re: [PATCH 1/4] Clean up of new format of -falign-FOO.
Hi Martin,
Your alignment patch breaks the arm port. In the file arm.c, function
'get_label_padding' the code uses:
static HOST_WIDE_INT
get_label_padding (rtx label)
{
HOST_WIDE_INT align, min_insn_size;
align = 1 << label_to_alignment (label);
min_insn_size = TARGET_THUMB ? 2 : 4;
return align > min_insn_size ? align - min_insn_size : 0;
}
Which breaks with your current change. I think this needs to be modified to:
'align = 1 << label_to_alignment (label).levels[0].log'
Regards,
Michael Collison
[committed] And now fixing alignment stuff for ARM
Just like the others.Committing to the trunk.
Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index bccc0c76b04..0249082f618 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,8 @@
2018-07-17 Jeff Law
+ * config/arm/arm.c (get_label_padding): Update for recent
+ changes to label_to_alignment.
+
PR tree-optimization/86010
* tree-ssa-dse.c (compute_trims): Fix typo/thinko.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index ec3abbcba9f..cf12aceb5fd 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -15938,7 +15938,7 @@ get_label_padding (rtx label)
{
HOST_WIDE_INT align, min_insn_size;
- align = 1 << label_to_alignment (label);
+ align = 1 << label_to_alignment (label).levels[0].log;
min_insn_size = TARGET_THUMB ? 2 : 4;
return align > min_insn_size ? align - min_insn_size : 0;
}
[PATCH] Introduce instance discriminators
This patch is a rewrite of an earlier patch submitted at https://gcc.gnu.org/ml/gcc-patches/2012-11/msg02340.html With -gnateS, the Ada compiler sets itself up to output discriminators for different instantiations of generics, but the middle and back ends have lacked support for that. This patch introduces the missing bits, translating the GNAT-internal representation of the instance map to an instance_table that maps ordinary line-map indices to instance discriminators. Instance discriminators are not compatible with LTO, in that the instance mapping is not preserved in LTO dumps. There are no plans to preserve discriminators in them. This patch (minus whitespace changes and tests) was regstrapped on x86_64-linux-gnu. The final form of the patch was tested with a non-bootstrap build, and a single-test check-gnat run. Ok to install? From: Olivier Hainque for libcpp/ChangeLog * include/line-map.h (ORDINARY_MAP_INDEX): New. for gcc/ChangeLog * einput.c: New file. Allow associating "line context" extension data to instruction location info, for sets of locations covered by an ordinary line_map structure. * einput.h: Likewise. * Makefile.in (OBJS): Add einput.o. * input.c (expand_location_1): On request, provide pointer to the line map that was used to resolve the input location. (map_expand_location): New function. Same as expand_location, also providing the map from which the input location was resolved. (expand_location, expand_location_to_spelling_point): Adjust calls to expand_location_1. (linemap_client_expand_location_to_spelling_point): Likewise. * input.h (map_expand_location): Declare. * emit-rtl.c (insn_location): Handle a location_lc* argument. * rtl.h (insn_location): Adjust prototype. * print-rtl.c (print_rtx): Adjust call to insn_location. * modulo-sched.c (dump_insn_location): Likewise. * tree-inline.c (copy_bb): Copy discriminator field as well. * flag-types.h (loc_discriminator_type): New enum, allowing BB or INSTANCE_ID discriminators. * common.opt (loc_discriminator_kind): New variable, conveying the kinf of discriminator we want to see emited with source locations. * final.c (bb_discriminator, last_bb_discriminator): New statics, to track basic block discriminators. (final_start_function_1): Initialize them. (final_scan_insn_1): On NOTE_INSN_BASIC_BLOCK, track bb_discriminator. (notice_source_line): If INSN_HAS_LOCATION, update current discriminator from BB or INSTANCE_ID depending on the kind we're requested to convey. When deciding to emit, account for both possible kinds of discriminators. for gcc/ada * trans.c (gigi): When requested so, allocate and populate the gcc table controlling the emission of per-instance debug info. From: Alexandre Oliva , Olivier Hainque for gcc/testsuite/ChangeLog * gnat.dg/dinst.adb: New. * gnat.dg/dinst_pkg.ads, gnat.dg/dinst_pkg.adb: New. --- gcc/Makefile.in |1 + gcc/ada/gcc-interface/trans.c | 10 ++ gcc/common.opt | 12 gcc/einput.c| 55 +++ gcc/einput.h| 50 gcc/emit-rtl.c | 11 +-- gcc/final.c | 29 +++--- gcc/flag-types.h| 14 + gcc/input.c | 32 +--- gcc/input.h |2 + gcc/modulo-sched.c |2 + gcc/print-rtl.c |2 + gcc/rtl.h |3 +- gcc/testsuite/gnat.dg/dinst.adb | 20 + gcc/testsuite/gnat.dg/dinst_pkg.adb |7 gcc/testsuite/gnat.dg/dinst_pkg.ads |4 +++ gcc/tree-inline.c |2 + libcpp/include/line-map.h |8 + 18 files changed, 247 insertions(+), 17 deletions(-) create mode 100644 gcc/einput.c create mode 100644 gcc/einput.h create mode 100644 gcc/testsuite/gnat.dg/dinst.adb create mode 100644 gcc/testsuite/gnat.dg/dinst_pkg.adb create mode 100644 gcc/testsuite/gnat.dg/dinst_pkg.ads diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 2a05a66ea9b87..f9a9fe8726b18 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1285,6 +1285,7 @@ OBJS = \ dwarf2cfi.o \ dwarf2out.o \ early-remat.o \ + einput.o \ emit-rtl.o \ et-forest.o \ except.o \ diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c index 31e098a0c707a..3ad3f83fd60f5 100644 --- a/gcc/ada/gcc-interface/trans.c +++ b/gcc/ada/gcc-interface/trans.c @@ -45,6 +45,7 @@ #include "tree-iterator.h" #include "gimplify.h
