* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro [not found] <003e01d04179$ccc38bc0$664aa340$@rt-rk.com> @ 2015-02-05 20:51 ` Matthew Fortune 2015-02-06 10:43 ` Maciej W. Rozycki 2015-02-06 15:12 ` Moore, Catherine 0 siblings, 2 replies; 19+ messages in thread From: Matthew Fortune @ 2015-02-05 20:51 UTC (permalink / raw) To: Petar Jovanovic, gcc-patches, 'Maciej W. Rozycki' Cc: Moore, Catherine (Catherine_Moore@mentor.com) Hi Petar, I've put your patch inline below and switched to plain text. I suspect your post was bounced by gcc-patches. I'm OK with this change but I'd like Catherine to comment before committing. It seems a shame to duplicate the block of code but it is probably just as ugly to define a macro for the la/dla instruction. For future reference, it is best not to include changelog content in a patch but instead just paste into the email. Also the ChangeLog you need for this change is gcc/ChangeLog (as confusing as that may be at first). Thanks, Matthew > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > Sent: 05 February 2015 19:28 > To: gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki'; Matthew Fortune > Subject: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > v2: > - add ChangeLog entry > - use DLA instead of LA for n64 > > PTAL. Thanks. > > Regards, > Petar --- ChangeLog | 5 +++++ gcc/config/mips/mips.h | 23 +++++++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c61c66..3a15f4a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-02-05 Petar Jovanovic <petar.jovanovic@rt-rk.com> + + * config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Fix the macro to use + la/jalr instead of jal. + 2015-02-02 Janis Johnson <janis.marie.johnson@gmail.com> * MAINTAINERS (Various Maintainers: testsuite): Remove myself. diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index ec69ed5..4bd83f5 100644 --- a/gcc/config/mips/mips.h +++ b/gcc/config/mips/mips.h @@ -3034,11 +3034,11 @@ while (0) nop\n\ 1: .cpload $31\n\ .set reorder\n\ - jal " USER_LABEL_PREFIX #FUNC "\n\ + la $25, " USER_LABEL_PREFIX #FUNC "\n\ + jalr $25\n\ .set pop\n\ " TEXT_SECTION_ASM_OP); -#elif ((defined _ABIN32 && _MIPS_SIM == _ABIN32) \ - || (defined _ABI64 && _MIPS_SIM == _ABI64)) +#elif (defined _ABIN32 && _MIPS_SIM == _ABIN32) #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC) \ asm (SECTION_OP "\n\ .set push\n\ @@ -3048,7 +3048,22 @@ while (0) nop\n\ 1: .set reorder\n\ .cpsetup $31, $2, 1b\n\ - jal " USER_LABEL_PREFIX #FUNC "\n\ + la $25, " USER_LABEL_PREFIX #FUNC "\n\ + jalr $25\n\ + .set pop\n\ + " TEXT_SECTION_ASM_OP); +#elif (defined _ABI64 && _MIPS_SIM == _ABI64) +#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC) \ + asm (SECTION_OP "\n\ + .set push\n\ + .set nomips16\n\ + .set noreorder\n\ + bal 1f\n\ + nop\n\ +1: .set reorder\n\ + .cpsetup $31, $2, 1b\n\ + dla $25, " USER_LABEL_PREFIX #FUNC "\n\ + jalr $25\n\ .set pop\n\ " TEXT_SECTION_ASM_OP); #endif -- 1.8.2.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-05 20:51 ` [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro Matthew Fortune @ 2015-02-06 10:43 ` Maciej W. Rozycki 2015-02-06 10:57 ` Matthew Fortune 2015-02-06 15:12 ` Moore, Catherine 1 sibling, 1 reply; 19+ messages in thread From: Maciej W. Rozycki @ 2015-02-06 10:43 UTC (permalink / raw) To: Matthew Fortune Cc: Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) On Thu, 5 Feb 2015, Matthew Fortune wrote: > I'm OK with this change but I'd like Catherine to comment before committing. > It seems a shame to duplicate the block of code but it is probably just as > ugly to define a macro for the la/dla instruction. Native systems have <sys/asm.h> for such ABI dependencies, including stuff to set up $gp. Perhaps we could reuse these bits, the licence I think allows us to. Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 10:43 ` Maciej W. Rozycki @ 2015-02-06 10:57 ` Matthew Fortune 2015-02-06 12:23 ` Maciej W. Rozycki 0 siblings, 1 reply; 19+ messages in thread From: Matthew Fortune @ 2015-02-06 10:57 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) Maciej W. Rozycki <macro@linux-mips.org> writes: > On Thu, 5 Feb 2015, Matthew Fortune wrote: > > > I'm OK with this change but I'd like Catherine to comment before > committing. > > It seems a shame to duplicate the block of code but it is probably just > as > > ugly to define a macro for the la/dla instruction. > > Native systems have <sys/asm.h> for such ABI dependencies, including > stuff to set up $gp. Perhaps we could reuse these bits, the licence I > think allows us to. That's a good idea. Perhaps I should take that on as part of some cleanup of the MIPS backend in the next stage1. I'm looking to rework how the ISA_HAS logic works so perhaps there would be value in doing this mostly in a header that can also be used for assembly programmers. That would naturally mean we get all the other nice assembly macros available in the backend of GCC too. Thanks, Matthew ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 10:57 ` Matthew Fortune @ 2015-02-06 12:23 ` Maciej W. Rozycki 2015-02-06 17:27 ` Mike Stump 0 siblings, 1 reply; 19+ messages in thread From: Maciej W. Rozycki @ 2015-02-06 12:23 UTC (permalink / raw) To: Matthew Fortune Cc: Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) On Fri, 6 Feb 2015, Matthew Fortune wrote: > > Native systems have <sys/asm.h> for such ABI dependencies, including > > stuff to set up $gp. Perhaps we could reuse these bits, the licence I > > think allows us to. > > That's a good idea. Perhaps I should take that on as part of some cleanup > of the MIPS backend in the next stage1. I'm looking to rework how the > ISA_HAS logic works so perhaps there would be value in doing this mostly in > a header that can also be used for assembly programmers. That would naturally > mean we get all the other nice assembly macros available in the backend of > GCC too. This consideration made me realise I've had a patch outstanding for some 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, x'. This has always been a good idea in case implementations recognised the special case and avoided involving branch prediction, and I believe it has become even more apparent with r6 calling it NAL. I'll see if I can submit it to glibc soon -- as you may have been aware 10 years ago wasn't exactly the most friendly period in glibc maintenance and hence I wasn't very prompt with patch submissions. Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 12:23 ` Maciej W. Rozycki @ 2015-02-06 17:27 ` Mike Stump 2015-02-06 17:41 ` Matthew Fortune 2015-02-06 17:46 ` Maciej W. Rozycki 0 siblings, 2 replies; 19+ messages in thread From: Mike Stump @ 2015-02-06 17:27 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Matthew Fortune, Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) On Feb 6, 2015, at 4:23 AM, Maciej W. Rozycki <macro@linux-mips.org> wrote: > This consideration made me realise I've had a patch outstanding for some > 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, x'. > This has always been a good idea in case implementations recognised the > special case and avoided involving branch prediction, and I believe it has > become even more apparent with r6 calling it NAL. Ick, no. ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 17:27 ` Mike Stump @ 2015-02-06 17:41 ` Matthew Fortune 2015-02-06 18:03 ` Mike Stump 2015-02-06 17:46 ` Maciej W. Rozycki 1 sibling, 1 reply; 19+ messages in thread From: Matthew Fortune @ 2015-02-06 17:41 UTC (permalink / raw) To: Mike Stump, Maciej W. Rozycki Cc: Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) Mike Stump <mikestump@comcast.net> writes: > On Feb 6, 2015, at 4:23 AM, Maciej W. Rozycki <macro@linux-mips.org> > wrote: > > This consideration made me realise I've had a patch outstanding for > > some > > 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, > x'. > > This has always been a good idea in case implementations recognised > > the special case and avoided involving branch prediction, and I > > believe it has become even more apparent with r6 calling it NAL. > > Ick, no. What part of this are you referring to? NAL (bizarre name or not) is the least intrusive way to obtain the PC on MIPS <= R5. The use of BAL for this, albeit common, has a high risk of affecting hardware optimisations like return predictors by introducing a call that will never return. This is a change that I am also planning to propagate to as many projects as possible. If you can see a problem with using BLTZAL for this purpose please could you explain as it may have been overlooked? Thanks, Matthew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 17:41 ` Matthew Fortune @ 2015-02-06 18:03 ` Mike Stump 2015-02-06 19:19 ` Maciej W. Rozycki 0 siblings, 1 reply; 19+ messages in thread From: Mike Stump @ 2015-02-06 18:03 UTC (permalink / raw) To: Matthew Fortune Cc: Maciej W. Rozycki, Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) On Feb 6, 2015, at 9:41 AM, Matthew Fortune <Matthew.Fortune@imgtec.com> wrote: > Mike Stump <mikestump@comcast.net> writes: >> On Feb 6, 2015, at 4:23 AM, Maciej W. Rozycki <macro@linux-mips.org> >> wrote: >>> This consideration made me realise I've had a patch outstanding for >>> some >>> 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, >> x'. >>> This has always been a good idea in case implementations recognised >>> the special case and avoided involving branch prediction, and I >>> believe it has become even more apparent with r6 calling it NAL. >> >> Ick, no. > > What part of this are you referring to? The first part. Ah, yes, I had mentally flipped the two cases. I mistakingly thought you wanted to change all the BALs to BLTZAL, which you don’t want to do. You only want to flip the non-calls to that form, which is perfectly reasonable. Personally, the call form of bal in my book should be called call, and the non-call form of it should be called bal, but, I realize it is likely to late to do much about now. If one went down this path, then even changing it away from bal is wrong. That’s the basis of why I fired up the first email. Maybe more a lament at this point. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 18:03 ` Mike Stump @ 2015-02-06 19:19 ` Maciej W. Rozycki 0 siblings, 0 replies; 19+ messages in thread From: Maciej W. Rozycki @ 2015-02-06 19:19 UTC (permalink / raw) To: Mike Stump Cc: Matthew Fortune, Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) On Fri, 6 Feb 2015, Mike Stump wrote: > Personally, the call form of bal in my book should be called call, and > the non-call form of it should be called bal, but, I realize it is > likely to late to do much about now. If one went down this path, then > even changing it away from bal is wrong. ThatÂ’s the basis of why I > fired up the first email. Maybe more a lament at this point. I'm confused, these all are conditional short-distance calls. And I suspect that they were included in the original MIPS architecture for the very purpose of letting relocatable code find its own position in memory at the run time -- and the rest was added to maximise the benefit of the opcode(s) that had to be there anyway. And it's just, owing to how the architecture has been structured, that there are exactly two cases (of the 64 encodings available) where the condition is fixed to either `true' or `false', respectively. And if anyhow, I'd call any assembly-language idiom of `BLTZAL $0, foo' just `MOVE $31, $pc' or maybe to avoid confusion `[D]ADDIU $31, $pc, 8'. Note that `foo' is never used in that operation (and that the MIPS16 and microMIPS, and now also r6 instruction sets actually have such direct operations that have a wider range of immediates available; although care has to be taken about them as they may have unusual alignment or other restrictions). Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 17:27 ` Mike Stump 2015-02-06 17:41 ` Matthew Fortune @ 2015-02-06 17:46 ` Maciej W. Rozycki 1 sibling, 0 replies; 19+ messages in thread From: Maciej W. Rozycki @ 2015-02-06 17:46 UTC (permalink / raw) To: Mike Stump Cc: Matthew Fortune, Petar Jovanovic, gcc-patches, Moore, Catherine (Catherine_Moore@mentor.com) On Fri, 6 Feb 2015, Mike Stump wrote: > > This consideration made me realise I've had a patch outstanding for some > > 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, x'. > > This has always been a good idea in case implementations recognised the > > special case and avoided involving branch prediction, and I believe it has > > become even more apparent with r6 calling it NAL. > > Ick, no. Hmm, have you hit `send' too quickly by any chance? There's surely a further part missing from your post where you'd explain what you mean. Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-05 20:51 ` [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro Matthew Fortune 2015-02-06 10:43 ` Maciej W. Rozycki @ 2015-02-06 15:12 ` Moore, Catherine 2015-02-13 0:28 ` Petar Jovanovic 1 sibling, 1 reply; 19+ messages in thread From: Moore, Catherine @ 2015-02-06 15:12 UTC (permalink / raw) To: Matthew Fortune, Petar Jovanovic, gcc-patches, 'Maciej W. Rozycki' > -----Original Message----- > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com] > Sent: Thursday, February 05, 2015 3:52 PM > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > > I've put your patch inline below and switched to plain text. I suspect your > post was bounced by gcc-patches. > > I'm OK with this change but I'd like Catherine to comment before committing. > It seems a shame to duplicate the block of code but it is probably just as ugly > to define a macro for the la/dla instruction. The patch itself is OK, although I agree with other's comments about the unfortunate duplication of code. Petar, would you please add your testcase to the gcc testsuite and repost the patch. Thanks, Catherine > > For future reference, it is best not to include changelog content in a patch > but instead just paste into the email. Also the ChangeLog you need for this > change is gcc/ChangeLog (as confusing as that may be at first). > > Thanks, > Matthew > > > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > > Sent: 05 February 2015 19:28 > > To: gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki'; Matthew Fortune > > Subject: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > > > v2: > > - add ChangeLog entry > > - use DLA instead of LA for n64 > > > > PTAL. Thanks. > > > > Regards, > > Petar > > --- > ChangeLog | 5 +++++ > gcc/config/mips/mips.h | 23 +++++++++++++++++++---- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 5c61c66..3a15f4a 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,8 @@ > +2015-02-05 Petar Jovanovic <petar.jovanovic@rt-rk.com> > + > + * config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Fix the macro > to use > + la/jalr instead of jal. > + > 2015-02-02 Janis Johnson <janis.marie.johnson@gmail.com> > > * MAINTAINERS (Various Maintainers: testsuite): Remove myself. > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index > ec69ed5..4bd83f5 100644 > --- a/gcc/config/mips/mips.h > +++ b/gcc/config/mips/mips.h > @@ -3034,11 +3034,11 @@ while (0) > nop\n\ > 1: .cpload $31\n\ > .set reorder\n\ > - jal " USER_LABEL_PREFIX #FUNC "\n\ > + la $25, " USER_LABEL_PREFIX #FUNC "\n\ > + jalr $25\n\ > .set pop\n\ > " TEXT_SECTION_ASM_OP); > -#elif ((defined _ABIN32 && _MIPS_SIM == _ABIN32) \ > - || (defined _ABI64 && _MIPS_SIM == _ABI64)) > +#elif (defined _ABIN32 && _MIPS_SIM == _ABIN32) > #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC) \ > asm (SECTION_OP "\n\ > .set push\n\ > @@ -3048,7 +3048,22 @@ while (0) > nop\n\ > 1: .set reorder\n\ > .cpsetup $31, $2, 1b\n\ > - jal " USER_LABEL_PREFIX #FUNC "\n\ > + la $25, " USER_LABEL_PREFIX #FUNC "\n\ > + jalr $25\n\ > + .set pop\n\ > + " TEXT_SECTION_ASM_OP); > +#elif (defined _ABI64 && _MIPS_SIM == _ABI64) > +#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC) \ > + asm (SECTION_OP "\n\ > + .set push\n\ > + .set nomips16\n\ > + .set noreorder\n\ > + bal 1f\n\ > + nop\n\ > +1: .set reorder\n\ > + .cpsetup $31, $2, 1b\n\ > + dla $25, " USER_LABEL_PREFIX #FUNC "\n\ > + jalr $25\n\ > .set pop\n\ > " TEXT_SECTION_ASM_OP); > #endif > -- > 1.8.2.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-06 15:12 ` Moore, Catherine @ 2015-02-13 0:28 ` Petar Jovanovic 2015-02-13 1:36 ` Moore, Catherine 0 siblings, 1 reply; 19+ messages in thread From: Petar Jovanovic @ 2015-02-13 0:28 UTC (permalink / raw) To: 'Moore, Catherine', 'Matthew Fortune', gcc-patches, 'Maciej W. Rozycki' -----Original Message----- From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] Sent: Friday, February 6, 2015 4:13 PM To: Matthew Fortune; Petar Jovanovic; gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki' Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > Petar, would you please add your testcase to the gcc testsuite and repost the patch. Hi Catherine, Sure, I can repost the patch and add the test case I used. My concern - and I would appreciate feedback from you and others on it - is that the test I originally used to report [1] the problem creates a large executable (380 MB). Here is a variant of it: /* { dg-do run } */ #include <stdio.h> #if defined(__GLIBC__) && (__GLIBC__ < 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 21)) #define BROKEN #endif int main (void) { #ifndef BROKEN asm(".fill 100000000, 4, 0x00000000\n"); #endif return 0; } The test also takes time to execute. Obviously, I can make it slightly more complex and make execute time short. Another path is to introduce and use options such as "Wl,-Ttext-segment" to make its size smaller. Any other ideas? Regards, Petar [1] https://sourceware.org/bugzilla/show_bug.cgi?id=17601 ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-13 0:28 ` Petar Jovanovic @ 2015-02-13 1:36 ` Moore, Catherine 2015-04-16 16:53 ` Petar Jovanovic 0 siblings, 1 reply; 19+ messages in thread From: Moore, Catherine @ 2015-02-13 1:36 UTC (permalink / raw) To: Petar Jovanovic, 'Matthew Fortune', gcc-patches, 'Maciej W. Rozycki' Cc: Moore, Catherine > -----Original Message----- > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > Sent: Thursday, February 12, 2015 7:28 PM > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > -----Original Message----- > From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] > Sent: Friday, February 6, 2015 4:13 PM > To: Matthew Fortune; Petar Jovanovic; gcc-patches@gcc.gnu.org; 'Maciej W. > Rozycki' > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > > > Petar, would you please add your testcase to the gcc testsuite and repost > the patch. > > Hi Catherine, > > Sure, I can repost the patch and add the test case I used. > My concern - and I would appreciate feedback from you and others on it - is > that the test I originally used to report [1] the problem creates a large > executable (380 MB). > > > The test also takes time to execute. Obviously, I can make it slightly more > complex and make execute time short. Another path is to introduce and use > options such as "Wl,-Ttext-segment" to make its size smaller. Any other > ideas? > Hi Petar, There isn't any need to execute a large testcase. Instead, try adding a short version of your test to the directory gcc/testsuite/gcc.target/mips. There are examples of other tests there they check for specific assembler sequences by using scan-assembler. See umips-swp-1.c (and others) for a specific example of how to do this. Let me know if you need additional help. Thanks, Catherine ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-02-13 1:36 ` Moore, Catherine @ 2015-04-16 16:53 ` Petar Jovanovic 2015-04-16 18:23 ` Maciej W. Rozycki 0 siblings, 1 reply; 19+ messages in thread From: Petar Jovanovic @ 2015-04-16 16:53 UTC (permalink / raw) To: 'Moore, Catherine', 'Matthew Fortune', gcc-patches, 'Maciej W. Rozycki' -----Original Message----- From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] Sent: Friday, February 13, 2015 2:37 AM To: Petar Jovanovic; 'Matthew Fortune'; gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki' Cc: Moore, Catherine Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > Hi Petar, > > There isn't any need to execute a large testcase. Instead, try adding a short version of your test to the directory gcc/testsuite/gcc.target/mips. > There are examples of other tests there they check for specific assembler sequences by using scan-assembler. See umips-swp-1.c (and others) for a specific example of how to do this. > Let me know if you need additional help. > Thanks, > Catherine Hi Catherine, Sorry for late response, I have missed to follow up on this. IIUC, scan-assembler will scan assembly file generated from a test file. This particular change will - on the other hand - modify crtend.o and thus init section. Is there a 'scan-assembler' functionality over a final linked executable, as that would actually test the change? Let me know. Thanks. Regards, Petar ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-04-16 16:53 ` Petar Jovanovic @ 2015-04-16 18:23 ` Maciej W. Rozycki 2015-04-16 20:38 ` Moore, Catherine 0 siblings, 1 reply; 19+ messages in thread From: Maciej W. Rozycki @ 2015-04-16 18:23 UTC (permalink / raw) To: Petar Jovanovic Cc: 'Moore, Catherine', 'Matthew Fortune', gcc-patches On Thu, 16 Apr 2015, Petar Jovanovic wrote: > > There isn't any need to execute a large testcase. Instead, try adding a > short version of your test to the directory gcc/testsuite/gcc.target/mips. > > There are examples of other tests there they check for specific assembler > sequences by using scan-assembler. See umips-swp-1.c (and others) for a > specific example of how to do this. > > Let me know if you need additional help. > > Thanks, > > Catherine > > Hi Catherine, > > Sorry for late response, I have missed to follow up on this. > IIUC, scan-assembler will scan assembly file generated from a test file. > This particular change will - on the other hand - modify crtend.o and thus > init section. Is there a 'scan-assembler' functionality over a final linked > executable, as that would actually test the change? You'd need `objdump' for doing that and there is no ready-to-use solution within the GCC test suite available, you'd have to cook something up yourself, perhaps starting with `[find_binutils_prog objdump]'. Another solution might be using an auxiliary linker script (`-Wl,-T,...' or maybe just an implicit linker script will do; see the LD manual for details) to place the sections the jump is made between apart manually at addresses appropriate for JAL to fail. They span does not have to be large -- when placed correctly, even `JAL .' can jump to the wrong place, which is what LD is supposed to catch as a relocation error -- so a test executable successfully linked with your correction in place won't be large, it doesn't have to take more than 2 virtual pages. The downside of the latter solution are possible portability issues with the linker script. You won't have to run your executable AFAICT from glibc BZ #17601 as you'll get a link error if a jump target is out of range. So you could make it a mere link test, no need to run it. Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-04-16 18:23 ` Maciej W. Rozycki @ 2015-04-16 20:38 ` Moore, Catherine 0 siblings, 0 replies; 19+ messages in thread From: Moore, Catherine @ 2015-04-16 20:38 UTC (permalink / raw) To: Maciej W. Rozycki, Petar Jovanovic; +Cc: 'Matthew Fortune', gcc-patches > -----Original Message----- > From: Maciej W. Rozycki [mailto:macro@linux-mips.org] > Sent: Thursday, April 16, 2015 2:23 PM > To: Petar Jovanovic > Cc: Moore, Catherine; 'Matthew Fortune'; gcc-patches@gcc.gnu.org > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > On Thu, 16 Apr 2015, Petar Jovanovic wrote: > > > > There isn't any need to execute a large testcase. Instead, try > > > adding a > > short version of your test to the directory gcc/testsuite/gcc.target/mips. > > > There are examples of other tests there they check for specific > > > assembler > > sequences by using scan-assembler. See umips-swp-1.c (and others) for a > > specific example of how to do this. > > > Let me know if you need additional help. > > > Thanks, > > > Catherine > > > > Hi Catherine, > > > > Sorry for late response, I have missed to follow up on this. > > IIUC, scan-assembler will scan assembly file generated from a test file. > > This particular change will - on the other hand - modify crtend.o and > > thus init section. Is there a 'scan-assembler' functionality over a > > final linked executable, as that would actually test the change? > Hi Petar, It looks like I answered a little too quickly the first time around. I'm sorry. In any case, Maciej is correct. I think a link-time test should do it. Thanks, Catherine > You'd need `objdump' for doing that and there is no ready-to-use solution > within the GCC test suite available, you'd have to cook something up > yourself, perhaps starting with `[find_binutils_prog objdump]'. > > Another solution might be using an auxiliary linker script (`-Wl,-T,...' > or maybe just an implicit linker script will do; see the LD manual for > details) to place the sections the jump is made between apart manually at > addresses appropriate for JAL to fail. They span does not have to be large -- > when placed correctly, even `JAL .' can jump to the wrong place, which is > what LD is supposed to catch as a relocation error -- so a test executable > successfully linked with your correction in place won't be large, it doesn't > have to take more than 2 virtual pages. > > The downside of the latter solution are possible portability issues with the > linker script. You won't have to run your executable AFAICT from glibc BZ > #17601 as you'll get a link error if a jump target is out of range. So you could > make it a mere link test, no need to run it. > > Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <6a22-55314b00-5-5cf51280@159592552>]
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro [not found] <6a22-55314b00-5-5cf51280@159592552> @ 2015-04-17 18:23 ` Petar Jovanovic 2015-04-17 18:35 ` Moore, Catherine 2015-04-17 19:36 ` Maciej W. Rozycki 1 sibling, 1 reply; 19+ messages in thread From: Petar Jovanovic @ 2015-04-17 18:23 UTC (permalink / raw) To: Petar Jovanovic Cc: Maciej W. Rozycki, Matthew Fortune, gcc-patches, Moore, Catherine Resending the previous message in a plain text format. > -------- Original Message -------- > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > Date: Thursday, April 16, 2015 10:38 PM CEST > From: "Moore, Catherine" <Catherine_Moore@mentor.com> > To: "Maciej W. Rozycki" <macro@linux-mips.org>, Petar Jovanovic<petar.jovanovic@rt-rk.com> > CC: 'Matthew Fortune' <Matthew.Fortune@imgtec.com>, "gcc-patches@gcc.gnu.org"<gcc-patches@gcc.gnu.org> > References: <003e01d04179$ccc38bc0$664aa340$@rt-rk.com><6D39441BF12EF246A7ABCE6654B0235320FCA3F1@LEMAIL01.le.imgtec.org><000201d04723$f69b1350$e3d139f0$@rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com> > Hi Petar, > It looks like I answered a little too quickly the first time around. I'm sorry. > In any case, Maciej is correct. I think a link-time test should do it. > Thanks, > Catherine > > > You'd need `objdump' for doing that and there is no ready-to-use solution > > within the GCC test suite available, you'd have to cook something up > > yourself, perhaps starting with `[find_binutils_prog objdump]'. > > > > Another solution might be using an auxiliary linker script (`-Wl,-T,...' > > or maybe just an implicit linker script will do; see the LD manual for > > details) to place the sections the jump is made between apart manually at > > addresses appropriate for JAL to fail. They span does not have to be large -- > > when placed correctly, even `JAL .' can jump to the wrong place, which is > > what LD is supposed to catch as a relocation error -- so a test executable > > successfully linked with your correction in place won't be large, it doesn't > > have to take more than 2 virtual pages. > > > > The downside of the latter solution are possible portability issues with the > > linker script. You won't have to run your executable AFAICT from glibc BZ > > #17601 as you'll get a link error if a jump target is out of range. So you could > > make it a mere link test, no need to run it. > > > > Maciej > > Hi Maciej, Catherine, This issue will not trigger a linker error (I believe it treats the symbol referred by the relocation as a local symbol). This is a follow up to GLIBC BZ #17601, the problem is seen only at runtime. So, I think this brings back the need to run the test. I can look into separating sections with -Wl,-T.. (that may also require extending mips_option_groups in mips/mips.exp), if running executable is OK. Regards, Petar ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-04-17 18:23 ` Petar Jovanovic @ 2015-04-17 18:35 ` Moore, Catherine 2015-04-21 18:05 ` Petar Jovanovic 0 siblings, 1 reply; 19+ messages in thread From: Moore, Catherine @ 2015-04-17 18:35 UTC (permalink / raw) To: Petar Jovanovic; +Cc: Maciej W. Rozycki, Matthew Fortune, gcc-patches > -----Original Message----- > From: Petar Jovanovic [mailto:Petar.Jovanovic@rt-rk.com] > Sent: Friday, April 17, 2015 2:23 PM > To: Petar Jovanovic > Cc: Maciej W. Rozycki; Matthew Fortune; gcc-patches@gcc.gnu.org; Moore, > Catherine > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > Resending the previous message in a plain text format. > > > -------- Original Message -------- > > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > Date: Thursday, April 16, 2015 10:38 PM CEST > > From: "Moore, Catherine" <Catherine_Moore@mentor.com> > > To: "Maciej W. Rozycki" <macro@linux-mips.org>, Petar > > Jovanovic<petar.jovanovic@rt-rk.com> > > CC: 'Matthew Fortune' <Matthew.Fortune@imgtec.com>, > > "gcc-patches@gcc.gnu.org"<gcc-patches@gcc.gnu.org> > > References: > > <003e01d04179$ccc38bc0$664aa340$@rt- > rk.com><6D39441BF12EF246A7ABCE6654 > > > B0235320FCA3F1@LEMAIL01.le.imgtec.org><000201d04723$f69b1350$e3d13 > 9f0$ > > @rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com> > > Hi Petar, > > It looks like I answered a little too quickly the first time around. I'm sorry. > > In any case, Maciej is correct. I think a link-time test should do it. > > Thanks, > > Catherine > > > > > You'd need `objdump' for doing that and there is no ready-to-use > > > solution within the GCC test suite available, you'd have to cook > > > something up yourself, perhaps starting with `[find_binutils_prog > objdump]'. > > > > > > Another solution might be using an auxiliary linker script (`-Wl,-T,...' > > > or maybe just an implicit linker script will do; see the LD manual > > > for > > > details) to place the sections the jump is made between apart > > > manually at addresses appropriate for JAL to fail. They span does > > > not have to be large -- when placed correctly, even `JAL .' can jump > > > to the wrong place, which is what LD is supposed to catch as a > > > relocation error -- so a test executable successfully linked with > > > your correction in place won't be large, it doesn't have to take more than > 2 virtual pages. > > > > > > The downside of the latter solution are possible portability issues > > > with the linker script. You won't have to run your executable AFAICT > > > from glibc BZ > > > #17601 as you'll get a link error if a jump target is out of range. > > > So you could make it a mere link test, no need to run it. > > > > > > Maciej > > > > > > Hi Maciej, Catherine, > > This issue will not trigger a linker error (I believe it treats the symbol > referred by the relocation as a local symbol). This is a follow up to GLIBC BZ > #17601, the problem is seen only at runtime. So, I think this brings back the > need to run the test. I can look into separating sections with -Wl,-T.. (that > may also require extending mips_option_groups in mips/mips.exp), if > running executable is OK. > Hi Petar, Running the executable is fine, but didn't you say that your test case takes hours to execute? If so, that's not acceptable. Is it possible to construct a test case that will run more quickly? Thanks, Catherine ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro 2015-04-17 18:35 ` Moore, Catherine @ 2015-04-21 18:05 ` Petar Jovanovic 0 siblings, 0 replies; 19+ messages in thread From: Petar Jovanovic @ 2015-04-21 18:05 UTC (permalink / raw) To: 'Moore, Catherine' Cc: 'Maciej W. Rozycki', 'Matthew Fortune', gcc-patches -----Original Message----- From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] Sent: Friday, April 17, 2015 8:36 PM To: Petar Jovanovic Cc: Maciej W. Rozycki; Matthew Fortune; gcc-patches@gcc.gnu.org Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > Hi Petar, > Running the executable is fine, but didn't you say that your test case takes hours to execute? > If so, that's not acceptable. Is it possible to construct a test case that will run more quickly? > Thanks, > Catherine Hi Catherine, I was just raising concerns about the original example (that did run long). Anyway, I have just uploaded a different example that triggers the issue. Regards, Petar ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro [not found] <6a22-55314b00-5-5cf51280@159592552> 2015-04-17 18:23 ` Petar Jovanovic @ 2015-04-17 19:36 ` Maciej W. Rozycki 1 sibling, 0 replies; 19+ messages in thread From: Maciej W. Rozycki @ 2015-04-17 19:36 UTC (permalink / raw) To: Petar Jovanovic; +Cc: Moore, Catherine, Matthew Fortune, gcc-patches On Fri, 17 Apr 2015, Petar Jovanovic wrote: > This issue will not trigger a linker error (I believe it treats the > symbol referred by the relocation as a local symbol). This is a follow > up to GLIBC BZ #17601, the problem is seen only at runtime. So, I think > this brings back the need to run the test. I can look into separating > sections with -Wl,-T.. (that may also require extending > mips_option_groups in mips/mips.exp), if running executable is OK. If the assembler or the linker knowingly (or under conditions where it can be determined) creates a broken executable, then it is a separate bug that needs to be filed against binutils. Due to the unusual constraints of absolute MIPS jump instructions any associated symbol references have to result in external relocations, to be resolved in the final static link only. If this is not the case or such a relocation resolves successfully despite a range overflow, then this has to be fixed in binutils. Can you double-check if this is the case? I see this in a dump from crtbegin.o: Disassembly of section .init: 00000000 <.init>: 0: 04110001 bal 8 <.init+0x8> 4: 00000000 nop 8: 0c00004b jal 12c <frame_dummy> 8: R_MIPS_26 .text c: 00000000 nop -- if `.text+0x12c' is out of range for JAL (R_MIPS_26) from `.init+0xc', that is the 4 most significant bits of both final addresses are not the same (the range calculation for this instruction/relocation is relative to the delay slot), then the static link is supposed to fail. I think this can be easily verified and if needed, converted to an LD test case. As to a GCC test case I agree with Catherine that a run-time test case will be fine, and actually inevitable if the linker fails to fail. Maciej ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-04-21 18:05 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <003e01d04179$ccc38bc0$664aa340$@rt-rk.com> 2015-02-05 20:51 ` [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro Matthew Fortune 2015-02-06 10:43 ` Maciej W. Rozycki 2015-02-06 10:57 ` Matthew Fortune 2015-02-06 12:23 ` Maciej W. Rozycki 2015-02-06 17:27 ` Mike Stump 2015-02-06 17:41 ` Matthew Fortune 2015-02-06 18:03 ` Mike Stump 2015-02-06 19:19 ` Maciej W. Rozycki 2015-02-06 17:46 ` Maciej W. Rozycki 2015-02-06 15:12 ` Moore, Catherine 2015-02-13 0:28 ` Petar Jovanovic 2015-02-13 1:36 ` Moore, Catherine 2015-04-16 16:53 ` Petar Jovanovic 2015-04-16 18:23 ` Maciej W. Rozycki 2015-04-16 20:38 ` Moore, Catherine [not found] <6a22-55314b00-5-5cf51280@159592552> 2015-04-17 18:23 ` Petar Jovanovic 2015-04-17 18:35 ` Moore, Catherine 2015-04-21 18:05 ` Petar Jovanovic 2015-04-17 19:36 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).