From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 570B5383E509 for ; Wed, 5 Jun 2024 08:13:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 570B5383E509 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 570B5383E509 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1717575204; cv=none; b=vKiBwz1FnpJBsnuNOtbOT9SWaeQxq+MMswewwREA2IMgVrqnk71P6X33zLVUsvXb39CulkS1MCLkGpG05z8njjp8Xi10acMEmZZ0YD7kuvr+FuiTBvygYrpZyftDKjygYxDWwEwxOzHLKHflr0I9DNMWus9bgljAVPQaoq1m+ww= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1717575204; c=relaxed/simple; bh=9rN4yyikWBKkM2PnjCknEuFAFvGL7wKHUwliFIXuED0=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=wddj+T/WaAP5btlmU3j2GNGG4J13nbTXLks6XVRtgetpkvqqHKDk7JlLUn46r4JWJg2LtSgsZ8gJr2KP+Uawxmk15TSHYN78TFQQ5Flh+4iirzZUxwMMPxrnSLPxLDwJzZqtO6NrpFnwNtqsB7kLU/Xpqw6QyWj4bwfH4KwDCzg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3F175DA7; Wed, 5 Jun 2024 01:13:46 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EB72E3F762; Wed, 5 Jun 2024 01:13:20 -0700 (PDT) From: Richard Sandiford To: Evgeny Karpov Mail-Followup-To: Evgeny Karpov ,"gcc-patches\@gcc.gnu.org" , "ubizjak\@gmail.com" , "Richard Earnshaw \(lists\)" , Maxim Kuvyrkov , Radek Barton , richard.sandiford@arm.com Cc: "gcc-patches\@gcc.gnu.org" , "ubizjak\@gmail.com" , "Richard Earnshaw \(lists\)" , Maxim Kuvyrkov , Radek Barton Subject: Re: [PATCH v1 0/6] Add DLL import/export implementation to AArch64 References: Date: Wed, 05 Jun 2024 09:13:19 +0100 In-Reply-To: (Evgeny Karpov's message of "Tue, 4 Jun 2024 20:10:30 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-19.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_NUMSUBJECT,KAM_STOCKGEN,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Evgeny Karpov writes: > Richard and Uros, could you please review the changes for v2? > Additionally, we have detected an issue with GCC GC in winnt-dll.cc. The fix will be included in v2. Would it be possible to have a more "purposeful" name than CMODEL_IS_NOT_LARGE_OR_MEDIUM_PIC? What's the property of large and medium PIC that needs to be handled differently? It'd be good to have the macro be a positive test rather than a negative test, so that we don't end up with !IS_NOT_FOO when testing for FOO. Otherwise it looks good to me. I never fully reviewed 1/6 or 6/6, sorry. My main comment there is that it would be good to avoid including config/mingw/winnt.h and config/mingw/winnt-dll.h in config/aarch64/aarch64-protos.h (or in other common AArch64 code). It's OK for common AArch64 code to have hooks that can be filled in by OS-specific headers, but there shouldn't be OS-specific includes or code in the common files themselves. >>> -ix86_handle_selectany_attribute (tree *node, tree name, tree, int, >>> +mingw_handle_selectany_attribute (tree *node, tree name, tree, int, >>> bool *no_add_attrs) > >> please reindent the parameters for the new name length. > > Richard, could you please clarify how it should be done? The "bool" on the second line should be directly under the "tree" on the first line (so one extra space before "bool"). Thanks, Richard > Thanks! > > Regards, > Evgeny > > > --- > gcc/config/aarch64/cygming.h | 6 +++++ > gcc/config/i386/cygming.h | 6 +++++ > gcc/config/i386/i386-expand.cc | 6 +++-- > gcc/config/i386/i386-expand.h | 2 -- > gcc/config/i386/i386.cc | 42 ++++++++++------------------------ > gcc/config/i386/i386.h | 2 ++ > gcc/config/mingw/winnt-dll.cc | 8 ++----- > gcc/config/mingw/winnt-dll.h | 2 +- > 8 files changed, 33 insertions(+), 41 deletions(-) > > diff --git a/gcc/config/aarch64/cygming.h b/gcc/config/aarch64/cygming.h > index 4beebf9e093..0ff475754e0 100644 > --- a/gcc/config/aarch64/cygming.h > +++ b/gcc/config/aarch64/cygming.h > @@ -183,4 +183,10 @@ still needed for compilation. */ > #undef MAX_OFILE_ALIGNMENT > #define MAX_OFILE_ALIGNMENT (8192 * 8) > > +#define CMODEL_IS_NOT_LARGE_OR_MEDIUM_PIC 0 > + > +#define HAVE_64BIT_POINTERS 1 > + > +#define GOT_ALIAS_SET mingw_GOT_alias_set () > + > #endif > diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h > index ee01e6bb6ce..cd240533dbc 100644 > --- a/gcc/config/i386/cygming.h > +++ b/gcc/config/i386/cygming.h > @@ -469,3 +469,9 @@ do { \ > #ifndef HAVE_GAS_ALIGNED_COMM > # define HAVE_GAS_ALIGNED_COMM 0 > #endif > + > +#define CMODEL_IS_NOT_LARGE_OR_MEDIUM_PIC ix86_cmodel != CM_LARGE_PIC && ix86_cmodel != CM_MEDIUM_PIC > + > +#define HAVE_64BIT_POINTERS TARGET_64BIT_DEFAULT > + > +#define GOT_ALIAS_SET mingw_GOT_alias_set () > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > index fb460e30d0a..267d0ba257b 100644 > --- a/gcc/config/i386/i386-expand.cc > +++ b/gcc/config/i386/i386-expand.cc > @@ -408,11 +408,12 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > : UNSPEC_GOT)); > op1 = gen_rtx_CONST (Pmode, op1); > op1 = gen_const_mem (Pmode, op1); > - set_mem_alias_set (op1, ix86_GOT_alias_set ()); > + set_mem_alias_set (op1, GOT_ALIAS_SET); > } > else > { > - tmp = ix86_legitimize_pe_coff_symbol (op1, addend != NULL_RTX); > +#if TARGET_PECOFF > + tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX); > if (tmp) > { > op1 = tmp; > @@ -424,6 +425,7 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > op1 = operands[1]; > break; > } > +#endif > } > > if (addend) > diff --git a/gcc/config/i386/i386-expand.h b/gcc/config/i386/i386-expand.h > index a8c20993954..5e02df1706d 100644 > --- a/gcc/config/i386/i386-expand.h > +++ b/gcc/config/i386/i386-expand.h > @@ -34,9 +34,7 @@ struct expand_vec_perm_d > }; > > rtx legitimize_tls_address (rtx x, enum tls_model model, bool for_mov); > -alias_set_type ix86_GOT_alias_set (void); > rtx legitimize_pic_address (rtx orig, rtx reg); > -rtx ix86_legitimize_pe_coff_symbol (rtx addr, bool inreg); > > bool insn_defines_reg (unsigned int regno1, unsigned int regno2, > rtx_insn *insn); > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > index 66845b30446..ee3a59ed498 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -11807,30 +11807,6 @@ constant_address_p (rtx x) > } > > > > -#if TARGET_PECOFF > -rtx ix86_legitimize_pe_coff_symbol (rtx addr, bool inreg) > -{ > - return legitimize_pe_coff_symbol (addr, inreg); > -} > - > -alias_set_type > -ix86_GOT_alias_set (void) > -{ > - return mingw_GOT_alias_set (); > -} > -#else > -rtx ix86_legitimize_pe_coff_symbol (rtx addr, bool inreg) > -{ > - return NULL_RTX; > -} > - > -alias_set_type > -ix86_GOT_alias_set (void) > -{ > - return -1; > -} > -#endif > - > /* Return a legitimate reference for ORIG (an address) using the > register REG. If REG is 0, a new pseudo is generated. > > @@ -11867,9 +11843,11 @@ legitimize_pic_address (rtx orig, rtx reg) > > if (TARGET_64BIT && TARGET_DLLIMPORT_DECL_ATTRIBUTES) > { > - rtx tmp = ix86_legitimize_pe_coff_symbol (addr, true); > +#if TARGET_PECOFF > + rtx tmp = legitimize_pe_coff_symbol (addr, true); > if (tmp) > return tmp; > +#endif > } > > if (TARGET_64BIT && legitimate_pic_address_disp_p (addr)) > @@ -11912,9 +11890,11 @@ legitimize_pic_address (rtx orig, rtx reg) > on VxWorks, see gotoff_operand. */ > || (TARGET_VXWORKS_RTP && GET_CODE (addr) == LABEL_REF)) > { > - rtx tmp = ix86_legitimize_pe_coff_symbol (addr, true); > +#if TARGET_PECOFF > + rtx tmp = legitimize_pe_coff_symbol (addr, true); > if (tmp) > return tmp; > +#endif > > /* For x64 PE-COFF there is no GOT table, > so we use address directly. */ > @@ -11929,7 +11909,7 @@ legitimize_pic_address (rtx orig, rtx reg) > UNSPEC_GOTPCREL); > new_rtx = gen_rtx_CONST (Pmode, new_rtx); > new_rtx = gen_const_mem (Pmode, new_rtx); > - set_mem_alias_set (new_rtx, ix86_GOT_alias_set ()); > + set_mem_alias_set (new_rtx, GOT_ALIAS_SET); > } > else > { > @@ -11951,7 +11931,7 @@ legitimize_pic_address (rtx orig, rtx reg) > new_rtx = gen_rtx_PLUS (Pmode, pic_offset_table_rtx, new_rtx); > > new_rtx = gen_const_mem (Pmode, new_rtx); > - set_mem_alias_set (new_rtx, ix86_GOT_alias_set ()); > + set_mem_alias_set (new_rtx, GOT_ALIAS_SET); > } > > new_rtx = copy_to_suggested_reg (new_rtx, reg, Pmode); > @@ -12328,7 +12308,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > if (pic) > off = gen_rtx_PLUS (tp_mode, pic, off); > off = gen_const_mem (tp_mode, off); > - set_mem_alias_set (off, ix86_GOT_alias_set ()); > + set_mem_alias_set (off, GOT_ALIAS_SET); > > if (TARGET_64BIT || TARGET_ANY_GNU_TLS) > { > @@ -12526,9 +12506,11 @@ ix86_legitimize_address (rtx x, rtx, machine_mode mode) > > if (TARGET_DLLIMPORT_DECL_ATTRIBUTES) > { > - rtx tmp = ix86_legitimize_pe_coff_symbol (x, true); > +#if TARGET_PECOFF > + rtx tmp = legitimize_pe_coff_symbol (x, true); > if (tmp) > return tmp; > +#endif > } > > if (flag_pic && SYMBOLIC_CONST (x)) > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h > index 969391d3013..2b3d0c8db75 100644 > --- a/gcc/config/i386/i386.h > +++ b/gcc/config/i386/i386.h > @@ -2261,6 +2261,8 @@ extern int const svr4_debugger_register_map[FIRST_PSEUDO_REGISTER]; > /* Which processor to tune code generation for. These must be in sync > with processor_cost_table in i386-options.cc. */ > > +#define GOT_ALIAS_SET -1 > + > enum processor_type > { > PROCESSOR_GENERIC = 0, > diff --git a/gcc/config/mingw/winnt-dll.cc b/gcc/config/mingw/winnt-dll.cc > index 591e89cb5c9..c91e95225d3 100644 > --- a/gcc/config/mingw/winnt-dll.cc > +++ b/gcc/config/mingw/winnt-dll.cc > @@ -1,6 +1,6 @@ > /* Expand a SYMBOL into its corresponding dllimport, far-address, > or refptr symbol. > -Copyright (C) 2024 Free Software Foundation, Inc. > +Copyright (C) 1988-2024 Free Software Foundation, Inc. > > GCC is free software; you can redistribute it and/or modify it under > the terms of the GNU General Public License as published by the Free > @@ -206,13 +206,9 @@ legitimize_pe_coff_symbol (rtx addr, bool inreg) > } > } > > -#if !defined (TARGET_AARCH64_MS_ABI) > - > - if (ix86_cmodel != CM_LARGE_PIC && ix86_cmodel != CM_MEDIUM_PIC) > + if (CMODEL_IS_NOT_LARGE_OR_MEDIUM_PIC) > return NULL_RTX; > > -#endif > - > if (GET_CODE (addr) == SYMBOL_REF > && !is_imported_p (addr) > && SYMBOL_REF_EXTERNAL_P (addr) > diff --git a/gcc/config/mingw/winnt-dll.h b/gcc/config/mingw/winnt-dll.h > index 19c16e747a2..0877f10d67e 100644 > --- a/gcc/config/mingw/winnt-dll.h > +++ b/gcc/config/mingw/winnt-dll.h > @@ -23,4 +23,4 @@ extern bool is_imported_p (rtx x); > extern alias_set_type mingw_GOT_alias_set (void); > extern rtx legitimize_pe_coff_symbol (rtx addr, bool inreg); > > -#endif > \ No newline at end of file > +#endif