[committed] Re: [PATCH] libstdc++: Add missing constexpr to simd
I pushed the attached patch. I kept the operator names... too late, there were already operator names in the stdx::simd implemenation anyway. ;) - Matthias On Monday, 22 May 2023 22:51:49 CEST Jonathan Wakely wrote: > On Mon, 22 May 2023 at 21:27, Matthias Kretz wrote: > > On Monday, 22 May 2023 18:25:15 CEST Jonathan Wakely wrote: > > > I note that using if (not __builtin_constant_evaluated()) will fail if > > > compiled with -fno-operator-names, which is why we don't use 'not', > > > > 'and', > > > > > etc. elsewhere in libstdc++. I don't know if (or why) anybody uses that > > > option though, so I don't think you need to hange anything in > > > stdx::simd. > > > > Ah, I just recently convinced myself that "operator-names" are more > > readable > > (=> easier to maintain). > > I tend to agree, but every time I decide to start using them some testcases > start to fail and I remember why we don't use them :-( > > > But OTOH a mix isn't necessarily better. I'm fine > > with keeping it consistent. > > > > > > * subscripting vector builtins is not allowed in constant expressions > > > > > > Is that just because nobody made it work (yet)? > > > > That is a good question. I guess I should open a PR. > > > > > * if the implementation needs/uses memcpy > > > > > > > * if the implementation would otherwise call SIMD intrinsics/builtins > > > > > > The indentation looks off here and in the _M_set member function > > > > following > > > > > it: > > Yes. I had to put an #if between an else and an if. Looks like this: > > else > > > > #ifdef _GLIBCXX_SIMD_USE_ALIASING_LOADS > > > > if (not __builtin_is_constant_evaluated()) > > return reinterpret_cast*>(this)[__i]; > > > > else > > > > #endif > > > > if constexpr (__is_scalar_abi<_Abi0>()) > > Ah yes, so the if is indented two spaces from the else above it. > What looks wrong to me is that the return is the at the same indentation as > the if controlling it. > > > Should the `if` be aligned to the `else` instead? > > How about moving the two else tokens? > > #ifdef _GLIBCXX_SIMD_USE_ALIASING_LOADS >else if (not __builtin_is_constant_evaluated()) > return reinterpret_cast*>(this)[__i]; > #endif >else if constexpr (__is_scalar_abi<_Abi0>()) > > I think that avoids the issue. > > > > Are the copyright years on > > > testsuite/experimental/simd/pr109261_constexpr_simd.cc correct, or just > > > copy&paste? > > > > Right, copy&paste. Should I simply remove the complete header? > > You could do. I don't think there's much in that test that's novel or worth > asserting copyright over - but if you disagree and want to assign whatever > is copyrightable to the FSF, keep the header but fix the years. Either way > is fine by me. > > OK for trunk and backports, with the comments above suitably resolved. -- ── Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de stdₓ::simd ── diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h index 224153ffbaf..b0571ca26c4 100644 --- a/libstdc++-v3/include/experimental/bits/simd.h +++ b/libstdc++-v3/include/experimental/bits/simd.h @@ -2675,7 +2675,14 @@ _SimdWrapper(_V __x) _GLIBCXX_SIMD_INTRINSIC constexpr void _M_set(size_t __i, _Tp __x) -{ _M_data[__i] = __x; } +{ + if (__builtin_is_constant_evaluated()) + _M_data = __generate_from_n_evaluations<_Width, _BuiltinType>([&](auto __j) { + return __j == __i ? __x : _M_data[__j()]; + }); + else + _M_data[__i] = __x; +} _GLIBCXX_SIMD_INTRINSIC constexpr bool @@ -3186,6 +3193,10 @@ resizing_simd_cast(const simd<_Up, _Ap>& __x) { if constexpr (is_same_v) return __x; +else if (__builtin_is_constant_evaluated()) + return _Tp([&](auto __i) constexpr { + return __i < simd_size_v<_Up, _Ap> ? __x[__i] : _Up(); + }); else if constexpr (simd_size_v<_Up, _Ap> == 1) { _Tp __r{}; @@ -3321,10 +3332,11 @@ __get_lvalue(const const_where_expression& __x) const_where_expression& operator=(const const_where_expression&) = delete; -_GLIBCXX_SIMD_INTRINSIC const_where_expression(const _M& __kk, const _Tp& dd) - : _M_k(__kk), _M_value(const_cast<_Tp&>(dd)) {} +_GLIBCXX_SIMD_INTRINSIC constexpr +const_where_expression(const _M& __kk, const _Tp& dd) +: _M_k(__kk), _M_value(const_cast<_Tp&>(dd)) {} -_GLIBCXX_SIMD_INTRINSIC _V +_GLIBCXX_SIMD_INTRINSIC _GLIBCXX_SIMD_CONSTEXPR _V operator-() const&& { return {__private_init, @@ -,7 +3345,7 @@ __get_lvalue(const const_where_expression& __x) } template - [[nodiscard]] _GLIBCXX_SIMD_INTRINSIC _V + [[nodiscard]] _GLIBCXX_SIMD_INTRINSIC _GLIB
Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization
Hi Juzhe, in general I find the revised structure quite logical and it is definitely an improvement. Some abstraction are still a bit leaky but we can always refactor "on the fly". Some comments on the general parts, skipping over the later details. > bool m_has_dest_p; Why does a store not have a destination (as commented below)? > /* It't true if the pattern uses all trues mask operand. */ > bool m_use_all_trues_mask_p; m_all_unmasked_p or m_fully_unmasked_p? > /* It's true if the pattern uses undefined merge operand. */ > bool m_use_undef_merge_p; Apart from the insn-centric name, couldn't we also decide this based on the context later? In the vector-builtins.cc we have use_real_mask_p and use_real_merge_p that do this. > bool m_has_avl_p; This means "has avl operand" I suppose? From the caller's point of view (and also the vsetvl pass) something like "needs avl" or so would be more descriptive but undecided here. > bool m_vlmax_p; > bool m_has_tail_policy_p; > bool m_has_mask_policy_p; Do we need to expose these in the constructor? As far as I can tell we can decide later whether the instruction has a policy or not (as I did in my patch, depending on whether all inputs are masks or so). > enum tail_policy m_tail_policy; > enum mask_policy m_mask_policy; > machine_mode m_dest_mode; > machine_mode m_mask_mode; Having the mask mode be automatically deduced from the destination is good, it was just obnoxious before to pass ... > Currently, we have "emit_vlmax_tany_many" and "emit_nonvlmax_tany_many". I don't particularly like the names ;) Going back to vlmax and nonvlmax I don't mind but do we really need to have the policies encoded in the name now? Especially since "many" is a word and the default is ANY anyway. Why not emit_vlmax_insn/emit_vlmax_op for now and add the tu/mu later? > #define RVV_BINOP_NUM 3 (number including the output) Could make this into an "instruction type" rather than just a number (i.e. RVV_BINOP) and then set the number of operands internally according to the type. This would also make it clearer in case we later want to set other options depending on the type. > Then just use emit_vlmax_tany_many (...RVV_BINOP_NUM...) > > So, if we support ternary operation in the future. It's quite simple: > #define RVV_TERNOP_NUM 4 (number including the output) > emit_vlmax_tany_many (...RVV_BINOP_NUM...) > > "*_tany_many" means we are using tail any and mask any. > > We will definitely need tail undisturbed or mask undisturbed when we support > these patterns > in middle-end. It's very simple to extend such helper base on current > framework: > > we can do that in the future like this: > > void > emit_nonvlmax_tu_mu (unsigned icode, int op_num, rtx *ops) > { > machine_mode data_mode = GET_MODE (ops[0]); > machine_mode mask_mode = get_mask_mode (data_mode).require (); > /* The number = 11 is because we have maximum 11 operands for > RVV instruction patterns according to vector.md. */ You can just drop the "The number = 11 is because" and say "We have a maximum of 11 operands for...". > insn_expander<11> e (/*OP_NUM*/ op_num, /*HAS_DEST_P*/ true, > /*USE_ALL_TRUES_MASK_P*/ true, > /*USE_UNDEF_MERGE_P*/ true, /*HAS_AVL_P*/ true, > /*VLMAX_P*/ false, > /*HAS_TAIL_POLICY_P*/ true, /*HAS_MASK_POLICY_P*/ true, > /*TAIL_POLICY*/ TAIL_UNDISTURBED, /*MASK_POLICY*/ > MASK_UNDISTURBED, > /*DEST_MODE*/ data_mode, /*MASK_MODE*/ mask_mode); > e.emit_insn ((enum insn_code) icode, ops); > } The eleven arguments seem a bit clunky here ;) I would suggest changing this again in the future bur for now let's just go ahead with it in order to make progress. > - riscv_vector::emit_len_op (code_for_pred_mov (mode), operands[0], > - operands[1], operands[2], mode); > + riscv_vector::emit_nonvlmax_tany_many (code_for_pred_mov (mode), > + RVV_UNOP_NUM, operands); >DONE; > }) The rtx operands[] array I like least of the changes in this patch. It's essentially an untyped array whose meaning is dependent on context containing source operands and the length that is sometimes empty and sometimes not. I can't think of something that wouldn't complicate things though but before we at least had functions called _len that would take a length (NULL or not) and _vlmax that wouldn't. It's pretty easy to mess up here on the caller's side. That said, again, we can always refactor again and let's not let perfect be the enemy of good here. All in all it is not more complicated now than it was before so we should go ahead IMHO. Regards Robin
[COMMITTED] ada: Crash on dispatching primitive referencing limited-with type
From: Javier Miranda The compiler crashes processing a compilation unit has limited-with context clauses, and the profile of some dispatching primitive references a type visible through a limited-with clause, and the dispatching primitive has class-wide preconditions. gcc/ada/ * sem_ch10.adb (Analyze_Required_Limited_With_Units): New subprogram. (Depends_On_Limited_Views): New subprogram. (Has_Limited_With_Clauses): New subprogram. (Analyze_Compilation_Unit): Call the new subprogram that performs the full analysis of required limited-with units. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_ch10.adb | 158 +++ 1 file changed, 158 insertions(+) diff --git a/gcc/ada/sem_ch10.adb b/gcc/ada/sem_ch10.adb index 13357924e64..c9bbd773424 100644 --- a/gcc/ada/sem_ch10.adb +++ b/gcc/ada/sem_ch10.adb @@ -85,6 +85,14 @@ package body Sem_Ch10 is procedure Analyze_Context (N : Node_Id); -- Analyzes items in the context clause of compilation unit + procedure Analyze_Required_Limited_With_Units (N : Node_Id); + -- Subsidiary of Analyze_Compilation_Unit. Perform full analysis of the + -- limited-with units of N when it is a package declaration that does not + -- require a package body, and the profile of some subprogram defined in N + -- depends on shadow incomplete type entities visible through limited-with + -- context clauses. This analysis is required to provide the backend with + -- the non-limited view of these shadow entities. + procedure Build_Limited_Views (N : Node_Id); -- Build and decorate the list of shadow entities for a package mentioned -- in a limited_with clause. If the package was not previously analyzed @@ -1390,6 +1398,13 @@ package body Sem_Ch10 is -- ensure that the pragma/aspect, if present, has been analyzed. Check_No_Elab_Code_All (N); + + -- If this is a main compilation containing a package declaration that + -- requires no package body, and the profile of some subprogram depends + -- on shadow incomplete entities then perform full analysis of its + -- limited-with units. + + Analyze_Required_Limited_With_Units (N); end Analyze_Compilation_Unit; - @@ -2024,6 +2039,149 @@ package body Sem_Ch10 is end if; end Analyze_Protected_Body_Stub; + - + -- Analyze_Required_Limited_With_Units -- + - + + procedure Analyze_Required_Limited_With_Units (N : Node_Id) is + Unit_Node : constant Node_Id := Unit (N); + Spec_Id : constant Entity_Id := Defining_Entity (Unit_Node); + + function Depends_On_Limited_Views (Pkg_Id : Entity_Id) return Boolean; + -- Determines whether the given package has some subprogram with a + -- profile that depends on shadow incomplete type entities of a + -- limited-with unit. + + function Has_Limited_With_Clauses return Boolean; + -- Determines whether the compilation unit N has limited-with context + -- clauses. + + -- + -- Has_Limited_With_Clauses -- + -- + + function Has_Limited_With_Clauses return Boolean is + Item : Node_Id := First (Context_Items (N)); + + begin + while Present (Item) loop +if Nkind (Item) = N_With_Clause + and then Limited_Present (Item) + and then not Implicit_With (Item) +then + return True; +end if; + +Next (Item); + end loop; + + return False; + end Has_Limited_With_Clauses; + + -- + -- Depends_On_Limited_Views -- + -- + + function Depends_On_Limited_Views (Pkg_Id : Entity_Id) return Boolean is + + function Has_Limited_View_Types (Subp : Entity_Id) return Boolean; + -- Determines whether the type of some formal of Subp, or its return + -- type, is a shadow incomplete entity of a limited-with unit. + + + -- Has_Limited_View_Types -- + + + function Has_Limited_View_Types (Subp : Entity_Id) return Boolean is +Formal : Entity_Id := First_Formal (Subp); + + begin +while Present (Formal) loop + if From_Limited_With (Etype (Formal)) + and then Has_Non_Limited_View (Etype (Formal)) + and then Ekind (Non_Limited_View (Etype (Formal))) += E_Incomplete_Type + then + return True; + end if; + + Formal := Next_Formal (Formal); +end loop; + +if Ekind (Subp) = E_Function + and then From_Limited_Wit
[COMMITTED] ada: Minor fix typo in comment
From: Piotr Trojanek Spotted while reviewing a patch with a similar typo. gcc/ada/ * libgnat/s-mmap.adb (Mapped_Region_Record): Fix typo in comment. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/s-mmap.adb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/ada/libgnat/s-mmap.adb b/gcc/ada/libgnat/s-mmap.adb index 60f0db30fb5..abb870ec1c0 100644 --- a/gcc/ada/libgnat/s-mmap.adb +++ b/gcc/ada/libgnat/s-mmap.adb @@ -75,7 +75,7 @@ package body System.Mmap is -- Whether this region is actually memory mapped Mutable : Boolean; - -- If the file is opened for reading, wheter this region is writable + -- If the file is opened for reading, whether this region is writable Buffer: System.Strings.String_Access; -- When this region is not actually memory mapped, contains the -- 2.40.0
[COMMITTED] ada: Small code cleanup
From: Eric Botcazou This just merges two conditional blocks depending on the same condition. gcc/ada/ * frontend.adb (Frontend): Merge two conditional blocks and adjust. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/frontend.adb | 21 - 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/gcc/ada/frontend.adb b/gcc/ada/frontend.adb index d964acd19f8..f2faa0960c6 100644 --- a/gcc/ada/frontend.adb +++ b/gcc/ada/frontend.adb @@ -426,24 +426,17 @@ begin -- Cleanup processing after completing main analysis --- In GNATprove_Mode we do not perform most expansions but body --- instantiation is needed. +pragma Assert (Operating_Mode in Check_Semantics | Generate_Code); -pragma Assert - (Operating_Mode = Generate_Code -or else Operating_Mode = Check_Semantics); +if Operating_Mode = Generate_Code or else GNATprove_Mode then + + -- In GNATprove_Mode we do not perform most expansions but body + -- instantiation is needed. -if Operating_Mode = Generate_Code - or else GNATprove_Mode -then Instantiate_Bodies; -end if; --- Analyze all inlined bodies, check access-before-elaboration --- rules, and remove ignored Ghost code when generating code or --- compiling for GNATprove. + -- Analyze inlined bodies if required -if Operating_Mode = Generate_Code or else GNATprove_Mode then if Inline_Processing_Required then Analyze_Inlined_Bodies; end if; @@ -455,6 +448,8 @@ begin Collect_Garbage_Entities; end if; + -- Check access-before-elaboration rules + if Legacy_Elaboration_Checks then Check_Elab_Calls; end if; -- 2.40.0
[COMMITTED] ada: Fix address arithmetic issues in the runtime
From: Eric Botcazou This is most notably the addition of addresses in Interfaces.C.Pointers and System.Bitfield_Utils. There is also a change to System.Stream_Attributes, which was representing a thin pointer as a record, which is not problematic per se, but is in the end, because the expanded code performs an unchecked conversion from it to the access type instead of accessing the component. gcc/ada/ * libgnat/i-cpoint.adb: Add clauses for System.Storage_Elements. (Addr): Delete. (Offset): New subtype of Storage_Offset. (To_Offset): New instance of Unchecked_Conversion. (To_Pointer): Adjust. (To_Addr): Likewise. (To_Ptrdiff): Likewise. ("+"): Call To_Offset on the offset. ("-"): Likewise. * libgnat/s-bituti.adb: Add clauses for System.Storage_Elements. (Val_Bytes): Change type to Storage_Count. (Get_Val_2): Add qualification to second operand of mod operator. (Set_Val_2): Likewise. (Copy_Bitfield): Likewise. Change type of Src_Adjust & Dest_Adjust. * libgnat/s-stratt.ads (Thin_Pointer): Change to subtype of Address. * libgnat/s-statxd.adb (I_AD): Adjust. (I_AS): Likewise. (W_AS): Likewise. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/i-cpoint.adb | 21 +++-- gcc/ada/libgnat/s-bituti.adb | 17 ++--- gcc/ada/libgnat/s-statxd.adb | 8 gcc/ada/libgnat/s-stratt.ads | 4 +--- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/gcc/ada/libgnat/i-cpoint.adb b/gcc/ada/libgnat/i-cpoint.adb index bf08e1a74ac..e1805f497de 100644 --- a/gcc/ada/libgnat/i-cpoint.adb +++ b/gcc/ada/libgnat/i-cpoint.adb @@ -29,19 +29,20 @@ -- -- -- -with Interfaces.C.Strings; use Interfaces.C.Strings; -with System; use System; +with Interfaces.C.Strings;use Interfaces.C.Strings; +with System.Storage_Elements; use System.Storage_Elements; +with System; use System; with Ada.Unchecked_Conversion; package body Interfaces.C.Pointers is - type Addr is mod 2 ** System.Parameters.ptr_bits; + subtype Offset is Storage_Offset; - function To_Pointer is new Ada.Unchecked_Conversion (Addr, Pointer); - function To_Addris new Ada.Unchecked_Conversion (Pointer, Addr); - function To_Addris new Ada.Unchecked_Conversion (ptrdiff_t, Addr); - function To_Ptrdiff is new Ada.Unchecked_Conversion (Addr, ptrdiff_t); + function To_Pointer is new Ada.Unchecked_Conversion (Address, Pointer); + function To_Addris new Ada.Unchecked_Conversion (Pointer, Address); + function To_Offset is new Ada.Unchecked_Conversion (ptrdiff_t, Offset); + function To_Ptrdiff is new Ada.Unchecked_Conversion (Offset,ptrdiff_t); Elmt_Size : constant ptrdiff_t := (Element_Array'Component_Size @@ -59,7 +60,7 @@ package body Interfaces.C.Pointers is raise Pointer_Error; end if; - return To_Pointer (To_Addr (Left) + To_Addr (Elmt_Size * Right)); + return To_Pointer (To_Addr (Left) + To_Offset (Elmt_Size * Right)); end "+"; function "+" (Left : ptrdiff_t; Right : Pointer) return Pointer is @@ -68,7 +69,7 @@ package body Interfaces.C.Pointers is raise Pointer_Error; end if; - return To_Pointer (To_Addr (Elmt_Size * Left) + To_Addr (Right)); + return To_Pointer (To_Offset (Elmt_Size * Left) + To_Addr (Right)); end "+"; - @@ -81,7 +82,7 @@ package body Interfaces.C.Pointers is raise Pointer_Error; end if; - return To_Pointer (To_Addr (Left) - To_Addr (Right * Elmt_Size)); + return To_Pointer (To_Addr (Left) - To_Offset (Right * Elmt_Size)); end "-"; function "-" (Left : Pointer; Right : Pointer) return ptrdiff_t is diff --git a/gcc/ada/libgnat/s-bituti.adb b/gcc/ada/libgnat/s-bituti.adb index 1b0acc18d68..28e41f36b14 100644 --- a/gcc/ada/libgnat/s-bituti.adb +++ b/gcc/ada/libgnat/s-bituti.adb @@ -29,11 +29,13 @@ -- -- -- +with System.Storage_Elements; use System.Storage_Elements; + package body System.Bitfield_Utils is package body G is - Val_Bytes : constant Address := Address (Val'Size / Storage_Unit); + Val_Bytes : constant Storage_Count := Val'Size / Storage_Unit; -- A Val_2 can cross a memory page boundary (e.g. an 8-byte Val_2 that -- starts 4 bytes before the end of a page). If the bit field also @@ -119,7 +121,7 @@ package body System.Bitfield_Utils is Size : Small_Size) return Val_2 is begin - pragma Assert (Src_Address mod Val'Alignment = 0); +
[COMMITTED] ada: Remove duplicate comment
From: Ronan Desplanques gcc/ada/ * sem_ch7.adb: Remove duplicate comment. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_ch7.adb | 4 1 file changed, 4 deletions(-) diff --git a/gcc/ada/sem_ch7.adb b/gcc/ada/sem_ch7.adb index 5021d0ee04f..2610a7bba4d 100644 --- a/gcc/ada/sem_ch7.adb +++ b/gcc/ada/sem_ch7.adb @@ -3197,10 +3197,6 @@ package body Sem_Ch7 is -- is simply that the initializing expression is missing. if not Has_Private_Declaration (Etype (Id)) then - - -- We assume that the user did not intend a deferred constant - -- declaration, and the expression is just missing. - Error_Msg_N ("constant declaration requires initialization expression", Parent (Id)); -- 2.40.0
[COMMITTED] ada: Transfer fix for pretty-printed parentheses from GNATprove to GNAT
From: Piotr Trojanek Expressions with parentheses are notoriously problematic to pretty-print. In GNATprove we had a post-processing fix for them, but it is better to have this fix in the GNAT frontend repository and apply it for CodePeer as well. gcc/ada/ * pprint.adb (Expression_Image): Move Count_Parentheses and Fix_Parentheses routines from GNATprove and apply them before returning the slice of a source code buffer. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/pprint.adb | 70 +- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/gcc/ada/pprint.adb b/gcc/ada/pprint.adb index 8add10c8f22..bcc7a257630 100644 --- a/gcc/ada/pprint.adb +++ b/gcc/ada/pprint.adb @@ -64,6 +64,19 @@ package body Pprint is -- Expand_Type is True and Expr is a type, try to expand Expr (an -- internally generated type) into a user understandable name. + function Count_Parentheses (S : String; C : Character) return Natural +with Pre => C in '(' | ')'; + -- Returns the number of times parenthesis character C should be added + -- to string S for getting a correctly parenthesized result. For C = '(' + -- this means prepending the character, for C = ')' this means appending + -- the character. + + function Fix_Parentheses (S : String) return String; + -- Counts the number of required opening and closing parentheses in S to + -- respectively prepend and append for getting correct parentheses. Then + -- returns S with opening parentheses prepended and closing parentheses + -- appended so that the result is correctly parenthesized. + Max_List_Depth : constant := 3; -- Limit number of nested lists to print @@ -650,6 +663,61 @@ package body Pprint is end case; end Expr_Name; + --- + -- Count_Parentheses -- + --- + + function Count_Parentheses (S : String; C : Character) return Natural is + + procedure Next_Char (Count : in out Natural; C, D, Ch : Character); + -- Process next character Ch and update the number Count of C + -- characters to add for correct parenthesizing, where D is the + -- opposite parenthesis. + + --- + -- Next_Char -- + --- + + procedure Next_Char (Count : in out Natural; C, D, Ch : Character) is + begin +if Ch = D then + Count := Count + 1; +elsif Ch = C and then Count > 0 then + Count := Count - 1; +end if; + end Next_Char; + + -- Local variables + + Count : Natural := 0; + + -- Start of processing for Count_Parentheses + + begin + if C = '(' then +for Ch of reverse S loop + Next_Char (Count, C, ')', Ch); +end loop; + else +for Ch of S loop + Next_Char (Count, C, '(', Ch); +end loop; + end if; + + return Count; + end Count_Parentheses; + + - + -- Fix_Parentheses -- + - + + function Fix_Parentheses (S : String) return String is + Count_Open : constant Natural := Count_Parentheses (S, '('); + Count_Close : constant Natural := Count_Parentheses (S, ')'); + begin + return (1 .. Count_Open => '(') & S & (1 .. Count_Close => ')'); + end Fix_Parentheses; + -- Local variables Left, Right : Source_Ptr; @@ -775,7 +843,7 @@ package body Pprint is Scn := Scn + 1; end loop; - return Buffer (1 .. Index); + return Fix_Parentheses (Buffer (1 .. Index)); end; end Expression_Image; -- 2.40.0
[COMMITTED] ada: Sync different variants of interrupt handler registration
From: Piotr Trojanek This patch propagates the apparently cleanest solutions between various variants of the runtime units for interrupt handler registration. In particular, the unnecessary default expressions for list cells with interrupt handler addresses are removed, the list is changed from doubly-linked to singly-linked, and assertion preventing registration of null addresses now appears in all runtimes. Effectively, it is just a code cleanup and minor optimization; behavior of the runtime unit is unaffected. gcc/ada/ * libgnarl/s-interr.adb (Registered_Handler): Remove default expression. (Registered_Handlers): Switch to singly-linked list. (Bind_Interrupt_To_Entry): Sync whitespace with other unit variants. (Is_Registered): Use singly-linked list. (Register_Interrupt_Handler): Use singly-linked list and initialized allocator; sync assertion with other unit variants. * libgnarl/s-interr__sigaction.adb: Likewise. * libgnarl/s-interr__vxworks.adb: Likewise. * libgnarl/s-interr__hwint.adb: Likewise. (Is_Registered): Remove repeated declaration. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnarl/s-interr.adb| 36 +--- gcc/ada/libgnarl/s-interr__hwint.adb | 34 +- gcc/ada/libgnarl/s-interr__sigaction.adb | 22 +++ gcc/ada/libgnarl/s-interr__vxworks.adb | 34 +- 4 files changed, 56 insertions(+), 70 deletions(-) diff --git a/gcc/ada/libgnarl/s-interr.adb b/gcc/ada/libgnarl/s-interr.adb index d28c8f9736b..7a231681272 100644 --- a/gcc/ada/libgnarl/s-interr.adb +++ b/gcc/ada/libgnarl/s-interr.adb @@ -187,20 +187,23 @@ package body System.Interrupts is -- needed to accomplish locking per Interrupt base. Also is needed to -- decide whether to create a new Server_Task. - -- Type and Head, Tail of the list containing Registered Interrupt - -- Handlers. These definitions are used to register the handlers - -- specified by the pragma Interrupt_Handler. + -- Type and the list containing Registered Interrupt Handlers. These + -- definitions are used to register the handlers specified by the pragma + -- Interrupt_Handler. + + -- + -- Handler Registration -- + -- type Registered_Handler; type R_Link is access all Registered_Handler; type Registered_Handler is record - H: System.Address := System.Null_Address; - Next : R_Link := null; + H: System.Address; + Next : R_Link; end record; - Registered_Handler_Head : R_Link := null; - Registered_Handler_Tail : R_Link := null; + Registered_Handlers : R_Link := null; Access_Hold : Server_Task_Access; -- Variable used to allocate Server_Task using "new" @@ -254,7 +257,6 @@ package body System.Interrupts is is Interrupt : constant Interrupt_ID := Interrupt_ID (Storage_Elements.To_Integer (Int_Ref)); - begin if Is_Reserved (Interrupt) then raise Program_Error with @@ -538,6 +540,7 @@ package body System.Interrupts is --- function Is_Registered (Handler : Parameterless_Handler) return Boolean is + Ptr : R_Link := Registered_Handlers; type Acc_Proc is access procedure; @@ -549,7 +552,6 @@ package body System.Interrupts is function To_Fat_Ptr is new Ada.Unchecked_Conversion (Parameterless_Handler, Fat_Ptr); - Ptr : R_Link; Fat : Fat_Ptr; begin @@ -559,7 +561,6 @@ package body System.Interrupts is Fat := To_Fat_Ptr (Handler); - Ptr := Registered_Handler_Head; while Ptr /= null loop if Ptr.H = Fat.Handler_Addr.all'Address then return True; @@ -600,8 +601,6 @@ package body System.Interrupts is - procedure Register_Interrupt_Handler (Handler_Addr : System.Address) is - New_Node_Ptr : R_Link; - begin -- This routine registers the Handler as usable for Dynamic Interrupt -- Handler. Routines attaching and detaching Handler dynamically should @@ -615,17 +614,8 @@ package body System.Interrupts is pragma Assert (Handler_Addr /= System.Null_Address); - New_Node_Ptr := new Registered_Handler; - New_Node_Ptr.H := Handler_Addr; - - if Registered_Handler_Head = null then - Registered_Handler_Head := New_Node_Ptr; - Registered_Handler_Tail := New_Node_Ptr; - - else - Registered_Handler_Tail.Next := New_Node_Ptr; - Registered_Handler_Tail := New_Node_Ptr; - end if; + Registered_Handlers := + new Registered_Handler'(H => Handler_Addr, Next => Registered_Handlers); end Register_Interrupt_Handler; --- diff --git a/gcc/ada/libgnarl/s-interr__hwint.adb b/gcc/ada/libgnarl/s-interr__hwint.adb index 441083
[COMMITTED] ada: Add new switch -gnatyz
From: Arnaud Charlet Improve -gnatyx to check additional complete conditions, and introduce a new switch -gnatyz to check for unnecessary parentheses according to operator precedence rules. Enable -gnatyz as part of -gnatyg. gcc/ada/ * par-ch5.adb, style.ads, styleg.adb, styleg.ads (Check_Xtra_Parens): Remove extra parameter Enable. (Check_Xtra_Parens_Precedence): New. (P_Case_Statement): Add -gnatyx style check. * sem_ch4.adb: Replace calls to Check_Xtra_Parens by Check_Xtra_Parens_Precedence. * stylesw.ads, stylesw.adb, usage.adb: Add support for -gnatyz. * doc/gnat_ugn/building_executable_programs_with_gnat.rst: Update -gnatyxzg doc. * sem_prag.adb, libgnat/s-regpat.adb, libgnarl/s-interr__hwint.adb, libgnarl/s-interr__vxworks.adb: Remove extra parens. * par-ch3.adb (P_Discrete_Range): Do not emit a style check if the expression is not a simple expression. * gnat_ugn.texi: Regenerate. Tested on x86_64-pc-linux-gnu, committed on master. --- ...building_executable_programs_with_gnat.rst | 23 ++- gcc/ada/gnat_ugn.texi | 27 +++- gcc/ada/libgnarl/s-interr__hwint.adb | 2 +- gcc/ada/libgnarl/s-interr__vxworks.adb| 2 +- gcc/ada/libgnat/s-regpat.adb | 2 +- gcc/ada/par-ch3.adb | 7 +- gcc/ada/par-ch5.adb | 12 +- gcc/ada/sem_ch4.adb | 8 +- gcc/ada/sem_prag.adb | 2 +- gcc/ada/style.ads | 12 +- gcc/ada/styleg.adb| 27 +++- gcc/ada/styleg.ads| 11 +- gcc/ada/stylesw.adb | 146 +- gcc/ada/stylesw.ads | 5 + gcc/ada/usage.adb | 2 + 15 files changed, 180 insertions(+), 108 deletions(-) diff --git a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst index 7968073a985..2838a302f2e 100644 --- a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst +++ b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst @@ -4706,7 +4706,7 @@ Style Checking .. index:: -gnaty (gcc) -The :switch:`-gnatyx` switch causes the compiler to +The :switch:`-gnaty` switch causes the compiler to enforce specified style rules. A limited set of style rules has been used in writing the GNAT sources themselves. This switch allows user programs to activate all or some of these checks. If the source program fails a @@ -4904,9 +4904,9 @@ checks to be performed. The following checks are defined: The set of style check switches is set to match that used by the GNAT sources. This may be useful when developing code that is eventually intended to be - incorporated into GNAT. Currently this is equivalent to :switch:`-gnatyydISux`) - but additional style switches may be added to this set in the future without - advance notice. + incorporated into GNAT. Currently this is equivalent to + :switch:`-gnatyydISuxz`) but additional style switches may be added to this + set in the future without advance notice. .. index:: -gnatyh (gcc) @@ -5207,9 +5207,9 @@ checks to be performed. The following checks are defined: :switch:`-gnatyx` *Check extra parentheses.* - Unnecessary extra level of parentheses (C-style) are not allowed - around conditions in ``if`` statements, ``while`` statements and - ``exit`` statements. + Unnecessary extra levels of parentheses (C-style) are not allowed + around conditions (or selection expressions) in ``if``, ``while``, + ``case``, and ``exit`` statements, as well as part of ranges. .. index:: -gnatyy (gcc) @@ -5223,6 +5223,15 @@ checks to be performed. The following checks are defined: :switch:`-gnatyS`, :switch:`-gnatyu`, and :switch:`-gnatyx`. +.. index:: -gnatyz (gcc) + +:switch:`-gnatyz` + *Check extra parentheses (operator precedence).* + + Extra levels of parentheses that are not required by operator precedence + rules are flagged. See also ``-gnatyx``. + + .. index:: -gnaty- (gcc) :switch:`-gnaty-` diff --git a/gcc/ada/gnat_ugn.texi b/gcc/ada/gnat_ugn.texi index b95519a8295..e1242068f56 100644 --- a/gcc/ada/gnat_ugn.texi +++ b/gcc/ada/gnat_ugn.texi @@ -13312,7 +13312,7 @@ temporary disabling of validity checks. @geindex -gnaty (gcc) -The @code{-gnatyx} switch causes the compiler to +The @code{-gnaty} switch causes the compiler to enforce specified style rules. A limited set of style rules has been used in writing the GNAT sources themselves. This switch allows user programs to activate all or some of these checks. If the source program fails a @@ -13569,9 +13569,9 @@ in the source text. The set of style check switches is set to match that used by the GNAT sources. Th
[COMMITTED] ada: Fix expression pretty-printer for SPARK counterexamples
From: Piotr Trojanek The expression pretty-printer that is used for SPARK counterexamples was essentially duplicating the logic of First_Node/Last_Node and First_Sloc/Last_Sloc routines. Now it simply reuses those routines. gcc/ada/ * errout.adb (Paren_Required): New subsidiary routine for better handling of parentheses in First_Node/Last_Node. (First_Sloc, Last_Sloc): Use Get_Source_File_Index to correctly handle generic instances and inlined subprograms; tune handling of parentheses; improve handling of literals. * pprint.adb (Expression_Image): Simplify using First_Sloc/Last_Sloc. * sem_ch6.adb (Analyze_Expression_Function): Remove parenthesis when relocating expression from expression function to simple return statement. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/errout.adb | 97 ++-- gcc/ada/pprint.adb | 377 ++-- gcc/ada/sem_ch6.adb | 7 + 3 files changed, 146 insertions(+), 335 deletions(-) diff --git a/gcc/ada/errout.adb b/gcc/ada/errout.adb index a82aff5266b..0a36a1d7466 100644 --- a/gcc/ada/errout.adb +++ b/gcc/ada/errout.adb @@ -50,6 +50,7 @@ with Sinfo.Nodes;use Sinfo.Nodes; with Sinfo.Utils;use Sinfo.Utils; with Snames; use Snames; with Stand; use Stand; +with Stringt;use Stringt; with Stylesw;use Stylesw; with System.OS_Lib; with Uname; use Uname; @@ -139,6 +140,11 @@ package body Errout is -- indicates if there are errors attached to the line, which forces -- listing on, even in the presence of pragma List (Off). + function Paren_Required (N : Node_Id) return Boolean; + -- Subsidiary to First_Sloc and Last_Sloc. Returns True iff parentheses + -- around node N are required by the Ada syntax, e.g. when N is an + -- expression of a qualified expression. + procedure Set_Msg_Insertion_Column; -- Handle column number insertion (@ insertion character) @@ -1845,7 +1851,7 @@ package body Errout is function First_Sloc (N : Node_Id) return Source_Ptr is - SI : constant Source_File_Index := Source_Index (Get_Source_Unit (N)); + SI : constant Source_File_Index := Get_Source_File_Index (Sloc (N)); SF : constant Source_Ptr:= Source_First (SI); SL : constant Source_Ptr:= Source_Last (SI); Src : constant Source_Buffer_Ptr := Source_Text (SI); @@ -1869,6 +1875,12 @@ package body Errout is -- values), but this is only for an error message so it is good enough. Node_Loop : loop + -- Include parentheses around the top node, except when they are + -- required by the syntax of the parent node. + + exit Node_Loop when F = N + and then Paren_Required (N); + Paren_Loop : for J in 1 .. Paren_Count (F) loop -- We don't look more than 12 characters behind the current @@ -1964,7 +1976,7 @@ package body Errout is --- function Last_Sloc (N : Node_Id) return Source_Ptr is - SI : constant Source_File_Index := Source_Index (Get_Source_Unit (N)); + SI : constant Source_File_Index := Get_Source_File_Index (Sloc (N)); SF : constant Source_Ptr:= Source_First (SI); SL : constant Source_Ptr:= Source_Last (SI); Src : constant Source_Buffer_Ptr := Source_Text (SI); @@ -1979,21 +1991,69 @@ package body Errout is return S; end if; - -- Skip past an identifier + -- For string and character literals simply forward the sloc by their + -- length including the closing quotes. Perhaps we should do something + -- special for multibyte characters, but this code is only used to emit + -- error messages, so it is not worth the effort. - while S in SF .. SL - 1 -and then Src (S + 1) - in -'0' .. '9' | 'a' .. 'z' | 'A' .. 'Z' | '.' | '_' - loop - S := S + 1; - end loop; + case Nkind (F) is + when N_String_Literal => +return S + Source_Ptr (String_Length (Strval (F))) + 1; + + when N_Character_Literal => +return S + 2; + + -- Skip past numeric literals, but they allow a wider set of + -- characters than keywords and identifiers. + + when N_Integer_Literal => +while S in SF .. SL - 1 + and then Src (S + 1) +in + '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' | '#' | '+' | '-' +loop + S := S + 1; +end loop; + + when N_Real_Literal => +declare + Dot_Seen : Boolean := False; +begin + while S in SF .. SL - 1 + and then + (Src (S + 1) in '0' .. '9' + | 'a' .. 'f' | 'A' .. 'F' +
[COMMITTED] ada: Revert to old pretty-printing of internal entities for CodePeer
From: Piotr Trojanek Add some defensive code to get pretty-printed CodePeer outputs for ACATS back to shape. At least some of this code appears to be redundant and perhaps unnecessary, but we can this up later. Expression pretty-printer should not be called with N_Defining_Identifier nodes at all, since they are not expressions. However, for those that come not from source, CodePeer expects that the Ident_Image routine will convert the internal names like "xL" to their corresponding human-readable representation like "x'Accesibility_Level". gcc/ada/ * pprint.adb (Expression_Image): Restore some of the old pretty-printing for CodePeer. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/pprint.adb | 22 +++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/gcc/ada/pprint.adb b/gcc/ada/pprint.adb index bcc7a257630..3843ec203b0 100644 --- a/gcc/ada/pprint.adb +++ b/gcc/ada/pprint.adb @@ -731,9 +731,11 @@ package body Pprint is -- itself, but for now simply return the default (if present) or print -- name of the defining identifier. - if Nkind (Expr) not in N_Subexpr then + if Nkind (Expr) = N_Defining_Identifier then pragma Assert (CodePeer_Mode); - if Nkind (Expr) = N_Defining_Identifier then + if Comes_From_Source (Expr) + or else Opt.Debug_Generated_Code + then if Default = "" then declare Nam : constant Name_Id := Chars (Expr); @@ -748,10 +750,24 @@ package body Pprint is return Default; end if; else -raise Program_Error; +declare + S : constant String := + Ident_Image + (Expr => Expr, Orig_Expr => Expr, Expand_Type => True); +begin + if S = "..." then + return Default; + else + return S; + end if; +end; end if; + else + pragma Assert (Nkind (Expr) in N_Subexpr); end if; + -- ??? The following should be primarily needed for CodePeer + if not Comes_From_Source (Expr) or else Opt.Debug_Generated_Code then -- 2.40.0
[COMMITTED] ada: Fix endings of pretty-printed numeric literals
From: Piotr Trojanek When looking for the end of an numeric literal we consumed '+' and '-' characters, because they might appear in the exponent part. This was too aggressive when they separated the number from the subsequent operand, like in "123+456". Now we skip past numeric literals by strictly following their grammar and only consume '+' and '-' when they belong to the exponent. gcc/ada/ * errout.adb (Last_Sloc): Rewrite skipping past numeric literals. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/errout.adb | 130 +++-- 1 file changed, 113 insertions(+), 17 deletions(-) diff --git a/gcc/ada/errout.adb b/gcc/ada/errout.adb index 5c3e76c5eca..6531410f0d2 100644 --- a/gcc/ada/errout.adb +++ b/gcc/ada/errout.adb @@ -2021,33 +2021,129 @@ package body Errout is when N_Character_Literal => return S + 2; - -- Skip past numeric literals, but they allow a wider set of - -- characters than keywords and identifiers. + -- Skip past integer literals, both decimal and based, integer and + -- real. We can't greedily accept all allowed character, because + -- we would consme too many of them in expressions like "123+ABC" + -- or "123..456", so we follow quite precisely the Ada grammar and + -- consume different characters depending on the context. when N_Integer_Literal => -while S in SF .. SL - 1 - and then Src (S + 1) -in - '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' | '#' | '+' | '-' + +-- Skip past the initial numeral, which either leads the decimal +-- literal or is the base of a based literal. + +while S < SL + and then Src (S + 1) in '0' .. '9' | '_' loop S := S + 1; end loop; - when N_Real_Literal => -declare - Dot_Seen : Boolean := False; -begin - while S in SF .. SL - 1 +-- Skip past #based_numeral#, if present + +if S < SL + and then Src (S + 1) = '#' +then + S := S + 1; + + while S < SL and then - (Src (S + 1) in '0' .. '9' - | 'a' .. 'f' | 'A' .. 'F' - | '_' | '#' | '+' | '-' - or else (Src (S + 1) = '.' and then not Dot_Seen)) + Src (S + 1) in '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' loop - Dot_Seen := Src (S + 1) = '.'; S := S + 1; end loop; -end; + + pragma Assert (S < SL and then Src (S + 1) = '#'); + + S := S + 1; +end if; + +-- Skip past exponent, if present + +if S < SL + 1 + and then Src (S + 1) in 'e' | 'E' +then + -- For positive exponents the plus sign is optional, but we + -- can simply skip past both plus and minus. + + if Src (S + 2) in '+' | '-' then + S := S + 1; + end if; + + -- Skip past the numeral part + + while S < SL + and then Src (S + 1) in '0' .. '9' | '_' + loop + S := S + 1; + end loop; +end if; + + when N_Real_Literal => +-- Skip past the initial numeral, which either leads the decimal +-- literal or is the base of a based literal. + +while S < SL + and then Src (S + 1) in '0' .. '9' | '_' +loop + S := S + 1; +end loop; + +if S < SL then + if Src (S + 1) = '.' then + while S < SL +and then Src (S + 1) in '0' .. '9' | '_' + loop + S := S + 1; + end loop; + else + pragma Assert (Src (S + 1) = '#'); + + S := S + 1; + + while S < SL +and then + Src (S + 1) in '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' + loop + S := S + 1; + end loop; + + pragma Assert (S < SL and then Src (S + 1) = '.'); + + S := S + 1; + + while S < SL +and then + Src (S + 1) in '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' + loop + S := S + 1; + end loop; + + pragma Assert (S < SL and then Src (S + 1) = '#'); + + S := S + 1; + end if; +end if; + +-- Skip past exponent, if present + +if S < S
[COMMITTED] ada: Fix address manipulation issue in the tasking runtime
From: Eric Botcazou The implementation of task attributes in the runtime defines an atomic clone of System.Address, which is awkward for targets where addresses and pointers have a specific representation, so this change replaces that with a pragma Atomic_Components on the Attribute_Array type. gcc/ada/ * libgnarl/s-taskin.ads (Atomic_Address): Delete. (Attribute_Array): Add pragma Atomic_Components. (Ada_Task_Control_Block): Adjust default value of Attributes. * libgnarl/s-tasini.adb (Finalize_Attributes): Adjust type of local variable. * libgnarl/s-tataat.ads (Deallocator): Adjust type of parameter. (To_Attribute): Adjust source type. * libgnarl/a-tasatt.adb: Add clauses for System.Storage_Elements. (New_Attribute): Adjust return type. (Deallocate): Adjust type of parameter. (To_Real_Attribute): Adjust source type. (To_Address): Add target type. (To_Attribute): Adjust source type. (Fast_Path): Adjust tested type. (Finalize): Compare with Null_Address. (Reference): Likewise. (Reinitialize): Likewise. (Set_Value): Likewise. Add conversion to Integer_Address. (Value): Likewise. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnarl/a-tasatt.adb | 51 ++- gcc/ada/libgnarl/s-tasini.adb | 2 +- gcc/ada/libgnarl/s-taskin.ads | 9 +++ gcc/ada/libgnarl/s-tataat.ads | 4 +-- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/gcc/ada/libgnarl/a-tasatt.adb b/gcc/ada/libgnarl/a-tasatt.adb index fb3ca682f15..6111f2987a5 100644 --- a/gcc/ada/libgnarl/a-tasatt.adb +++ b/gcc/ada/libgnarl/a-tasatt.adb @@ -29,6 +29,7 @@ -- -- -- +with System.Storage_Elements; with System.Tasking; with System.Tasking.Initialization; with System.Tasking.Task_Attributes; @@ -43,6 +44,7 @@ with Ada.Unchecked_Deallocation; package body Ada.Task_Attributes is use System, + System.Storage_Elements, System.Tasking.Initialization, System.Tasking, System.Tasking.Task_Attributes; @@ -75,34 +77,32 @@ package body Ada.Task_Attributes is -- System.Tasking.Task_Attributes.Attribute_Record and to allow unchecked -- conversions between Attribute_Access and Real_Attribute_Access. - function New_Attribute (Val : Attribute) return Atomic_Address; + function New_Attribute (Val : Attribute) return System.Address; -- Create a new Real_Attribute using Val, and return its address. The -- returned value can be converted via To_Real_Attribute. - procedure Deallocate (Ptr : Atomic_Address); + procedure Deallocate (Ptr : System.Address); -- Free memory associated with Ptr, a Real_Attribute_Access in reality function To_Real_Attribute is new - Ada.Unchecked_Conversion (Atomic_Address, Real_Attribute_Access); + Ada.Unchecked_Conversion (System.Address, Real_Attribute_Access); pragma Warnings (Off); -- Kill warning about possible size mismatch function To_Address is new - Ada.Unchecked_Conversion (Attribute, Atomic_Address); + Ada.Unchecked_Conversion (Attribute, System.Address); function To_Attribute is new - Ada.Unchecked_Conversion (Atomic_Address, Attribute); + Ada.Unchecked_Conversion (System.Address, Attribute); type Unsigned is mod 2 ** Integer'Size; - function To_Address is new - Ada.Unchecked_Conversion (Attribute, System.Address); function To_Unsigned is new Ada.Unchecked_Conversion (Attribute, Unsigned); pragma Warnings (On); function To_Address is new - Ada.Unchecked_Conversion (Real_Attribute_Access, Atomic_Address); + Ada.Unchecked_Conversion (Real_Attribute_Access, System.Address); pragma Warnings (Off); -- Kill warning about possible aliasing @@ -121,12 +121,12 @@ package body Ada.Task_Attributes is Fast_Path : constant Boolean := (Attribute'Size = Integer'Size - and then Attribute'Alignment <= Atomic_Address'Alignment + and then Attribute'Alignment <= System.Address'Alignment and then To_Unsigned (Initial_Value) = 0) or else (Attribute'Size = System.Address'Size - and then Attribute'Alignment <= Atomic_Address'Alignment - and then To_Address (Initial_Value) = System.Null_Address); - -- If the attribute fits in an Atomic_Address (both size and alignment) + and then Attribute'Alignment <= System.Address'Alignment + and then To_Address (Initial_Value) = Null_Address); + -- If the attribute fits in a System.Address (both size and alignment) -- and Initial_Value is 0 (or null), then we will map the attribute -- directly in
[COMMITTED] ada: Add mention of what LSP stands for
From: Ronan Desplanques There are multiple possible interpretations of "LSP". For example, "Language Server Protocol". This patch clarifies that the occurrences of "LSP" in GNAT's source code refer to the Liskov Substitution Principle. gcc/ada/ * einfo.ads: Mention full name of LSP. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/einfo.ads | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/ada/einfo.ads b/gcc/ada/einfo.ads index 0cc4b495bd9..d346eddac57 100644 --- a/gcc/ada/einfo.ads +++ b/gcc/ada/einfo.ads @@ -4786,7 +4786,8 @@ package Einfo is -- Defined in subprogram entities. Set on wrappers created to handle -- inherited class-wide pre/post conditions that call overridden -- primitives. It references the parent primitive that has the --- class-wide pre/post conditions. +-- class-wide pre/post conditions. LSP stands for Liskov Substitution +-- Principle. --- -- Renaming and Aliasing -- -- 2.40.0
[COMMITTED] ada: Suppress warning about Subprogram_Variant failing at run time
From: Piotr Trojanek Warning about check failing at run time is likely spurious for mutually recursive subprograms with multiple variant clauses. These will be non-trivial to detect, so we simply suppress the warning altogether for all subprogram variants. gcc/ada/ * exp_prag.adb (Expand_Pragma_Check): Suppress warning for checks of subprogram variants. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_prag.adb | 7 +++ 1 file changed, 7 insertions(+) diff --git a/gcc/ada/exp_prag.adb b/gcc/ada/exp_prag.adb index ceb27848dad..c6b3bed5425 100644 --- a/gcc/ada/exp_prag.adb +++ b/gcc/ada/exp_prag.adb @@ -564,6 +564,13 @@ package body Exp_Prag is then null; + -- For Subprogram_Variant suppress the warning altogether, because + -- for mutually recursive subprograms with multiple variant clauses + -- some of the clauses might have expressions that are only meant for + -- verification and would always fail when executed. + + elsif Nam = Name_Subprogram_Variant then +null; elsif Nam = Name_Assert then Error_Msg_N ("?.a?assertion will fail at run time", N); else -- 2.40.0
[COMMITTED] ada: Spurious errors on class-wide preconditions of private types
From: Javier Miranda The compiler reports spurious errors processing the class-wide preconditions of a dispatching primitive of a private type T, when the class-wide precondition invokes another dispatching primitive of T that has the same name as a record component of T. gcc/ada/ * sem_prag.adb (Analyze_Pre_Post_Condition_In_Decl_Part): Remove call to preanalyze class-wide conditions since here it is too early; they must be preanalyzed when full views of private types have been analyzed. * sem_ch7.adb (Analyze_Package_Specification): Preanalyze class-wide conditions of dispatching primitives defined in nested packages. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_ch7.adb | 14 ++ gcc/ada/sem_prag.adb | 14 -- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gcc/ada/sem_ch7.adb b/gcc/ada/sem_ch7.adb index 2610a7bba4d..aed09f30934 100644 --- a/gcc/ada/sem_ch7.adb +++ b/gcc/ada/sem_ch7.adb @@ -1936,6 +1936,20 @@ package body Sem_Ch7 is end; end if; + -- Preanalyze class-wide conditions of dispatching primitives defined + -- in nested packages. For library packages, class-wide pre- and + -- postconditions are preanalyzed when the primitives are frozen + -- (see Merge_Class_Conditions); for nested packages, the end of the + -- package does not cause freezing (and hence they must be analyzed + -- now to ensure the correct visibility of referenced entities). + + if not Is_Compilation_Unit (Id) + and then Is_Dispatching_Operation (E) + and then Present (Contract (E)) + then +Preanalyze_Class_Conditions (E); + end if; + Next_Entity (E); end loop; diff --git a/gcc/ada/sem_prag.adb b/gcc/ada/sem_prag.adb index 234b02d7b72..ca55a6f8290 100644 --- a/gcc/ada/sem_prag.adb +++ b/gcc/ada/sem_prag.adb @@ -26269,20 +26269,6 @@ package body Sem_Prag is Check_Postcondition_Use_In_Inlined_Subprogram (N, Spec_Id); Set_Is_Analyzed_Pragma (N); - -- If the subprogram is frozen then its class-wide pre- and post- - -- conditions have been preanalyzed (see Merge_Class_Conditions); - -- otherwise they must be preanalyzed now to ensure the correct - -- visibility of their referenced entities. This scenario occurs - -- when the subprogram is defined in a nested package (since the - -- end of the package does not cause freezing). - - if Class_Present (N) -and then Is_Dispatching_Operation (Spec_Id) -and then not Is_Frozen (Spec_Id) - then - Preanalyze_Class_Conditions (Spec_Id); - end if; - Restore_Ghost_Region (Saved_GM, Saved_IGR); end Analyze_Pre_Post_Condition_In_Decl_Part; -- 2.40.0
[COMMITTED] ada: Make string interpolation part of the core extensions
From: Raphael Amiard gcc/ada/ * scng.adb (Scan): Replace occurrences of All_Extensions_Allowed by Core_Extensions_Allowed. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/scng.adb | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gcc/ada/scng.adb b/gcc/ada/scng.adb index d1230e2efe1..abf9c68cd3d 100644 --- a/gcc/ada/scng.adb +++ b/gcc/ada/scng.adb @@ -1527,10 +1527,10 @@ package body Scng is end if; -- Left curly bracket, treated as right paren but proper delimiter - -- of interpolated string literals when all extensions are allowed. + -- of interpolated string literals when core extensions are allowed. when '{' => -if All_Extensions_Allowed then +if Core_Extensions_Allowed then Scan_Ptr := Scan_Ptr + 1; Token := Tok_Left_Curly_Bracket; @@ -1962,10 +1962,10 @@ package body Scng is return; -- Right curly bracket, treated as right paren but proper delimiter - -- of interpolated string literals when all extensions are allowed. + -- of interpolated string literals when core extensions are allowed. when '}' => -if All_Extensions_Allowed then +if Core_Extensions_Allowed then Token := Tok_Right_Curly_Bracket; else @@ -2125,7 +2125,7 @@ package body Scng is -- Lower case letters when 'a' .. 'z' => -if All_Extensions_Allowed +if Core_Extensions_Allowed and then Source (Scan_Ptr) = 'f' and then Source (Scan_Ptr + 1) = '"' then @@ -2145,7 +2145,7 @@ package body Scng is -- Upper case letters when 'A' .. 'Z' => -if All_Extensions_Allowed +if Core_Extensions_Allowed and then Source (Scan_Ptr) = 'F' and then Source (Scan_Ptr + 1) = '"' then -- 2.40.0
[COMMITTED] ada: Fix internal error on quantified expression with predicated type
From: Eric Botcazou The problem is that the special function created by the compiler to check the predicate does not inherit the public status of the type, because it is generated as part of the freezing of the quantified expression, which occurs from within a couple of intermediate internal scopes. gcc/ada/ * sem_ch13.adb (Build_Predicate_Function_Declaration): Adjust the commentary to the current implementation. * sem_util.ads (Current_Scope_No_Loops): Move around. (Current_Scope_No_Loops_No_Blocks): New declaration. (Add_Block_Identifier): Fix formatting. * sem_util.adb (Add_Block_Identifier): Likewise. (Current_Scope_No_Loops_No_Blocks): New function. (Set_Public_Status): Call Current_Scope_No_Loops_No_Blocks instead of Current_Scope to get the current scope. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_ch13.adb | 4 +--- gcc/ada/sem_util.adb | 35 ++- gcc/ada/sem_util.ads | 15 +-- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb index 9ece773304a..d1458f58784 100644 --- a/gcc/ada/sem_ch13.adb +++ b/gcc/ada/sem_ch13.adb @@ -133,9 +133,7 @@ package body Sem_Ch13 is function Build_Predicate_Function_Declaration (Typ : Entity_Id) return Node_Id; -- Build the declaration for a predicate function. The declaration is built - -- at the end of the declarative part containing the type definition, which - -- may be before the freeze point of the type. The predicate expression is - -- preanalyzed at this point, to catch visibility errors. + -- at the same time as the body but inserted before, as explained below. procedure Build_Predicate_Function (Typ : Entity_Id; N : Node_Id); -- If Typ has predicates (indicated by Has_Predicates being set for Typ), diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb index 391cade9eac..c8599d47593 100644 --- a/gcc/ada/sem_util.adb +++ b/gcc/ada/sem_util.adb @@ -312,11 +312,12 @@ package body Sem_Util is -- procedure Add_Block_Identifier - (N : Node_Id; -Id : out Entity_Id; -Scope : Entity_Id := Current_Scope) + (N : Node_Id; + Id: out Entity_Id; + Scope : Entity_Id := Current_Scope) is Loc : constant Source_Ptr := Sloc (N); + begin pragma Assert (Nkind (N) = N_Block_Statement); @@ -331,7 +332,6 @@ package body Sem_Util is Id := New_Internal_Entity (E_Block, Scope, Loc, 'B'); Set_Etype (Id, Standard_Void_Type); Set_Parent (Id, N); - Set_Identifier (N, New_Occurrence_Of (Id, Loc)); Set_Block_Node (Id, Identifier (N)); end if; @@ -6721,6 +6721,31 @@ package body Sem_Util is return S; end Current_Scope_No_Loops; + -- + -- Current_Scope_No_Loops_No_Blocks -- + -- + + function Current_Scope_No_Loops_No_Blocks return Entity_Id is + S : Entity_Id; + + begin + -- Examine the scope stack starting from the current scope and skip any + -- internally generated loops and blocks. + + S := Current_Scope; + while Present (S) and then S /= Standard_Standard loop + if Ekind (S) in E_Loop | E_Block + and then not Comes_From_Source (S) + then +S := Scope (S); + else +exit; + end if; + end loop; + + return S; + end Current_Scope_No_Loops_No_Blocks; + -- Current_Subprogram -- @@ -27724,7 +27749,7 @@ package body Sem_Util is --- procedure Set_Public_Status (Id : Entity_Id) is - S : constant Entity_Id := Current_Scope; + S : constant Entity_Id := Current_Scope_No_Loops_No_Blocks; function Within_HSS_Or_If (E : Entity_Id) return Boolean; -- Determines if E is defined within handled statement sequence or diff --git a/gcc/ada/sem_util.ads b/gcc/ada/sem_util.ads index 7bb8cdbe3f3..3edc158c749 100644 --- a/gcc/ada/sem_util.ads +++ b/gcc/ada/sem_util.ads @@ -639,18 +639,21 @@ package Sem_Util is function Current_Scope return Entity_Id; -- Get entity representing current scope + function Current_Scope_No_Loops return Entity_Id; + -- Return the current scope ignoring internally generated loops + + function Current_Scope_No_Loops_No_Blocks return Entity_Id; + -- Return the current scope ignoring internally generated loops and blocks + procedure Add_Block_Identifier - (N : Node_Id; -Id : out Entity_Id; -Scope : Entity_Id := Current_Scope); + (N : Node_Id; + Id: out Entity_Id; + Scope : Entity_Id := Current_Scope); -- Given a block statement N, generate an internal E_Block label and make -- it the identifier of the bloc
[COMMITTED] ada: Remove special-case for parentheses in expansion for GNATprove
From: Piotr Trojanek When expanding of inequality operators for GNAT we were adding extra parens, supposedly to "fix Sprint output" (source print); for GNATprove we didn't, as these extra parens were leading to wrong columns for check messages. Adding these extra parens couldn't be a right, because Sprint should produce reasonable output regardless of the parens, especially since we don't care about parens when creating AST in expansion. Avoiding them for GNATprove couldn't be right either, because source printing should work regardless of the GNAT/GNATprove mode. The proper fix for GNAT is rather to not add parens at all and tune source printing, if necessary. The proper fix for GNATprove is not have them either, as it confuses the heuristic in First_Sloc, which is then used by Compute_Sloc. gcc/ada/ * exp_ch4.adb (Expand_N_Op_Ne): Simply don't add extra parens. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_ch4.adb | 8 1 file changed, 8 deletions(-) diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb index f197c2ef570..70e779d0406 100644 --- a/gcc/ada/exp_ch4.adb +++ b/gcc/ada/exp_ch4.adb @@ -10119,14 +10119,6 @@ package body Exp_Ch4 is Left_Opnd => Left_Opnd (N), Right_Opnd => Right_Opnd (N))); --- The level of parentheses is useless in GNATprove mode, and --- bumping its level here leads to wrong columns being used in --- check messages, hence skip it in this mode. - -if not GNATprove_Mode then - Set_Paren_Count (Right_Opnd (Neg), 1); -end if; - if Scope (Ne) /= Standard_Standard then Set_Entity (Right_Opnd (Neg), Corresponding_Equality (Ne)); end if; -- 2.40.0
[COMMITTED] ada: Add default value at initialization for CodePeer
From: Yannick Moy Avoid spurious alarm by CodePeer analysis by adding default value for a variable initialization. gcc/ada/ * sem_util.adb (Check_Node): Add default init on local Id. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_util.adb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb index c8599d47593..baf4cefdfb3 100644 --- a/gcc/ada/sem_util.adb +++ b/gcc/ada/sem_util.adb @@ -2618,7 +2618,8 @@ package body Sem_Util is function Check_Node (N : Node_Id) return Traverse_Result is Is_Writable_Actual : Boolean := False; -Id : Entity_Id; +Id : Entity_Id := Empty; +-- Default init of Id for CodePeer begin if Nkind (N) = N_Identifier then -- 2.40.0
[COMMITTED] ada: Add tags to warnings controlled by Warn_On_Redundant_Constructs
From: Piotr Trojanek Some of the calls to Error_Msg_N controlled by the flag Warn_On_Redundant_Constructs missed the "?r?" tag in their message string. This caused a misleading "[enabled by default]" label to appear next to the error message. Spotted while adding a warning about duplicated choices in exception handlers. gcc/ada/ * freeze.adb (Freeze_Record_Type): Add tag for redundant pragma Pack. * sem_aggr.adb (Resolve_Record_Aggregate): Add tag for redundant OTHERS choice. * sem_ch8.adb (Use_One_Type): Add tag for redundant USE clauses. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/freeze.adb | 2 +- gcc/ada/sem_aggr.adb | 4 ++-- gcc/ada/sem_ch8.adb | 10 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gcc/ada/freeze.adb b/gcc/ada/freeze.adb index 990bace4fba..6014f71e661 100644 --- a/gcc/ada/freeze.adb +++ b/gcc/ada/freeze.adb @@ -5500,7 +5500,7 @@ package body Freeze is if Warn_On_Redundant_Constructs then Error_Msg_N -- CODEFIX - ("??pragma Pack has no effect, no unplaced components", + ("?r?pragma Pack has no effect, no unplaced components", Get_Rep_Pragma (Rec, Name_Pack)); end if; end if; diff --git a/gcc/ada/sem_aggr.adb b/gcc/ada/sem_aggr.adb index b44708a3e8c..f1511b70648 100644 --- a/gcc/ada/sem_aggr.adb +++ b/gcc/ada/sem_aggr.adb @@ -5834,9 +5834,9 @@ package body Sem_Aggr is ("OTHERS must represent at least one component", Selectr); elsif Others_Box = 1 and then Warn_On_Redundant_Constructs then - Error_Msg_N ("OTHERS choice is redundant?", Box_Node); + Error_Msg_N ("OTHERS choice is redundant?r?", Box_Node); Error_Msg_N -("\previous choices cover all components?", Box_Node); +("\previous choices cover all components?r?", Box_Node); end if; exit Verification; diff --git a/gcc/ada/sem_ch8.adb b/gcc/ada/sem_ch8.adb index 876dbacc951..212c13e12fd 100644 --- a/gcc/ada/sem_ch8.adb +++ b/gcc/ada/sem_ch8.adb @@ -10755,7 +10755,7 @@ package body Sem_Ch8 is Error_Msg_Sloc := Sloc (Clause1); Error_Msg_NE -- CODEFIX ("& is already use-visible through previous " -& "use_type_clause #??", Clause2, T); +& "use_type_clause #?r?", Clause2, T); return; end if; @@ -10827,7 +10827,7 @@ package body Sem_Ch8 is Error_Msg_NE -- CODEFIX ("& is already use-visible through previous " -& "use_type_clause #??", Err_No, Id); +& "use_type_clause #?r?", Err_No, Id); end if; end Use_Clause_Known; @@ -10837,7 +10837,7 @@ package body Sem_Ch8 is else Error_Msg_NE -- CODEFIX ("& is already use-visible through previous " - & "use_type_clause??", Id, T); + & "use_type_clause?r?", Id, T); end if; -- The package where T is declared is already used @@ -10852,7 +10852,7 @@ package body Sem_Ch8 is Error_Msg_Sloc := Sloc (Find_First_Use (Current_Use_Clause (Scope (T; Error_Msg_NE -- CODEFIX - ("& is already use-visible through package use clause #??", + ("& is already use-visible through package use clause #?r?", Id, T); end if; @@ -10861,7 +10861,7 @@ package body Sem_Ch8 is else Error_Msg_Node_2 := Scope (T); Error_Msg_NE -- CODEFIX - ("& is already use-visible inside package &??", Id, T); + ("& is already use-visible inside package &?r?", Id, T); end if; end if; end Use_One_Type; -- 2.40.0
[COMMITTED] ada: Cleanup inconsistent iteration over exception handlers
From: Piotr Trojanek When detecting duplicate choices in exception handlers we had inconsistent pairs of First/Next_Non_Pragma and First_Non_Pragma/Next. This was harmless, because exception choices don't allow pragmas at all, e.g.: when Program_Error | Constraint_Error | ...; -- pragma not allowed and exception handlers only allow pragmas to appear as the first item on the list, e.g.: exception pragma Inspection_Point; -- first item on the list of handlers when Program_Error => pragma Inspection_Point; -- last item on the list of statements when Constraint_Error => ... However, it still seems cleaner to have consistent pairs of First/Next and First_Non_Pragma/Next_Non_Pragma. gcc/ada/ * sem_ch11.adb (Check_Duplication): Fix inconsistent iteration. (Others_Present): Iterate over handlers using First_Non_Pragma and Next_Non_Pragma just like in Check_Duplication. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_ch11.adb | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/gcc/ada/sem_ch11.adb b/gcc/ada/sem_ch11.adb index 6d519ebdd41..547d682 100644 --- a/gcc/ada/sem_ch11.adb +++ b/gcc/ada/sem_ch11.adb @@ -136,10 +136,10 @@ package body Sem_Ch11 is end if; end if; - Next_Non_Pragma (Id1); + Next (Id1); end loop; -Next (Handler); +Next_Non_Pragma (Handler); end loop; end Check_Duplication; @@ -151,15 +151,13 @@ package body Sem_Ch11 is H : Node_Id; begin - H := First (L); + H := First_Non_Pragma (L); while Present (H) loop -if Nkind (H) /= N_Pragma - and then Nkind (First (Exception_Choices (H))) = N_Others_Choice -then +if Nkind (First (Exception_Choices (H))) = N_Others_Choice then return True; end if; -Next (H); +Next_Non_Pragma (H); end loop; return False; -- 2.40.0
[COMMITTED] ada: Ignore accessibility actuals in expression pretty-printer
From: Piotr Trojanek Extra actual parameters for accessibility checks are confusing for the expression pretty-printer that is used by CodePeer. It seems that nodes created for the accessibility checks should use the Sloc of the source expression of accessibility checks, not the target. However, this is problematic to achieve with the current implementation of accessibility checks, so with this patch we will simply ignore the accessibility actuals when computing Slocs for expression pretty-printing. gcc/ada/ * errout.adb (First_And_Last_Nodes): Ignore accessibility parameters. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/errout.adb | 18 ++ 1 file changed, 18 insertions(+) diff --git a/gcc/ada/errout.adb b/gcc/ada/errout.adb index 0a36a1d7466..5c3e76c5eca 100644 --- a/gcc/ada/errout.adb +++ b/gcc/ada/errout.adb @@ -1769,6 +1769,24 @@ package body Errout is Loc : constant Source_Ptr := Sloc (Norig); begin + -- ??? For assignments that require accessiblity checks, e.g.: + -- + --Y := Func (123); + -- + -- the function call gets an extra actual parameter association with + -- Sloc of the assigned name "Y": + -- + --Y := Func (123, A8b => 2); + -- + -- We can simply ignore those extra actual parameters when + -- determining the Sloc range of the "Func (123)" expression. + + if Nkind (N) = N_Parameter_Association + and then Is_Accessibility_Actual (N) + then +return Skip; + end if; + -- Check for earlier if Loc < Eloc -- 2.40.0
[COMMITTED] ada: Facilitate proof of Interfaces.C.To_Ada
From: Yannick Moy Nightly runs of GNATprove fail on proof of the assertion following the loop. Add a loop invariant to facilitate that proof. gcc/ada/ * libgnat/i-c.adb (To_Ada): Add loop invariant. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/i-c.adb | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/ada/libgnat/i-c.adb b/gcc/ada/libgnat/i-c.adb index 9236189fc15..63aa2a2d53b 100644 --- a/gcc/ada/libgnat/i-c.adb +++ b/gcc/ada/libgnat/i-c.adb @@ -605,6 +605,7 @@ is pragma Loop_Invariant (for all J in Item'First .. From when J /= From => Item (J) /= char32_nul); +pragma Loop_Invariant (From <= Item'First + C_Length_Ghost (Item)); pragma Loop_Variant (Increases => From); if From > Item'Last then -- 2.40.0
[COMMITTED] ada: ICE on BIP call in class-wide function return within instance
From: Gary Dismukes The compiler blows up (such as with a Storage_Error or Assert_Failure) on a call to a limited build-in-place function occurring in the return for a function with a limited class-wide result. Such a function should include extra formals for a task master and activation chain (because it's possible for a limited class-wide type to have values with task parts), but when the enclosing function occurs within an instantiation and the result subtype comes from a formal type, the extra formals were missing for the enclosing function. As a result, the attempt to retrieve the task master formal for passing along to a BIP call in the return failed when calling Build_In_Place_Formal to loop through the formals. When determining the need for the formals in Create_Extra_Formals, Needs_BIP_Actual_Task_Actuals was returning False, because Might_Have_Tasks incorrectly returned False due to the test of Is_Limited_Record flag on the class-wide generic actual subtype's Etype being False. Is_Limited_Record was not being properly inherited by the class-wide type in the case of private extensions, because Make_Class_Wide_Type was called in Analyze_Private_Extension_Declaration before certain flags (such as Is_Limited_Record and Is_Controlled_Active) are inherited later in Build_Derived_Record_Type (which will also call Make_Class_Wide_Type). This is corrected by removing the early call to Make_Class_Wide_Type. gcc/ada/ * exp_ch6.adb (Might_Have_Tasks): Remove unneeded Etype call from call to Is_Limited_Record, since that flag is now properly inherited by class-wide types. * sem_ch3.adb (Analyze_Private_Extension_Declaration): Remove call to Make_Class_Wide_Type, which is done too early, and will later be done in Build_Derived_Record_Type after flags such as Is_Limited_Record and Is_Controlled_Active have been set on the derived type. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_ch6.adb | 2 +- gcc/ada/sem_ch3.adb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb index 28b746ba2c4..fce10d5e946 100644 --- a/gcc/ada/exp_ch6.adb +++ b/gcc/ada/exp_ch6.adb @@ -9326,7 +9326,7 @@ package body Exp_Ch6 is and then not No_Run_Time_Mode and then (Has_Task (Typ) or else (Is_Class_Wide_Type (Typ) - and then Is_Limited_Record (Etype (Typ)) + and then Is_Limited_Record (Typ) and then not Has_Aspect (Etype (Typ), Aspect_No_Task_Parts))); end Might_Have_Tasks; diff --git a/gcc/ada/sem_ch3.adb b/gcc/ada/sem_ch3.adb index 50ccb390363..a0783195b8b 100644 --- a/gcc/ada/sem_ch3.adb +++ b/gcc/ada/sem_ch3.adb @@ -5376,7 +5376,6 @@ package body Sem_Ch3 is Set_Convention (T, Convention (Parent_Type)); Set_First_Rep_Item (T, First_Rep_Item (Parent_Type)); Set_Is_First_Subtype (T); - Make_Class_Wide_Type (T); -- Set the SPARK mode from the current context -- 2.40.0
[COMMITTED] ada: Rework fix for internal error on quantified expression with predicated type
From: Eric Botcazou It turns out that skipping compiler-generated block scopes is problematic when computing the public status of a subprogram, because this subprogram may end up being nested in the elaboration procedure of a package spec or body, in which case it may not be public. This replaces the original fix with a pair of Push_Scope/Pop_Scope in the Build_Predicate_Function procedure, as done elsewhere in similar cases. gcc/ada/ * sem_ch13.adb (Build_Predicate_Functions): If the current scope is not that of the type, push this scope and pop it at the end. * sem_util.ads (Current_Scope_No_Loops_No_Blocks): Delete. * sem_util.adb (Current_Scope_No_Loops_No_Blocks): Likewise. (Set_Public_Status): Call again Current_Scope. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_ch13.adb | 26 -- gcc/ada/sem_util.adb | 27 +-- gcc/ada/sem_util.ads | 3 --- 3 files changed, 21 insertions(+), 35 deletions(-) diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb index d1458f58784..983f877e001 100644 --- a/gcc/ada/sem_ch13.adb +++ b/gcc/ada/sem_ch13.adb @@ -9921,6 +9921,10 @@ package body Sem_Ch13 is procedure Build_Predicate_Function (Typ : Entity_Id; N : Node_Id) is Loc : constant Source_Ptr := Sloc (Typ); + Saved_GM : constant Ghost_Mode_Type := Ghost_Mode; + Saved_IGR : constant Node_Id := Ignored_Ghost_Region; + -- Save the Ghost-related attributes to restore on exit + Expr : Node_Id; -- This is the expression for the result of the function. It is -- is build by connecting the component predicates with AND THEN. @@ -9939,6 +9943,9 @@ package body Sem_Ch13 is SId : Entity_Id; -- Its entity + Restore_Scope : Boolean; + -- True if the current scope must be restored on exit + Ancestor_Predicate_Function_Called : Boolean := False; -- Does this predicate function include a call to the -- predication function of an ancestor subtype? @@ -10190,12 +10197,6 @@ package body Sem_Ch13 is Replace_Type_References (N, Typ); end Replace_Current_Instance_References; - -- Local variables - - Saved_GM : constant Ghost_Mode_Type := Ghost_Mode; - Saved_IGR : constant Node_Id := Ignored_Ghost_Region; - -- Save the Ghost-related attributes to restore on exit - -- Start of processing for Build_Predicate_Function begin @@ -10234,6 +10235,15 @@ package body Sem_Ch13 is return; end if; + -- Ensure that the declarations are added to the scope of the type + + if Scope (Typ) /= Current_Scope then + Push_Scope (Scope (Typ)); + Restore_Scope := True; + else + Restore_Scope := False; + end if; + -- The related type may be subject to pragma Ghost. Set the mode now to -- ensure that the predicate functions are properly marked as Ghost. @@ -10652,6 +10662,10 @@ package body Sem_Ch13 is end if; Restore_Ghost_Region (Saved_GM, Saved_IGR); + + if Restore_Scope then + Pop_Scope; + end if; end Build_Predicate_Function; -- diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb index 22dc9376b92..9a0197cb45c 100644 --- a/gcc/ada/sem_util.adb +++ b/gcc/ada/sem_util.adb @@ -6722,31 +6722,6 @@ package body Sem_Util is return S; end Current_Scope_No_Loops; - -- - -- Current_Scope_No_Loops_No_Blocks -- - -- - - function Current_Scope_No_Loops_No_Blocks return Entity_Id is - S : Entity_Id; - - begin - -- Examine the scope stack starting from the current scope and skip any - -- internally generated loops and blocks. - - S := Current_Scope; - while Present (S) and then S /= Standard_Standard loop - if Ekind (S) in E_Loop | E_Block - and then not Comes_From_Source (S) - then -S := Scope (S); - else -exit; - end if; - end loop; - - return S; - end Current_Scope_No_Loops_No_Blocks; - -- Current_Subprogram -- @@ -27763,7 +27738,7 @@ package body Sem_Util is --- procedure Set_Public_Status (Id : Entity_Id) is - S : constant Entity_Id := Current_Scope_No_Loops_No_Blocks; + S : constant Entity_Id := Current_Scope; function Within_HSS_Or_If (E : Entity_Id) return Boolean; -- Determines if E is defined within handled statement sequence or diff --git a/gcc/ada/sem_util.ads b/gcc/ada/sem_util.ads index 3edc158c749..253d1dadeee 100644 --- a/gcc/ada/sem_util.ads +++ b/gcc/ada/sem_util.ads @@ -642,9 +642,6 @@ package Sem_Util is function Current_Scope_No_Loops return Entity_Id; -- Return the current sc
[COMMITTED] ada: Remove unnecessary call to Detach.
From: Vadim Godunko Holder object is constant and protected from modification by tampering rules. gcc/ada/ * libgnat/a-coinho__shared.adb (Constant_Reference): Remove call of Detach (Query_Element): Likewise. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/a-coinho__shared.adb | 4 1 file changed, 4 deletions(-) diff --git a/gcc/ada/libgnat/a-coinho__shared.adb b/gcc/ada/libgnat/a-coinho__shared.adb index 367089048af..f49ac4ad817 100644 --- a/gcc/ada/libgnat/a-coinho__shared.adb +++ b/gcc/ada/libgnat/a-coinho__shared.adb @@ -149,8 +149,6 @@ package body Ada.Containers.Indefinite_Holders is raise Constraint_Error with "container is empty"; end if; - Detach (Container); - declare Ref : constant Constant_Reference_Type := (Element => Container.Reference.Element.all'Access, @@ -305,8 +303,6 @@ package body Ada.Containers.Indefinite_Holders is raise Constraint_Error with "container is empty"; end if; - Detach (Container); - B := B + 1; begin -- 2.40.0
[COMMITTED] ada: Fix bogus error on predicated limited record declared in protected type
From: Eric Botcazou This happens when the limited record is initialized with a function call because of a couple of issues: incorrect tree sharing when building the predicate check and too late freezing for a compiler-generated subtype. It turns out that building the predicate check manually is redundant here, since predicate checks are automatically generated during the expansion of assignment statements, and the late freezing can be easily fixed. gcc/ada/ * exp_ch3.adb (Build_Record_Init_Proc.Build_Assignment): Do not manually generate a predicate check. Call Unqualify before doing pattern matching on the expression. * sem_ch3.adb (Analyze_Object_Declaration): Also freeze the actual subtype when it is built in the definite case. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_ch3.adb | 31 +-- gcc/ada/sem_ch3.adb | 1 + 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb index 3a023092532..b992a587433 100644 --- a/gcc/ada/exp_ch3.adb +++ b/gcc/ada/exp_ch3.adb @@ -2082,8 +2082,8 @@ package body Exp_Ch3 is Typ : constant Entity_Id := Underlying_Type (Etype (Id)); Adj_Call : Node_Id; - Exp : Node_Id := Default; - Kind : Node_Kind := Nkind (Default); + Exp : Node_Id; + Exp_Q: Node_Id; Lhs : Node_Id; Res : List_Id; @@ -2094,13 +2094,14 @@ package body Exp_Ch3 is Selector_Name => New_Occurrence_Of (Id, Default_Loc)); Set_Assignment_OK (Lhs); - -- Take a copy of Exp to ensure that later copies of this component + -- Take copy of Default to ensure that later copies of this component -- declaration in derived types see the original tree, not a node -- rewritten during expansion of the init_proc. If the copy contains -- itypes, the scope of the new itypes is the init_proc being built. declare Map : Elist_Id := No_Elist; + begin if Has_Late_Init_Comp then -- Map the type to the _Init parameter in order to @@ -2131,7 +2132,7 @@ package body Exp_Ch3 is end if; end if; -Exp := New_Copy_Tree (Exp, New_Scope => Proc_Id, Map => Map); +Exp := New_Copy_Tree (Default, New_Scope => Proc_Id, Map => Map); end; Res := New_List ( @@ -2141,6 +2142,8 @@ package body Exp_Ch3 is Set_No_Ctrl_Actions (First (Res)); + Exp_Q := Unqualify (Exp); + -- Adjust the tag if tagged (because of possible view conversions). -- Suppress the tag adjustment when not Tagged_Type_Expansion because -- tags are represented implicitly in objects, and when the record is @@ -2148,9 +2151,7 @@ package body Exp_Ch3 is if Is_Tagged_Type (Typ) and then Tagged_Type_Expansion - and then Nkind (Exp) /= N_Raise_Expression - and then (Nkind (Exp) /= N_Qualified_Expression - or else Nkind (Expression (Exp)) /= N_Raise_Expression) + and then Nkind (Exp_Q) /= N_Raise_Expression then Append_To (Res, Make_Assignment_Statement (Default_Loc, @@ -2173,12 +2174,8 @@ package body Exp_Ch3 is -- Adjust the component if controlled except if it is an aggregate -- that will be expanded inline. - if Kind = N_Qualified_Expression then -Kind := Nkind (Expression (Default)); - end if; - if Needs_Finalization (Typ) - and then Kind not in N_Aggregate | N_Extension_Aggregate + and then Nkind (Exp_Q) not in N_Aggregate | N_Extension_Aggregate and then not Is_Build_In_Place_Function_Call (Exp) then Adj_Call := @@ -2194,16 +2191,6 @@ package body Exp_Ch3 is end if; end if; - -- If a component type has a predicate, add check to the component - -- assignment. Discriminants are handled at the point of the call, - -- which provides for a better error message. - - if Comes_From_Source (Exp) - and then Predicate_Enabled (Typ) - then -Append (Make_Predicate_Check (Typ, Exp), Res); - end if; - return Res; exception diff --git a/gcc/ada/sem_ch3.adb b/gcc/ada/sem_ch3.adb index 2ebbe36abc6..bace2cf616a 100644 --- a/gcc/ada/sem_ch3.adb +++ b/gcc/ada/sem_ch3.adb @@ -4971,6 +4971,7 @@ package body Sem_Ch3 is end if; Rewrite (Object_Definition (N), New_Occurrence_Of (Act_T, Loc)); + Freeze_Before (N, Act_T); elsif Nkind (E) = N_Function_Call and then Constant_Present (N) -- 2.40.0
[COMMITTED] ada: Accept and analyze new aspect Exceptional_Cases
From: Piotr Trojanek Add new aspect Exceptional_Cases, which is intended for SPARK and describes in which cases an exception will be raised, and optionally supply a postcondition that shall be verified in this case. The implementation is heavily modeled after Subprogram_Variant, which in turn was heavily modeled after Contract_Cases. Currently the aspect is only analysed; the code infrastructure required to expand it is prepared but empty. This is enough for the aspect to be verified by GNATprove. gcc/ada/ * aspects.ads (Aspect_Id): Add aspect identifier. (Aspect_Argument): New aspect accepts an expression. (Is_Representation_Aspect): New aspect is not a representation aspect. (Aspect_Names): Associate name with the new aspect identifier. (Aspect_Delay): New aspect is never delayed. * contracts.adb (Add_Contract_Item): Store new aspect among contract items. (Analyze_Entry_Or_Subprogram_Contract): Likewise. (Analyze_Subprogram_Body_Stub_Contract): Likewise. (Process_Contract_Cases): Expand new aspect, if present. * contracts.ads (Analyze_Entry_Or_Subprogram_Body_Contract): Mention new aspect in spec. (Analyze_Entry_Or_Subprogram_Contract): Likewise. * einfo-utils.adb (Get_Pragma): Allow new aspect to be picked by the backend. * einfo-utils.ads (Get_Pragma): Mention new aspect in spec. * exp_prag.adb (Expand_Pragma_Exceptional_Cases): Dummy expansion routine. * exp_prag.ads (Expand_Pragma_Exceptional_Cases): Add spec for expansion routine. * inline.adb (Remove_Aspects_And_Pragmas): Remove aspect from bodies to inline. * par-prag.adb (Par.Prag): Accept pragma in the parser, so it will be checked later. * sem_ch12.adb (Implementation of Generic Contracts): Mention new aspect in comment. * sem_ch13.adb (Analyze_Aspect_Specifications): Transform new aspect info a corresponding pragma. * sem_prag.adb (Analyze_Exceptional_Cases_In_Decl_Part): Analyze aspect expression; heavily inspired by the existing code for analysis of Subprogram_Variant and exception handlers. (Analyze_Pragma): Analyze pragma corresponding to the new aspect. (Is_Non_Significant_Pragma_Reference): Add new pragma to the table. * sem_prag.ads (Assertion_Expression_Pragma): New pragma acts as an assertion expression, even though it is not currently expanded. (Analyze_Exceptional_Cases_In_Decl_Part): Add spec. * sem_util.adb (Is_Subprogram_Contract_Annotation): Mark new annotation is a subprogram contract, so the subprogram with it won't be inlined. * sem_util.ads (Is_Subprogram_Contract_Annotation): Mention new aspect in comment. * sinfo.ads (Contract_Test_Cases): Mention new aspect in comment. * snames.ads-tmpl: Add entries for the new name and pragma. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/aspects.ads | 5 + gcc/ada/contracts.adb | 18 +- gcc/ada/contracts.ads | 2 + gcc/ada/einfo-utils.adb | 1 + gcc/ada/einfo-utils.ads | 1 + gcc/ada/exp_prag.adb| 41 gcc/ada/exp_prag.ads| 4 + gcc/ada/inline.adb | 2 + gcc/ada/par-prag.adb| 1 + gcc/ada/sem_ch12.adb| 3 +- gcc/ada/sem_ch13.adb| 28 ++- gcc/ada/sem_prag.adb| 429 gcc/ada/sem_prag.ads| 8 + gcc/ada/sem_util.adb| 1 + gcc/ada/sem_util.ads| 1 + gcc/ada/sinfo.ads | 4 +- gcc/ada/snames.ads-tmpl | 2 + 17 files changed, 538 insertions(+), 13 deletions(-) diff --git a/gcc/ada/aspects.ads b/gcc/ada/aspects.ads index 36957d466e6..6670b64ca49 100644 --- a/gcc/ada/aspects.ads +++ b/gcc/ada/aspects.ads @@ -96,6 +96,7 @@ package Aspects is Aspect_Dynamic_Predicate, Aspect_Effective_Reads, -- GNAT Aspect_Effective_Writes, -- GNAT + Aspect_Exceptional_Cases, -- GNAT Aspect_Extensions_Visible,-- GNAT Aspect_External_Name, Aspect_External_Tag, @@ -389,6 +390,7 @@ package Aspects is Aspect_Dynamic_Predicate => Expression, Aspect_Effective_Reads=> Optional_Expression, Aspect_Effective_Writes => Optional_Expression, + Aspect_Exceptional_Cases => Expression, Aspect_Extensions_Visible => Optional_Expression, Aspect_External_Name => Expression, Aspect_External_Tag => Expression, @@ -496,6 +498,7 @@ package Aspects is Aspect_Dynamic_Predicate=> False, Aspect_Effective_Reads => False, Aspect_Effective_Writes => False, + Aspect_Exceptional_Cases
[COMMITTED] ada: Fix oversight in latest change
From: Eric Botcazou The resolution must be identical inside and outside the System hierarchy. gcc/ada/ * sem_res.adb (Resolve_Intrinsic_Operator): Always perform the same resolution for the special mod operator of System.Storage_Elements. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_res.adb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb index f1d9a97452a..3b7d821158c 100644 --- a/gcc/ada/sem_res.adb +++ b/gcc/ada/sem_res.adb @@ -9778,11 +9778,13 @@ package body Sem_Res is -- If the result or operand types are private, rewrite with unchecked -- conversions on the operands and the result, to expose the proper - -- underlying numeric type. + -- underlying numeric type. Likewise for the special mod operator of + -- System.Storage_Elements, to expose the modified base type. if Is_Private_Type (Typ) or else Is_Private_Type (Etype (Left_Opnd (N))) or else Is_Private_Type (Etype (Right_Opnd (N))) +or else Is_Stoele_Mod then Arg1 := Convert_Operand (Left_Opnd (N)); -- 2.40.0
[COMMITTED] ada: Turn assertions into defensive code in error locations
From: Piotr Trojanek We pretty-print numeric literals that do not come from source by relying on their Sloc. This generally works well, but sporadically the Sloc is set wrongly. We might want to trace and fix such occurrences, but for now it is simpler to replace an otherwise reasonable assertions with defensive code. gcc/ada/ * errout.adb (Last_Sloc): Refactor a heavily repeated "S := S + 1" statement into a subprogram; replace assertions with defensive code; fix few more off-by-one errors. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/errout.adb | 89 +++--- 1 file changed, 61 insertions(+), 28 deletions(-) diff --git a/gcc/ada/errout.adb b/gcc/ada/errout.adb index 6531410f0d2..1c6222b3a29 100644 --- a/gcc/ada/errout.adb +++ b/gcc/ada/errout.adb @@ -1994,6 +1994,20 @@ package body Errout is --- function Last_Sloc (N : Node_Id) return Source_Ptr is + procedure Skip_Char (S : in out Source_Ptr); + -- Skip one character of the source buffer at location S + + --- + -- Skip_Char -- + --- + + procedure Skip_Char (S : in out Source_Ptr) is + begin + S := S + 1; + end Skip_Char; + + -- Local variables + SI : constant Source_File_Index := Get_Source_File_Index (Sloc (N)); SF : constant Source_Ptr:= Source_First (SI); SL : constant Source_Ptr:= Source_Last (SI); @@ -2001,6 +2015,8 @@ package body Errout is F : Node_Id; S : Source_Ptr; + -- Start of processing for Last_Sloc + begin F := Last_Node (N); S := Sloc (F); @@ -2035,7 +2051,7 @@ package body Errout is while S < SL and then Src (S + 1) in '0' .. '9' | '_' loop - S := S + 1; + Skip_Char (S); end loop; -- Skip past #based_numeral#, if present @@ -2043,30 +2059,36 @@ package body Errout is if S < SL and then Src (S + 1) = '#' then - S := S + 1; + Skip_Char (S); while S < SL and then Src (S + 1) in '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' loop - S := S + 1; + Skip_Char (S); end loop; - pragma Assert (S < SL and then Src (S + 1) = '#'); - - S := S + 1; + if S < SL + and then Src (S + 1) = '#' + then + Skip_Char (S); + end if; end if; -- Skip past exponent, if present -if S < SL + 1 +if S < SL and then Src (S + 1) in 'e' | 'E' then + Skip_Char (S); + -- For positive exponents the plus sign is optional, but we -- can simply skip past both plus and minus. - if Src (S + 2) in '+' | '-' then - S := S + 1; + if S < SL + and then Src (S + 1) in '+' | '-' + then + Skip_Char (S); end if; -- Skip past the numeral part @@ -2074,7 +2096,7 @@ package body Errout is while S < SL and then Src (S + 1) in '0' .. '9' | '_' loop - S := S + 1; + Skip_Char (S); end loop; end if; @@ -2085,55 +2107,66 @@ package body Errout is while S < SL and then Src (S + 1) in '0' .. '9' | '_' loop - S := S + 1; + Skip_Char (S); end loop; if S < SL then + + -- Skip the dot and continue with a decimal literal + if Src (S + 1) = '.' then + Skip_Char (S); + while S < SL and then Src (S + 1) in '0' .. '9' | '_' loop - S := S + 1; + Skip_Char (S); end loop; - else - pragma Assert (Src (S + 1) = '#'); - S := S + 1; + -- Skip the hash and continue with a based literal + + elsif Src (S + 1) = '#' then + Skip_Char (S); while S < SL and then Src (S + 1) in '0' .. '9' | 'a' .. 'f' | 'A' .. 'F' | '_' loop - S := S + 1; + Skip_Char (S); end loop; - pragma Assert (S < SL and then Src (S + 1) = '.'); - - S := S + 1; + if S < SL +and then Src (S + 1) = '.' + then + Skip_Char (S); +
[COMMITTED] ada: Fix minor address arithmetic issues in System.Dwarf_Lines
From: Eric Botcazou Offset calculations should use the operator of System.Storage_Elements. gcc/ada/ * libgnat/s-dwalin.adb (Enable_Cache): Use the subtract operator of System.Storage_Elements to compute the offset. (Symbolic_Address): Likewise. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/s-dwalin.adb | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gcc/ada/libgnat/s-dwalin.adb b/gcc/ada/libgnat/s-dwalin.adb index d38bc058e3b..d35d03a8a2f 100644 --- a/gcc/ada/libgnat/s-dwalin.adb +++ b/gcc/ada/libgnat/s-dwalin.adb @@ -1542,7 +1542,7 @@ package body System.Dwarf_Lines is exit when Ar_Start = Null_Address and Ar_Len = 0; Len := uint32 (Ar_Len); - Start := uint32 (Address'(Ar_Start - C.Low)); + Start := uint32 (Storage_Count'(Ar_Start - C.Low)); -- Search START in the array @@ -1762,7 +1762,7 @@ package body System.Dwarf_Lines is if C.Cache /= null then declare -Addr_Off : constant uint32 := uint32 (Address'(Addr - C.Low)); +Off : constant uint32 := uint32 (Storage_Count'(Addr - C.Low)); First, Last, Mid : Natural; begin @@ -1772,17 +1772,17 @@ package body System.Dwarf_Lines is while First <= Last loop Mid := First + (Last - First) / 2; - if Addr_Off < C.Cache (Mid).First then + if Off < C.Cache (Mid).First then Last := Mid - 1; - elsif Addr_Off >= C.Cache (Mid).First + C.Cache (Mid).Size then + elsif Off >= C.Cache (Mid).First + C.Cache (Mid).Size then First := Mid + 1; else exit; end if; end loop; -if Addr_Off >= C.Cache (Mid).First - and then Addr_Off < C.Cache (Mid).First + C.Cache (Mid).Size +if Off >= C.Cache (Mid).First + and then Off < C.Cache (Mid).First + C.Cache (Mid).Size then Line_Offset := Offset (C.Cache (Mid).Line); S := Read_Symbol (C.Obj.all, Offset (C.Cache (Mid).Sym)); -- 2.40.0
Re: [PATCH] Fix handling of non-integral bit-fields in native_encode_initializer
> OK. Thanks! > Can we handle non-integer bitfields by recursing with a temporary buffer to > encode it byte-aligned and then apply shifting and masking to get it in > place? Or is that not worth it? Certainly doable, something along these lines is implemented in varasm.c to output these non-integral bit-fields (output_constructor et al) so we could even try to share some code. However, in practice, these cases turn out to be rare because the tree_output_constant_def path in gimplify_init_constructor is well guarded. -- Eric Botcazou
[COMMITTED] ada: Remove the body of System.Storage_Elements
From: Eric Botcazou All the subprograms declared in the unit have convention Intrinsic and their current implementation makes some implicit assumptions that are not valid universally, so it is replaced by a direct expansion. This is mostly straightforward because Resolve_Intrinsic_Operator already contains the required circuitry, but a few adjustements are necessary. gcc/ada/ * exp_ch4.adb (Expand_N_Op_Mod): Deal with the special mod operator of System.Storage_Elements. * exp_intr.adb (Expand_To_Integer): New procedure. (Expand_Intrinsic_Call): Call Expand_To_Integer appropriately. (Expand_To_Address): Deal with an argument with modular type. * sem_ch3.adb (Derive_Subprogram): Also set convention Intrinsic on a derived intrinsic subprogram. * sem_res.adb (Resolve_Arithmetic_Op): Deal with intrinsic operators not coming from source exactly as those coming from source and also generate a reference in both cases. (Resolve_Op_Expon): Likewise. (Resolve_Intrinsic_Operator): Call Implementation_Base_Type to get a nonprivate base type. * snames.ads-tmpl (Name_To_Integer): New intrinsic name. * libgnat/s-stoele.ads: Replace pragma Convention with pragma Import throughout and remove pragma Inline_Always and Pure_Function. * libgnat/s-stoele.adb: Replace entire contents with pragma No_Body. * libgnat/s-atacco.adb: Adjust comment about pragma No_Body. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_ch4.adb | 28 +- gcc/ada/exp_intr.adb | 27 ++ gcc/ada/libgnat/s-atacco.adb | 6 +-- gcc/ada/libgnat/s-stoele.adb | 101 ++- gcc/ada/libgnat/s-stoele.ads | 36 +++-- gcc/ada/sem_ch3.adb | 1 + gcc/ada/sem_res.adb | 10 ++-- gcc/ada/snames.ads-tmpl | 3 +- 8 files changed, 75 insertions(+), 137 deletions(-) diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb index 70e779d0406..c974a9e8d44 100644 --- a/gcc/ada/exp_ch4.adb +++ b/gcc/ada/exp_ch4.adb @@ -9560,6 +9560,12 @@ package body Exp_Ch4 is Typ : constant Entity_Id := Etype (N); DDC : constant Boolean:= Do_Division_Check (N); + Is_Stoele_Mod : constant Boolean := +Is_RTE (First_Subtype (Typ), RE_Storage_Offset) + and then Nkind (Left_Opnd (N)) = N_Unchecked_Type_Conversion + and then Is_RTE (Etype (Expression (Left_Opnd (N))), RE_Address); + -- True if this is the special mod operator of System.Storage_Elements + Left : Node_Id; Right : Node_Id; @@ -9593,7 +9599,10 @@ package body Exp_Ch4 is end if; end if; - if Is_Integer_Type (Typ) then + -- For the special mod operator of System.Storage_Elements, the checks + -- are subsumed into the handling of the negative case below. + + if Is_Integer_Type (Typ) and then not Is_Stoele_Mod then Apply_Divide_Checks (N); -- All done if we don't have a MOD any more, which can happen as a @@ -9663,6 +9672,23 @@ package body Exp_Ch4 is return; end if; + -- The negative case makes no sense since it is a case of a mod where + -- the left argument is unsigned and the right argument is signed. In + -- accordance with the (spirit of the) permission of RM 13.7.1(16), + -- we raise CE, and also include the zero case here. Yes, the RM says + -- PE, but this really is so obviously more like a constraint error. + + if Is_Stoele_Mod and then (not ROK or else Rlo <= 0) then +Insert_Action (N, + Make_Raise_Constraint_Error (Loc, +Condition => + Make_Op_Le (Loc, +Left_Opnd => Duplicate_Subexpr_No_Checks (Right), +Right_Opnd => Make_Integer_Literal (Loc, 0)), +Reason => CE_Overflow_Check_Failed)); +return; + end if; + -- If we still have a mod operator and we are in Modify_Tree_For_C -- mode, and we have a signed integer type, then here is where we do -- the rewrite in terms of Rem. Note this rewrite bypasses the need diff --git a/gcc/ada/exp_intr.adb b/gcc/ada/exp_intr.adb index a1e55882391..2eee892605e 100644 --- a/gcc/ada/exp_intr.adb +++ b/gcc/ada/exp_intr.adb @@ -102,6 +102,12 @@ package body Exp_Intr is -- N_Free_Statement and appropriate context. procedure Expand_To_Address (N : Node_Id); + -- Expand a call to corresponding function from System.Storage_Elements or + -- declared in an instance of System.Address_To_Access_Conversions. + + procedure Expand_To_Integer (N : Node_Id); + -- Expand a call to corresponding function from System.Storage_Elements + procedure Expand_To_Pointer (N : Node_Id); -- Expand a call to corresponding function, dec
[COMMITTED] ada: Update ghost code for proof of integer input functions
From: Claire Dross Introduce new ghost helper functions to facilitate proof. gcc/ada/ * libgnat/s-valueu.adb (Scan_Raw_Unsigned): Use new helpers. * libgnat/s-vauspe.ads (Raw_Unsigned_Starts_As_Based_Ghost, Raw_Unsigned_Is_Based_Ghost): New ghost helper functions. (Is_Raw_Unsigned_Format_Ghost, Scan_Split_No_Overflow_Ghost, Scan_Split_Value_Ghost, Raw_Unsigned_Last_Ghost): Use new helpers. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/s-valueu.adb | 10 ++ gcc/ada/libgnat/s-vauspe.ads | 63 ++-- 2 files changed, 41 insertions(+), 32 deletions(-) diff --git a/gcc/ada/libgnat/s-valueu.adb b/gcc/ada/libgnat/s-valueu.adb index 062ad33b1e9..9c77caa3bcb 100644 --- a/gcc/ada/libgnat/s-valueu.adb +++ b/gcc/ada/libgnat/s-valueu.adb @@ -140,10 +140,7 @@ package body System.Value_U is Spec.Scan_Based_Number_Ghost (Str, Ptr.all, Last_Num_Init) with Ghost; Starts_As_Based : constant Boolean := -Last_Num_Init < Max - 1 -and then Str (Last_Num_Init + 1) in '#' | ':' -and then Str (Last_Num_Init + 2) in -'0' .. '9' | 'a' .. 'f' | 'A' .. 'F' +Spec.Raw_Unsigned_Starts_As_Based_Ghost (Str, Last_Num_Init, Max) with Ghost; Last_Num_Based : constant Integer := (if Starts_As_Based @@ -151,9 +148,8 @@ package body System.Value_U is else Last_Num_Init) with Ghost; Is_Based: constant Boolean := -Starts_As_Based -and then Last_Num_Based < Max -and then Str (Last_Num_Based + 1) = Str (Last_Num_Init + 1) +Spec.Raw_Unsigned_Is_Based_Ghost + (Str, Last_Num_Init, Last_Num_Based, Max) with Ghost; Based_Val : constant Spec.Uns_Option := (if Starts_As_Based and then not Init_Val.Overflow diff --git a/gcc/ada/libgnat/s-vauspe.ads b/gcc/ada/libgnat/s-vauspe.ads index 117769d20cb..960ad8ec486 100644 --- a/gcc/ada/libgnat/s-vauspe.ads +++ b/gcc/ada/libgnat/s-vauspe.ads @@ -279,24 +279,50 @@ is Exponent_Unsigned_Ghost (Value * Base, Exp - 1, Base)); -- Normal case: exponentiation without overflows + function Raw_Unsigned_Starts_As_Based_Ghost + (Str : String; + Last_Num_Init, To : Integer) + return Boolean + is + (Last_Num_Init < To - 1 + and then Str (Last_Num_Init + 1) in '#' | ':' + and then Str (Last_Num_Init + 2) in + '0' .. '9' | 'a' .. 'f' | 'A' .. 'F') + with Ghost, + Pre => Last_Num_Init in Str'Range + and then To in Str'Range; + -- Return True if Str starts as a based number + + function Raw_Unsigned_Is_Based_Ghost + (Str: String; + Last_Num_Init : Integer; + Last_Num_Based : Integer; + To : Integer) + return Boolean + is + (Raw_Unsigned_Starts_As_Based_Ghost (Str, Last_Num_Init, To) + and then Last_Num_Based < To + and then Str (Last_Num_Based + 1) = Str (Last_Num_Init + 1)) + with Ghost, + Pre => Last_Num_Init in Str'Range + and then Last_Num_Based in Last_Num_Init .. Str'Last + and then To in Str'Range; + -- Return True if Str is a based number + function Is_Raw_Unsigned_Format_Ghost (Str : String) return Boolean is (Is_Natural_Format_Ghost (Str) and then (declare Last_Num_Init : constant Integer := Last_Number_Ghost (Str); Starts_As_Based : constant Boolean := - Last_Num_Init < Str'Last - 1 - and then Str (Last_Num_Init + 1) in '#' | ':' - and then Str (Last_Num_Init + 2) in - '0' .. '9' | 'a' .. 'f' | 'A' .. 'F'; + Raw_Unsigned_Starts_As_Based_Ghost (Str, Last_Num_Init, Str'Last); Last_Num_Based : constant Integer := (if Starts_As_Based then Last_Hexa_Ghost (Str (Last_Num_Init + 2 .. Str'Last)) else Last_Num_Init); Is_Based: constant Boolean := - Starts_As_Based - and then Last_Num_Based < Str'Last - and then Str (Last_Num_Based + 1) = Str (Last_Num_Init + 1); + Raw_Unsigned_Is_Based_Ghost + (Str, Last_Num_Init, Last_Num_Based, Str'Last); First_Exp : constant Integer := (if Is_Based then Last_Num_Based + 2 else Last_Num_Init + 1); begin @@ -330,10 +356,7 @@ is Init_Val: constant Uns_Option := Scan_Based_Number_Ghost (Str, From, Last_Num_Init); Starts_As_Based : constant Boolean := - Last_Num_Init < To - 1 - and then Str (Last_Num_Init + 1) in '#' | ':' - and then Str (Last_Num_Init + 2) in - '0' .. '9' | 'a' .. 'f' | 'A' .. 'F'; + Raw_Unsigned_Starts_As_Based_Ghost (Str, Last_Num_Init, To); Last_Num_Based : constant Integer := (if Starts_As_Based then Last_Hexa_G
[COMMITTED] ada: Fix reference to Ada issue in comment
From: Ronan Desplanques gcc/ada/ * sem_disp.adb: Fix reference to Ada issue in comment. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_disp.adb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/ada/sem_disp.adb b/gcc/ada/sem_disp.adb index b01e3d4186e..ab409d3a4e4 100644 --- a/gcc/ada/sem_disp.adb +++ b/gcc/ada/sem_disp.adb @@ -1392,7 +1392,7 @@ package body Sem_Disp is -- 4. Wrappers built for inherited operations with inherited class- -- wide conditions, where the conditions include calls to other -- overridden primitives. The wrappers include checks on these - -- modified conditions. (AI12-113). + -- modified conditions. (AI12-195). -- 5. Declarations built for subprograms without separate specs that -- are eligible for inlining in GNATprove (inside -- 2.40.0
[COMMITTED] ada: Fix latent issue in support for protected entries
From: Eric Botcazou The problem is that, unlike for protected subprograms, the expansion of cleanups for protected entries is not delayed when they contain package instances with a body, so the cleanups are generated twice and this may yield two finalizers if the secondary stack is used in the entry body. This restores the delaying, which uncovers the missing propagation of the Uses_Sec_Stack flag as is done for protected subprograms, which in turn requires using a Corresponding_Spec field as for protected subprograms. This also gets rid of the Delay_Subprogram_Descriptors flag on entities, whose only remaining use in Expand_Cleanup_Actions was unreachable. The last change is to unconditionally reset the scopes in the case of protected subprograms when they are expanded, as is done in the case of protected entries. This makes it possible to remove the code adjusting the scope on the fly in Cleanup_Scopes but requires a few adjustments. gcc/ada/ * einfo.ads (Delay_Subprogram_Descriptors): Delete. * gen_il-fields.ads (Opt_Field_Enum): Remove Delay_Subprogram_Descriptors. * gen_il-gen-gen_entities.adb (Gen_Entities): Likewise. * gen_il-gen-gen_nodes.adb (N_Entry_Body): Add Corresponding_Spec. * sinfo.ads (Corresponding_Spec): Document new use. (N_Entry_Body): Likewise. * exp_ch6.adb (Expand_Protected_Object_Reference): Be prepared for protected subprograms that have been expanded. * exp_ch7.adb (Expand_Cleanup_Actions): Remove unreachable code. * exp_ch9.adb (Build_Protected_Entry): Add a local variable for the new block and propagate Uses_Sec_Stack from the corresponding spec. (Expand_N_Protected_Body) : Unconditionally reset the scopes of top-level entities in the new body. * inline.adb (Cleanup_Scopes): Do not adjust the scope on the fly. * sem_ch9.adb (Analyze_Entry_Body): Set Corresponding_Spec. * sem_ch12.adb (Analyze_Package_Instantiation): Remove obsolete code setting Delay_Subprogram_Descriptors and tidy up. * sem_util.adb (Scope_Within): Deal with protected subprograms that have been expanded. (Scope_Within_Or_Same): Likewise. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/einfo.ads | 21 - gcc/ada/exp_ch6.adb | 9 +--- gcc/ada/exp_ch7.adb | 10 - gcc/ada/exp_ch9.adb | 35 - gcc/ada/gen_il-fields.ads | 1 - gcc/ada/gen_il-gen-gen_entities.adb | 1 - gcc/ada/gen_il-gen-gen_nodes.adb| 3 ++- gcc/ada/inline.adb | 10 - gcc/ada/sem_ch12.adb| 27 +- gcc/ada/sem_ch9.adb | 1 + gcc/ada/sem_util.adb| 16 + gcc/ada/sinfo.ads | 5 +++-- 12 files changed, 48 insertions(+), 91 deletions(-) diff --git a/gcc/ada/einfo.ads b/gcc/ada/einfo.ads index d346eddac57..78a1534c749 100644 --- a/gcc/ada/einfo.ads +++ b/gcc/ada/einfo.ads @@ -871,23 +871,6 @@ package Einfo is -- entity must be delayed, since the insertion of the generic body -- may affect cleanup generation (see Inline for further details). ---Delay_Subprogram_Descriptors --- Defined in entities for which exception subprogram descriptors --- are generated (subprograms, package declarations and package --- bodies). Defined if there are pending generic body instantiations --- for the corresponding entity. If this flag is set, then generation --- of the subprogram descriptor for the corresponding entities must --- be delayed, since the insertion of the generic body may add entries --- to the list of handlers. --- --- Note: for subprograms, Delay_Subprogram_Descriptors is set if and --- only if Delay_Cleanups is set. But Delay_Cleanups can be set for a --- a block (in which case Delay_Subprogram_Descriptors is set for the --- containing subprogram). In addition Delay_Subprogram_Descriptors is --- set for a library level package declaration or body which contains --- delayed instantiations (in this case the descriptor refers to the --- enclosing elaboration procedure). - --Delta_Value -- Defined in fixed and decimal types. Points to a universal real -- that holds value of delta for the type, as given in the declaration @@ -5552,7 +5535,6 @@ package Einfo is --Contains_Ignored_Ghost_Code --Default_Expressions_Processed --Delay_Cleanups - --Delay_Subprogram_Descriptors --Discard_Names --Elaboration_Entity_Required --Has_Completion @@ -5801,7 +5783,6 @@ package Einfo is --Body_Needed_For_Inlining --Body_Needed_For_SAL --Contains_Ignored_Ghost_Code - --Delay_Subprogram_Descriptors --Discard_Names --
[COMMITTED] ada: A discriminant of a variable is not a variable
From: Steve Baird gcc/ada/ * sem_util.adb (Is_Variable): Correctly return False for a selected component name of the form Some_Object.Some_Discriminant, even if Some_Object is a variable. We don't want to allow such a name as an actual parameter in a call if the corresponding formal parameter's mode is not "in". Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/sem_util.adb | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb index baf4cefdfb3..7e302897888 100644 --- a/gcc/ada/sem_util.adb +++ b/gcc/ada/sem_util.adb @@ -21196,11 +21196,8 @@ package body Sem_Util is return Is_Variable_Prefix (Prefix (Orig_Node)); when N_Selected_Component => - return (Is_Variable (Selector_Name (Orig_Node)) -and then Is_Variable_Prefix (Prefix (Orig_Node))) - or else - (Nkind (N) = N_Expanded_Name - and then Scope (Entity (N)) = Entity (Prefix (N))); + return Is_Variable (Selector_Name (Orig_Node)) + and then Is_Variable_Prefix (Prefix (Orig_Node)); -- For an explicit dereference, the type of the prefix cannot -- be an access to constant or an access to subprogram. -- 2.40.0
[COMMITTED] ada: Remove redundant parentheses from System.Stack_Checking.Operations
From: Patrick Bernardi gcc/ada/ * libgnat/s-stchop.adb (Stack_Check): Remove redundant parentheses. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/s-stchop.adb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gcc/ada/libgnat/s-stchop.adb b/gcc/ada/libgnat/s-stchop.adb index 8d8cc60ffb0..e0efcefa217 100644 --- a/gcc/ada/libgnat/s-stchop.adb +++ b/gcc/ada/libgnat/s-stchop.adb @@ -234,11 +234,10 @@ package body System.Stack_Checking.Operations is -- it is essential to use our local copy of Stack. begin - if (Stack_Grows_Down and then - (not (Frame_Address <= My_Stack.Base))) + if (Stack_Grows_Down and then not (Frame_Address <= My_Stack.Base)) or else (not Stack_Grows_Down and then - (not (Frame_Address >= My_Stack.Base))) + not (Frame_Address >= My_Stack.Base)) then -- The returned Base is lower than the stored one, so assume that -- the original one wasn't right and use the current Frame_Address -- 2.40.0
[COMMITTED] ada: Fix address arithmetic issues in the expanded code
From: Eric Botcazou This is most notably the addition of addresses in Expand_Interface_Thunk. There is also a small change to Expand_Dispatching_Call, which was directly accessing a class-wide interface object as a tag, thus giving rise later to unchecked conversions between either the root or the equivalent record type and access types. gcc/ada/ * exp_disp.adb (Expand_Dispatching_Call): In the abstract interface class-wide case, use 'Tag of the object as the controlling tag. (Expand_Interface_Thunk): Perform address arithmetic using operators of System.Storage_Elements. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_disp.adb | 69 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/gcc/ada/exp_disp.adb b/gcc/ada/exp_disp.adb index 1fb15fb7b02..e7cae38d553 100644 --- a/gcc/ada/exp_disp.adb +++ b/gcc/ada/exp_disp.adb @@ -1040,10 +1040,11 @@ package body Exp_Disp is -- Ada 2005 (AI-251): Abstract interface class-wide type - elsif Is_Interface (Ctrl_Typ) -and then Is_Class_Wide_Type (Ctrl_Typ) - then - Controlling_Tag := Duplicate_Subexpr (Ctrl_Arg); + elsif Is_Interface (Ctrl_Typ) and then Is_Class_Wide_Type (Ctrl_Typ) then + Controlling_Tag := + Make_Attribute_Reference (Loc, + Prefix => Duplicate_Subexpr (Ctrl_Arg), + Attribute_Name => Name_Tag); elsif Is_Access_Type (Ctrl_Typ) then Controlling_Tag := @@ -2030,8 +2031,8 @@ package body Exp_Disp is then -- Generate: -- type T is access all <> --- S : Storage_Offset := Storage_Offset!(Formal) ---+ Offset_To_Top (address!(Formal)) +-- S : constant Address := Address!(Formal) +-- + Offset_To_Top (Address!(Formal)) Decl_2 := Make_Full_Type_Declaration (Loc, @@ -2063,16 +2064,20 @@ package body Exp_Disp is Defining_Identifier => Make_Temporary (Loc, 'S'), Constant_Present=> True, Object_Definition => - New_Occurrence_Of (RTE (RE_Storage_Offset), Loc), + New_Occurrence_Of (RTE (RE_Address), Loc), Expression => - Make_Op_Add (Loc, -Left_Opnd => - Unchecked_Convert_To -(RTE (RE_Storage_Offset), - New_Occurrence_Of - (Defining_Identifier (Formal), Loc)), - Right_Opnd => - Offset_To_Top)); + Make_Function_Call (Loc, +Name => + Make_Expanded_Name (Loc, +Chars => Name_Op_Add, +Prefix => + New_Occurrence_Of +(RTU_Entity (System_Storage_Elements), Loc), +Selector_Name => + Make_Identifier (Loc, Name_Op_Add)), +Parameter_Associations => New_List ( + New_Copy_Tree (New_Arg), + Offset_To_Top))); Append_To (Decl, Decl_2); Append_To (Decl, Decl_1); @@ -2088,16 +2093,15 @@ package body Exp_Disp is elsif Is_Controlling_Formal (Target_Formal) then -- Generate: --- S1 : Storage_Offset := Storage_Offset!(Formal'Address) --- + Offset_To_Top (Formal'Address) --- S2 : Addr_Ptr := Addr_Ptr!(S1) +-- S1 : constant Address := Formal'Address +--+ Offset_To_Top (Formal'Address) +-- S2 : constant Addr_Ptr := Addr_Ptr!(S1) New_Arg := Make_Attribute_Reference (Loc, Prefix => New_Occurrence_Of (Defining_Identifier (Formal), Loc), -Attribute_Name => - Name_Address); +Attribute_Name => Name_Address); if not RTE_Available (RE_Offset_To_Top) then Offset_To_Top := @@ -2114,19 +2118,20 @@ package body Exp_Disp is Defining_Identifier => Make_Temporary (Loc, 'S'), Constant_Present=> True, Object_Definition => - New_Occurrence_Of (RTE (RE_Storage_Offset), Loc), + New_Occurrence_Of (RTE (RE_Address), Loc), Expression => - Make_Op_Add (Loc, -Left_Opnd => - Unchecked_Convert_To -(RTE (RE_Storage_Offset), - Make_Attribute_Reference (Loc, - Prefix => -
Re: [PATCH] libiberty: On Windows pass a >32k cmdline through a response file.
On 5/22/23 13:25, Costas Argyris wrote: Currently on Windows, when CreateProcess is called with a command-line that exceeds the 32k Windows limit, we get a very bad error: "CreateProcess: No such file or directory" This patch detects the case where this would happen and writes the long command-line to a temporary response file and calls CreateProcess with @file instead. Looks OK to me. I will commit it around next week if there are no objections.
[COMMITTED] ada: Fix resolution of mod operator of System.Storage_Elements
From: Eric Botcazou This operator is special because the left operand is of Address type while the right operand and the result are of Storage_Offset type (but we raise Constraint_Error if the right operand is not positive) and it needs to be resolved to the type of the left operand to implement the correct semantics. gcc/ada/ * exp_ch4.adb (Expand_N_Op_Mod): Adjust the detection of the special operator of System.Storage_Elements. Do not rewrite it into a rem. * sem_res.adb (Resolve_Intrinsic_Operator): Use the base type of the left operand for the special mod operator of System.Storage_Elements Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_ch4.adb | 11 +++ gcc/ada/sem_res.adb | 23 +++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb index c974a9e8d44..c7727904df2 100644 --- a/gcc/ada/exp_ch4.adb +++ b/gcc/ada/exp_ch4.adb @@ -9561,9 +9561,10 @@ package body Exp_Ch4 is DDC : constant Boolean:= Do_Division_Check (N); Is_Stoele_Mod : constant Boolean := -Is_RTE (First_Subtype (Typ), RE_Storage_Offset) - and then Nkind (Left_Opnd (N)) = N_Unchecked_Type_Conversion - and then Is_RTE (Etype (Expression (Left_Opnd (N))), RE_Address); +Is_RTE (Typ, RE_Address) + and then Nkind (Right_Opnd (N)) = N_Unchecked_Type_Conversion + and then +Is_RTE (Etype (Expression (Right_Opnd (N))), RE_Storage_Offset); -- True if this is the special mod operator of System.Storage_Elements Left : Node_Id; @@ -9633,6 +9634,7 @@ package body Exp_Ch4 is and then ((Llo >= 0 and then Rlo >= 0) or else (Lhi <= 0 and then Rhi <= 0)) +and then not Is_Stoele_Mod then Rewrite (N, Make_Op_Rem (Sloc (N), @@ -9683,7 +9685,8 @@ package body Exp_Ch4 is Make_Raise_Constraint_Error (Loc, Condition => Make_Op_Le (Loc, -Left_Opnd => Duplicate_Subexpr_No_Checks (Right), +Left_Opnd => + Duplicate_Subexpr_No_Checks (Expression (Right)), Right_Opnd => Make_Integer_Literal (Loc, 0)), Reason => CE_Overflow_Check_Failed)); return; diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb index a99bed00118..f1d9a97452a 100644 --- a/gcc/ada/sem_res.adb +++ b/gcc/ada/sem_res.adb @@ -9710,10 +9710,19 @@ package body Sem_Res is procedure Resolve_Intrinsic_Operator (N : Node_Id; Typ : Entity_Id) is - Btyp : constant Entity_Id := Implementation_Base_Type (Typ); - Op : Entity_Id; - Arg1 : Node_Id; - Arg2 : Node_Id; + Is_Stoele_Mod : constant Boolean := +Nkind (N) = N_Op_Mod + and then Is_RTE (First_Subtype (Typ), RE_Storage_Offset) + and then Is_RTE (Etype (Left_Opnd (N)), RE_Address); + -- True if this is the special mod operator of System.Storage_Elements, + -- which needs to be resolved to the type of the left operand in order + -- to implement the correct semantics. + + Btyp : constant Entity_Id := +(if Is_Stoele_Mod + then Implementation_Base_Type (Etype (Left_Opnd (N))) + else Implementation_Base_Type (Typ)); + -- The base type to be used for the operator function Convert_Operand (Opnd : Node_Id) return Node_Id; -- If the operand is a literal, it cannot be the expression in a @@ -9742,6 +9751,12 @@ package body Sem_Res is return Res; end Convert_Operand; + -- Local variables + + Arg1 : Node_Id; + Arg2 : Node_Id; + Op : Entity_Id; + -- Start of processing for Resolve_Intrinsic_Operator begin -- 2.40.0
Re: [C PATCH v3] Fix ICEs related to VM types in C 2/2
On Tue, May 23, 2023 at 8:24 AM Martin Uecker wrote: > > Am Dienstag, dem 23.05.2023 um 08:13 +0200 schrieb Richard Biener: > > On Mon, May 22, 2023 at 7:24 PM Martin Uecker via Gcc-patches > > wrote: > > > > > > > > > > > > This version contains the middle-end changes for PR109450 > > > and test cases as before. The main middle-end change is that > > > we use gimplify_type_sizes also for parameters and remove > > > the special code that also walked into pointers (which is > > > incorrect). > > > > > > In addition, in the C FE this patch now also adds DECL_EXPR > > > for vm-types which are pointed-to by parameters declared > > > as arrays. The new function created contains the exact > > > code previously used only for regular pointers, and is > > > now also called for parameters declared as arrays. > > > > > > > > > Martin > > > > > > > > > > > > > > > > > > > > > > > > Fix ICEs related to VM types in C 2/2 [PR109450] > > > > > > Size expressions were sometimes lost and not gimplified correctly, > > > leading to ICEs and incorrect evaluation order. Fix this by 1) not > > > recursing pointers when gimplifying parameters in the middle-end > > > (the code is merged with gimplify_type_sizes), which is incorrect > > > because it might access variables declared later for incomplete > > > structs, and 2) adding a decl expr for variably-modified arrays > > > that are pointed to by parameters declared as arrays. > > > > > > PR c/109450 > > > > > > gcc/ > > > * c/c-decl.cc (add_decl_expr): New function. > > > (grokdeclarator): Add decl expr for size expression in > > > types pointed to by parameters declared as arrays. > > > * function.cc (gimplify_parm_type): Remove function. > > > (gimplify_parameters): Call gimplify_parm_sizes. > > > * gimplify.cc (gimplify_type_sizes): Make function static. > > > (gimplify_parm_sizes): New function. > > > > > > gcc/testsuite/ > > > * gcc.dg/pr109450-1.c: New test. > > > * gcc.dg/pr109450-2.c: New test. > > > * gcc.dg/vla-26.c: New test. > > > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > > > index 494d3cf1747..c35347734b2 100644 > > > --- a/gcc/c/c-decl.cc > > > +++ b/gcc/c/c-decl.cc > > > @@ -6490,6 +6490,55 @@ smallest_type_quals_location (const location_t > > > *locations, > > >return loc; > > > } > > > > > > + > > > +/* We attach an artificial TYPE_DECL to pointed-to type > > > + and arrange for it to be included in a DECL_EXPR. This > > > + forces the sizes evaluation at a safe point and ensures it > > > + is not deferred until e.g. within a deeper conditional context. > > > + > > > + PARM contexts have no enclosing statement list that > > > + can hold the DECL_EXPR, so we need to use a BIND_EXPR > > > + instead, and add it to the list of expressions that > > > + need to be evaluated. > > > + > > > + TYPENAME contexts do have an enclosing statement list, > > > + but it would be incorrect to use it, as the size should > > > + only be evaluated if the containing expression is > > > + evaluated. We might also be in the middle of an > > > + expression with side effects on the pointed-to type size > > > + "arguments" prior to the pointer declaration point and > > > + the fake TYPE_DECL in the enclosing context would force > > > + the size evaluation prior to the side effects. We therefore > > > + use BIND_EXPRs in TYPENAME contexts too. */ > > > +static void > > > +add_decl_expr(location_t loc, enum decl_context decl_context, tree type, > > > tree *expr) > > > +{ > > > + tree bind = NULL_TREE; > > > + if (decl_context == TYPENAME || decl_context == PARM || decl_context > > > == FIELD) > > > +{ > > > + bind = build3 (BIND_EXPR, void_type_node, NULL_TREE, NULL_TREE, > > > NULL_TREE); > > > + TREE_SIDE_EFFECTS (bind) = 1; > > > + BIND_EXPR_BODY (bind) = push_stmt_list (); > > > + push_scope (); > > > +} > > > + > > > + tree decl = build_decl (loc, TYPE_DECL, NULL_TREE, type); > > > + pushdecl (decl); > > > + DECL_ARTIFICIAL (decl) = 1; > > > + add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl)); > > > + TYPE_NAME (type) = decl; > > > + > > > + if (bind) > > > +{ > > > + pop_scope (); > > > + BIND_EXPR_BODY (bind) = pop_stmt_list (BIND_EXPR_BODY (bind)); > > > + if (*expr) > > > + *expr = build2 (COMPOUND_EXPR, void_type_node, *expr, bind); > > > + else > > > + *expr = bind; > > > +} > > > +} > > > + > > > /* Given declspecs and a declarator, > > > determine the name and type of the object declared > > > and construct a ..._DECL node for it. > > > @@ -7474,58 +7523,9 @@ grokdeclarator (const struct c_declarator > > > *declarator, > > > > > >This is expected to happen automatically when the > > > pointed-to > > >
Re: Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization
Hi, Robin. >> Why does a store not have a destination (as commented below)? OK, V2 patch will have more comments. >> m_all_unmasked_p or m_fully_unmasked_p? OK. >> Apart from the insn-centric name, couldn't we also decide this >> based on the context later? In the vector-builtins.cc we have >> use_real_mask_p and use_real_merge_p that do this. Ok. V2 will follow builtin framework >> This means "has avl operand" I suppose? From the caller's point >> of view (and also the vsetvl pass) something like "needs avl" or so >> would be more descriptive but undecided here. Ok. >> Do we need to expose these in the constructor? As far as I can >> tell we can decide later whether the instruction has a policy >> or not (as I did in my patch, depending on whether all inputs >> are masks or so). Maybe, we can add helpers to set policies. I will send V2 let you see. >> Having the mask mode be automatically deduced from the destination >>is good, it was just obnoxious before to pass ... Ok >> I don't particularly like the names ;) Going back to vlmax and >> nonvlmax I don't mind but do we really need to have the policies >> encoded in the name now? Especially since "many" is a word and >> the default is ANY anyway. Why not emit_vlmax_insn/emit_vlmax_op >> for now and add the tu/mu later? Ok >> You can just drop the "The number = 11 is because" and say >> "We have a maximum of 11 operands for...". >> The eleven arguments seem a bit clunky here ;) I would suggest >> changing this again in the future bur for now let's just go ahead >> with it in order to make progress. Ok >> The rtx operands[] array I like least of the changes in this patch. >> It's essentially an untyped array whose meaning is dependent on context >> containing source operands and the length that is sometimes empty and >> sometimes not. I can't think of something that wouldn't complicate things >> though but before we at least had functions called _len that would take >> a length (NULL or not) and _vlmax that wouldn't. It's pretty easy to mess >> up here on the caller's side. ARM uses rtx operands[] in many places and I personally prefer this way since it will make codes much cleaner. I dislike the way making the function argument with multiple operand ,like this: void func(rtx dest, rtx src1, rtx src2, ) If we are doing this, we will need to add helpers forever... Sending V2 patch soon. Thanks. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-05-23 16:06 To: juzhe.zhong; gcc-patches CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; jeffreyalaw Subject: Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization Hi Juzhe, in general I find the revised structure quite logical and it is definitely an improvement. Some abstraction are still a bit leaky but we can always refactor "on the fly". Some comments on the general parts, skipping over the later details. > bool m_has_dest_p; Why does a store not have a destination (as commented below)? > /* It't true if the pattern uses all trues mask operand. */ > bool m_use_all_trues_mask_p; m_all_unmasked_p or m_fully_unmasked_p? > /* It's true if the pattern uses undefined merge operand. */ > bool m_use_undef_merge_p; Apart from the insn-centric name, couldn't we also decide this based on the context later? In the vector-builtins.cc we have use_real_mask_p and use_real_merge_p that do this. > bool m_has_avl_p; This means "has avl operand" I suppose? From the caller's point of view (and also the vsetvl pass) something like "needs avl" or so would be more descriptive but undecided here. > bool m_vlmax_p; > bool m_has_tail_policy_p; > bool m_has_mask_policy_p; Do we need to expose these in the constructor? As far as I can tell we can decide later whether the instruction has a policy or not (as I did in my patch, depending on whether all inputs are masks or so). > enum tail_policy m_tail_policy; > enum mask_policy m_mask_policy; > machine_mode m_dest_mode; > machine_mode m_mask_mode; Having the mask mode be automatically deduced from the destination is good, it was just obnoxious before to pass ... > Currently, we have "emit_vlmax_tany_many" and "emit_nonvlmax_tany_many". I don't particularly like the names ;) Going back to vlmax and nonvlmax I don't mind but do we really need to have the policies encoded in the name now? Especially since "many" is a word and the default is ANY anyway. Why not emit_vlmax_insn/emit_vlmax_op for now and add the tu/mu later? > #define RVV_BINOP_NUM 3 (number including the output) Could make this into an "instruction type" rather than just a number (i.e. RVV_BINOP) and then set the number of operands internally according to the type. This would also make it clearer in case we later want to set other options depending on the type. > Then just use emit_vlmax_tany_many (...RVV_BINOP_NUM...) > > So, if we support ternary operation in the future. It's quite simple: > #define RVV_TER
Re: Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization
> ARM uses rtx operands[] in many places and I personally prefer this way since > it will make codes much cleaner. > I dislike the way making the function argument with multiple operand ,like > this: > void func(rtx dest, rtx src1, rtx src2, ) > If we are doing this, we will need to add helpers forever... Don't forget we are using C++, so we have function overloading or default arguments :)
[PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization
From: Juzhe-Zhong This patch is to refactor the framework of RVV auto-vectorization. Since we find out are keep adding helpers && wrappers when implementing auto-vectorization. It will make the RVV auto-vectorizaiton very messy. After double check my downstream RVV GCC, assemble all auto-vectorization patterns we are going to have. Base on these informations, I refactor the RVV framework to make it is easier and flexible for future use. For example, we will definitely implement len_mask_load/len_mask_store patterns which have both length && mask operand and use undefine merge operand. len_cond_div or cond_div will have length or mask operand and use a real merge operand instead of undefine merge operand. Also, we will have some patterns will use tail undisturbed and mask any. etc. We will defintely have various features. Base on these circumstances, we add these following private members: int m_op_num; /* It't true when the pattern has a dest operand. Most of the patterns have dest operand wheras some patterns like STOREs does not have dest operand. */ bool m_has_dest_p; bool m_fully_unmasked_p; bool m_use_real_merge_p; bool m_has_avl_p; bool m_vlmax_p; bool m_has_tail_policy_p; bool m_has_mask_policy_p; enum tail_policy m_tail_policy; enum mask_policy m_mask_policy; machine_mode m_dest_mode; machine_mode m_mask_mode; These variables I believe can cover all potential situations. And the instruction generater wrapper is "emit_insn" which will add operands and emit instruction according to the variables I mentioned above. After this is done. We will easily add helpers without changing any base class "insn_expand". Currently, we have "emit_vlmax_tany_many" and "emit_nonvlmax_tany_many". For example, when we want to emit a binary operations: We have #define RVV_BINOP_NUM 3 (number including the output) Then just use emit_vlmax_tany_many (...RVV_BINOP_NUM...) So, if we support ternary operation in the future. It's quite simple: #define RVV_TERNOP_NUM 4 (number including the output) emit_vlmax_tany_many (...RVV_BINOP_NUM...) "*_tany_many" means we are using tail any and mask any. We will definitely need tail undisturbed or mask undisturbed when we support these patterns in middle-end. It's very simple to extend such helper base on current framework: we can do that in the future like this: void emit_nonvlmax_tu_mu (unsigned icode, int op_num, rtx *ops) { machine_mode data_mode = GET_MODE (ops[0]); machine_mode mask_mode = get_mask_mode (data_mode).require (); /* The number = 11 is because we have maximum 11 operands for RVV instruction patterns according to vector.md. */ insn_expander<11> e (/*OP_NUM*/ op_num, /*HAS_DEST_P*/ true, /*USE_ALL_TRUES_MASK_P*/ true, /*USE_UNDEF_MERGE_P*/ true, /*HAS_AVL_P*/ true, /*VLMAX_P*/ false, /*HAS_TAIL_POLICY_P*/ true, /*HAS_MASK_POLICY_P*/ true, /*TAIL_POLICY*/ TAIL_UNDISTURBED, /*MASK_POLICY*/ MASK_UNDISTURBED, /*DEST_MODE*/ data_mode, /*MASK_MODE*/ mask_mode); e.emit_insn ((enum insn_code) icode, ops); } That's enough (I have tested it fully in my downstream RVV GCC). I didn't add it in this patch. Thanks. gcc/ChangeLog: * config/riscv/autovec.md: Refactor the framework of RVV auto-vectorization. * config/riscv/riscv-protos.h (RVV_MISC_OP_NUM): Ditto. (RVV_UNOP_NUM): New macro. (RVV_BINOP_NUM): Ditto. (legitimize_move): Refactor the framework of RVV auto-vectorization. (emit_vlmax_op): Ditto. (emit_vlmax_reg_op): Ditto. (emit_len_op): Ditto. (emit_len_binop): Ditto. (emit_vlmax_tany_many): Ditto. (emit_nonvlmax_tany_many): Ditto. (sew64_scalar_helper): Ditto. (expand_tuple_move): Ditto. * config/riscv/riscv-v.cc (emit_pred_op): Ditto. (emit_pred_binop): Ditto. (emit_vlmax_op): Ditto. (emit_vlmax_tany_many): New function. (emit_len_op): Remove. (emit_nonvlmax_tany_many): New function. (emit_vlmax_reg_op): Remove. (emit_len_binop): Ditto. (emit_index_op): Ditto. (expand_vec_series): Refactor the framework of RVV auto-vectorization. (expand_const_vector): Ditto. (legitimize_move): Ditto. (sew64_scalar_helper): Ditto. (expand_tuple_move): Ditto. (expand_vector_init_insert_elems): Ditto. * config/riscv/riscv.cc (vector_zero_call_used_regs): Ditto. * config/riscv/vector.md: Ditto. --- gcc/config/riscv/autovec.md | 40 +--- gcc/config/riscv/riscv-protos.h | 19 +- gcc/config/riscv/riscv-v.cc | 354 ++-- gcc/config/riscv/riscv.cc | 8 +- gcc/config/riscv/vector.md | 40 +--- 5 files changed, 232 insertions(+), 229 deletions(-) diff --git a/gcc/config/riscv/autovec.md b/g
Re: Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization
Yeah. I know. Like ARM does everywhere: (define_expand "vcond" [(set (match_operand:SVE_ALL 0 "register_operand") (if_then_else:SVE_ALL (match_operator 3 "comparison_operator" [(match_operand:SVE_I 4 "register_operand") (match_operand:SVE_I 5 "nonmemory_operand")]) (match_operand:SVE_ALL 1 "nonmemory_operand") (match_operand:SVE_ALL 2 "nonmemory_operand")))] "TARGET_SVE && == " { aarch64_expand_sve_vcond (mode, mode, operands); DONE; } ) passing "operands" looks codes much cleaner. Hi, kito. Could you take a look at the V2 refactor patch: https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619291.html This is important for us since we can't post more autovec patches without refactor patch. Thanks juzhe.zh...@rivai.ai From: Kito Cheng Date: 2023-05-23 16:45 To: juzhe.zh...@rivai.ai CC: Robin Dapp; gcc-patches; Kito.cheng; palmer; palmer; jeffreyalaw Subject: Re: Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization > ARM uses rtx operands[] in many places and I personally prefer this way since > it will make codes much cleaner. > I dislike the way making the function argument with multiple operand ,like > this: > void func(rtx dest, rtx src1, rtx src2, ) > If we are doing this, we will need to add helpers forever... Don't forget we are using C++, so we have function overloading or default arguments :)
Re: [C PATCH v3] Fix ICEs related to VM types in C 2/2
Am Dienstag, dem 23.05.2023 um 10:18 +0200 schrieb Richard Biener: > On Tue, May 23, 2023 at 8:24 AM Martin Uecker > wrote: > > > > Am Dienstag, dem 23.05.2023 um 08:13 +0200 schrieb Richard Biener: > > > On Mon, May 22, 2023 at 7:24 PM Martin Uecker via Gcc-patches > > > wrote: > > > > > > > > > > > > > > > > This version contains the middle-end changes for PR109450 > > > > and test cases as before. The main middle-end change is that > > > > we use gimplify_type_sizes also for parameters and remove > > > > the special code that also walked into pointers (which is > > > > incorrect). > > > > > > > > In addition, in the C FE this patch now also adds DECL_EXPR > > > > for vm-types which are pointed-to by parameters declared > > > > as arrays. The new function created contains the exact > > > > code previously used only for regular pointers, and is > > > > now also called for parameters declared as arrays. > > > > > > > > > > > > Martin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Fix ICEs related to VM types in C 2/2 [PR109450] > > > > > > > > Size expressions were sometimes lost and not gimplified > > > > correctly, > > > > leading to ICEs and incorrect evaluation order. Fix this > > > > by 1) not > > > > recursing pointers when gimplifying parameters in the > > > > middle-end > > > > (the code is merged with gimplify_type_sizes), which is > > > > incorrect > > > > because it might access variables declared later for > > > > incomplete > > > > structs, and 2) adding a decl expr for variably-modified > > > > arrays > > > > that are pointed to by parameters declared as arrays. > > > > > > > > PR c/109450 > > > > > > > > gcc/ > > > > * c/c-decl.cc (add_decl_expr): New function. > > > > (grokdeclarator): Add decl expr for size expression > > > > in > > > > types pointed to by parameters declared as arrays. > > > > * function.cc (gimplify_parm_type): Remove > > > > function. > > > > (gimplify_parameters): Call gimplify_parm_sizes. > > > > * gimplify.cc (gimplify_type_sizes): Make function > > > > static. > > > > (gimplify_parm_sizes): New function. > > > > > > > > gcc/testsuite/ > > > > * gcc.dg/pr109450-1.c: New test. > > > > * gcc.dg/pr109450-2.c: New test. > > > > * gcc.dg/vla-26.c: New test. > > > > > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > > > > index 494d3cf1747..c35347734b2 100644 > > > > --- a/gcc/c/c-decl.cc > > > > +++ b/gcc/c/c-decl.cc > > > > @@ -6490,6 +6490,55 @@ smallest_type_quals_location (const > > > > location_t *locations, > > > > return loc; > > > > } > > > > > > > > + > > > > +/* We attach an artificial TYPE_DECL to pointed-to type > > > > + and arrange for it to be included in a DECL_EXPR. This > > > > + forces the sizes evaluation at a safe point and ensures it > > > > + is not deferred until e.g. within a deeper conditional > > > > context. > > > > + > > > > + PARM contexts have no enclosing statement list that > > > > + can hold the DECL_EXPR, so we need to use a BIND_EXPR > > > > + instead, and add it to the list of expressions that > > > > + need to be evaluated. > > > > + > > > > + TYPENAME contexts do have an enclosing statement list, > > > > + but it would be incorrect to use it, as the size should > > > > + only be evaluated if the containing expression is > > > > + evaluated. We might also be in the middle of an > > > > + expression with side effects on the pointed-to type size > > > > + "arguments" prior to the pointer declaration point and > > > > + the fake TYPE_DECL in the enclosing context would force > > > > + the size evaluation prior to the side effects. We > > > > therefore > > > > + use BIND_EXPRs in TYPENAME contexts too. */ > > > > +static void > > > > +add_decl_expr(location_t loc, enum decl_context decl_context, > > > > tree type, tree *expr) > > > > +{ > > > > + tree bind = NULL_TREE; > > > > + if (decl_context == TYPENAME || decl_context == PARM || > > > > decl_context == FIELD) > > > > + { > > > > + bind = build3 (BIND_EXPR, void_type_node, NULL_TREE, > > > > NULL_TREE, NULL_TREE); > > > > + TREE_SIDE_EFFECTS (bind) = 1; > > > > + BIND_EXPR_BODY (bind) = push_stmt_list (); > > > > + push_scope (); > > > > + } > > > > + > > > > + tree decl = build_decl (loc, TYPE_DECL, NULL_TREE, type); > > > > + pushdecl (decl); > > > > + DECL_ARTIFICIAL (decl) = 1; > > > > + add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), > > > > DECL_EXPR, decl)); > > > > + TYPE_NAME (type) = decl; > > > > + > > > > + if (bind) > > > > + { > > > > + pop_scope (); > > > > + BIND_EXPR_BODY (bind) = pop_stmt_list (BIND_EXPR_BODY > > > > (bind)); > > > > + if (*expr) > > > > + *expr = build2 (COMPOUND_EXPR, void_type_node, *expr, > > > > bind); > > > > +
Re: [PATCH] libatomic: Provide gthr.h default implementation
On 10.01.23 16:38, Sebastian Huber wrote: On 19/12/2022 17:02, Sebastian Huber wrote: Build libatomic for all targets. Use gthr.h to provide a default implementation. If the thread model is "single", then this implementation will not work if for example atomic operations are used for thread/interrupt synchronization. Is this and the related -fprofile-update=atomic patch something for GCC 14? Now that the GCC 14 development is in progress, what about this patch? -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 fax: +49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/
[PATCH] testsuite, analyzer: Fix testcases with fclose
The gcc.dg/analyzer/data-model-4.c and gcc.dg/analyzer/torture/conftest-1.c fail with recent glibc headers and succeed with older headers. The new error message is: warning: use of possibly-NULL 'f' where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument] Like similar previous fixes in this area, this patch updates the testcase so that this warning isn't reported. 2023-05-23 Christophe Lyon gcc/testsuite/ * gcc.dg/analyzer/data-model-4.c: Exit if fopen returns NULL. * gcc.dg/analyzer/torture/conftest-1.c: Likewise. --- gcc/testsuite/gcc.dg/analyzer/data-model-4.c | 2 ++ gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-4.c b/gcc/testsuite/gcc.dg/analyzer/data-model-4.c index 33f90871dfb..d41868d6dbc 100644 --- a/gcc/testsuite/gcc.dg/analyzer/data-model-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/data-model-4.c @@ -8,6 +8,8 @@ int main () { FILE *f = fopen ("conftest.out", "w"); + if (f == NULL) +return 1; return ferror (f) || fclose (f) != 0; ; diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c b/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c index 0cf85f0ebe1..9631bcf73e0 100644 --- a/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c @@ -3,6 +3,8 @@ int main () { FILE *f = fopen ("conftest.out", "w"); + if (f == NULL) +return 1; return ferror (f) || fclose (f) != 0; ; -- 2.34.1
Re: [PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization
Hi Juzhe, thanks, IMHO it's clearer with the changes now. There are still things that could be improved but it is surely an improvement over what we currently have. Therefore I'd vote to go ahead so we can continue with more expanders and changes. Still, we should be prepared for more refactoring changes in the future. Regards Robin
Re: Re: [PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization
Oh, Thanks. Let's wait for Kito's final approved. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-05-23 17:44 To: juzhe.zhong; gcc-patches CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; jeffreyalaw Subject: Re: [PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization Hi Juzhe, thanks, IMHO it's clearer with the changes now. There are still things that could be improved but it is surely an improvement over what we currently have. Therefore I'd vote to go ahead so we can continue with more expanders and changes. Still, we should be prepared for more refactoring changes in the future. Regards Robin
[PATCH] tree-optimization/109849 - missed code hoisting
The following fixes code hoisting to properly consider ANTIC_OUT instead of ANTIC_IN. That's a bit expensive to re-compute but since we no longer iterate we're doing this only once per BB which should be acceptable. This avoids missing hoistings to the end of blocks where something in the block clobbers the hoisted value. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/109849 * tree-ssa-pre.cc (do_hoist_insertion): Compute ANTIC_OUT and use that to determine what to hoist. * gcc.dg/tree-ssa/ssa-hoist-8.c: New testcase. --- gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-8.c | 22 ++ gcc/tree-ssa-pre.cc | 48 ++--- 2 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-8.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-8.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-8.c new file mode 100644 index 000..66bb48e0dc1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-8.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-pre-stats" } */ + +int mem; +void foo (); +int bar (int flag) +{ + int res; + foo (); + /* Hoist the load of mem here even though foo () clobbers it. */ + if (flag) +res = mem; + else +{ + res = mem; + mem = 2; +} + return res; +} + +/* { dg-final { scan-tree-dump "HOIST inserted: 1" "pre" } } */ +/* { dg-final { scan-tree-dump-times " = mem;" 1 "pre" } } */ diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc index 1f7eea93c16..d56431b4145 100644 --- a/gcc/tree-ssa-pre.cc +++ b/gcc/tree-ssa-pre.cc @@ -3622,19 +3622,51 @@ do_hoist_insertion (basic_block block) && stmt_ends_bb_p (gsi_stmt (last))) return false; - /* Compute the set of hoistable expressions from ANTIC_IN. First compute + /* We have multiple successors, compute ANTIC_OUT by taking the intersection + of all of ANTIC_IN translating through PHI nodes. Note we do not have to + worry about iteration stability here so just intersect the expression sets + as well. This is a simplification of what we do in compute_antic_aux. */ + bitmap_set_t ANTIC_OUT = bitmap_set_new (); + bool first = true; + FOR_EACH_EDGE (e, ei, block->succs) +{ + if (first) + { + phi_translate_set (ANTIC_OUT, ANTIC_IN (e->dest), e); + first = false; + } + else if (!gimple_seq_empty_p (phi_nodes (e->dest))) + { + bitmap_set_t tmp = bitmap_set_new (); + phi_translate_set (tmp, ANTIC_IN (e->dest), e); + bitmap_and_into (&ANTIC_OUT->values, &tmp->values); + bitmap_and_into (&ANTIC_OUT->expressions, &tmp->expressions); + bitmap_set_free (tmp); + } + else + { + bitmap_and_into (&ANTIC_OUT->values, &ANTIC_IN (e->dest)->values); + bitmap_and_into (&ANTIC_OUT->expressions, + &ANTIC_IN (e->dest)->expressions); + } +} + + /* Compute the set of hoistable expressions from ANTIC_OUT. First compute hoistable values. */ bitmap_set hoistable_set; - /* A hoistable value must be in ANTIC_IN(block) + /* A hoistable value must be in ANTIC_OUT(block) but not in AVAIL_OUT(BLOCK). */ bitmap_initialize (&hoistable_set.values, &grand_bitmap_obstack); bitmap_and_compl (&hoistable_set.values, - &ANTIC_IN (block)->values, &AVAIL_OUT (block)->values); + &ANTIC_OUT->values, &AVAIL_OUT (block)->values); /* Short-cut for a common case: hoistable_set is empty. */ if (bitmap_empty_p (&hoistable_set.values)) -return false; +{ + bitmap_set_free (ANTIC_OUT); + return false; +} /* Compute which of the hoistable values is in AVAIL_OUT of at least one of the successors of BLOCK. */ @@ -3652,11 +3684,14 @@ do_hoist_insertion (basic_block block) /* Short-cut for a common case: availout_in_some is empty. */ if (bitmap_empty_p (&availout_in_some)) -return false; +{ + bitmap_set_free (ANTIC_OUT); + return false; +} /* Hack hoitable_set in-place so we can use sorted_array_from_bitmap_set. */ bitmap_move (&hoistable_set.values, &availout_in_some); - hoistable_set.expressions = ANTIC_IN (block)->expressions; + hoistable_set.expressions = ANTIC_OUT->expressions; /* Now finally construct the topological-ordered expression set. */ vec exprs = sorted_array_from_bitmap_set (&hoistable_set); @@ -3731,6 +3766,7 @@ do_hoist_insertion (basic_block block) } exprs.release (); + bitmap_set_free (ANTIC_OUT); return new_stuff; } -- 2.35.3
Re: Re: [PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization
Lgtm, we can always improve later, I am not intend to block things too :) juzhe.zh...@rivai.ai 於 2023年5月23日 週二 17:46 寫道: > Oh, Thanks. > Let's wait for Kito's final approved. > > > > juzhe.zh...@rivai.ai > > From: Robin Dapp > Date: 2023-05-23 17:44 > To: juzhe.zhong; gcc-patches > CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; jeffreyalaw > Subject: Re: [PATCH V2] RISC-V: Refactor the framework of RVV > auto-vectorization > Hi Juzhe, > > thanks, IMHO it's clearer with the changes now. There are still > things that could be improved but it is surely an improvement over > what we currently have. Therefore I'd vote to go ahead so we can > continue with more expanders and changes. > > Still, we should be prepared for more refactoring changes in the future. > > Regards > Robin > >
[PATCH][committed] aarch64: PR target/109855 Add predicate and constraints to define_subst in aarch64-simd.md
Hi all, In this PR we ICE because the substituted pattern for mla "lost" its predicate and constraint for operand 0 because the define_subst template: [(set (match_operand: 0) (vec_concat: (match_dup 1) (match_operand:VDZ 2 "aarch64_simd_or_scalar_imm_zero")))]) Uses match_operand instead of match_dup for operand 0. We can't use match_dup 0 for it because we need to specify the widened mode. The problem is fixed by adding a "register_operand" predicate and "=w" constraint to the match_operand. This makes sense conceptually too as the transformation we're targeting only applies to instructions that write a "w" register. With this change the mddump pattern that ICEs goes from: (define_insn ("aarch64_mlav4hi_vec_concatz_le") [ (set (match_operand:V8HI 0 ("") ("")) <<-- Missing constraint! (vec_concat:V8HI (plus:V4HI (mult:V4HI (match_operand:V4HI 2 ("register_operand") ("w")) (match_operand:V4HI 3 ("register_operand") ("w"))) (match_operand:V4HI 1 ("register_operand") ("0"))) (match_operand:V4HI 4 ("aarch64_simd_or_scalar_imm_zero") ("" ] ("(!BYTES_BIG_ENDIAN) && (TARGET_SIMD)") ("mla\t%0.4h, %2.4h, %3.4h") to the proper: (define_insn ("aarch64_mlav4hi_vec_concatz_le") [ (set (match_operand:V8HI 0 ("register_operand") ("=w")) << Constraint in the right place (vec_concat:V8HI (plus:V4HI (mult:V4HI (match_operand:V4HI 2 ("register_operand") ("w")) (match_operand:V4HI 3 ("register_operand") ("w"))) (match_operand:V4HI 1 ("register_operand") ("0"))) (match_operand:V4HI 4 ("aarch64_simd_or_scalar_imm_zero") ("" ] ("(!BYTES_BIG_ENDIAN) && (TARGET_SIMD)") ("mla\t%0.4h, %2.4h, %3.4h") This seems to do the right thing for multi-alternative patterns as well, the annotated pattern for aarch64_cmltv8qi is: (define_insn ("aarch64_cmltv8qi") [ (set (match_operand:V8QI 0 ("register_operand") ("=w,w")) (neg:V8QI (lt:V8QI (match_operand:V8QI 1 ("register_operand") ("w,w")) (match_operand:V8QI 2 ("aarch64_simd_reg_or_zero") ("w,ZDz") ] whereas the substituted version now looks like: (define_insn ("aarch64_cmltv8qi_vec_concatz_le") [ (set (match_operand:V16QI 0 ("register_operand") ("=w,w")) (vec_concat:V16QI (neg:V8QI (lt:V8QI (match_operand:V8QI 1 ("register_operand") ("w,w")) (match_operand:V8QI 2 ("aarch64_simd_reg_or_zero") ("w,ZDz" (match_operand:V8QI 3 ("aarch64_simd_or_scalar_imm_zero") ("" ] Bootstrapped and tested on aarch64-none-linux-gnu. Pushing to trunk. Thanks, Kyrill gcc/ChangeLog: PR target/109855 * config/aarch64/aarch64-simd.md (add_vec_concat_subst_le): Add predicate and constraint for operand 0. (add_vec_concat_subst_be): Likewise. gcc/testsuite/ChangeLog: PR target/109855 * gcc.target/aarch64/pr109855.c: New test. subst-pred.patch Description: subst-pred.patch
[PATCH] RISC-V: Fix warning of vxrm pattern
From: Juzhe-Zhong I just notice the warning: ../../../riscv-gcc/gcc/config/riscv/vector.md:618:1: warning: source missing a mode? gcc/ChangeLog: * config/riscv/vector.md: Add mode. --- gcc/config/riscv/vector.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md index ac244430970..13b94862693 100644 --- a/gcc/config/riscv/vector.md +++ b/gcc/config/riscv/vector.md @@ -617,7 +617,7 @@ ;; Set VXRM (define_insn "vxrmsi" [(set (reg:SI VXRM_REGNUM) - (match_operand 0 "const_int_operand" "i"))] + (match_operand:SI 0 "const_int_operand" "i"))] "TARGET_VECTOR" "csrwi\tvxrm,%0" [(set_attr "type" "wrvxrm") -- 2.36.3
RE: Re: [PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization
Committed, thanks Kito. Pan -Original Message- From: Gcc-patches On Behalf Of Kito Cheng via Gcc-patches Sent: Tuesday, May 23, 2023 5:57 PM To: 钟居哲 Cc: Robin Dapp ; gcc-patches ; Kito.cheng ; palmer ; palmer ; jeffreyalaw Subject: Re: Re: [PATCH V2] RISC-V: Refactor the framework of RVV auto-vectorization Lgtm, we can always improve later, I am not intend to block things too :) juzhe.zh...@rivai.ai 於 2023年5月23日 週二 17:46 寫道: > Oh, Thanks. > Let's wait for Kito's final approved. > > > > juzhe.zh...@rivai.ai > > From: Robin Dapp > Date: 2023-05-23 17:44 > To: juzhe.zhong; gcc-patches > CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; jeffreyalaw > Subject: Re: [PATCH V2] RISC-V: Refactor the framework of RVV > auto-vectorization Hi Juzhe, > > thanks, IMHO it's clearer with the changes now. There are still > things that could be improved but it is surely an improvement over > what we currently have. Therefore I'd vote to go ahead so we can > continue with more expanders and changes. > > Still, we should be prepared for more refactoring changes in the future. > > Regards > Robin > >
[PATCH 2/2] aarch64: Provide FPR alternatives for some bit insertions [PR109632]
At -O2, and so with SLP vectorisation enabled: struct complx_t { float re, im; }; complx_t add(complx_t a, complx_t b) { return {a.re + b.re, a.im + b.im}; } generates: fmovw3, s1 fmovx0, d0 fmovx1, d2 fmovw2, s3 bfi x0, x3, 32, 32 fmovd31, x0 bfi x1, x2, 32, 32 fmovd30, x1 faddv31.2s, v31.2s, v30.2s fmovx1, d31 lsr x0, x1, 32 fmovs1, w0 lsr w0, w1, 0 fmovs0, w0 ret This is because complx_t is passed and returned in FPRs, but GCC gives it DImode. We therefore “need” to assemble a DImode pseudo from the two individual floats, bitcast it to a vector, do the arithmetic, bitcast it back to a DImode pseudo, then extract the individual floats. There are many problems here. The most basic is that we shouldn't use SLP for such a trivial example. But SLP should in principle be beneficial for more complicated examples, so preventing SLP for the example above just changes the reproducer needed. A more fundamental problem is that it doesn't make sense to use single DImode pseudos in a testcase like this. I have a WIP patch to allow re and im to be stored in individual SFmode pseudos instead, but it's quite an invasive change and might end up going nowhere. A simpler problem to tackle is that we allow DImode pseudos to be stored in FPRs, but we don't provide any patterns for inserting values into them, even though INS makes that easy for element-like insertions. This patch adds some patterns for that. Doing that showed that aarch64_modes_tieable_p was too strict: it didn't allow SFmode and DImode values to be tied, even though both of them occupy a single GPR and FPR, and even though we allow both classes to change between the modes. The *aarch64_bfidi_subreg_ pattern is especially ugly, but it's not clear what target-independent code ought to simplify it to, if it was going to simplify it. We should probably do the same thing for extractions, but that's left as future work. After the patch we generate: ins v0.s[1], v1.s[0] ins v2.s[1], v3.s[0] faddv0.2s, v0.2s, v2.2s fmovx0, d0 ushrd1, d0, 32 lsr w0, w0, 0 fmovs0, w0 ret which seems like a step in the right direction. All in all, there's nothing elegant about this patchh. It just seems like the least worst option. Tested on aarch64-linux-gnu and aarch64_be-elf (including ILP32). Pushed to trunk. Richard gcc/ PR target/109632 * config/aarch64/aarch64.cc (aarch64_modes_tieable_p): Allow subregs between any scalars that are 64 bits or smaller. * config/aarch64/iterators.md (SUBDI_BITS): New int iterator. (bits_etype): New int attribute. * config/aarch64/aarch64.md (*insv_reg_) (*aarch64_bfi_): New patterns. (*aarch64_bfidi_subreg_): Likewise. gcc/testsuite/ * gcc.target/aarch64/ins_bitfield_1.c: New test. * gcc.target/aarch64/ins_bitfield_2.c: Likewise. * gcc.target/aarch64/ins_bitfield_3.c: Likewise. * gcc.target/aarch64/ins_bitfield_4.c: Likewise. * gcc.target/aarch64/ins_bitfield_5.c: Likewise. * gcc.target/aarch64/ins_bitfield_6.c: Likewise. --- gcc/config/aarch64/aarch64.cc | 12 ++ gcc/config/aarch64/aarch64.md | 62 +++ gcc/config/aarch64/iterators.md | 4 + .../gcc.target/aarch64/ins_bitfield_1.c | 142 .../gcc.target/aarch64/ins_bitfield_2.c | 142 .../gcc.target/aarch64/ins_bitfield_3.c | 156 ++ .../gcc.target/aarch64/ins_bitfield_4.c | 156 ++ .../gcc.target/aarch64/ins_bitfield_5.c | 139 .../gcc.target/aarch64/ins_bitfield_6.c | 139 9 files changed, 952 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_1.c create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_3.c create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_4.c create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_5.c create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_6.c diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index d6fc94015fa..146c2ad4988 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -24827,6 +24827,18 @@ aarch64_modes_tieable_p (machine_mode mode1, machine_mode mode2) if (GET_MODE_CLASS (mode1) == GET_MODE_CLASS (mode2)) return true; + /* Allow changes between scalar modes if both modes fit within 64 bits. + This is because: + + - We allow all such modes for both FPRs and GPRs. + - They occupy a single register for both FPRs and GPRs. + - We can
[PATCH 1/2] md: Allow to refer to the value of int iterator FOO
In a follow-up patch, I wanted to use an int iterator to iterate over various possible values of a const_int. But one problem with int iterators was that there was no way of referring to the current value of the iterator. This is unlike modes and codes, which provide automatic "mode", "MODE", "code" and "CODE" attribbutes. These automatic definitions are the equivalent of an explicit: (define_mode_attr mode [(QI "qi") (HI "hi") ...]) We obviously can't do that for every possible value of an int. One option would have been to go for some kind of lazily-populated attribute. But that sounds quite complicated. This patch instead goes for the simpler approach of allowing to refer to the current value of FOO. In principle it would be possible to allow the same thing for mode and code iterators. But for modes there are at least 4 realistic possiblities: - the E_* enumeration value (which is what this patch would give) - the user-facing C token, like DImode, SFmode, etc. - the equivalent of - the equivalent of Because of this ambiguity, it seemed better to stick to the current approach for modes. For codes it's less clear-cut, but and are both realistic possibilities, so again it seemed better to be explicit. The patch also removes “Each @var{int} must have the same rtx format. @xref{RTL Classes}.”, which was erroneously copied from the code iterator section. Tested on aarch64-linux-gnu and x86_64-linux-gnu. Also tested by checking that no port had an int iterator whose name matched an existing attribute. Pushed to trunk. Richard gcc/ * doc/md.texi: Document that can be used to refer to the numerical value of an int iterator FOO. Tweak other parts of the int iterator documentation. * read-rtl.cc (iterator_group::has_self_attr): New field. (map_attr_string): When has_self_attr is true, make expand to the current value of iterator FOO. (initialize_iterators): Set has_self_attr for int iterators. --- gcc/doc/md.texi | 15 +-- gcc/read-rtl.cc | 26 ++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi index 8ebce31ba78..6a435eb4461 100644 --- a/gcc/doc/md.texi +++ b/gcc/doc/md.texi @@ -11502,11 +11502,10 @@ The construct: @end smallexample defines a pseudo integer constant @var{name} that can be instantiated as -@var{inti} if condition @var{condi} is true. Each @var{int} must have the -same rtx format. @xref{RTL Classes}. Int iterators can appear in only -those rtx fields that have 'i', 'n', 'w', or 'p' as the specifier. This -means that each @var{int} has to be a constant defined using define_constant -or define_c_enum. +@var{inti} if condition @var{condi} is true. Int iterators can appear in +only those rtx fields that have `i', `n', `w', or `p' as the specifier. +This means that each @var{int} has to be a constant defined using +@samp{define_constant} or @samp{define_c_enum}. As with mode and code iterators, each pattern that uses @var{name} will be expanded @var{n} times, once with all uses of @var{name} replaced by @@ -11517,9 +11516,13 @@ It is possible to define attributes for ints as well as for codes and modes. Attributes are defined using: @smallexample -(define_int_attr @var{name} [(@var{int1} "@var{value1}") @dots{} (@var{intn} "@var{valuen}")]) +(define_int_attr @var{attr_name} [(@var{int1} "@var{value1}") @dots{} (@var{intn} "@var{valuen}")]) @end smallexample +In additon to these user-defined attributes, it is possible to use +@samp{<@var{name}>} to refer to the current expansion of iterator +@var{name} (such as @var{int1}, @var{int2}, and so on). + Here's an example of int iterators in action, taken from the ARM port: @smallexample diff --git a/gcc/read-rtl.cc b/gcc/read-rtl.cc index 8cb25aebdbb..292f8b72d43 100644 --- a/gcc/read-rtl.cc +++ b/gcc/read-rtl.cc @@ -80,6 +80,12 @@ struct iterator_group { /* Return the C token for the given standard mode, code, etc. */ const char *(*get_c_token) (int); + + /* True if each iterator name should be treated as an attribute that + maps to the C token produced by get_c_token. This means that for + an iterator ITER, can be used in strings to refer to the + current value of ITER, as a C token. */ + bool has_self_attr; }; /* Records one use of an iterator. */ @@ -472,6 +478,25 @@ map_attr_string (file_location loc, const char *p, mapping **iterator_out = 0) || iterator->name[iterator_name_len] != 0)) continue; + if (iterator->group->has_self_attr + && strcmp (attr, iterator->name) == 0) + { + if (iterator_out) + *iterator_out = iterator; + int number = iterator->current_value->number; + const char *string = iterator->group->get_c_token (number); + if (res && strcmp (string, res->string) != 0) + { + error_at (loc, "ambiguous attr
Re: [PATCH 2/2] aarch64: Provide FPR alternatives for some bit insertions [PR109632]
On Tue, May 23, 2023 at 12:38 PM Richard Sandiford via Gcc-patches wrote: > > At -O2, and so with SLP vectorisation enabled: > > struct complx_t { float re, im; }; > complx_t add(complx_t a, complx_t b) { > return {a.re + b.re, a.im + b.im}; > } > > generates: > > fmovw3, s1 > fmovx0, d0 > fmovx1, d2 > fmovw2, s3 > bfi x0, x3, 32, 32 > fmovd31, x0 > bfi x1, x2, 32, 32 > fmovd30, x1 > faddv31.2s, v31.2s, v30.2s > fmovx1, d31 > lsr x0, x1, 32 > fmovs1, w0 > lsr w0, w1, 0 > fmovs0, w0 > ret > > This is because complx_t is passed and returned in FPRs, but GCC gives > it DImode. Isn't that the choice of the target? Of course "FPRs" might mean a single FPR here and arguably DFmode would be similarly bad? That said, to the ppc folks who also recently tried to change how argument passing materializes I suggested to piggy-back on a SRA style analysis (you could probably simply build the access tree for all function parameters using its infrastructure) to drive RTL expansion heuristics (it's heuristics after all...) what exact (set of) pseudo / stack slot we want to form from the actual argument hard registers. > We therefore “need” to assemble a DImode pseudo from the > two individual floats, bitcast it to a vector, do the arithmetic, > bitcast it back to a DImode pseudo, then extract the individual floats. > > There are many problems here. The most basic is that we shouldn't > use SLP for such a trivial example. But SLP should in principle be > beneficial for more complicated examples, so preventing SLP for the > example above just changes the reproducer needed. A more fundamental > problem is that it doesn't make sense to use single DImode pseudos in a > testcase like this. I have a WIP patch to allow re and im to be stored > in individual SFmode pseudos instead, but it's quite an invasive change > and might end up going nowhere. > > A simpler problem to tackle is that we allow DImode pseudos to be stored > in FPRs, but we don't provide any patterns for inserting values into > them, even though INS makes that easy for element-like insertions. > This patch adds some patterns for that. > > Doing that showed that aarch64_modes_tieable_p was too strict: > it didn't allow SFmode and DImode values to be tied, even though > both of them occupy a single GPR and FPR, and even though we allow > both classes to change between the modes. > > The *aarch64_bfidi_subreg_ pattern is > especially ugly, but it's not clear what target-independent > code ought to simplify it to, if it was going to simplify it. > > We should probably do the same thing for extractions, but that's left > as future work. > > After the patch we generate: > > ins v0.s[1], v1.s[0] > ins v2.s[1], v3.s[0] > faddv0.2s, v0.2s, v2.2s > fmovx0, d0 > ushrd1, d0, 32 > lsr w0, w0, 0 > fmovs0, w0 > ret > > which seems like a step in the right direction. > > All in all, there's nothing elegant about this patchh. It just > seems like the least worst option. > > Tested on aarch64-linux-gnu and aarch64_be-elf (including ILP32). > Pushed to trunk. > > Richard > > > gcc/ > PR target/109632 > * config/aarch64/aarch64.cc (aarch64_modes_tieable_p): Allow > subregs between any scalars that are 64 bits or smaller. > * config/aarch64/iterators.md (SUBDI_BITS): New int iterator. > (bits_etype): New int attribute. > * config/aarch64/aarch64.md (*insv_reg_) > (*aarch64_bfi_): New patterns. > (*aarch64_bfidi_subreg_): Likewise. > > gcc/testsuite/ > * gcc.target/aarch64/ins_bitfield_1.c: New test. > * gcc.target/aarch64/ins_bitfield_2.c: Likewise. > * gcc.target/aarch64/ins_bitfield_3.c: Likewise. > * gcc.target/aarch64/ins_bitfield_4.c: Likewise. > * gcc.target/aarch64/ins_bitfield_5.c: Likewise. > * gcc.target/aarch64/ins_bitfield_6.c: Likewise. > --- > gcc/config/aarch64/aarch64.cc | 12 ++ > gcc/config/aarch64/aarch64.md | 62 +++ > gcc/config/aarch64/iterators.md | 4 + > .../gcc.target/aarch64/ins_bitfield_1.c | 142 > .../gcc.target/aarch64/ins_bitfield_2.c | 142 > .../gcc.target/aarch64/ins_bitfield_3.c | 156 ++ > .../gcc.target/aarch64/ins_bitfield_4.c | 156 ++ > .../gcc.target/aarch64/ins_bitfield_5.c | 139 > .../gcc.target/aarch64/ins_bitfield_6.c | 139 > 9 files changed, 952 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_1.c > create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_2.c > create mode 100644 gcc/testsuite/gcc.target/aa
[PATCH] Dump ANTIC_OUT before pruning it
This dumps ANTIC_OUT before pruning clobbered mems from it as part of the ANTIC_IN compute. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. * tree-ssa-pre.cc (compute_antic_aux): Dump the correct ANTIC_OUT. --- gcc/tree-ssa-pre.cc | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc index d56431b4145..b1ceea90a8e 100644 --- a/gcc/tree-ssa-pre.cc +++ b/gcc/tree-ssa-pre.cc @@ -2216,6 +2216,10 @@ compute_antic_aux (basic_block block, bool block_has_abnormal_pred_edge) } } + /* Dump ANTIC_OUT before it's pruned. */ + if (dump_file && (dump_flags & TDF_DETAILS)) +print_bitmap_set (dump_file, ANTIC_OUT, "ANTIC_OUT", block->index); + /* Prune expressions that are clobbered in block and thus become invalid if translated from ANTIC_OUT to ANTIC_IN. */ prune_clobbered_mems (ANTIC_OUT, block); @@ -2270,9 +2274,6 @@ compute_antic_aux (basic_block block, bool block_has_abnormal_pred_edge) maybe_dump_sets: if (dump_file && (dump_flags & TDF_DETAILS)) { - if (ANTIC_OUT) - print_bitmap_set (dump_file, ANTIC_OUT, "ANTIC_OUT", block->index); - if (changed) fprintf (dump_file, "[changed] "); print_bitmap_set (dump_file, ANTIC_IN (block), "ANTIC_IN", -- 2.35.3
Re: [PATCH 2/2] aarch64: Provide FPR alternatives for some bit insertions [PR109632]
Richard Biener writes: > On Tue, May 23, 2023 at 12:38 PM Richard Sandiford via Gcc-patches > wrote: >> >> At -O2, and so with SLP vectorisation enabled: >> >> struct complx_t { float re, im; }; >> complx_t add(complx_t a, complx_t b) { >> return {a.re + b.re, a.im + b.im}; >> } >> >> generates: >> >> fmovw3, s1 >> fmovx0, d0 >> fmovx1, d2 >> fmovw2, s3 >> bfi x0, x3, 32, 32 >> fmovd31, x0 >> bfi x1, x2, 32, 32 >> fmovd30, x1 >> faddv31.2s, v31.2s, v30.2s >> fmovx1, d31 >> lsr x0, x1, 32 >> fmovs1, w0 >> lsr w0, w1, 0 >> fmovs0, w0 >> ret >> >> This is because complx_t is passed and returned in FPRs, but GCC gives >> it DImode. > > Isn't that the choice of the target? Of course "FPRs" might mean a > single FPR here and arguably DFmode would be similarly bad? Yeah, the problem is really the "single register" aspect, rather than the exact choice of mode. We're happy to store DImode values in FPRs if it makes sense (and we will do for this example, after the patch). V2SFmode or DFmode would be just as bad, like you say. > That said, to the ppc folks who also recently tried to change how > argument passing materializes I suggested to piggy-back on a > SRA style analysis (you could probably simply build the access > tree for all function parameters using its infrastructure) to drive > RTL expansion heuristics (it's heuristics after all...) what exact > (set of) pseudo / stack slot we want to form from the actual > argument hard registers. My long-term plan is to allow a DECL_RTL to be a PARALLEL of pseudos, just like the DECL_INCOMING_RTL of a PARM_DECL can be a PARALLEL of hard registers. This also makes it possible to store things that are currently BLKmode (but still passed and returned in registers). E.g. it means that a 12-byte structure can be stored in registers rather than being forced to the stack frame. The idea (at least at first) is to handle only those cases that make sense from an ABI point of view. We'd still be relying on SRA to split up operations on individual fields. I have a WIP patch that gives some promising improvements, but it needs more time. Thanks, Richard
RE: [PATCH] arm: Fix ICE due to infinite splitting [PR109800]
Hi Alex, > -Original Message- > From: Alex Coplan > Sent: Thursday, May 11, 2023 12:15 PM > To: gcc-patches@gcc.gnu.org > Cc: ni...@redhat.com; Richard Earnshaw ; > Ramana Radhakrishnan ; Kyrylo Tkachov > > Subject: [PATCH] arm: Fix ICE due to infinite splitting [PR109800] > > Hi, > > In r11-966-g9a182ef9ee011935d827ab5c6c9a7cd8e22257d8 we introduce a > simplification to emit_move_insn that attempts to simplify moves of the > form: > > (set (subreg:M1 (reg:M2 ...)) (constant C)) > > where M1 and M2 are of equal mode size. That is problematic for the splitter > vfp.md:no_literal_pool_df_immediate in the arm backend, which tries to pun > an > lvalue DFmode pseudo into DImode and assign a constant to it with > emit_move_insn, as the new transformation simply undoes this, and we end > up > splitting indefinitely. > > This patch changes things around in the arm backend so that we use a > DImode temporary (instead of DFmode) and first load the DImode constant > into the pseudo, and then pun the pseudo into DFmode as an rvalue in a > reg -> reg move. I believe this should be semantically equivalent but > avoids the pathalogical behaviour seen in the PR. > > Bootstrapped/regtested on arm-linux-gnueabihf, regtested on > arm-none-eabi and armeb-none-eabi. > > OK for trunk and backports? Ok but the testcase... > > Thanks, > Alex > > gcc/ChangeLog: > > PR target/109800 > * config/arm/arm.md (movdf): Generate temporary pseudo in > DImode > instead of DFmode. > * config/arm/vfp.md (no_literal_pool_df_immediate): Rather than > punning an > lvalue DFmode pseudo into DImode, use a DImode pseudo and pun it > into > DFmode as an rvalue. > > gcc/testsuite/ChangeLog: > > PR target/109800 > * gcc.target/arm/pr109800.c: New test. diff --git a/gcc/testsuite/gcc.target/arm/pr109800.c b/gcc/testsuite/gcc.target/arm/pr109800.c new file mode 100644 index 000..71d1ede13dd --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr109800.c @@ -0,0 +1,3 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=armv7-m -mfloat-abi=hard -mfpu=fpv4-sp-d16 -mbig-endian -mpure-code" } */ +double f() { return 5.0; } ... The arm testsuite options are kinda hard to get right with all the effective targets and multilibs and such hardcoded abi and march options tend to break in some target. I suggest you put this testcase in gcc.target/arm/pure-code and add a dg-skip-if to skip the test if the multilib options specify a different float-abi. Thanks, Kyrill
Re: [PATCH] c-family: implement -ffp-contract=on
On Tue, 23 May 2023, Richard Biener wrote: > > Ah, no, I deliberately decided against that, because that way we would go > > via gimplify_arg, which would emit all side effects in *pre_p. That seems > > wrong if arguments had side-effects that should go in *post_p. > > Ah, true - that warrants a comment though. Incrementally fixed up in my tree like this: diff --git a/gcc/c-family/c-gimplify.cc b/gcc/c-family/c-gimplify.cc index f7635d3b0c..17b0610a89 100644 --- a/gcc/c-family/c-gimplify.cc +++ b/gcc/c-family/c-gimplify.cc @@ -803,6 +803,7 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED, else ops[2] = build1 (NEGATE_EXPR, type, ops[2]); } + /* Avoid gimplify_arg: it emits all side effects into *PRE_P. */ for (auto &&op : ops) if (gimplify_expr (&op, pre_p, post_p, is_gimple_val, fb_rvalue) == GS_ERROR) Alexander
Re: [PATCH] RISC-V: Refactor the framework of RVV auto-vectorization
"juzhe.zh...@rivai.ai" writes: > Yeah. I know. > Like ARM does everywhere: > (define_expand "vcond" > [(set (match_operand:SVE_ALL 0 "register_operand") > (if_then_else:SVE_ALL > (match_operator 3 "comparison_operator" > [(match_operand:SVE_I 4 "register_operand") >(match_operand:SVE_I 5 "nonmemory_operand")]) > (match_operand:SVE_ALL 1 "nonmemory_operand") > (match_operand:SVE_ALL 2 "nonmemory_operand")))] > "TARGET_SVE && == " > { > aarch64_expand_sve_vcond (mode, mode, operands); > DONE; > } > ) > > passing "operands" looks codes much cleaner. FWIW, I think we only do that when we're reusing optab patterns. The handling of operand 3 is forced by the definition of vcond_optab. When there's a choice, we generally use "@" patterns instead, and pass codes and modes to the expander. Thanks, Richard
Re: [PATCH] Fix type error of 'switch (SUBREG_BYTE (op)).'
Jeff Law via Gcc-patches writes: > On 5/17/23 03:03, Jin Ma wrote: >> For example: >> (define_insn "mov_lowpart_sidi2" >>[(set (match_operand:SI0 "register_operand" "=r") >> (subreg:SI (match_operand:DI 1 "register_operand" " r") 0))] >>"TARGET_64BIT" >>"mov\t%0,%1") >> >> (define_insn "mov_highpart_sidi2" >>[(set (match_operand:SI0 "register_operand" "=r") >> (subreg:SI (match_operand:DI 1 "register_operand" " r") 1))] >>"TARGET_64BIT" >>"movh\t%0,%1") >> >> When defining the above patterns, the generated file insn-recog.cc will >> appear 'switch (SUBREG_BYTE (op))', but since the return value of >> SUBREG_BYTE is poly_uint16_pod, the following error will occur: >> "error: switch quantity not an integer". >> >> gcc/ChangeLog: >> >> * genrecog.cc (print_nonbool_test): Fix type error of >> 'switch (SUBREG_BYTE (op))'. > Thanks. Installed. We shouldn't add to_constant just because it's a convenient way of getting rid of errors :) There has to be a good reason in principle why the value is known at compile time. So I think this should be reverted. Nothing guarantees that SUBREG_BYTEs are constant on AArch64 and RISC-V. And for SVE it's common for them not to be. If we want to support the above, I think we need to make the generator use known_eq instead. The patterns don't look right though. An SI subreg of a DI can't have a SUBREG_BYTE of 1. And the lowpart SUBREG_BYTE depends on endianness. So I think a better way of writing the lowpart pattern above is to use subreg_lowpart_operator (which riscv already has). The high part can't be done using subregs though. Thanks, Richard
Re: [PATCH V12] VECT: Add decrement IV iteration loop control by variable amount support
Bootstrap on X86 passed. Ok for trunk? Thanks. juzhe.zh...@rivai.ai From: juzhe.zhong Date: 2023-05-22 16:38 To: gcc-patches CC: richard.sandiford; rguenther; Ju-Zhe Zhong Subject: [PATCH V12] VECT: Add decrement IV iteration loop control by variable amount support From: Ju-Zhe Zhong gcc/ChangeLog: * tree-vect-loop-manip.cc (vect_adjust_loop_lens_control): New function. (vect_set_loop_controls_directly): Add decrement IV support. (vect_set_loop_condition_partial_vectors): Ditto. * tree-vect-loop.cc: Ditto. * tree-vectorizer.h (LOOP_VINFO_USING_DECREMENTING_IV_P): New macro. --- gcc/tree-vect-loop-manip.cc | 184 +++- gcc/tree-vect-loop.cc | 10 ++ gcc/tree-vectorizer.h | 8 ++ 3 files changed, 199 insertions(+), 3 deletions(-) diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc index ff6159e08d5..94b38d1e0fb 100644 --- a/gcc/tree-vect-loop-manip.cc +++ b/gcc/tree-vect-loop-manip.cc @@ -385,6 +385,66 @@ vect_maybe_permute_loop_masks (gimple_seq *seq, rgroup_controls *dest_rgm, return false; } +/* Try to use adjust loop lens for non-SLP multiple-rgroups. + + _36 = MIN_EXPR ; + + First length (MIN (X, VF/N)): + loop_len_15 = MIN_EXPR <_36, VF/N>; + + Second length: + tmp = _36 - loop_len_15; + loop_len_16 = MIN (tmp, VF/N); + + Third length: + tmp2 = tmp - loop_len_16; + loop_len_17 = MIN (tmp2, VF/N); + + Last length: + loop_len_18 = tmp2 - loop_len_17; +*/ + +static void +vect_adjust_loop_lens_control (tree iv_type, gimple_seq *seq, +rgroup_controls *dest_rgm, +rgroup_controls *src_rgm, tree step) +{ + tree ctrl_type = dest_rgm->type; + poly_uint64 nitems_per_ctrl += TYPE_VECTOR_SUBPARTS (ctrl_type) * dest_rgm->factor; + tree length_limit = build_int_cst (iv_type, nitems_per_ctrl); + + for (unsigned int i = 0; i < dest_rgm->controls.length (); ++i) +{ + if (!step) + step = src_rgm->controls[i / dest_rgm->controls.length ()]; + tree ctrl = dest_rgm->controls[i]; + if (i == 0) + { + /* First iteration: MIN (X, VF/N) capped to the range [0, VF/N]. */ + gassign *assign + = gimple_build_assign (ctrl, MIN_EXPR, step, length_limit); + gimple_seq_add_stmt (seq, assign); + } + else if (i == dest_rgm->controls.length () - 1) + { + /* Last iteration: Remain capped to the range [0, VF/N]. */ + gassign *assign = gimple_build_assign (ctrl, MINUS_EXPR, step, + dest_rgm->controls[i - 1]); + gimple_seq_add_stmt (seq, assign); + } + else + { + /* (MIN (remain, VF*I/N)) capped to the range [0, VF/N]. */ + step = gimple_build (seq, MINUS_EXPR, iv_type, step, +dest_rgm->controls[i - 1]); + gassign *assign + = gimple_build_assign (ctrl, MIN_EXPR, step, length_limit); + gimple_seq_add_stmt (seq, assign); + } +} +} + /* Helper for vect_set_loop_condition_partial_vectors. Generate definitions for all the rgroup controls in RGC and return a control that is nonzero when the loop needs to iterate. Add any new preheader statements to @@ -468,9 +528,78 @@ vect_set_loop_controls_directly (class loop *loop, loop_vec_info loop_vinfo, gimple_stmt_iterator incr_gsi; bool insert_after; standard_iv_increment_position (loop, &incr_gsi, &insert_after); - create_iv (build_int_cst (iv_type, 0), PLUS_EXPR, nitems_step, NULL_TREE, - loop, &incr_gsi, insert_after, &index_before_incr, - &index_after_incr); + if (LOOP_VINFO_USING_DECREMENTING_IV_P (loop_vinfo)) +{ + nitems_total = gimple_convert (preheader_seq, iv_type, nitems_total); + tree step = make_ssa_name (iv_type); + /* Create decrement IV. */ + create_iv (nitems_total, MINUS_EXPR, step, NULL_TREE, loop, &incr_gsi, + insert_after, &index_before_incr, &index_after_incr); + tree temp = gimple_build (header_seq, MIN_EXPR, iv_type, + index_before_incr, nitems_step); + gimple_seq_add_stmt (header_seq, gimple_build_assign (step, temp)); + + if (rgc->max_nscalars_per_iter == 1) + { + /* single rgroup: + ... + _10 = (unsigned long) count_12(D); + ... + # ivtmp_9 = PHI + _36 = MIN_EXPR ; + ... + vect__4.8_28 = .LEN_LOAD (_17, 32B, _36, 0); + ... + ivtmp_35 = ivtmp_9 - _36; + ... + if (ivtmp_35 != 0) +goto ; [83.33%] + else +goto ; [16.67%] + */ + gassign *assign = gimple_build_assign (rgc->controls[0], step); + gimple_seq_add_stmt (header_seq, assign); + } + else + { + /* Multiple rgroup (SLP): + ... + _38 = (unsigned long) bnd.7_29; + _39 = _38 * 2; + ... + # ivtmp_41 = PHI + ... + _43 = MIN_EXPR ; + loop_len_26 = MIN_EXPR <_43, 16>; + loop_len_25 = _43 - loop_len_26; + ... + .LEN_STORE (_6, 8B, loop_len_26, ...); + ... + .LEN_STORE (_25, 8B, loop_len_25, ...); + _33 = loop_len_26 / 2; + ... +
[patch]: Implement PR104327 for avr
PR target/104327 not only affects s390 but also avr: The avr backend pre-sets some options depending on optimization level. The inliner then thinks that always_inline functions are not eligible for inlining and terminates with an error. Proposing the following patch that implements TARGET_CAN_INLINE_P. Ok to apply? Johann -- target/104327: Allow more inlining between different optimization levels. avr-common.cc introduces the following options that are set depending on optimization level: -mgas-isr-prologues, -mmain-is-OS-task and -fsplit-wide-types-early. The inliner thinks that different options disallow cross-optimization inlining, so provide can_inline_p. gcc/ PR target/104327 * config/avr/avr.cc (avr_can_inline_p): New static function. (TARGET_CAN_INLINE_P): Define to that function. diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc index 9fa50ca230d..55b48f63865 100644 --- a/gcc/config/avr/avr.cc +++ b/gcc/config/avr/avr.cc @@ -1018,6 +1018,22 @@ avr_no_gccisr_function_p (tree func) return avr_lookup_function_attribute1 (func, "no_gccisr"); } + +/* Implement `TARGET_CAN_INLINE_P'. */ +/* Some options like -mgas_isr_prologues depend on optimization level, + and the inliner might think that due to different options, inlining + is not permitted; see PR104327. */ + +static bool +avr_can_inline_p (tree /* caller */, tree callee) +{ + // For now, dont't allow to inline ISRs. If the user actually wants + // to inline ISR code, they have to turn the body of the ISR into an + // ordinary function. + + return ! avr_interrupt_function_p (callee); +} + /* Implement `TARGET_SET_CURRENT_FUNCTION'. */ /* Sanity cheching for above function attributes. */ @@ -14713,6 +14729,9 @@ avr_float_lib_compare_returns_bool (machine_mode mode, enum rtx_code) #undef TARGET_MD_ASM_ADJUST #define TARGET_MD_ASM_ADJUST avr_md_asm_adjust +#undef TARGET_CAN_INLINE_P +#define TARGET_CAN_INLINE_P avr_can_inline_p + struct gcc_target targetm = TARGET_INITIALIZER;
[COMMITTED] Remove buggy special case in irange::invert [PR109934].
[Andrew, do you remotely remember what if anything this did? It came from a wholesale merge from our long forgotten branch, so there's no history on the specifics of it. Not important, I'm just curious. It was probably me high on something.] This patch removes a buggy special case in irange::invert which seems to have been broken for a while, and probably never triggered because the legacy code was handled elsewhere, and the non-legacy code was using an int_range_max of int_range<255> which made it extremely likely for num_ranges == 255. However, with auto-resizing ranges, int_range_max will start off at 3 and can hit this bogus code in the unswitching code. PR tree-optimization/109934 gcc/ChangeLog: * value-range.cc (irange::invert): Remove buggy special case. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/pr109934.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/pr109934.c | 22 ++ gcc/value-range.cc | 8 2 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr109934.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr109934.c b/gcc/testsuite/gcc.dg/tree-ssa/pr109934.c new file mode 100644 index 000..08bd5ce95c6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr109934.c @@ -0,0 +1,22 @@ +// { dg-do run } +// { dg-options "-O3" } + +int printf(const char *, ...); +short a; +long b = 3, c; +int d(int e) { + switch (e) + case 111: + case 222: + case 44: +return 0; + return e; +} +int main() { + for (; a >= 0; --a) +if (d(c + 23) - 23) + b = 0; + + if (b != 3) +__builtin_abort (); +} diff --git a/gcc/value-range.cc b/gcc/value-range.cc index 45b1e655967..874a1843ebf 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -1650,14 +1650,6 @@ irange::invert () wide_int type_min = wi::min_value (prec, sign); wide_int type_max = wi::max_value (prec, sign); m_nonzero_mask = wi::minus_one (prec); - if (m_num_ranges == m_max_ranges - && lower_bound () != type_min - && upper_bound () != type_max) -{ - m_base[1] = type_max; - m_num_ranges = 1; - return; -} // At this point, we need one extra sub-range to represent the // inverse. -- 2.40.1
Re: [COMMITTED] Remove buggy special case in irange::invert [PR109934].
BTW, we should probably backport this to god knows how many branches. Aldy On Tue, May 23, 2023 at 2:58 PM Aldy Hernandez wrote: > > [Andrew, do you remotely remember what if anything this did? It came > from a wholesale merge from our long forgotten branch, so there's no > history on the specifics of it. Not important, I'm just curious. It > was probably me high on something.] > > This patch removes a buggy special case in irange::invert which seems > to have been broken for a while, and probably never triggered because > the legacy code was handled elsewhere, and the non-legacy code was > using an int_range_max of int_range<255> which made it extremely > likely for num_ranges == 255. However, with auto-resizing ranges, > int_range_max will start off at 3 and can hit this bogus code in the > unswitching code. > > PR tree-optimization/109934 > > gcc/ChangeLog: > > * value-range.cc (irange::invert): Remove buggy special case. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/pr109934.c: New test. > --- > gcc/testsuite/gcc.dg/tree-ssa/pr109934.c | 22 ++ > gcc/value-range.cc | 8 > 2 files changed, 22 insertions(+), 8 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr109934.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr109934.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr109934.c > new file mode 100644 > index 000..08bd5ce95c6 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr109934.c > @@ -0,0 +1,22 @@ > +// { dg-do run } > +// { dg-options "-O3" } > + > +int printf(const char *, ...); > +short a; > +long b = 3, c; > +int d(int e) { > + switch (e) > + case 111: > + case 222: > + case 44: > +return 0; > + return e; > +} > +int main() { > + for (; a >= 0; --a) > +if (d(c + 23) - 23) > + b = 0; > + > + if (b != 3) > +__builtin_abort (); > +} > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > index 45b1e655967..874a1843ebf 100644 > --- a/gcc/value-range.cc > +++ b/gcc/value-range.cc > @@ -1650,14 +1650,6 @@ irange::invert () >wide_int type_min = wi::min_value (prec, sign); >wide_int type_max = wi::max_value (prec, sign); >m_nonzero_mask = wi::minus_one (prec); > - if (m_num_ranges == m_max_ranges > - && lower_bound () != type_min > - && upper_bound () != type_max) > -{ > - m_base[1] = type_max; > - m_num_ranges = 1; > - return; > -} > >// At this point, we need one extra sub-range to represent the >// inverse. > -- > 2.40.1 >
Re: [PATCH] RISC-V: Fix warning of vxrm pattern
On 5/23/23 04:09, juzhe.zh...@rivai.ai wrote: From: Juzhe-Zhong I just notice the warning: ../../../riscv-gcc/gcc/config/riscv/vector.md:618:1: warning: source missing a mode? gcc/ChangeLog: * config/riscv/vector.md: Add mode. While I'm a big fan of the gen* warnings, I do wish they had a bit more smarts to avoid the missing mode warnings for arguments which are restricted to CONST_INTs which don't have modes. Oh well. OK for the trunk. jeff
[PATCH V2] RISC-V: Add RVV comparison autovectorization
From: Juzhe-Zhong This patch enable RVV auto-vectorization including floating-point unorder and order comparison. The testcases are leveraged from Richard. So include Richard as co-author. Co-Authored-By: Richard Sandiford gcc/ChangeLog: * config/riscv/autovec.md (@vcond_mask_): New pattern. (vec_cmp): Ditto. (vec_cmpu): Ditto. (vcond): Ditto. (vcondu): Ditto. * config/riscv/riscv-protos.h (enum insn_type): Add new enum. (emit_vlmax_merge_insn): New function. (emit_vlmax_cmp_insn): Ditto. (expand_vec_cmp): Ditto. (expand_vec_cmp_float):Ditto. (expand_vcond):Ditto. * config/riscv/riscv-v.cc (emit_vlmax_merge_insn): Ditto. (emit_vlmax_cmp_insn): Ditto. (get_cmp_insn_code): Ditto. (expand_vec_cmp): Ditto. (expand_vec_cmp_float): Ditto. (expand_vcond): Ditto. gcc/testsuite/ChangeLog: * gcc.target/riscv/rvv/rvv.exp: Add RVV comparison testcases. * gcc.target/riscv/rvv/autovec/cmp/vcond-1.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond-2.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond-3.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond_run-1.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond_run-2.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond_run-3.c: New test. --- gcc/config/riscv/autovec.md | 112 gcc/config/riscv/riscv-protos.h | 7 + gcc/config/riscv/riscv-v.cc | 258 +- .../riscv/rvv/autovec/cmp/vcond-1.c | 157 +++ .../riscv/rvv/autovec/cmp/vcond-2.c | 75 + .../riscv/rvv/autovec/cmp/vcond-3.c | 13 + .../riscv/rvv/autovec/cmp/vcond_run-1.c | 49 .../riscv/rvv/autovec/cmp/vcond_run-2.c | 76 ++ .../riscv/rvv/autovec/cmp/vcond_run-3.c | 6 + gcc/testsuite/gcc.target/riscv/rvv/rvv.exp| 2 + 10 files changed, 754 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond-3.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond_run-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond_run-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond_run-3.c diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md index 04b4459222a..e0258e8b798 100644 --- a/gcc/config/riscv/autovec.md +++ b/gcc/config/riscv/autovec.md @@ -162,3 +162,115 @@ riscv_vector::RVV_BINOP, operands); DONE; }) + +;; = +;; == Comparisons and selects +;; = + +;; - +;; [INT,FP] Select based on masks +;; - +;; Includes merging patterns for: +;; - vmerge.vv +;; - vmerge.vx +;; - vfmerge.vf +;; - + +(define_expand "@vcond_mask_" + [(match_operand:V 0 "register_operand") + (match_operand: 3 "register_operand") + (match_operand:V 1 "nonmemory_operand") + (match_operand:V 2 "register_operand")] + "TARGET_VECTOR" + { +/* The order of vcond_mask is opposite to pred_merge. */ +std::swap (operands[1], operands[2]); +riscv_vector::emit_vlmax_merge_insn (code_for_pred_merge (mode), + riscv_vector::RVV_MERGE_OP, operands); +DONE; + } +) + +;; - +;; [INT,FP] Comparisons +;; - +;; Includes: +;; - vms. +;; - + +(define_expand "vec_cmp" + [(set (match_operand: 0 "register_operand") + (match_operator: 1 "comparison_operator" + [(match_operand:VI 2 "register_operand") + (match_operand:VI 3 "register_operand")]))] + "TARGET_VECTOR" + { +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), + operands[2], operands[3]); +DONE; + } +) + +(define_expand "vec_cmpu" + [(set (match_operand: 0 "register_operand") + (match_operator: 1 "comparison_operator" + [(match_operand:VI 2 "register_operand") + (match_operand:VI 3 "register_operand")]))] + "TARGET_VECTOR" + { +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), + operands[2], operands[3]); +DONE; + } +) + +(define_expand "vec_cmp" + [(set (match_ope
Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization
> +(define_expand "vec_cmp" > + [(set (match_operand: 0 "register_operand") > + (match_operator: 1 "comparison_operator" > + [(match_operand:VI 2 "register_operand") > +(match_operand:VI 3 "register_operand")]))] > + "TARGET_VECTOR" > + { > +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), > + operands[2], operands[3]); > +DONE; > + } > +) > + > +(define_expand "vec_cmpu" > + [(set (match_operand: 0 "register_operand") > + (match_operator: 1 "comparison_operator" > + [(match_operand:VI 2 "register_operand") > +(match_operand:VI 3 "register_operand")]))] > + "TARGET_VECTOR" > + { > +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), > + operands[2], operands[3]); > +DONE; > + } > +) > + > +(define_expand "vec_cmp" > + [(set (match_operand: 0 "register_operand") > + (match_operator: 1 "comparison_operator" > + [(match_operand:VF 2 "register_operand") > +(match_operand:VF 3 "register_operand")]))] > + "TARGET_VECTOR" > + { > +riscv_vector::expand_vec_cmp_float (operands[0], GET_CODE (operands[1]), > + operands[2], operands[3], false); > +DONE; > + } > +) Don't you want to use your shiny new operand passing style here as with the other expanders? > + /* We have a maximum of 11 operands for RVV instruction patterns according > to > + * vector.md. */ > + insn_expander<11> e (/*OP_NUM*/ op_num, /*HAS_DEST_P*/ true, > +/*FULLY_UNMASKED_P*/ false, > +/*USE_REAL_MERGE_P*/ false, /*HAS_AVL_P*/ true, > +/*VLMAX_P*/ true, > +/*DEST_MODE*/ data_mode, /*MASK_MODE*/ mask_mode); > + e.set_policy (TAIL_ANY); > + e.emit_insn ((enum insn_code) icode, ops); > +} I don't think we need the same comment in each of these. Same for /*DEST_MODE*/ and /*MASK_MODE*/ which would be redundant if data_mode were called dest_mode. > +/* Expand an RVV comparison. */ > + > +void > +expand_vec_cmp (rtx target, rtx_code code, rtx op0, rtx op1) > +{ > + machine_mode mask_mode = GET_MODE (target); > + machine_mode data_mode = GET_MODE (op0); > + insn_code icode = get_cmp_insn_code (code, data_mode); > + > + if (code == LTGT) > +{ > + rtx gt = gen_reg_rtx (mask_mode); > + rtx lt = gen_reg_rtx (mask_mode); > + expand_vec_cmp (gt, GT, op0, op1); > + expand_vec_cmp (lt, LT, op0, op1); > + icode = code_for_pred (IOR, mask_mode); > + rtx ops[3] = {target, gt, lt}; > + emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); > + return; > +} Swap lt and gt here for consistency's sake. Regards Robin
RE: [PATCH] RISC-V: Fix warning of vxrm pattern
Committed, thanks Jeff. Pan -Original Message- From: Gcc-patches On Behalf Of Jeff Law via Gcc-patches Sent: Tuesday, May 23, 2023 9:43 PM To: juzhe.zh...@rivai.ai; gcc-patches@gcc.gnu.org Cc: kito.ch...@gmail.com; kito.ch...@sifive.com; pal...@dabbelt.com; pal...@rivosinc.com; rdapp@gmail.com Subject: Re: [PATCH] RISC-V: Fix warning of vxrm pattern On 5/23/23 04:09, juzhe.zh...@rivai.ai wrote: > From: Juzhe-Zhong > > I just notice the warning: > ../../../riscv-gcc/gcc/config/riscv/vector.md:618:1: warning: source missing > a mode? > > gcc/ChangeLog: > > * config/riscv/vector.md: Add mode. While I'm a big fan of the gen* warnings, I do wish they had a bit more smarts to avoid the missing mode warnings for arguments which are restricted to CONST_INTs which don't have modes. Oh well. OK for the trunk. jeff
Re: [PATCH] vect: Missed opportunity to use [SU]ABD
> > + if (reject_unsigned && TYPE_UNSIGNED (abs_type)) > > +return false; > > + if (!ANY_INTEGRAL_TYPE_P (abs_type) || TYPE_OVERFLOW_WRAPS (abs_type)) > > +return false; > > Could you explain the reject_unsigned behaviour? I'd have expected > TYPE_OVERFLOW_WRAPS (abs_type) to reject the unsigned case anyway. When REJECT_UNSIGNED is true, cases wherein the abs type is unsigned or when the unpromoted diff type is both not equal in precision to the abs type and unsigned the statement is rejected. vect_recog_absolute_difference replaces some of the logic in vect_recog_sad_pattern and is used by vect_recog_abd_pattern. vect_recog_sad_pattern aborts if the abs type is unsigned or when the unprom diff type isn't the same precision as abs type and unsigned. vect_recog_abd_pattern doesn't do the same, so REJECT_UNSIGNED is a flag for this. I found it to be unnecessary as you suggested, so it's been dropped. > > + if (half_type) > > +{ > > + if (!SAME_TYPE (unprom[0].type, unprom[1].type)) > > + return NULL; > > I wouldn't have expected this to be unecessary. half_type is supposed > to be a common type that can hold all values of unprom[0].type and > unprom[1].type. We should just be able to do: > > > + tree diff_type = TREE_TYPE (diff_oprnds[0]); > > + if (TYPE_PRECISION (out_type) != TYPE_PRECISION (diff_type)) > > + { > > + tree vectype = get_vectype_for_scalar_type (vinfo, half_type); > > + vect_convert_inputs (vinfo, stmt_vinfo, 2, abd_oprnds, > > + half_type, unprom, vectype); > > ...this vect_convert_inputs unconditionally. We need to check that > the get_vectype_for_scalar_type call succeeds though. > > So does it work as: > > if (half_type) > { > tree vectype = get_vectype_for_scalar_type (vinfo, half_type); > if (!vectype) > return false; > vect_convert_inputs (vinfo, stmt_vinfo, 2, abd_oprnds, >half_type, unprom, vectype); > } > > ? The proposed solution works. > > + } > > + else > > + { > > + abd_oprnds[0] = diff_oprnds[0]; > > + abd_oprnds[1] = diff_oprnds[1]; > > + } > > +} > > + else > > +{ > > + if (unprom[0].op && unprom[1].op > > + && (!SAME_TYPE (unprom[0].type, unprom[1].type) > > + || !SAME_TYPE (unprom[0].type, out_type))) > > + return NULL; > > AIUI, the !half_type case shouldn't look at unprom, since it's handling > simple MINUS_EXPRs. I think we can just delete this "if" statement. If statement removed. > > + unprom[0].op = diff_oprnds[0]; > > + unprom[1].op = diff_oprnds[1]; > > + tree signed_out = signed_type_for (out_type); > > + tree signed_out_vectype = get_vectype_for_scalar_type (vinfo, > > signed_out); > > We need to check for success here too. Add a check. > > + vect_convert_inputs (vinfo, stmt_vinfo, 2, abd_oprnds, > > + signed_out, unprom, signed_out_vectype); > > + > > + if (!SAME_TYPE (TREE_TYPE (diff_oprnds[0]), TREE_TYPE > > (abd_oprnds[0]))) > > + return NULL; > > I don't think this is needed. Statement removed. > > +} > > + > > + if (!SAME_TYPE (TREE_TYPE (abd_oprnds[0]), TREE_TYPE (abd_oprnds[1])) > > + || !SAME_TYPE (TREE_TYPE (abd_oprnds[0]), out_type)) > > +return NULL; > > I also don't think this is needed. AIUI, the previous code has done > all the necessary correctness checks. Statements removed. > > + vect_pattern_detected ("vect_recog_abd_pattern", last_stmt); > > + > > + tree vectype = get_vectype_for_scalar_type (vinfo, out_type); > > I think instead we want the vector types computed above. That is: > > - The ABD should be done on the vector version of half_type > if the subtraction was on promoted inputs. The result of > the ABD should then be zero-extended (using vect_convert_output) > to out_type. > > In particular, it's the sign of HALF_TYPE that decides whether > it's signed or unsigned ABD. > > - The ABD should be done on the vector version of signed_outtype > if the subtraction was on unpromoted inputs. We then might need > to sign-cast it to outtype, if outtype is unsigned. We can > use vect_convert_output for that too. > > In other words, this case must use signed ABD. In the half_type case out_type is set to be half_type. Patch is in the next response.
[PATCH] [arm] testsuite: make mve_intrinsic_type_overloads-int.c libc-agnostic
Glibc defines int32_t as 'int' while newlib defines it as 'long int'. Although these correspond to the same size, g++ complains when using the 'wrong' version: invalid conversion from 'long int*' to 'int32_t*' {aka 'int*'} [-fpermissive] or invalid conversion from 'int*' to 'int32_t*' {aka 'long int*'} [-fpermissive] when calling vst1q(int32*, int32x4_t) with a first parameter of type 'long int *' (resp. 'int *') To make this test pass with any type of toolchain, this patch defines 'word_type' according to which libc is in use. 2023-05-23 Christophe Lyon gcc/testsuite/ * gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c: Support both definitions of int32_t. --- .../mve_intrinsic_type_overloads-int.c| 28 ++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c index 7947dc024bc..ab51cc8b323 100644 --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c @@ -47,14 +47,22 @@ foo2 (short * addr, int16x8_t value) vst1q (addr, value); } -void -foo3 (int * addr, int32x4_t value) -{ - vst1q (addr, value); /* { dg-warning "invalid conversion" "" { target c++ } } */ -} +/* Glibc defines int32_t as 'int' while newlib defines it as 'long int'. + + Although these correspond to the same size, g++ complains when using the + 'wrong' version: + invalid conversion from 'long int*' to 'int32_t*' {aka 'int*'} [-fpermissive] + + The trick below is to make this test pass whether using glibc-based or + newlib-based toolchains. */ +#if defined(__GLIBC__) +#define word_type int +#else +#define word_type long int +#endif void -foo4 (long * addr, int32x4_t value) +foo3 (word_type * addr, int32x4_t value) { vst1q (addr, value); } @@ -78,13 +86,7 @@ foo7 (unsigned short * addr, uint16x8_t value) } void -foo8 (unsigned int * addr, uint32x4_t value) -{ - vst1q (addr, value); /* { dg-warning "invalid conversion" "" { target c++ } } */ -} - -void -foo9 (unsigned long * addr, uint32x4_t value) +foo8 (unsigned word_type * addr, uint32x4_t value) { vst1q (addr, value); } -- 2.34.1
Re: [PATCH] [arm] testsuite: make mve_intrinsic_type_overloads-int.c libc-agnostic
On 23/05/2023 15:41, Christophe Lyon wrote: Glibc defines int32_t as 'int' while newlib defines it as 'long int'. Although these correspond to the same size, g++ complains when using the 'wrong' version: invalid conversion from 'long int*' to 'int32_t*' {aka 'int*'} [-fpermissive] or invalid conversion from 'int*' to 'int32_t*' {aka 'long int*'} [-fpermissive] when calling vst1q(int32*, int32x4_t) with a first parameter of type 'long int *' (resp. 'int *') To make this test pass with any type of toolchain, this patch defines 'word_type' according to which libc is in use. Thank you for spotting this! I think this fix is needed on all of GCC12,13,trunk btw (it should apply cleanly) 2023-05-23 Christophe Lyon gcc/testsuite/ * gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c: Support both definitions of int32_t. --- .../mve_intrinsic_type_overloads-int.c| 28 ++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c index 7947dc024bc..ab51cc8b323 100644 --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c @@ -47,14 +47,22 @@ foo2 (short * addr, int16x8_t value) vst1q (addr, value); } -void -foo3 (int * addr, int32x4_t value) -{ - vst1q (addr, value); /* { dg-warning "invalid conversion" "" { target c++ } } */ -} +/* Glibc defines int32_t as 'int' while newlib defines it as 'long int'. + + Although these correspond to the same size, g++ complains when using the + 'wrong' version: + invalid conversion from 'long int*' to 'int32_t*' {aka 'int*'} [-fpermissive] + + The trick below is to make this test pass whether using glibc-based or + newlib-based toolchains. */ +#if defined(__GLIBC__) +#define word_type int +#else +#define word_type long int +#endif void -foo4 (long * addr, int32x4_t value) +foo3 (word_type * addr, int32x4_t value) { vst1q (addr, value); } @@ -78,13 +86,7 @@ foo7 (unsigned short * addr, uint16x8_t value) } void -foo8 (unsigned int * addr, uint32x4_t value) -{ - vst1q (addr, value); /* { dg-warning "invalid conversion" "" { target c++ } } */ -} - -void -foo9 (unsigned long * addr, uint32x4_t value) +foo8 (unsigned word_type * addr, uint32x4_t value) { vst1q (addr, value); }
[PATCH 1/2] Missed opportunity to use [SU]ABD
From: oluade01 This adds a recognition pattern for the non-widening absolute difference (ABD). gcc/ChangeLog: * doc/md.texi (sabd, uabd): Document them. * internal-fn.def (ABD): Use new optab. * optabs.def (sabd_optab, uabd_optab): New optabs, * tree-vect-patterns.cc (vect_recog_absolute_difference): Recognize the following idiom abs (a - b). (vect_recog_sad_pattern): Refactor to use vect_recog_absolute_difference. (vect_recog_abd_pattern): Use patterns found by vect_recog_absolute_difference to build a new ABD internal call. --- gcc/doc/md.texi | 10 ++ gcc/internal-fn.def | 3 + gcc/optabs.def| 2 + gcc/tree-vect-patterns.cc | 231 +- 4 files changed, 215 insertions(+), 31 deletions(-) diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi index 07bf8bdebffb2e523f25a41f2b57e43c0276b745..77c77c6ecb0c29ef764e914e88d1090c45fb9a9e 100644 --- a/gcc/doc/md.texi +++ b/gcc/doc/md.texi @@ -5778,6 +5778,16 @@ Other shift and rotate instructions, analogous to the Vector shift and rotate instructions that take vectors as operand 2 instead of a scalar type. +@cindex @code{uabd@var{m}} instruction pattern +@cindex @code{sabd@var{m}} instruction pattern +@item @samp{uabd@var{m}}, @samp{sabd@var{m}} +Signed and unsigned absolute difference instructions. These +instructions find the difference between operands 1 and 2 +then return the absolute value. A C code equivalent would be: +@smallexample +op0 = op1 > op2 ? op1 - op2 : op2 - op1; +@end smallexample + @cindex @code{avg@var{m}3_floor} instruction pattern @cindex @code{uavg@var{m}3_floor} instruction pattern @item @samp{avg@var{m}3_floor} diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def index 7fe742c2ae713e7152ab05cfdfba86e4e0aa3456..0f1724ecf37a31c231572edf90b5577e2d82f468 100644 --- a/gcc/internal-fn.def +++ b/gcc/internal-fn.def @@ -167,6 +167,9 @@ DEF_INTERNAL_OPTAB_FN (FMS, ECF_CONST, fms, ternary) DEF_INTERNAL_OPTAB_FN (FNMA, ECF_CONST, fnma, ternary) DEF_INTERNAL_OPTAB_FN (FNMS, ECF_CONST, fnms, ternary) +DEF_INTERNAL_SIGNED_OPTAB_FN (ABD, ECF_CONST | ECF_NOTHROW, first, + sabd, uabd, binary) + DEF_INTERNAL_SIGNED_OPTAB_FN (AVG_FLOOR, ECF_CONST | ECF_NOTHROW, first, savg_floor, uavg_floor, binary) DEF_INTERNAL_SIGNED_OPTAB_FN (AVG_CEIL, ECF_CONST | ECF_NOTHROW, first, diff --git a/gcc/optabs.def b/gcc/optabs.def index 695f5911b300c9ca5737de9be809fa01aabe5e01..29bc92281a2175f898634cbe6af63c18021e5268 100644 --- a/gcc/optabs.def +++ b/gcc/optabs.def @@ -359,6 +359,8 @@ OPTAB_D (mask_fold_left_plus_optab, "mask_fold_left_plus_$a") OPTAB_D (extract_last_optab, "extract_last_$a") OPTAB_D (fold_extract_last_optab, "fold_extract_last_$a") +OPTAB_D (uabd_optab, "uabd$a3") +OPTAB_D (sabd_optab, "sabd$a3") OPTAB_D (savg_floor_optab, "avg$a3_floor") OPTAB_D (uavg_floor_optab, "uavg$a3_floor") OPTAB_D (savg_ceil_optab, "avg$a3_ceil") diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc index a49b09539776c0056e77f99b10365d0a8747fbc5..3a2248263cf67834a1cb41167a1783a3b6400014 100644 --- a/gcc/tree-vect-patterns.cc +++ b/gcc/tree-vect-patterns.cc @@ -770,6 +770,93 @@ vect_split_statement (vec_info *vinfo, stmt_vec_info stmt2_info, tree new_rhs, } } +/* Look for the following pattern + X = x[i] + Y = y[i] + DIFF = X - Y + DAD = ABS_EXPR + + ABS_STMT should point to a statement of code ABS_EXPR or ABSU_EXPR. + If REJECT_UNSIGNED is true it aborts if the type of ABS_STMT is unsigned. + HALF_TYPE and UNPROM will be set should the statement be found to + be a widened operation. + DIFF_OPRNDS will be set to the two inputs of the MINUS_EXPR preceding + ABS_STMT, otherwise it will be set the operations found by + vect_widened_op_tree. + */ +static bool +vect_recog_absolute_difference (vec_info *vinfo, gassign *abs_stmt, + tree *half_type, + vect_unpromoted_value unprom[2], + tree diff_oprnds[2]) +{ + if (!abs_stmt) +return false; + + /* FORNOW. Can continue analyzing the def-use chain when this stmt in a phi + inside the loop (in case we are analyzing an outer-loop). */ + enum tree_code code = gimple_assign_rhs_code (abs_stmt); + if (code != ABS_EXPR && code != ABSU_EXPR) +return false; + + tree abs_oprnd = gimple_assign_rhs1 (abs_stmt); + tree abs_type = TREE_TYPE (abs_oprnd); + if (!abs_oprnd) +return false; + if (!ANY_INTEGRAL_TYPE_P (abs_type) + || TYPE_OVERFLOW_WRAPS (abs_type) + || TYPE_UNSIGNED (abs_type)) +return false; + + /* Peel off conversions from the ABS input. This can involve sign + changes (e.g. from an unsigned subtraction to a signed ABS input) + or signed promotion, but it can't include unsigned promotion. + (Note that ABS of
[PATCH V3] RISC-V: Add RVV comparison autovectorization
From: Juzhe-Zhong This patch enable RVV auto-vectorization including floating-point unorder and order comparison. The testcases are leveraged from Richard. So include Richard as co-author. Co-Authored-By: Richard Sandiford gcc/ChangeLog: * config/riscv/autovec.md (@vcond_mask_): New pattern. (vec_cmp): Ditto. (vec_cmpu): Ditto. (vcond): Ditto. (vcondu): Ditto. * config/riscv/riscv-protos.h (enum insn_type): Add new enum. (emit_vlmax_merge_insn): New function. (emit_vlmax_cmp_insn): Ditto. (expand_vec_cmp): Ditto. (expand_vec_cmp_float):Ditto. (expand_vcond):Ditto. * config/riscv/riscv-v.cc (emit_vlmax_merge_insn): Ditto. (emit_vlmax_cmp_insn): Ditto. (get_cmp_insn_code): Ditto. (expand_vec_cmp): Ditto. (expand_vec_cmp_float): Ditto. (expand_vcond): Ditto. gcc/testsuite/ChangeLog: * gcc.target/riscv/rvv/rvv.exp: Add RVV comparison testcases. * gcc.target/riscv/rvv/autovec/cmp/vcond-1.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond-2.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond-3.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond_run-1.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond_run-2.c: New test. * gcc.target/riscv/rvv/autovec/cmp/vcond_run-3.c: New test. --- gcc/config/riscv/autovec.md | 112 gcc/config/riscv/riscv-protos.h | 7 + gcc/config/riscv/riscv-v.cc | 266 +- .../riscv/rvv/autovec/cmp/vcond-1.c | 157 +++ .../riscv/rvv/autovec/cmp/vcond-2.c | 75 + .../riscv/rvv/autovec/cmp/vcond-3.c | 13 + .../riscv/rvv/autovec/cmp/vcond_run-1.c | 49 .../riscv/rvv/autovec/cmp/vcond_run-2.c | 76 + .../riscv/rvv/autovec/cmp/vcond_run-3.c | 6 + gcc/testsuite/gcc.target/riscv/rvv/rvv.exp| 2 + 10 files changed, 756 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond-3.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond_run-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond_run-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cmp/vcond_run-3.c diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md index 04b4459222a..e0258e8b798 100644 --- a/gcc/config/riscv/autovec.md +++ b/gcc/config/riscv/autovec.md @@ -162,3 +162,115 @@ riscv_vector::RVV_BINOP, operands); DONE; }) + +;; = +;; == Comparisons and selects +;; = + +;; - +;; [INT,FP] Select based on masks +;; - +;; Includes merging patterns for: +;; - vmerge.vv +;; - vmerge.vx +;; - vfmerge.vf +;; - + +(define_expand "@vcond_mask_" + [(match_operand:V 0 "register_operand") + (match_operand: 3 "register_operand") + (match_operand:V 1 "nonmemory_operand") + (match_operand:V 2 "register_operand")] + "TARGET_VECTOR" + { +/* The order of vcond_mask is opposite to pred_merge. */ +std::swap (operands[1], operands[2]); +riscv_vector::emit_vlmax_merge_insn (code_for_pred_merge (mode), + riscv_vector::RVV_MERGE_OP, operands); +DONE; + } +) + +;; - +;; [INT,FP] Comparisons +;; - +;; Includes: +;; - vms. +;; - + +(define_expand "vec_cmp" + [(set (match_operand: 0 "register_operand") + (match_operator: 1 "comparison_operator" + [(match_operand:VI 2 "register_operand") + (match_operand:VI 3 "register_operand")]))] + "TARGET_VECTOR" + { +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), + operands[2], operands[3]); +DONE; + } +) + +(define_expand "vec_cmpu" + [(set (match_operand: 0 "register_operand") + (match_operator: 1 "comparison_operator" + [(match_operand:VI 2 "register_operand") + (match_operand:VI 3 "register_operand")]))] + "TARGET_VECTOR" + { +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), + operands[2], operands[3]); +DONE; + } +) + +(define_expand "vec_cmp" + [(set (match_ope
Re: Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization
Hi, Robin. >> Don't you want to use your shiny new operand passing style here as >> with the other expanders? H, I do this just following ARM code style. You can see I do pass rtx[] for expand_vcond and pass rtx,rtx,rtx for expand_vec_cmp. Well, I just follow ARM SVE implementation (You can check aarch64-sve.md, we are the same) :) If don't like it, could give me more information then I change it for you. >> I don't think we need the same comment in each of these. Same for >> /*DEST_MODE*/ and /*MASK_MODE*/ which would be redundant if data_mode >> were called dest_mode. Ok >> Swap lt and gt here for consistency's sake. Ok. I have fixed as you suggested. Would you mind review V3 patch: https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619324.html Thanks. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-05-23 22:12 To: juzhe.zhong; gcc-patches CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; jeffreyalaw; Richard Sandiford Subject: Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization > +(define_expand "vec_cmp" > + [(set (match_operand: 0 "register_operand") > + (match_operator: 1 "comparison_operator" > + [(match_operand:VI 2 "register_operand") > +(match_operand:VI 3 "register_operand")]))] > + "TARGET_VECTOR" > + { > +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), > + operands[2], operands[3]); > +DONE; > + } > +) > + > +(define_expand "vec_cmpu" > + [(set (match_operand: 0 "register_operand") > + (match_operator: 1 "comparison_operator" > + [(match_operand:VI 2 "register_operand") > +(match_operand:VI 3 "register_operand")]))] > + "TARGET_VECTOR" > + { > +riscv_vector::expand_vec_cmp (operands[0], GET_CODE (operands[1]), > + operands[2], operands[3]); > +DONE; > + } > +) > + > +(define_expand "vec_cmp" > + [(set (match_operand: 0 "register_operand") > + (match_operator: 1 "comparison_operator" > + [(match_operand:VF 2 "register_operand") > +(match_operand:VF 3 "register_operand")]))] > + "TARGET_VECTOR" > + { > +riscv_vector::expand_vec_cmp_float (operands[0], GET_CODE (operands[1]), > + operands[2], operands[3], false); > +DONE; > + } > +) Don't you want to use your shiny new operand passing style here as with the other expanders? > + /* We have a maximum of 11 operands for RVV instruction patterns according > to > + * vector.md. */ > + insn_expander<11> e (/*OP_NUM*/ op_num, /*HAS_DEST_P*/ true, > +/*FULLY_UNMASKED_P*/ false, > +/*USE_REAL_MERGE_P*/ false, /*HAS_AVL_P*/ true, > +/*VLMAX_P*/ true, > +/*DEST_MODE*/ data_mode, /*MASK_MODE*/ mask_mode); > + e.set_policy (TAIL_ANY); > + e.emit_insn ((enum insn_code) icode, ops); > +} I don't think we need the same comment in each of these. Same for /*DEST_MODE*/ and /*MASK_MODE*/ which would be redundant if data_mode were called dest_mode. > +/* Expand an RVV comparison. */ > + > +void > +expand_vec_cmp (rtx target, rtx_code code, rtx op0, rtx op1) > +{ > + machine_mode mask_mode = GET_MODE (target); > + machine_mode data_mode = GET_MODE (op0); > + insn_code icode = get_cmp_insn_code (code, data_mode); > + > + if (code == LTGT) > +{ > + rtx gt = gen_reg_rtx (mask_mode); > + rtx lt = gen_reg_rtx (mask_mode); > + expand_vec_cmp (gt, GT, op0, op1); > + expand_vec_cmp (lt, LT, op0, op1); > + icode = code_for_pred (IOR, mask_mode); > + rtx ops[3] = {target, gt, lt}; > + emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); > + return; > +} Swap lt and gt here for consistency's sake. Regards Robin ail-LinkSize:2273655 QQMail-LineLen:76 QQMail-BreakType:1 QQMail-Key:cbdff912c7f03cb40444ad0dccf1f041 QQMail-MD5:6754fd07de754a129fff82b243962497 QQMail-LinkEnd --=_Part_2195_841924464.1657529212753--0eWxlPSJjb2xvcjojMDAwMDAwIj48Zm9udCB5YWhlaT0i Ij48c3Ryb25nPlRlbDo8L3N0cm9uZz4mbmJzcDs4Ni0yOC02ODM3MzE2NiA2ODM3MzE4OCZuYnNw OzxiciAvPgo8c3Ryb25nPkZheDo8L3N0cm9uZz4mbmJzcDs4Ni0yOC02ODM3MzE2Ni04MDQmbmJz cDs8YnIgLz4KPHN0cm9uZz5BZGQ6PC9zdHJvbmc+NzE4LE5vLjEwLDEgTm9ydGgsMiBSaW5nLENo ZW5nZHUsQ2hpbmEsPC9mb250PjwvZm9udD48L2ZvbnQ+PGJyIG1pY3Jvc29mdD0iIiBzdHlsZT0i Y29sb3I6IzAwMDAwMCIgeWFoZWk9IiIgLz4KPGZvbnQgbWljcm9zb2Z0PSIiPjxmb250IHN0eWxl PSJjb2xvcjojMDAwMDAwIj48Zm9udCB5YWhlaT0iIj48c3Ryb25nPlBvc3RhbCBjb2RlOjwvc3Ry b25nPjYxMDAzMTwvZm9udD48L2ZvbnQ+PC9mb250PjxiciBtaWNyb3NvZnQ9IiIgc3R5bGU9ImNv bG9yOiMwMDAwMDAiIHlhaGVpPSIiIC8+CiZuYnNwOzxpbWcgYWx0PSIiIHNyYz0iL2VudHNvZnQv RXRBY3Rpb24uZW50Y3JtP21ldGhvZD10ZSZtYWlsSUQ9ODgwNTgzJmFzcF9jb2Q9JmNfdGFza051 bT0iIGhlaWdodD0wIHdpZHRoPTA+PC9CT0RZPjwvSFRNTD4= --=_Part_8340_683676631.1684738404743-- --=_Part_8339_2046897854.1684738404722 Content-Type: image/jpeg;name="1669700265737.jpg.jpeg" Content-Transfer-Encoding: base64 Content-ID: <2023052214532474264814...@entsoft.net> /9j/4AAQSkZJRgABAQEBLAEsAAD/2wBDAAYEBQYFBAYGBQYHBwYIChAKCgkJChQODwwQFxQYGBcU FhYaHSUfGhsjHBYWICwgIyYnKSopGR8tMC0oMCUoKSj/2wBDAQcHBwoIChMKChMoGhYaKC
[PATCH] Generic vector op costing adjustment
This is a small adjustment to the work done for PR108752 and better reflects the cost of the generated sequence. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/108752 * tree-vect-stmts.cc (vectorizable_operation): For bit operations with generic word_mode vectors do not cost an extra stmt. For plus, minus and negate also cost the constant materialization. --- gcc/tree-vect-stmts.cc | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index 0022b878767..127b987cd62 100644 --- a/gcc/tree-vect-stmts.cc +++ b/gcc/tree-vect-stmts.cc @@ -6466,8 +6466,8 @@ vectorizable_operation (vec_info *vinfo, { /* The above vect_model_simple_cost call handles constants in the prologue and (mis-)costs one of the stmts as -vector stmt. See tree-vect-generic.cc:do_plus_minus/do_negate -for the actual lowering that will be applied. */ +vector stmt. See below for the actual lowering that will +be applied. */ unsigned n = slp_node ? SLP_TREE_NUMBER_OF_VEC_STMTS (slp_node) : ncopies; switch (code) @@ -6481,9 +6481,20 @@ vectorizable_operation (vec_info *vinfo, case NEGATE_EXPR: n *= 4; break; - default:; + default: + /* Bit operations do not have extra cost and are accounted +as vector stmt by vect_model_simple_cost. */ + n = 0; + break; + } + if (n != 0) + { + /* We also need to materialize two large constants. */ + record_stmt_cost (cost_vec, 2, scalar_stmt, stmt_info, + 0, vect_prologue); + record_stmt_cost (cost_vec, n, scalar_stmt, stmt_info, + 0, vect_body); } - record_stmt_cost (cost_vec, n, scalar_stmt, stmt_info, 0, vect_body); } return true; } -- 2.35.3
[PATCH] tree-optimization/109747 - SLP cost of CTORs
The x86 backend looks at the SLP node passed to the add_stmt_cost hook when costing vec_construct, looking for elements that require a move from a GPR to a vector register and cost that. But since vect_prologue_cost_for_slp decomposes the cost for an external SLP node into individual pieces this cost gets applied N times without a chance for the backend to know it's just dealing with a part of the SLP node. Just looking at a part is also not perfect since the GPR to XMM move cost applies only once per distinct element so handling the whole SLP node one more correctly reflects cost (albeit without considering other external SLP nodes). The following addresses the issue by passing down the SLP node only for one piece and nullptr for the rest. The x86 backend is currently the only one looking at it. In the future the cost of external elements is something to deal with globally but that would require the full SLP tree be available to costing. It's difficult to write a testcase, at the tipping point not vectorizing is better so I'll followup with x86 specific adjustments and will see to add a testcase later. Bootstrapped and tested on x86_64-unknown-linux-gnu. Richard, we talked about this issue two weeks ago and I was looking for a solution that would be OK for backporting if the need arises. The following is what I could come up with that retains the whole SLP-node wide "CSE" of the element move cost. Is that OK until we come up with a better plan for trunk at some point? Thanks, Richard. PR tree-optimization/109747 * tree-vect-slp.cc (vect_prologue_cost_for_slp): Pass down the SLP node only once to the cost hook. --- gcc/tree-vect-slp.cc | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc index e5c9d7e766e..a6f277c5e21 100644 --- a/gcc/tree-vect-slp.cc +++ b/gcc/tree-vect-slp.cc @@ -6069,6 +6069,7 @@ vect_prologue_cost_for_slp (slp_tree node, } /* ??? We're just tracking whether vectors in a single node are the same. Ideally we'd do something more global. */ + bool passed = false; for (unsigned int start : starts) { vect_cost_for_stmt kind; @@ -6078,7 +6079,15 @@ vect_prologue_cost_for_slp (slp_tree node, kind = scalar_to_vec; else kind = vec_construct; - record_stmt_cost (cost_vec, 1, kind, node, vectype, 0, vect_prologue); + /* The target cost hook has no idea which part of the SLP node +we are costing so avoid passing it down more than once. Pass +it to the first vec_construct or scalar_to_vec part since for those +the x86 backend tries to account for GPR to XMM register moves. */ + record_stmt_cost (cost_vec, 1, kind, + (kind != vector_load && !passed) ? node : nullptr, + vectype, 0, vect_prologue); + if (kind != vector_load) + passed = true; } } -- 2.35.3
[PATCH] Account for vector splat GPR->XMM move cost
The following also accounts for a GPR->XMM move cost for splat operations and properly guards eliding the cost when moving from memory only for SSE4.1 or HImode or larger operands. This doesn't fix the PR fully yet. Bootstrapped and tested on x86_64-unknown-linux-gnu, OK? Thanks, Richard. PR target/109944 * config/i386/i386.cc (ix86_vector_costs::add_stmt_cost): For vector construction or splats apply GPR->XMM move costing. QImode memory can be handled directly only with SSE4.1 pinsrb. --- gcc/config/i386/i386.cc | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 38125ce284a..011a1fb0d6d 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -23654,7 +23654,7 @@ ix86_vector_costs::add_stmt_cost (int count, vect_cost_for_stmt kind, stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign); stmt_cost *= (TYPE_VECTOR_SUBPARTS (vectype) + 1); } - else if (kind == vec_construct + else if ((kind == vec_construct || kind == scalar_to_vec) && node && SLP_TREE_DEF_TYPE (node) == vect_external_def && INTEGRAL_TYPE_P (TREE_TYPE (vectype))) @@ -23687,7 +23687,9 @@ ix86_vector_costs::add_stmt_cost (int count, vect_cost_for_stmt kind, Likewise with a BIT_FIELD_REF extracting from a vector register we can hope to avoid using a GPR. */ if (!is_gimple_assign (def) - || (!gimple_assign_load_p (def) + || ((!gimple_assign_load_p (def) + || (!TARGET_SSE4_1 + && GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op))) == 1)) && (gimple_assign_rhs_code (def) != BIT_FIELD_REF || !VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (gimple_assign_rhs1 (def), 0)) -- 2.35.3
Re: [PATCH] Account for vector splat GPR->XMM move cost
On Tue, May 23, 2023 at 5:18 PM Richard Biener wrote: > > The following also accounts for a GPR->XMM move cost for splat > operations and properly guards eliding the cost when moving from > memory only for SSE4.1 or HImode or larger operands. This > doesn't fix the PR fully yet. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK? > > Thanks, > Richard. > > PR target/109944 > * config/i386/i386.cc (ix86_vector_costs::add_stmt_cost): > For vector construction or splats apply GPR->XMM move > costing. QImode memory can be handled directly only > with SSE4.1 pinsrb. OK. Thanks, Uros. > --- > gcc/config/i386/i386.cc | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > index 38125ce284a..011a1fb0d6d 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -23654,7 +23654,7 @@ ix86_vector_costs::add_stmt_cost (int count, > vect_cost_for_stmt kind, >stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign); >stmt_cost *= (TYPE_VECTOR_SUBPARTS (vectype) + 1); > } > - else if (kind == vec_construct > + else if ((kind == vec_construct || kind == scalar_to_vec) >&& node >&& SLP_TREE_DEF_TYPE (node) == vect_external_def >&& INTEGRAL_TYPE_P (TREE_TYPE (vectype))) > @@ -23687,7 +23687,9 @@ ix86_vector_costs::add_stmt_cost (int count, > vect_cost_for_stmt kind, > Likewise with a BIT_FIELD_REF extracting from a vector > register we can hope to avoid using a GPR. */ > if (!is_gimple_assign (def) > - || (!gimple_assign_load_p (def) > + || ((!gimple_assign_load_p (def) > + || (!TARGET_SSE4_1 > + && GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op))) == 1)) > && (gimple_assign_rhs_code (def) != BIT_FIELD_REF > || !VECTOR_TYPE_P (TREE_TYPE > (TREE_OPERAND (gimple_assign_rhs1 (def), > 0)) > -- > 2.35.3
[COMMITTED] i386: Add V8QI and V4QImode partial vector shift operations
Add V8QImode and V4QImode vector shift patterns that call into ix86_expand_vecop_qihi_partial. Generate special sequences for constant count operands. The patch regresses g++.dg/pr91838.C - as explained in PR91838, the test returns different results, depending on whether V8QImode shift pattern is present in target *.md files. The tree optimizers produce: V f (V x) { V _2; [local count: 1073741824]: _2 = x_1(D) >> 8; return _2; } and without the named expander: V f (V x) { [local count: 1073741824]: return { 0, 0, 0, 0, 0, 0, 0, 0 }; } RTL part just expands from there. gcc/ChangeLog: * config/i386/i386-expand.cc (ix86_expand_vecop_qihi_partial): Call ix86_expand_vec_shift_qihi_constant for shifts with constant count operand. * config/i386/i386.cc (ix86_shift_rotate_cost): Handle V4QImode and V8QImode. * config/i386/mmx.md (v8qi3): New insn pattern. (v4qi3): Ditto. gcc/testsuite/ChangeLog: * gcc.target/i386/vect-shiftv4qi.c: New test. * gcc.target/i386/vect-shiftv8qi.c: New test. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Uros. diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc index 50d9d34ebcb..ff3d382f1b4 100644 --- a/gcc/config/i386/i386-expand.cc +++ b/gcc/config/i386/i386-expand.cc @@ -23294,6 +23294,16 @@ ix86_expand_vecop_qihi_partial (enum rtx_code code, rtx dest, rtx op1, rtx op2) else qop2 = op2; + qdest = gen_reg_rtx (V16QImode); + + if (CONST_INT_P (op2) + && (code == ASHIFT || code == LSHIFTRT || code == ASHIFTRT) + && ix86_expand_vec_shift_qihi_constant (code, qdest, qop1, qop2)) +{ + emit_move_insn (dest, gen_lowpart (qimode, qdest)); + return; +} + switch (code) { case MULT: @@ -23358,8 +23368,6 @@ ix86_expand_vecop_qihi_partial (enum rtx_code code, rtx dest, rtx op1, rtx op2) bool ok; int i; - qdest = gen_reg_rtx (V16QImode); - /* Merge the data back into the right place. */ d.target = qdest; d.op0 = qres; diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 38125ce284a..2710c6dfc56 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -20580,6 +20580,37 @@ ix86_shift_rotate_cost (const struct processor_costs *cost, switch (mode) { + case V4QImode: + case V8QImode: + if (TARGET_AVX2) + /* Use vpbroadcast. */ + extra = cost->sse_op; + else + extra = cost->sse_load[2]; + + if (constant_op1) + { + if (code == ASHIFTRT) + { + count = 4; + extra *= 2; + } + else + count = 2; + } + else if (TARGET_AVX512BW && TARGET_AVX512VL) + { + count = 3; + return ix86_vec_cost (mode, cost->sse_op * count); + } + else if (TARGET_SSE4_1) + count = 4; + else if (code == ASHIFTRT) + count = 5; + else + count = 4; + return ix86_vec_cost (mode, cost->sse_op * count) + extra; + case V16QImode: if (TARGET_XOP) { @@ -20600,7 +20631,12 @@ ix86_shift_rotate_cost (const struct processor_costs *cost, } /* FALLTHRU */ case V32QImode: - extra = (mode == V16QImode) ? cost->sse_load[2] : cost->sse_load[3]; + if (TARGET_AVX2) + /* Use vpbroadcast. */ + extra = cost->sse_op; + else + extra = (mode == V16QImode) ? cost->sse_load[2] : cost->sse_load[3]; + if (constant_op1) { if (code == ASHIFTRT) diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md index 45773673049..a37811f 100644 --- a/gcc/config/i386/mmx.md +++ b/gcc/config/i386/mmx.md @@ -2680,6 +2680,28 @@ (const_string "0"))) (set_attr "mode" "TI")]) +(define_expand "v8qi3" + [(set (match_operand:V8QI 0 "register_operand") + (any_shift:V8QI (match_operand:V8QI 1 "register_operand") + (match_operand:DI 2 "nonmemory_operand")))] + "TARGET_MMX_WITH_SSE" +{ + ix86_expand_vecop_qihi_partial (, operands[0], + operands[1], operands[2]); + DONE; +}) + +(define_expand "v4qi3" + [(set (match_operand:V4QI 0 "register_operand") + (any_shift:V4QI (match_operand:V4QI 1 "register_operand") + (match_operand:DI 2 "nonmemory_operand")))] + "TARGET_SSE2" +{ + ix86_expand_vecop_qihi_partial (, operands[0], + operands[1], operands[2]); + DONE; +}) + (define_insn_and_split "v2qi3" [(set (match_operand:V2QI 0 "register_operand" "=Q") (any_shift:V2QI diff --git a/gcc/testsuite/gcc.target/i386/vect-shiftv4qi.c b/gcc/testsuite/gcc.target/i386/vect-shiftv4qi.c new file mode 100644 index 000..c06dfb87bd1 --- /dev/null +++ b/gcc/te
Re: [PATCH] tree-optimization/109747 - SLP cost of CTORs
Richard Biener writes: > The x86 backend looks at the SLP node passed to the add_stmt_cost > hook when costing vec_construct, looking for elements that require > a move from a GPR to a vector register and cost that. But since > vect_prologue_cost_for_slp decomposes the cost for an external > SLP node into individual pieces this cost gets applied N times > without a chance for the backend to know it's just dealing with > a part of the SLP node. Just looking at a part is also not perfect > since the GPR to XMM move cost applies only once per distinct > element so handling the whole SLP node one more correctly reflects > cost (albeit without considering other external SLP nodes). > > The following addresses the issue by passing down the SLP node > only for one piece and nullptr for the rest. The x86 backend > is currently the only one looking at it. > > In the future the cost of external elements is something to deal > with globally but that would require the full SLP tree be available > to costing. > > It's difficult to write a testcase, at the tipping point not > vectorizing is better so I'll followup with x86 specific adjustments > and will see to add a testcase later. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Richard, we talked about this issue two weeks ago and I was looking > for a solution that would be OK for backporting if the need arises. > The following is what I could come up with that retains the whole > SLP-node wide "CSE" of the element move cost. Is that OK until > we come up with a better plan for trunk at some point? Yeah, seems like a neat workaround to me FWIW. Thanks, Richard > > Thanks, > Richard. > > PR tree-optimization/109747 > * tree-vect-slp.cc (vect_prologue_cost_for_slp): Pass down > the SLP node only once to the cost hook. > --- > gcc/tree-vect-slp.cc | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc > index e5c9d7e766e..a6f277c5e21 100644 > --- a/gcc/tree-vect-slp.cc > +++ b/gcc/tree-vect-slp.cc > @@ -6069,6 +6069,7 @@ vect_prologue_cost_for_slp (slp_tree node, > } >/* ??? We're just tracking whether vectors in a single node are the same. > Ideally we'd do something more global. */ > + bool passed = false; >for (unsigned int start : starts) > { >vect_cost_for_stmt kind; > @@ -6078,7 +6079,15 @@ vect_prologue_cost_for_slp (slp_tree node, > kind = scalar_to_vec; >else > kind = vec_construct; > - record_stmt_cost (cost_vec, 1, kind, node, vectype, 0, vect_prologue); > + /* The target cost hook has no idea which part of the SLP node > + we are costing so avoid passing it down more than once. Pass > + it to the first vec_construct or scalar_to_vec part since for those > + the x86 backend tries to account for GPR to XMM register moves. */ > + record_stmt_cost (cost_vec, 1, kind, > + (kind != vector_load && !passed) ? node : nullptr, > + vectype, 0, vect_prologue); > + if (kind != vector_load) > + passed = true; > } > }
[avr,committed] Fix cost computation for bit insertions.
Applied this patchlet that implements proper cost computation of (set (zero_extract (...) ...)) kind patterns that do single-bit (inverted) bit insertions. Johann -- Improve cost computation for single-bit bit insertions. Some miscomputation of rtx_costs lead to sub-optimal code for single-bit bit insertions. This patch implements TARGET_INSN_COST, which has a chance to see the whole insn during insn combination; in particular the SET_DEST of (set (zero_extract (...) ...)). gcc/ * config/avr/avr.cc (avr_insn_cost): New static function. (TARGET_INSN_COST): Define to that function. diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc index 9fa50ca230d..4fa6f5309b2 100644 --- a/gcc/config/avr/avr.cc +++ b/gcc/config/avr/avr.cc @@ -11514,6 +11514,52 @@ avr_rtx_costs (rtx x, machine_mode mode, int outer_code, } +/* Implement `TARGET_INSN_COST'. */ +/* For some insns, it is not enough to look at the cost of the SET_SRC. + In that case, have a look at the entire insn, e.g. during insn combine. */ + +static int +avr_insn_cost (rtx_insn *insn, bool speed) +{ + const int unknown_cost = -1; + int cost = unknown_cost; + + rtx set = single_set (insn); + + if (set + && ZERO_EXTRACT == GET_CODE (SET_DEST (set))) +{ + // Try find anything that would flip the extracted bit. + bool not_bit_p = false; + + subrtx_iterator::array_type array; + FOR_EACH_SUBRTX (iter, array, SET_SRC (set), NONCONST) + { + enum rtx_code code = GET_CODE (*iter); + not_bit_p |= code == NOT || code == XOR || code == GE; + } + + // Don't go too deep into the analysis. In almost all cases, + // using BLD/BST is the best we can do for single-bit moves, + // even considering CSE. + cost = COSTS_N_INSNS (2 + not_bit_p); +} + + if (cost != unknown_cost) +{ + if (avr_log.rtx_costs) + avr_edump ("\n%? (%s) insn_cost=%d\n%r\n", + speed ? "speed" : "size", cost, insn); + return cost; +} + + // Resort to what rtlanal.cc::insn_cost() implements as a default + // when targetm.insn_cost() is not implemented. + + return pattern_cost (PATTERN (insn), speed); +} + + /* Implement `TARGET_ADDRESS_COST'. */ static int @@ -14574,6 +14620,8 @@ avr_float_lib_compare_returns_bool (machine_mode mode, enum rtx_code) #undef TARGET_ASM_FINAL_POSTSCAN_INSN #define TARGET_ASM_FINAL_POSTSCAN_INSN avr_asm_final_postscan_insn +#undef TARGET_INSN_COST +#define TARGET_INSN_COST avr_insn_cost #undef TARGET_REGISTER_MOVE_COST #define TARGET_REGISTER_MOVE_COST avr_register_move_cost #undef TARGET_MEMORY_MOVE_COST
Re: [patch] mcore: Fix sprintf length warning
On 5/22/23 17:58, Jan-Benedict Glaw wrote: Hi! One of the supplied argument strings is unneccesarily long (c-sky, using basically the same code, fixed it to a shorter length) and this fixes overflow warnings, as GCC fails to deduce that the full 256 bytes for load_op[] are not used at all. make[1]: Entering directory '/var/lib/laminar/run/gcc-mcore-elf/38/toolchain-build/gcc' [...] /usr/lib/gcc-snapshot/bin/g++ -fno-PIE -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace -o mcore.o -MT mcore.o -MMD -MP -MF ./.deps/mcore.TPo ../../gcc/gcc/config/mcore/mcore.cc ../../gcc/gcc/config/mcore/mcore.cc: In function 'const char* output_inline_const(machine_mode, rtx_def**)': ../../gcc/gcc/config/mcore/mcore.cc:1264:24: error: ' ixw ' directive writing 6 bytes into a region of size between 1 and 256 [-Werror=format-overflow=] 1264 | sprintf (buf, "%s\n\tixw\t%s,%s\t// %ld 0x%lx", load_op, dst_fmt, dst_fmt, value, value); |^ ../../gcc/gcc/config/mcore/mcore.cc:1264:21: note: using the range [0, 18446744073709551615] for directive argument 1264 | sprintf (buf, "%s\n\tixw\t%s,%s\t// %ld 0x%lx", load_op, dst_fmt, dst_fmt, value, value); | ^~~~ ../../gcc/gcc/config/mcore/mcore.cc:1264:15: note: 'sprintf' output between 21 and 310 bytes into a destination of size 256 1264 | sprintf (buf, "%s\n\tixw\t%s,%s\t// %ld 0x%lx", load_op, dst_fmt, dst_fmt, value, value); | ^~~~ ../../gcc/gcc/config/mcore/mcore.cc:1261:24: error: ' ixh ' directive writing 6 bytes into a region of size between 1 and 256 [-Werror=format-overflow=] 1261 | sprintf (buf, "%s\n\tixh\t%s,%s\t// %ld 0x%lx", load_op, dst_fmt, dst_fmt, value, value); |^ ../../gcc/gcc/config/mcore/mcore.cc:1261:21: note: using the range [0, 18446744073709551615] for directive argument 1261 | sprintf (buf, "%s\n\tixh\t%s,%s\t// %ld 0x%lx", load_op, dst_fmt, dst_fmt, value, value); | ^~~~ ../../gcc/gcc/config/mcore/mcore.cc:1261:15: note: 'sprintf' output between 21 and 310 bytes into a destination of size 256 1261 | sprintf (buf, "%s\n\tixh\t%s,%s\t// %ld 0x%lx", load_op, dst_fmt, dst_fmt, value, value); | ^~~~ ../../gcc/gcc/config/mcore/mcore.cc:1258:24: error: ' lsli' directive writing 7 bytes into a region of size between 1 and 256 [-Werror=format-overflow=] 1258 | sprintf (buf, "%s\n\tlsli\t%s,%%2\t// %ld 0x%lx", load_op, dst_fmt, value, value); |^~ ../../gcc/gcc/config/mcore/mcore.cc:1258:21: note: using the range [0, 18446744073709551615] for directive argument 1258 | sprintf (buf, "%s\n\tlsli\t%s,%%2\t// %ld 0x%lx", load_op, dst_fmt, value, value); | ^~ ../../gcc/gcc/config/mcore/mcore.cc:1258:15: note: 'sprintf' output between 22 and 311 bytes into a destination of size 256 1258 | sprintf (buf, "%s\n\tlsli\t%s,%%2\t// %ld 0x%lx", load_op, dst_fmt, value, value); | ^ ../../gcc/gcc/config/mcore/mcore.cc:1255:24: error: ' rotli ' directive writing 8 bytes into a region of size between 1 and 256 [-Werror=format-overflow=] 1255 | sprintf (buf, "%s\n\trotli\t%s,%%2\t// %ld 0x%lx", load_op, dst_fmt, value, value); |^~~ ../../gcc/gcc/config/mcore/mcore.cc:1255:21: note: using the range [0, 18446744073709551615] for directive argument 1255 | sprintf (buf, "%s\n\trotli\t%s,%%2\t// %ld 0x%lx", load_op, dst_fmt, value, value); | ^~~ ../../gcc/gcc/config/mcore/mcore.cc:1255:15: note: 'sprintf' output between 23 and 312 bytes into a destination of size 256 1255 | sprintf (buf, "%s\n\trotli\t%s,%%2\t// %ld 0x%lx", load_op, dst_fmt, value, value); | ^~ ../../gcc/gcc/config/mcore/mcore.cc:1252:24: error: ' bcl
[PATCH] PR middle-end/109840: Preserve popcount/parity type in match.pd.
PR middle-end/109840 is a regression introduced by my recent patch to fold popcount(bswap(x)) as popcount(x). When the bswap and the popcount have the same precision, everything works fine, but this optimization also allowed a zero-extension between the two. The oversight is that we need to be strict with type conversions, both to avoid accidentally changing the argument type to popcount, and also to reflect the effects of argument/return-value promotion in the call to bswap, so this zero extension needs to be preserved/explicit in the optimized form. Interestingly, match.pd should (in theory) be able to narrow calls to popcount and parity, removing a zero-extension from its argument, but that is an independent optimization, that needs to check IFN_ support. Many thanks to Andrew Pinski for his help/fixes with these transformations. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=unix{-m32} with no new failures. Ok for mainline? 2023-05-23 Roger Sayle gcc/ChangeLog PR middle-end/109840 * match.pd : Preserve zero-extension when optimizing popcount((T)bswap(x)) and popcount((T)rotate(x,y)) as popcount((T)x), so the popcount's argument keeps the same type. : Likewise preserve extensions when simplifying parity((T)bswap(x)) and parity((T)rotate(x,y)) as parity((T)x), so that the parity's argument type is the same. gcc/testsuite/ChangeLog PR middle-end/109840 * gcc.dg/fold-parity-8.c: New test. * gcc.dg/fold-popcount-11.c: Likewise. Thanks in advance, and apologies for any inconvenience. Roger -- diff --git a/gcc/match.pd b/gcc/match.pd index 1fe0559..6e32f47 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7865,10 +7865,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (popcount (convert?@0 (bswap:s@1 @2))) (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P (TREE_TYPE (@1))) - (with { unsigned int prec0 = TYPE_PRECISION (TREE_TYPE (@0)); - unsigned int prec1 = TYPE_PRECISION (TREE_TYPE (@1)); } - (if (prec0 == prec1 || (prec0 > prec1 && TYPE_UNSIGNED (TREE_TYPE (@1 - (popcount @2))) + (with { tree type0 = TREE_TYPE (@0); + tree type1 = TREE_TYPE (@1); + unsigned int prec0 = TYPE_PRECISION (type0); + unsigned int prec1 = TYPE_PRECISION (type1); } + (if (prec0 == prec1 || (prec0 > prec1 && TYPE_UNSIGNED (type1))) + (popcount (convert:type0 (convert:type1 @2) /* popcount(rotate(X Y)) is popcount(X). */ (for popcount (POPCOUNT) @@ -7878,10 +7880,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P (TREE_TYPE (@1)) && (GIMPLE || !TREE_SIDE_EFFECTS (@3))) - (with { unsigned int prec0 = TYPE_PRECISION (TREE_TYPE (@0)); - unsigned int prec1 = TYPE_PRECISION (TREE_TYPE (@1)); } - (if (prec0 == prec1 || (prec0 > prec1 && TYPE_UNSIGNED (TREE_TYPE (@1 - (popcount @2))) + (with { tree type0 = TREE_TYPE (@0); + tree type1 = TREE_TYPE (@1); + unsigned int prec0 = TYPE_PRECISION (type0); + unsigned int prec1 = TYPE_PRECISION (type1); } + (if (prec0 == prec1 || (prec0 > prec1 && TYPE_UNSIGNED (type1))) + (popcount (convert:type0 @2 /* Canonicalize POPCOUNT(x)&1 as PARITY(X). */ (simplify @@ -7923,7 +7927,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && INTEGRAL_TYPE_P (TREE_TYPE (@1)) && TYPE_PRECISION (TREE_TYPE (@0)) >= TYPE_PRECISION (TREE_TYPE (@1))) - (parity @2) + (with { tree type0 = TREE_TYPE (@0); + tree type1 = TREE_TYPE (@1); } + (parity (convert:type0 (convert:type1 @2 /* parity(rotate(X Y)) is parity(X). */ (for parity (PARITY) @@ -7935,7 +7941,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && (GIMPLE || !TREE_SIDE_EFFECTS (@3)) && TYPE_PRECISION (TREE_TYPE (@0)) >= TYPE_PRECISION (TREE_TYPE (@1))) - (parity @2) + (with { tree type0 = TREE_TYPE (@0); } + (parity (convert:type0 @2))) /* parity(X)^parity(Y) is parity(X^Y). */ (simplify diff --git a/gcc/testsuite/gcc.dg/fold-parity-8.c b/gcc/testsuite/gcc.dg/fold-parity-8.c new file mode 100644 index 000..48e1f7f --- /dev/null +++ b/gcc/testsuite/gcc.dg/fold-parity-8.c @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +int foo(unsigned short x) +{ + unsigned short t1 = __builtin_bswap16(x); + unsigned int t2 = t1; + return __builtin_parity (t2); +} + +int fool(unsigned short x) +{ + unsigned short t1 = __builtin_bswap16(x); + unsigned long t2 = t1; + return __builtin_parityl (t2); +} + +int fooll(unsigned short x) +{ + unsigned short t1 = __builtin
[PATCH] libstdc++: use using instead of typedef for type_traits
Since the type_traits header is a C++11 header file, using can be used instead of typedef. This patch provides more readability, especially for long type names. libstdc++-v3/ChangeLog: * include/std/type_traits: Use using instead of typedef --- libstdc++-v3/include/std/type_traits | 158 +-- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits index bc6982f9e64..0e7a9c9c7f3 100644 --- a/libstdc++-v3/include/std/type_traits +++ b/libstdc++-v3/include/std/type_traits @@ -61,9 +61,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template struct integral_constant { - static constexpr _Tp value = __v; - typedef _Tp value_type; - typedef integral_constant<_Tp, __v> type; + static constexpr _Tp value = __v; + using value_type = _Tp; + using type = integral_constant<_Tp, __v>; constexpr operator value_type() const noexcept { return value; } #if __cplusplus > 201103L @@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Partial specialization for true. template struct enable_if -{ typedef _Tp type; }; +{ using type = _Tp; }; // __enable_if_t (std::enable_if_t for C++11) template @@ -946,7 +946,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct __is_destructible_impl : public __do_is_destructible_impl { - typedef decltype(__test<_Tp>(0)) type; + using type = decltype(__test<_Tp>(0)); }; template(0)) type; + using type = decltype(__test<_Tp>(0)); }; template())) type; + using type = decltype(__test(declval<_Tp>())); }; template @@ -1422,7 +1422,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION is_array<_To>>::value> struct __is_convertible_helper { - typedef typename is_void<_To>::type type; + using type = typename is_void<_To>::type; }; #pragma GCC diagnostic push @@ -1443,7 +1443,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __test(...); public: - typedef decltype(__test<_From, _To>(0)) type; + using type = decltype(__test<_From, _To>(0)); }; #pragma GCC diagnostic pop @@ -1521,20 +1521,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// remove_const template struct remove_const -{ typedef _Tp type; }; +{ using type = _Tp; }; template struct remove_const<_Tp const> -{ typedef _Tp type; }; +{ using type = _Tp; }; /// remove_volatile template struct remove_volatile -{ typedef _Tp type; }; +{ using type = _Tp; }; template struct remove_volatile<_Tp volatile> -{ typedef _Tp type; }; +{ using type = _Tp; }; /// remove_cv #if __has_builtin(__remove_cv) @@ -1658,83 +1658,83 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template struct __cv_selector<_Unqualified, false, false> -{ typedef _Unqualified __type; }; +{ using __type = _Unqualified; }; template struct __cv_selector<_Unqualified, false, true> -{ typedef volatile _Unqualified __type; }; +{ using __type = volatile _Unqualified; }; template struct __cv_selector<_Unqualified, true, false> -{ typedef const _Unqualified __type; }; +{ using __type = const _Unqualified; }; template struct __cv_selector<_Unqualified, true, true> -{ typedef const volatile _Unqualified __type; }; +{ using __type = const volatile _Unqualified; }; template::value, bool _IsVol = is_volatile<_Qualified>::value> class __match_cv_qualifiers { - typedef __cv_selector<_Unqualified, _IsConst, _IsVol> __match; + using __match = __cv_selector<_Unqualified, _IsConst, _IsVol>; public: - typedef typename __match::__type __type; + using __type = typename __match::__type; }; // Utility for finding the unsigned versions of signed integral types. template struct __make_unsigned -{ typedef _Tp __type; }; +{ using __type = _Tp; }; template<> struct __make_unsigned -{ typedef unsigned char __type; }; +{ using __type = unsigned char; }; template<> struct __make_unsigned -{ typedef unsigned char __type; }; +{ using __type = unsigned char; }; template<> struct __make_unsigned -{ typedef unsigned short __type; }; +{ using __type = unsigned short; }; template<> struct __make_unsigned -{ typedef unsigned int __type; }; +{ using __type = unsigned int; }; template<> struct __make_unsigned -{ typedef unsigned long __type; }; +{ using __type = unsigned long; }; template<> struct __make_unsigned -{ typedef unsigned long long __type; }; +{ using __type = unsigned long long; }; #if defined(__GLIBCXX_TYPE_INT_N_0) __extension__ template<> struct __make_unsigned<__GLIBCXX_TYPE_INT_N_0> -{ typedef unsigned __GLIBCXX_TYPE_
RISC-V: Use extension instructions instead of bitwise "and"
In the case where the target supports extension instructions, it is preferable to use that instead of doing the same in other ways. For the following case void foo (unsigned long a, unsigned long* ptr) { ptr[0] = a & 0xUL; ptr[1] &= 0xUL; } GCC generates foo: li a5,-1 srlia5,a5,32 and a0,a0,a5 sd a0,0(a1) ld a4,8(a1) and a5,a4,a5 sd a5,8(a1) ret but it will be profitable to generate this one foo: zext.w a0,a0 sd a0,0(a1) lwu a5,8(a1) sd a5,8(a1) ret This patch fixes mentioned issue. It supports HI -> DI, HI->SI and SI -> DI extensions. gcc/ChangeLog: * config/riscv/riscv.md (and3): New expander. (*and3) New pattern. * config/riscv/predicates.md (arith_operand_or_mode_mask): New predicate. gcc/testsuite/ChangeLog: * gcc.target/riscv/and-extend-1.c: New test * gcc.target/riscv/and-extend-2.c: New test -- With the best regards Jivan Hakobyan diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md index ffcbb9a7589..70f570153ae 100644 --- a/gcc/config/riscv/predicates.md +++ b/gcc/config/riscv/predicates.md @@ -27,6 +27,12 @@ (ior (match_operand 0 "const_arith_operand") (match_operand 0 "register_operand"))) +(define_predicate "arith_operand_or_mode_mask" + (ior (match_operand 0 "arith_operand") + (and (match_code "const_int") +(match_test "INTVAL (op) == GET_MODE_MASK (HImode) + || INTVAL (op) == GET_MODE_MASK (SImode)" + (define_predicate "lui_operand" (and (match_code "const_int") (match_test "LUI_OPERAND (INTVAL (op))"))) diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index 124d8c95804..6492812 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -1342,9 +1342,46 @@ ;; For RV64, we don't expose the SImode operations to the rtl expanders, ;; but SImode versions exist for combine. +(define_expand "and3" + [(set (match_operand:X0 "register_operand") +(and:X (match_operand:X 1 "register_operand") + (match_operand:X 2 "arith_operand_or_mode_mask")))] + "" +{ + if (CONST_INT_P (operands[2])) + { +enum machine_mode tmode = VOIDmode; +if (INTVAL (operands[2]) == GET_MODE_MASK (HImode)) + tmode = HImode; +else if (INTVAL (operands[2]) == GET_MODE_MASK (SImode)) + tmode = SImode; + +if (tmode != VOIDmode) +{ + rtx tmp = gen_lowpart (tmode, operands[1]); + emit_insn (gen_extend_insn (operands[0], tmp, mode, tmode, 1)); + DONE; +} + } + else + { +emit_move_insn (operands[0], gen_rtx_AND (mode, operands[1], operands[2])); +DONE; + } +}) + +(define_insn "*and3" + [(set (match_operand:X0 "register_operand" "=r,r") + (and:X (match_operand:X 1 "register_operand" "%r,r") + (match_operand:X 2 "arith_operand"" r,I")))] + "" + "and%i2\t%0,%1,%2" + [(set_attr "type" "logical") + (set_attr "mode" "")]) + (define_insn "3" [(set (match_operand:X0 "register_operand" "=r,r") - (any_bitwise:X (match_operand:X 1 "register_operand" "%r,r") + (any_or:X (match_operand:X 1 "register_operand" "%r,r") (match_operand:X 2 "arith_operand"" r,I")))] "" "%i2\t%0,%1,%2" diff --git a/gcc/testsuite/gcc.target/riscv/and-extend-1.c b/gcc/testsuite/gcc.target/riscv/and-extend-1.c new file mode 100644 index 000..a270d287374 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/and-extend-1.c @@ -0,0 +1,30 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zba_zbb -mabi=lp64" } */ +/* { dg-skip-if "" { *-*-* } { "-O0" } } */ + +void +foo(unsigned long a, unsigned long* ptr) +{ +ptr[0] = a & 0xUL; +ptr[1] &= 0xUL; +} + +void +foo2(unsigned long a, unsigned long* ptr) +{ +ptr[0] = a & 0x; +ptr[1] &= 0x; +} + +void +foo3(unsigned int a, unsigned int* ptr) +{ +ptr[0] = a & 0x; +ptr[1] &= 0x; +} + +/* { dg-final { scan-assembler-times "zext.w" 1 } } */ +/* { dg-final { scan-assembler-times "zext.h" 2 } } */ +/* { dg-final { scan-assembler-times "lwu" 1 } } */ +/* { dg-final { scan-assembler-times "lhu" 2 } } */ +/* { dg-final { scan-assembler-not "and\t" } } */ diff --git a/gcc/testsuite/gcc.target/riscv/and-extend-2.c b/gcc/testsuite/gcc.target/riscv/and-extend-2.c new file mode 100644 index 000..fe639cd1e82 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/and-extend-2.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv32gc_zba_zbb -mabi=ilp32" } */ +/* { dg-skip-if "" { *-*-* } { "-O0" } } */ + +void +foo(unsigned long a, unsigned long* ptr) +{ +ptr[0] = a & 0xUL; +ptr[1] &= 0xUL; +} + +void +foo2(unsigned long a, unsigned long* ptr) +{ +ptr[0] = a & 0x; +ptr[1] &= 0x; +} + +void +foo3(unsigned int a, unsigned int*
Re: [PATCH v2] xtensa: Optimize '(x & CST1_POW2) != 0 ? CST2_POW2 : 0'
On Mon, May 22, 2023 at 10:48 PM Takayuki 'January June' Suwa wrote: > > On 2023/05/23 11:27, Max Filippov wrote: > > Hi Suwa-san, > > Hi! > > > This change introduces a bunch of test failures on big endian configuration. > > I believe that's because the starting bit position for zero_extract is > > counted > > from different ends depending on the endianness. > > Oops, what a stupid mistake... X( > > === > This patch decreses one machine instruction from "single bit extraction > with shifting" operation, and tries to eliminate the conditional > branch if CST2_POW2 doesn't fit into signed 12 bits with the help > of ifcvt optimization. > > /* example #1 */ > int test0(int x) { > return (x & 1048576) != 0 ? 1024 : 0; > } > extern int foo(void); > int test1(void) { > return (foo() & 1048576) != 0 ? 16777216 : 0; > } > > ;; before > test0: > movia9, 0x400 > sraia2, a2, 10 > and a2, a2, a9 > ret.n > test1: > addisp, sp, -16 > s32i.n a0, sp, 12 > call0 foo > extui a2, a2, 20, 1 > sllia2, a2, 20 > beqz.n a2, .L2 > movi.n a2, 1 > sllia2, a2, 24 > .L2: > l32i.n a0, sp, 12 > addisp, sp, 16 > ret.n > > ;; after > test0: > extui a2, a2, 20, 1 > sllia2, a2, 10 > ret.n > test1: > addisp, sp, -16 > s32i.n a0, sp, 12 > call0 foo > l32i.n a0, sp, 12 > extui a2, a2, 20, 1 > sllia2, a2, 24 > addisp, sp, 16 > ret.n > > In addition, if the left shift amount ('exact_log2(CST2_POW2)') is > between 1 through 3 and a either addition or subtraction with another > register follows, emit a ADDX[248] or SUBX[248] machine instruction > instead of separate left shift and add/subtract ones. > > /* example #2 */ > int test2(int x, int y) { > return ((x & 1048576) != 0 ? 4 : 0) + y; > } > int test3(int x, int y) { > return ((x & 2) != 0 ? 8 : 0) - y; > } > > ;; before > test2: > movi.n a9, 4 > sraia2, a2, 18 > and a2, a2, a9 > add.n a2, a2, a3 > ret.n > test3: > movi.n a9, 8 > sllia2, a2, 2 > and a2, a2, a9 > sub a2, a2, a3 > ret.n > > ;; after > test2: > extui a2, a2, 20, 1 > addx4 a2, a2, a3 > ret.n > test3: > extui a2, a2, 1, 1 > subx8 a2, a2, a3 > ret.n > > gcc/ChangeLog: > > * config/xtensa/predicates.md (addsub_operator): New. > * config/xtensa/xtensa.md (*extzvsi-1bit_ashlsi3, > *extzvsi-1bit_addsubx): New insn_and_split patterns. > * config/xtensa/xtensa.cc (xtensa_rtx_costs): > Add a special case about ifcvt 'noce_try_cmove()' to handle > constant loads that do not fit into signed 12 bits in the > patterns added above. > --- > gcc/config/xtensa/predicates.md | 3 ++ > gcc/config/xtensa/xtensa.cc | 3 +- > gcc/config/xtensa/xtensa.md | 83 + > 3 files changed, 88 insertions(+), 1 deletion(-) Regtested for target=xtensa-linux-uclibc, no new regressions. Committed to master. -- Thanks. -- Max
Re: [PATCH 2/2] xtensa: Merge '*addx' and '*subx' insn patterns into one
On Mon, May 22, 2023 at 12:06 AM Takayuki 'January June' Suwa wrote: > > By making use of the 'addsub_operator' added in the last patch. > > gcc/ChangeLog: > > * config/xtensa/xtensa.md (*addsubx): Rename from '*addx', > and change to also accept '*subx' pattern. > (*subx): Remove. > --- > gcc/config/xtensa/xtensa.md | 31 +-- > 1 file changed, 13 insertions(+), 18 deletions(-) Regtested for target=xtensa-linux-uclibc, no new regressions. Committed to master. -- Thanks. -- Max
Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization
>>> Don't you want to use your shiny new operand passing style here as >>> with the other expanders? > H, I do this just following ARM code style. > You can see I do pass rtx[] for expand_vcond and pass rtx,rtx,rtx for > expand_vec_cmp. > Well, I just follow ARM SVE implementation (You can check aarch64-sve.md, we > are the same) :) > If don't like it, could give me more information then I change it for you. It doesn't matter that much in the end. I just wondered that we just introduced a new style of passing operands to the insn_expander and then immediately not use it in the first follow up :) Nit: + e.set_policy (op_num == RVV_CMP_OP ? MASK_UNDISTURBED : MASK_ANY); This looks weird in an emit__cmp_insn. Without a comment it's unclear why anything else but a CMP_OP would ever be used here. The double meaning of the enum (that I wanted to be an instruction type rather than a "number of operands") doesn't help. But well, fixable in the future. We just need to make sure not to accumulate too many of these warts. >From the expander side V3 looks clean now. The integer parts look OK to me but I haven't checked the FP side at all. Regards Robin
[PATCH] libstdc++: Add missing constexpr to simd_neon
Signed-off-by: Matthias Kretz libstdc++-v3/ChangeLog: PR libstdc++/109261 * include/experimental/bits/simd_neon.h (_S_reduce): Add constexpr and make NEON implementation conditional on not __builtin_is_constant_evaluated. --- .../include/experimental/bits/simd_neon.h | 76 +-- 1 file changed, 36 insertions(+), 40 deletions(-) -- ── Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de stdₓ::simd ──diff --git a/libstdc++-v3/include/experimental/bits/simd_neon.h b/libstdc++-v3/include/experimental/bits/simd_neon.h index 637b121b130..8f732d7587b 100644 --- a/libstdc++-v3/include/experimental/bits/simd_neon.h +++ b/libstdc++-v3/include/experimental/bits/simd_neon.h @@ -84,50 +84,46 @@ _S_masked_store_nocvt(_SimdWrapper<_Tp, _Np> __v, _Tp* __mem, // }}} // _S_reduce {{{ template - _GLIBCXX_SIMD_INTRINSIC static _Tp + _GLIBCXX_SIMD_INTRINSIC static constexpr _Tp _S_reduce(simd<_Tp, _Abi> __x, _BinaryOperation&& __binary_op) { - constexpr size_t _Np = __x.size(); - if constexpr (sizeof(__x) == 16 && _Np >= 4 - && !_Abi::template _S_is_partial<_Tp>) - { - const auto __halves = split>>(__x); - const auto __y = __binary_op(__halves[0], __halves[1]); - return _SimdImplNeon>::_S_reduce( - __y, static_cast<_BinaryOperation&&>(__binary_op)); - } - else if constexpr (_Np == 8) - { - __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( - __vector_permute<1, 0, 3, 2, 5, 4, 7, 6>( - __x._M_data))); - __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( - __vector_permute<3, 2, 1, 0, 7, 6, 5, 4>( - __x._M_data))); - __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( - __vector_permute<7, 6, 5, 4, 3, 2, 1, 0>( - __x._M_data))); - return __x[0]; - } - else if constexpr (_Np == 4) - { - __x - = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( - __vector_permute<1, 0, 3, 2>(__x._M_data))); - __x - = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( - __vector_permute<3, 2, 1, 0>(__x._M_data))); - return __x[0]; - } - else if constexpr (_Np == 2) + if (not __builtin_is_constant_evaluated()) { - __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( - __vector_permute<1, 0>(__x._M_data))); - return __x[0]; + constexpr size_t _Np = __x.size(); + if constexpr (sizeof(__x) == 16 && _Np >= 4 + && !_Abi::template _S_is_partial<_Tp>) + { + const auto __halves = split>>(__x); + const auto __y = __binary_op(__halves[0], __halves[1]); + return _SimdImplNeon>::_S_reduce( + __y, static_cast<_BinaryOperation&&>(__binary_op)); + } + else if constexpr (_Np == 8) + { + __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( + __vector_permute<1, 0, 3, 2, 5, 4, 7, 6>(__x._M_data))); + __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( + __vector_permute<3, 2, 1, 0, 7, 6, 5, 4>(__x._M_data))); + __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( + __vector_permute<7, 6, 5, 4, 3, 2, 1, 0>(__x._M_data))); + return __x[0]; + } + else if constexpr (_Np == 4) + { + __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( + __vector_permute<1, 0, 3, 2>(__x._M_data))); + __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( + __vector_permute<3, 2, 1, 0>(__x._M_data))); + return __x[0]; + } + else if constexpr (_Np == 2) + { + __x = __binary_op(__x, _Base::template _M_make_simd<_Tp, _Np>( + __vector_permute<1, 0>(__x._M_data))); + return __x[0]; + } } - else - return _Base::_S_reduce(__x, - static_cast<_BinaryOperation&&>(__binary_op)); + return _Base::_S_reduce(__x, static_cast<_BinaryOperation&&>(__binary_op)); } // }}}
Re: [PATCH v2] rs6000: Add buildin for mffscrn instructions
On 5/23/23 12:24 AM, Kewen.Lin wrote: > on 2023/5/23 01:31, Carl Love wrote: >> The builtins were requested for use in GLibC. As of version 2.31 they >> were added as inline asm. They requested a builtin so the asm could be >> removed. > > So IMHO we also want the similar support for mffscrn, that is to make > use of mffscrn and mffscrni on Power9 and later, but falls back to > __builtin_set_fpscr_rn + mffs similar on older platforms. So __builtin_set_fpscr_rn everything we want (sets the RN bits) and uses mffscrn/mffscrni on P9 and later and uses older insns on pre-P9. The only problem is we don't return the current FPSCR bits, as the bif is defined to return void. Crazy idea, but could we extend the built-in with an overload that returns the FPSCR bits? To be honest, I like the __builtin_set_fpscr_rn name better than __builtin_mffscrn[i]. The built-in machinery can see that the usage is expecting a return value or not and for the pre-P9 code, can skip generating the ending mffs if we don't want the return value. Peter
[PATCH] Dump if a pattern fails after having printed applying it
While trying to understand how to use the ! operand for match patterns, I noticed that the debug dumps would print out applying a pattern but nothing when it was rejected in the end. This was confusing me. This adds that by emitting a dump for the failed case. Note the patch is little more complex as we don't want to print out if debug counter rejected the pattern and then we need to fix up when we mark needing a label or not. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. gcc/ChangeLog: * genmatch.cc (needs_label): New variable (expr::gen_transform): Set needs_label if we use the local_label. (dt_simplify::gen_1): Use `_1` for the debug count label. After the local label, emit debug print for the failure. Emit `_1` label if needed. --- gcc/genmatch.cc | 28 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index 177c13d87cb..2ea80d341a2 100644 --- a/gcc/genmatch.cc +++ b/gcc/genmatch.cc @@ -2433,6 +2433,7 @@ capture_info::walk_c_expr (c_expr *e) /* The current label failing the current matched pattern during code generation. */ static char *fail_label; +static bool needs_label; /* Code generation off the decision tree and the refered AST nodes. */ @@ -2611,6 +2612,7 @@ expr::gen_transform (FILE *f, int indent, const char *dest, bool gimple, fprintf_indent (f, indent, "if (!_r%d) goto %s;\n", depth, fail_label); + needs_label = true; if (*opr == CONVERT_EXPR) { indent -= 4; @@ -2640,11 +2642,13 @@ expr::gen_transform (FILE *f, int indent, const char *dest, bool gimple, { fprintf_indent (f, indent, "if (!_r%d)\n", depth); fprintf_indent (f, indent, " goto %s;\n", fail_label); + needs_label = true; } if (force_leaf) { fprintf_indent (f, indent, "if (EXPR_P (_r%d))\n", depth); fprintf_indent (f, indent, " goto %s;\n", fail_label); + needs_label = true; } if (*opr == CONVERT_EXPR) { @@ -3409,7 +3413,8 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) char local_fail_label[256]; snprintf (local_fail_label, 256, "next_after_fail%u", ++fail_label_cnt); fail_label = local_fail_label; - bool needs_label = false; + needs_label = false; + bool needs_label_1 = false; /* Analyze captures and perform early-outs on the incoming arguments that cover cases we cannot handle. */ @@ -3484,8 +3489,8 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) if (s->kind == simplify::SIMPLIFY) { - fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label); - needs_label = true; + fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s_1;\n", fail_label); + needs_label_1 = true; } fprintf_indent (f, indent, "if (UNLIKELY (debug_dump)) " @@ -3718,7 +3723,22 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) indent -= 2; fprintf_indent (f, indent, "}\n"); if (needs_label) -fprintf (f, "%s:;\n", fail_label); +{ + fprintf (f, "%s:;\n", fail_label); + if (s->kind == simplify::SIMPLIFY) + { + fprintf_indent (f, indent, "if (UNLIKELY (debug_dump)) " + "fprintf (dump_file, \"Pattern failed "); + fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); + output_line_directive (f, +result ? result->location : s->match->location, true, +true); + fprintf (f, ", __FILE__, __LINE__);\n"); + } +} + if (needs_label_1) +fprintf (f, "%s_1:;\n", fail_label); + needs_label = false; fail_label = NULL; } -- 2.31.1
[PATCH] RISC-V: Fix magic number of RVV auto-vectorization expander
From: Juzhe-Zhong This simple patch fixes the magic number, replaced by enum to make code more reasonable. Ok for trunk ? gcc/ChangeLog: * config/riscv/riscv-v.cc (expand_vec_series): Fix magic number. (expand_const_vector): Ditto. (legitimize_move): Ditto. (expand_vector_init_insert_elems): Ditto. --- gcc/config/riscv/riscv-v.cc | 39 + 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc index 478a052a779..524e8c7f858 100644 --- a/gcc/config/riscv/riscv-v.cc +++ b/gcc/config/riscv/riscv-v.cc @@ -406,14 +406,14 @@ expand_vec_series (rtx dest, rtx base, rtx step) int shift = exact_log2 (INTVAL (step)); rtx shift_amount = gen_int_mode (shift, Pmode); insn_code icode = code_for_pred_scalar (ASHIFT, mode); - rtx ops[3] = {step_adj, vid, shift_amount}; - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); + rtx ops[RVV_BINOP] = {step_adj, vid, shift_amount}; + emit_vlmax_insn (icode, RVV_BINOP, ops); } else { insn_code icode = code_for_pred_scalar (MULT, mode); - rtx ops[3] = {step_adj, vid, step}; - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); + rtx ops[RVV_BINOP] = {step_adj, vid, step}; + emit_vlmax_insn (icode, RVV_BINOP, ops); } } @@ -428,8 +428,8 @@ expand_vec_series (rtx dest, rtx base, rtx step) { rtx result = gen_reg_rtx (mode); insn_code icode = code_for_pred_scalar (PLUS, mode); - rtx ops[3] = {result, step_adj, base}; - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); + rtx ops[RVV_BINOP] = {result, step_adj, base}; + emit_vlmax_insn (icode, RVV_BINOP, ops); emit_move_insn (dest, result); } } @@ -445,8 +445,8 @@ expand_const_vector (rtx target, rtx src) gcc_assert ( const_vec_duplicate_p (src, &elt) && (rtx_equal_p (elt, const0_rtx) || rtx_equal_p (elt, const1_rtx))); - rtx ops[2] = {target, src}; - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, ops); + rtx ops[RVV_UNOP] = {target, src}; + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); return; } @@ -458,16 +458,14 @@ expand_const_vector (rtx target, rtx src) we use vmv.v.i instruction. */ if (satisfies_constraint_vi (src) || satisfies_constraint_Wc0 (src)) { - rtx ops[2] = {tmp, src}; - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, - ops); + rtx ops[RVV_UNOP] = {tmp, src}; + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); } else { elt = force_reg (elt_mode, elt); - rtx ops[2] = {tmp, elt}; - emit_vlmax_insn (code_for_pred_broadcast (mode), - riscv_vector::RVV_UNOP, ops); + rtx ops[RVV_UNOP] = {tmp, elt}; + emit_vlmax_insn (code_for_pred_broadcast (mode), RVV_UNOP, ops); } if (tmp != target) @@ -536,9 +534,8 @@ legitimize_move (rtx dest, rtx src) rtx tmp = gen_reg_rtx (mode); if (MEM_P (src)) { - rtx ops[2] = {tmp, src}; - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, - ops); + rtx ops[RVV_UNOP] = {tmp, src}; + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); } else emit_move_insn (tmp, src); @@ -548,8 +545,8 @@ legitimize_move (rtx dest, rtx src) if (satisfies_constraint_vu (src)) return false; - rtx ops[2] = {dest, src}; - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, ops); + rtx ops[RVV_UNOP] = {dest, src}; + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); return true; } @@ -1281,8 +1278,8 @@ expand_vector_init_insert_elems (rtx target, const rvv_builder &builder, unsigned int unspec = FLOAT_MODE_P (mode) ? UNSPEC_VFSLIDE1DOWN : UNSPEC_VSLIDE1DOWN; insn_code icode = code_for_pred_slide (unspec, mode); - rtx ops[3] = {target, target, builder.elt (i)}; - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); + rtx ops[RVV_BINOP] = {target, target, builder.elt (i)}; + emit_vlmax_insn (icode, RVV_BINOP, ops); } } -- 2.36.3
Re: Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization
Ok. Let's wait for Kito's more comments. Thanks. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-05-24 05:07 To: 钟居哲; gcc-patches CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; Jeff Law; richard.sandiford Subject: Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization >>> Don't you want to use your shiny new operand passing style here as >>> with the other expanders? > H, I do this just following ARM code style. > You can see I do pass rtx[] for expand_vcond and pass rtx,rtx,rtx for > expand_vec_cmp. > Well, I just follow ARM SVE implementation (You can check aarch64-sve.md, we > are the same) :) > If don't like it, could give me more information then I change it for you. It doesn't matter that much in the end. I just wondered that we just introduced a new style of passing operands to the insn_expander and then immediately not use it in the first follow up :) Nit: + e.set_policy (op_num == RVV_CMP_OP ? MASK_UNDISTURBED : MASK_ANY); This looks weird in an emit__cmp_insn. Without a comment it's unclear why anything else but a CMP_OP would ever be used here. The double meaning of the enum (that I wanted to be an instruction type rather than a "number of operands") doesn't help. But well, fixable in the future. We just need to make sure not to accumulate too many of these warts. From the expander side V3 looks clean now. The integer parts look OK to me but I haven't checked the FP side at all. Regards Robin