| Message ID | DBBPR83MB0613B6ECB03F495B4CFCE09BF8D72@DBBPR83MB0613.EURPRD83.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | i386: Fix regression after refactoring legitimize_pe_coff_symbol, ix86_GOT_alias_set and PE_COFF_LEGITIMIZE_EXTERN_DECL | expand |
On Thu, Jun 27, 2024 at 9:16 AM Evgeny Karpov <Evgeny.Karpov@microsoft.com> wrote: > > Thank you for reporting the issues and discussing the root causes. > It helped in preparing the patch. > > This patch fixes 3 bugs reported after merging > the "Add DLL import/export implementation to AArch64" series. > https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html > The series refactors the i386 codebase to reuse it in AArch64, which > triggers some bugs. > > Bug 115661 - [15 Regression] wrong code at -O{2,3} on > x86_64-linux-gnu since r15-1599-g63512c72df09b4 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661 > > Bug 115635 - [15 regression] Bootstrap fails with failed > self-test with the rust fe (diagnostic-path.cc:1153: > test_empty_path: FAIL: ASSERT_FALSE > ((path.interprocedural_p ()))) since r15-1599-g63512c72df09b4 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635 > > Issue 1. In some code, i386 has been relying on the > legitimize_pe_coff_symbol call on all platforms and should return > NULL_RTX if it is not supported. > > Fix: NULL_RTX handling has been added when the target does not > support PECOFF. > > Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be > extracted to mingw. > > Fix: ix86_GOT_alias_set has been returned as it was and is used on > all platforms for i386. > > Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks > x86_64-w64-mingw32 build cannot represent relocation type > BFD_RELOC_64 since r15-1602-ged20feebd9ea31 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643 > > Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used > with a negative operator for a complex expression without braces. > > Fix: Braces has been added, and > PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to > PE_COFF_LEGITIMIZE_EXTERN_DECL. > > > The patch has been attached as a text file because it contains special > characters that are usually removed by the mail client. > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > index 5dfa7d49f58..20adb42e17b 100644 > --- a/gcc/config/i386/i386-expand.cc > +++ b/gcc/config/i386/i386-expand.cc > @@ -414,6 +414,10 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > { > #if TARGET_PECOFF > tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX); > +#else > + tmp = NULL_RTX; > +#endif > + > if (tmp) > { > op1 = tmp; > @@ -425,7 +429,6 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > op1 = operands[1]; > break; > } > -#endif > } > > if (addend) tmp can only be set by legitimize_pe_coff_symbol, so !TARGET_PECOFF will always get to the "else" part. Do this change simply by moving #endif, like the below: --cut here-- iff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc index 5dfa7d49f58..407db6c215b 100644 --- a/gcc/config/i386/i386-expand.cc +++ b/gcc/config/i386/i386-expand.cc @@ -421,11 +421,11 @@ ix86_expand_move (machine_mode mode, rtx operands[]) break; } else +#endif { op1 = operands[1]; break; } -#endif } if (addend) --cut here-- Side note, legitimize_pe_coff_symbol is always called from #if TARGET_PECOFF, so: rtx legitimize_pe_coff_symbol (rtx addr, bool inreg) { if (!TARGET_PECOFF) return NULL_RTX; should be removed or converted to gcc_assert. > +alias_set_type > +ix86_GOT_alias_set (void) > +{ > + static alias_set_type set = -1; Please add a line of vertical space here. > + if (set == -1) > + set = new_alias_set (); > + return set; OK, but please allow RichartB to look at the alias_set changes. Thanks, Uros.
Hi Evgeny, Minor comments: - the patch title should end with [PRnnnnn, ...] (choose the most relevant bug number) - ChangeLog should mention every bug with PR component/nnnnn so that the bugzilla hooks will notice the commit. See https://gcc.gnu.org/contribute.html#patches (but I can do it for you before pushing the patch) Thanks, Christophe On Thu, 27 Jun 2024 at 09:16, Evgeny Karpov <Evgeny.Karpov@microsoft.com> wrote: > > Thank you for reporting the issues and discussing the root causes. > It helped in preparing the patch. > > This patch fixes 3 bugs reported after merging > the "Add DLL import/export implementation to AArch64" series. > https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html > The series refactors the i386 codebase to reuse it in AArch64, which > triggers some bugs. > > Bug 115661 - [15 Regression] wrong code at -O{2,3} on > x86_64-linux-gnu since r15-1599-g63512c72df09b4 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661 > > Bug 115635 - [15 regression] Bootstrap fails with failed > self-test with the rust fe (diagnostic-path.cc:1153: > test_empty_path: FAIL: ASSERT_FALSE > ((path.interprocedural_p ()))) since r15-1599-g63512c72df09b4 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635 > > Issue 1. In some code, i386 has been relying on the > legitimize_pe_coff_symbol call on all platforms and should return > NULL_RTX if it is not supported. > > Fix: NULL_RTX handling has been added when the target does not > support PECOFF. > > Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be > extracted to mingw. > > Fix: ix86_GOT_alias_set has been returned as it was and is used on > all platforms for i386. > > Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks > x86_64-w64-mingw32 build cannot represent relocation type > BFD_RELOC_64 since r15-1602-ged20feebd9ea31 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643 > > Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used > with a negative operator for a complex expression without braces. > > Fix: Braces has been added, and > PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to > PE_COFF_LEGITIMIZE_EXTERN_DECL. > > > The patch has been attached as a text file because it contains special > characters that are usually removed by the mail client. > > Regards, > Evgeny
Can this process be a little bit simpler in the future? Get Outlook for Android<https://aka.ms/AAb9ysg>
On Thu, Jun 27, 2024 at 10:40 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Thu, Jun 27, 2024 at 9:16 AM Evgeny Karpov > <Evgeny.Karpov@microsoft.com> wrote: > > > > Thank you for reporting the issues and discussing the root causes. > > It helped in preparing the patch. > > > > This patch fixes 3 bugs reported after merging > > the "Add DLL import/export implementation to AArch64" series. > > https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html > > The series refactors the i386 codebase to reuse it in AArch64, which > > triggers some bugs. > > > > Bug 115661 - [15 Regression] wrong code at -O{2,3} on > > x86_64-linux-gnu since r15-1599-g63512c72df09b4 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661 > > > > Bug 115635 - [15 regression] Bootstrap fails with failed > > self-test with the rust fe (diagnostic-path.cc:1153: > > test_empty_path: FAIL: ASSERT_FALSE > > ((path.interprocedural_p ()))) since r15-1599-g63512c72df09b4 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635 > > > > Issue 1. In some code, i386 has been relying on the > > legitimize_pe_coff_symbol call on all platforms and should return > > NULL_RTX if it is not supported. > > > > Fix: NULL_RTX handling has been added when the target does not > > support PECOFF. > > > > Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be > > extracted to mingw. > > > > Fix: ix86_GOT_alias_set has been returned as it was and is used on > > all platforms for i386. > > > > Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks > > x86_64-w64-mingw32 build cannot represent relocation type > > BFD_RELOC_64 since r15-1602-ged20feebd9ea31 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643 > > > > Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used > > with a negative operator for a complex expression without braces. > > > > Fix: Braces has been added, and > > PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to > > PE_COFF_LEGITIMIZE_EXTERN_DECL. > > > > > > The patch has been attached as a text file because it contains special > > characters that are usually removed by the mail client. > > > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > > index 5dfa7d49f58..20adb42e17b 100644 > > --- a/gcc/config/i386/i386-expand.cc > > +++ b/gcc/config/i386/i386-expand.cc > > @@ -414,6 +414,10 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > > { > > #if TARGET_PECOFF > > tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX); > > +#else > > + tmp = NULL_RTX; > > +#endif > > + > > if (tmp) > > { > > op1 = tmp; > > @@ -425,7 +429,6 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > > op1 = operands[1]; > > break; > > } > > -#endif > > } > > > > if (addend) > > tmp can only be set by legitimize_pe_coff_symbol, so !TARGET_PECOFF > will always get to the "else" part. Do this change simply by moving > #endif, like the below: > > --cut here-- > iff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > index 5dfa7d49f58..407db6c215b 100644 > --- a/gcc/config/i386/i386-expand.cc > +++ b/gcc/config/i386/i386-expand.cc > @@ -421,11 +421,11 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > break; > } > else > +#endif > { > op1 = operands[1]; > break; > } > -#endif > } > > if (addend) > --cut here-- > > Side note, legitimize_pe_coff_symbol is always called from #if > TARGET_PECOFF, so: > > rtx > legitimize_pe_coff_symbol (rtx addr, bool inreg) > { > if (!TARGET_PECOFF) > return NULL_RTX; > > should be removed or converted to gcc_assert. > > > +alias_set_type > > +ix86_GOT_alias_set (void) > > +{ > > + static alias_set_type set = -1; > > Please add a line of vertical space here. > > > + if (set == -1) > > + set = new_alias_set (); > > + return set; > > OK, but please allow RichartB to look at the alias_set changes. I see that it basically reverts to the old behavior so OK from my side. Richard. > Thanks, > Uros.
Evgeny Karpov <Evgeny.Karpov@microsoft.com> writes: > Thank you for reporting the issues and discussing the root causes. > It helped in preparing the patch. Thanks. I'll test it shortly but it looks equivalent to my local changes, so LGTM. > > This patch fixes 3 bugs reported after merging > the "Add DLL import/export implementation to AArch64" series. > https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html > The series refactors the i386 codebase to reuse it in AArch64, which > triggers some bugs. > > Bug 115661 - [15 Regression] wrong code at -O{2,3} on > x86_64-linux-gnu since r15-1599-g63512c72df09b4 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661 > > Bug 115635 - [15 regression] Bootstrap fails with failed > self-test with the rust fe (diagnostic-path.cc:1153: > test_empty_path: FAIL: ASSERT_FALSE > ((path.interprocedural_p ()))) since r15-1599-g63512c72df09b4 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635 > > Issue 1. In some code, i386 has been relying on the > legitimize_pe_coff_symbol call on all platforms and should return > NULL_RTX if it is not supported. > > Fix: NULL_RTX handling has been added when the target does not > support PECOFF. > > Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be > extracted to mingw. > > Fix: ix86_GOT_alias_set has been returned as it was and is used on > all platforms for i386. > > Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks > x86_64-w64-mingw32 build cannot represent relocation type > BFD_RELOC_64 since r15-1602-ged20feebd9ea31 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643 > > Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used > with a negative operator for a complex expression without braces. > > Fix: Braces has been added, and > PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to > PE_COFF_LEGITIMIZE_EXTERN_DECL. > > > The patch has been attached as a text file because it contains special > characters that are usually removed by the mail client. > > Regards, > Evgeny > > [2. 0001-i386-Fix-regression-after-refactoring-legitimize_pe_.txt --- text/plain; 0001-i386-Fix-regression-after-refactoring-legitimize_pe_.txt]...
Thursday, June 27, 2024 10:39 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > > index 5dfa7d49f58..20adb42e17b 100644 > > --- a/gcc/config/i386/i386-expand.cc > > +++ b/gcc/config/i386/i386-expand.cc > > @@ -414,6 +414,10 @@ ix86_expand_move (machine_mode mode, rtx > operands[]) > > { > > #if TARGET_PECOFF > > tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX); > > +#else > > + tmp = NULL_RTX; > > +#endif > > + > > if (tmp) > > { > > op1 = tmp; > > @@ -425,7 +429,6 @@ ix86_expand_move (machine_mode mode, rtx > operands[]) > > op1 = operands[1]; > > break; > > } > > -#endif > > } > > > > if (addend) > > tmp can only be set by legitimize_pe_coff_symbol, so !TARGET_PECOFF > will always get to the "else" part. Do this change simply by moving > #endif, like the below: > > --cut here-- > iff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > index 5dfa7d49f58..407db6c215b 100644 > --- a/gcc/config/i386/i386-expand.cc > +++ b/gcc/config/i386/i386-expand.cc > @@ -421,11 +421,11 @@ ix86_expand_move (machine_mode mode, rtx > operands[]) > break; > } > else > +#endif > { > op1 = operands[1]; > break; > } > -#endif > } > > if (addend) > --cut here-- > I would prefer readability in the original version if there are no objections. > Side note, legitimize_pe_coff_symbol is always called from #if > TARGET_PECOFF, so: > > rtx > legitimize_pe_coff_symbol (rtx addr, bool inreg) > { > if (!TARGET_PECOFF) > return NULL_RTX; > > should be removed or converted to gcc_assert. > Ok, it will be replaced with gcc_assert. Regards, Evgeny
Thursday, June 27, 2024 11:31 AM Christophe Lyon <christophe.lyon@linaro.org> wrote: > > Hi Evgeny, > > Minor comments: > - the patch title should end with [PRnnnnn, ...] (choose the most > relevant bug number) > - ChangeLog should mention every bug with PR component/nnnnn > so that the bugzilla hooks will notice the commit. > See > https://gcc.gn/ > u.org%2Fcontribute.html%23patches&data=05%7C02%7CEvgeny.Karpov%40m > icrosoft.com%7C5a0dc959feac45f679db08dc968bdafc%7C72f988bf86f141af9 > 1ab2d7cd011db47%7C1%7C0%7C638550774683835608%7CUnknown%7CT > WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ > XVCI6Mn0%3D%7C0%7C%7C%7C&sdata=v5rvSCIFzfLI9yyVt1jSwGsCPaaQNOZ > n5J3RoEdJGRw%3D&reserved=0 > > (but I can do it for you before pushing the patch) > > Thanks, > > Christophe > Thanks Christophe, I will update the patch with required references. Regards, Evgeny
On Thu, Jun 27, 2024 at 12:50 PM Evgeny Karpov <Evgeny.Karpov@microsoft.com> wrote: > > Thursday, June 27, 2024 10:39 AM > Uros Bizjak <ubizjak@gmail.com> wrote: > > > > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > > > index 5dfa7d49f58..20adb42e17b 100644 > > > --- a/gcc/config/i386/i386-expand.cc > > > +++ b/gcc/config/i386/i386-expand.cc > > > @@ -414,6 +414,10 @@ ix86_expand_move (machine_mode mode, rtx > > operands[]) > > > { > > > #if TARGET_PECOFF > > > tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX); > > > +#else > > > + tmp = NULL_RTX; > > > +#endif > > > + > > > if (tmp) > > > { > > > op1 = tmp; > > > @@ -425,7 +429,6 @@ ix86_expand_move (machine_mode mode, rtx > > operands[]) > > > op1 = operands[1]; > > > break; > > > } > > > -#endif > > > } > > > > > > if (addend) > > > > tmp can only be set by legitimize_pe_coff_symbol, so !TARGET_PECOFF > > will always get to the "else" part. Do this change simply by moving > > #endif, like the below: > > > > --cut here-- > > iff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > > index 5dfa7d49f58..407db6c215b 100644 > > --- a/gcc/config/i386/i386-expand.cc > > +++ b/gcc/config/i386/i386-expand.cc > > @@ -421,11 +421,11 @@ ix86_expand_move (machine_mode mode, rtx > > operands[]) > > break; > > } > > else > > +#endif > > { > > op1 = operands[1]; > > break; > > } > > -#endif > > } > > > > if (addend) > > --cut here-- > > > > I would prefer readability in the original version if there are no objections. The proposed form is how existing TARGET_MACHO handles similar issue. Please see e.g. i386.cc, around line 6216 and elsewhere: #if TARGET_MACHO if (TARGET_MACHO) { switch_to_section (darwin_sections[picbase_thunk_section]); fputs ("\t.weak_definition\t", asm_out_file); assemble_name (asm_out_file, name); fputs ("\n\t.private_extern\t", asm_out_file); assemble_name (asm_out_file, name); putc ('\n', asm_out_file); ASM_OUTPUT_LABEL (asm_out_file, name); DECL_WEAK (decl) = 1; } else #endif if (USE_HIDDEN_LINKONCE) ... So, there is no problem having #endif just after else. Anyway, it's your call, this is not a hill I'm willing to die on. ;) Thanks, Uros.
Thursday, June 27, 2024 8:13 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > So, there is no problem having #endif just after else. > > Anyway, it's your call, this is not a hill I'm willing to die on. ;) > > Thanks, > Uros. It looks like the patch resolves 3 reported issues. Uros, I suggest merging the patch as it is, without minor refactoring, to avoid triggering another round of testing, if you agree. Thanks. Regards, Evgeny
On Fri, Jun 28, 2024 at 1:41 PM Evgeny Karpov <Evgeny.Karpov@microsoft.com> wrote: > > Thursday, June 27, 2024 8:13 PM > Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > So, there is no problem having #endif just after else. > > > > Anyway, it's your call, this is not a hill I'm willing to die on. ;) > > > > Thanks, > > Uros. > > It looks like the patch resolves 3 reported issues. > Uros, I suggest merging the patch as it is, without minor refactoring, to avoid triggering another round of testing, if you agree. Yes, please go ahead. Thanks, Uros.
diff --git a/gcc/config/aarch64/cygming.h b/gcc/config/aarch64/cygming.h index e26488735db..9ce140a356f 100644 --- a/gcc/config/aarch64/cygming.h +++ b/gcc/config/aarch64/cygming.h @@ -186,7 +186,7 @@ still needed for compilation. */ #undef GOT_ALIAS_SET #define GOT_ALIAS_SET mingw_GOT_alias_set () -#define PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED 1 +#define PE_COFF_LEGITIMIZE_EXTERN_DECL 1 #define HAVE_64BIT_POINTERS 1 diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h index 0493b3be875..9c8c7e33cc2 100644 --- a/gcc/config/i386/cygming.h +++ b/gcc/config/i386/cygming.h @@ -470,10 +470,7 @@ do { \ # define HAVE_GAS_ALIGNED_COMM 0 #endif -#undef GOT_ALIAS_SET -#define GOT_ALIAS_SET mingw_GOT_alias_set () - -#define PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED \ - ix86_cmodel == CM_LARGE_PIC || ix86_cmodel == CM_MEDIUM_PIC +#define PE_COFF_LEGITIMIZE_EXTERN_DECL \ + (ix86_cmodel == CM_LARGE_PIC || ix86_cmodel == CM_MEDIUM_PIC) #define HAVE_64BIT_POINTERS TARGET_64BIT_DEFAULT diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc index 5dfa7d49f58..20adb42e17b 100644 --- a/gcc/config/i386/i386-expand.cc +++ b/gcc/config/i386/i386-expand.cc @@ -414,6 +414,10 @@ ix86_expand_move (machine_mode mode, rtx operands[]) { #if TARGET_PECOFF tmp = legitimize_pe_coff_symbol (op1, addend != NULL_RTX); +#else + tmp = NULL_RTX; +#endif + if (tmp) { op1 = tmp; @@ -425,7 +429,6 @@ 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 5e02df1706d..56bee29253b 100644 --- a/gcc/config/i386/i386-expand.h +++ b/gcc/config/i386/i386-expand.h @@ -34,6 +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); bool insn_defines_reg (unsigned int regno1, unsigned int regno2, diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 1f71ed04be6..aa5d188ada0 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -11242,6 +11242,17 @@ ix86_cannot_force_const_mem (machine_mode mode, rtx x) return !ix86_legitimate_constant_p (mode, x); } +/* Return a unique alias set for the GOT. */ + +alias_set_type +ix86_GOT_alias_set (void) +{ + static alias_set_type set = -1; + if (set == -1) + set = new_alias_set (); + return set; +} + /* Nonzero if the constant value X is a legitimate general operand when generating PIC code. It is given that flag_pic is on and that X satisfies CONSTANT_P. */ diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 9ed225ec587..147b12cd014 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -2259,7 +2259,7 @@ 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 +#define GOT_ALIAS_SET ix86_GOT_alias_set () enum processor_type { diff --git a/gcc/config/mingw/winnt-dll.cc b/gcc/config/mingw/winnt-dll.cc index 66c445cba77..f74495b7fda 100644 --- a/gcc/config/mingw/winnt-dll.cc +++ b/gcc/config/mingw/winnt-dll.cc @@ -139,7 +139,7 @@ get_dllimport_decl (tree decl, bool beimport) } rtl = gen_const_mem (Pmode, rtl); - set_mem_alias_set (rtl, mingw_GOT_alias_set ()); + set_mem_alias_set (rtl, GOT_ALIAS_SET); SET_DECL_RTL (to, rtl); SET_DECL_ASSEMBLER_NAME (to, get_identifier (name)); @@ -206,7 +206,7 @@ legitimize_pe_coff_symbol (rtx addr, bool inreg) } } - if (!PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED) + if (!PE_COFF_LEGITIMIZE_EXTERN_DECL) return NULL_RTX; if (GET_CODE (addr) == SYMBOL_REF diff --git a/gcc/config/mingw/winnt-dll.h b/gcc/config/mingw/winnt-dll.h index 14ca743b69f..659d9c16b7c 100644 --- a/gcc/config/mingw/winnt-dll.h +++ b/gcc/config/mingw/winnt-dll.h @@ -22,6 +22,7 @@ http://www.gnu.org/licenses/. */ #ifndef USED_FOR_TARGET extern bool is_imported_p (rtx x); +extern alias_set_type ix86_GOT_alias_set (void); extern alias_set_type mingw_GOT_alias_set (void); extern rtx legitimize_pe_coff_symbol (rtx addr, bool inreg);
Thank you for reporting the issues and discussing the root causes. It helped in preparing the patch. This patch fixes 3 bugs reported after merging the "Add DLL import/export implementation to AArch64" series. https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html The series refactors the i386 codebase to reuse it in AArch64, which triggers some bugs. Bug 115661 - [15 Regression] wrong code at -O{2,3} on x86_64-linux-gnu since r15-1599-g63512c72df09b4 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661 Bug 115635 - [15 regression] Bootstrap fails with failed self-test with the rust fe (diagnostic-path.cc:1153: test_empty_path: FAIL: ASSERT_FALSE ((path.interprocedural_p ()))) since r15-1599-g63512c72df09b4 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635 Issue 1. In some code, i386 has been relying on the legitimize_pe_coff_symbol call on all platforms and should return NULL_RTX if it is not supported. Fix: NULL_RTX handling has been added when the target does not support PECOFF. Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be extracted to mingw. Fix: ix86_GOT_alias_set has been returned as it was and is used on all platforms for i386. Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks x86_64-w64-mingw32 build cannot represent relocation type BFD_RELOC_64 since r15-1602-ged20feebd9ea31 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643 Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used with a negative operator for a complex expression without braces. Fix: Braces has been added, and PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to PE_COFF_LEGITIMIZE_EXTERN_DECL. The patch has been attached as a text file because it contains special characters that are usually removed by the mail client. Regards, Evgeny From 6b1448e9ba651657dccdb41590a51a7c3c371e50 Mon Sep 17 00:00:00 2001 From: Evgeny Karpov <eukarpov@gmail.com> Date: Wed, 26 Jun 2024 22:10:55 +0200 Subject: [PATCH] i386: Fix regression after refactoring legitimize_pe_coff_symbol, ix86_GOT_alias_set and PE_COFF_LEGITIMIZE_EXTERN_DECL This patch fixes 3 bugs reported after merging the "Add DLL import/export implementation to AArch64" series. https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653955.html The series refactors the i386 codebase to reuse it in AArch64, which triggers some bugs. Bug 115661 - [15 Regression] wrong code at -O{2,3} on x86_64-linux-gnu since r15-1599-g63512c72df09b4 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115661 Bug 115635 - [15 regression] Bootstrap fails with failed self-test with the rust fe (diagnostic-path.cc:1153: test_empty_path: FAIL: ASSERT_FALSE ((path.interprocedural_p ()))) since r15-1599-g63512c72df09b4 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635 Issue 1. In some code, i386 has been relying on the legitimize_pe_coff_symbol call on all platforms and should return NULL_RTX if it is not supported. Fix: NULL_RTX handling has been added when the target does not support PECOFF. Issue 2. ix86_GOT_alias_set is used on all platforms and cannot be extracted to mingw. Fix: ix86_GOT_alias_set has been returned as it was and is used on all platforms for i386. Bug 115643 - [15 regression] aarch64-w64-mingw32 support today breaks x86_64-w64-mingw32 build cannot represent relocation type BFD_RELOC_64 since r15-1602-ged20feebd9ea31 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115643 Issue 3. PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been added and used with a negative operator for a complex expression without braces. Fix: Braces has been added, and PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED has been renamed to PE_COFF_LEGITIMIZE_EXTERN_DECL. gcc/ChangeLog: * config/aarch64/cygming.h (PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED): Rename to PE_COFF_LEGITIMIZE_EXTERN_DECL. (PE_COFF_LEGITIMIZE_EXTERN_DECL): Likewise. * config/i386/cygming.h (GOT_ALIAS_SET): Remove the diffinition to reuse it from i386.h. (PE_COFF_EXTERN_DECL_SHOULD_BE_LEGITIMIZED): Rename to PE_COFF_LEGITIMIZE_EXTERN_DECL. (PE_COFF_LEGITIMIZE_EXTERN_DECL): Likewise. * config/i386/i386-expand.cc (ix86_expand_move): Return ix86_GOT_alias_set. * config/i386/i386-expand.h (ix86_GOT_alias_set): Likewise. * config/i386/i386.cc (ix86_GOT_alias_set): Likewise. * config/i386/i386.h (GOT_ALIAS_SET): Likewise. * config/mingw/winnt-dll.cc (get_dllimport_decl): Use GOT_ALIAS_SET. (legitimize_pe_coff_symbol): Rename to PE_COFF_LEGITIMIZE_EXTERN_DECL. * config/mingw/winnt-dll.h (ix86_GOT_alias_set): Declare ix86_GOT_alias_set. --- gcc/config/aarch64/cygming.h | 2 +- gcc/config/i386/cygming.h | 7 ++----- gcc/config/i386/i386-expand.cc | 5 ++++- gcc/config/i386/i386-expand.h | 1 + gcc/config/i386/i386.cc | 11 +++++++++++ gcc/config/i386/i386.h | 2 +- gcc/config/mingw/winnt-dll.cc | 4 ++-- gcc/config/mingw/winnt-dll.h | 1 + 8 files changed, 23 insertions(+), 10 deletions(-)