* Reviving SH FDPIC target @ 2015-09-02 18:36 Rich Felker 2015-09-02 20:00 ` Joseph Myers 0 siblings, 1 reply; 12+ messages in thread From: Rich Felker @ 2015-09-02 18:36 UTC (permalink / raw) To: gcc-patches I've started work on reviving the FDPIC support patch for the SH target, which was proposed upstream in 2010 then abandoned: https://gcc.gnu.org/ml/gcc-patches/2010-08/msg01464.html Right now I'm in the process of determining what parts can be applied as-is to current gcc, and what parts need to be adapted or rewritten to account for changes in gcc between the 4.5 era and now (5.2/6.0). The original patch as posted contained a significant amount of changes that were unrelated to FDPIC code generation but rather for adding sh*-uclinux tuples, and some things that even look like they're associated with the old bFLT format. I am omitting these parts for now since I'm unfamiliar with the old uClinux stuff and it's unmaintained. If anyone else wants to use it, I think it would make more sense factored as a separate patch anyway. The target I have in mind is SH-2/J2 with musl libc, but uClibc or even glibc could be made to work with it. I will submit the patches for musl support (basically, just hooking up the dynamic linker name for fdpic) separately; I believe they'll need merging with the already-pending musl support patch from Szabolcs Nagy. One question I'd like to ask now in case it's a problem that takes a while to work out -- is copyright assignment already handled for the old patch? The contributors listed in it are (all codesourcery): - Daniel Jacobowitz - Joseph Myers - Mark Shinwell - Andrew Stubbs Also, according to Joseph Myers, there was some unresolved disagreement that stalled (and eventually sunk) the old patch, so if anyone's still around who has objections to it, could you speak up and let me know what's wrong? Kaz Kojima seems to have approved the patch at the time so I'm confused what the issue was/is. Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-02 18:36 Reviving SH FDPIC target Rich Felker @ 2015-09-02 20:00 ` Joseph Myers 2015-09-02 21:14 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: Joseph Myers @ 2015-09-02 20:00 UTC (permalink / raw) To: Rich Felker; +Cc: gcc-patches On Wed, 2 Sep 2015, Rich Felker wrote: > Also, according to Joseph Myers, there was some unresolved > disagreement that stalled (and eventually sunk) the old patch, so if > anyone's still around who has objections to it, could you speak up and > let me know what's wrong? Kaz Kojima seems to have approved the patch > at the time so I'm confused what the issue was/is. It's patch 1/3 (architecture-independent) that had the disagreement (and patch 3/3 depends on patch 1/3). https://gcc.gnu.org/ml/gcc-patches/2010-08/msg01462.html -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-02 20:00 ` Joseph Myers @ 2015-09-02 21:14 ` Rich Felker 2015-09-03 4:33 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: Rich Felker @ 2015-09-02 21:14 UTC (permalink / raw) To: Joseph Myers; +Cc: gcc-patches On Wed, Sep 02, 2015 at 07:59:45PM +0000, Joseph Myers wrote: > On Wed, 2 Sep 2015, Rich Felker wrote: > > > Also, according to Joseph Myers, there was some unresolved > > disagreement that stalled (and eventually sunk) the old patch, so if > > anyone's still around who has objections to it, could you speak up and > > let me know what's wrong? Kaz Kojima seems to have approved the patch > > at the time so I'm confused what the issue was/is. > > It's patch 1/3 (architecture-independent) that had the disagreement (and > patch 3/3 depends on patch 1/3). > > https://gcc.gnu.org/ml/gcc-patches/2010-08/msg01462.html So this is only for __fpscr_values? In that case I think the right solution is just to follow up with getting rid of __fpscr_values, if it's not already done: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60138 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53513 53513 is marked fixed, but I didn't follow up to confirm that the actual problems I reported in 60138 are fixed; I'll do some more research on this. But if all goes well, we can just drop 1/3. Thanks for the quick reply! Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-02 21:14 ` Rich Felker @ 2015-09-03 4:33 ` Rich Felker 2015-09-03 14:59 ` Joseph Myers 0 siblings, 1 reply; 12+ messages in thread From: Rich Felker @ 2015-09-03 4:33 UTC (permalink / raw) To: Joseph Myers; +Cc: gcc-patches On Wed, Sep 02, 2015 at 05:05:35PM -0400, Rich Felker wrote: > On Wed, Sep 02, 2015 at 07:59:45PM +0000, Joseph Myers wrote: > > On Wed, 2 Sep 2015, Rich Felker wrote: > > > > > Also, according to Joseph Myers, there was some unresolved > > > disagreement that stalled (and eventually sunk) the old patch, so if > > > anyone's still around who has objections to it, could you speak up and > > > let me know what's wrong? Kaz Kojima seems to have approved the patch > > > at the time so I'm confused what the issue was/is. > > > > It's patch 1/3 (architecture-independent) that had the disagreement (and > > patch 3/3 depends on patch 1/3). > > > > https://gcc.gnu.org/ml/gcc-patches/2010-08/msg01462.html > > So this is only for __fpscr_values? In that case I think the right > solution is just to follow up with getting rid of __fpscr_values, if > it's not already done: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60138 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53513 > > 53513 is marked fixed, but I didn't follow up to confirm that the > actual problems I reported in 60138 are fixed; I'll do some more > research on this. But if all goes well, we can just drop 1/3. I've confirmed that gcc 5.2 does not produce references to __fpscr_values; instead, it does: mov.l .L4,r3 ... sts fpscr,r1 xor r3,r1 lds r1,fpscr ... .L4: .long 524288 So if __fpscr_values was the only reason for patch 1/3 in the FDPIC patchset, I think we can safely drop it. And patch 2/3 was already committed, so 3/3, the one I was originally looking at, seems to be all we need. It was approved at the time, so I'll proceed with merging it with 5.2.0. Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-03 4:33 ` Rich Felker @ 2015-09-03 14:59 ` Joseph Myers 2015-09-03 15:59 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: Joseph Myers @ 2015-09-03 14:59 UTC (permalink / raw) To: Rich Felker; +Cc: gcc-patches On Wed, 2 Sep 2015, Rich Felker wrote: > So if __fpscr_values was the only reason for patch 1/3 in the FDPIC > patchset, I think we can safely drop it. And patch 2/3 was already > committed, so 3/3, the one I was originally looking at, seems to be > all we need. It was approved at the time, so I'll proceed with merging > it with 5.2.0. Well, obviously if trying dropping patch 1/3 you need to remove everything related to use_initial_val (the feature added in patch 1/3) from patch 3/3. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-03 14:59 ` Joseph Myers @ 2015-09-03 15:59 ` Rich Felker 2015-09-04 20:22 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: Rich Felker @ 2015-09-03 15:59 UTC (permalink / raw) To: Joseph Myers; +Cc: gcc-patches On Thu, Sep 03, 2015 at 02:58:39PM +0000, Joseph Myers wrote: > On Wed, 2 Sep 2015, Rich Felker wrote: > > > So if __fpscr_values was the only reason for patch 1/3 in the FDPIC > > patchset, I think we can safely drop it. And patch 2/3 was already > > committed, so 3/3, the one I was originally looking at, seems to be > > all we need. It was approved at the time, so I'll proceed with merging > > it with 5.2.0. > > Well, obviously if trying dropping patch 1/3 you need to remove everything > related to use_initial_val (the feature added in patch 1/3) from patch > 3/3. As far as I can tell, the only "use" of use_initial_val is defining the pseudo-instruction in the md file, which causes the code in patch 1/3 to use it. I see no other references to it. As I understand, the breakage from not having it (in the original 4.5-era patch) would be when introducing references to __fpscr_values later, and no longer having the GOT pointer, but that code is gone now. Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-03 15:59 ` Rich Felker @ 2015-09-04 20:22 ` Rich Felker 2015-09-04 23:08 ` Segher Boessenkool 2015-09-11 4:05 ` Rich Felker 0 siblings, 2 replies; 12+ messages in thread From: Rich Felker @ 2015-09-04 20:22 UTC (permalink / raw) To: Joseph Myers; +Cc: gcc-patches On Thu, Sep 03, 2015 at 11:53:45AM -0400, Rich Felker wrote: > On Thu, Sep 03, 2015 at 02:58:39PM +0000, Joseph Myers wrote: > > On Wed, 2 Sep 2015, Rich Felker wrote: > > > > > So if __fpscr_values was the only reason for patch 1/3 in the FDPIC > > > patchset, I think we can safely drop it. And patch 2/3 was already > > > committed, so 3/3, the one I was originally looking at, seems to be > > > all we need. It was approved at the time, so I'll proceed with merging > > > it with 5.2.0. > > > > Well, obviously if trying dropping patch 1/3 you need to remove everything > > related to use_initial_val (the feature added in patch 1/3) from patch > > 3/3. > > As far as I can tell, the only "use" of use_initial_val is defining > the pseudo-instruction in the md file, which causes the code in patch > 1/3 to use it. I see no other references to it. As I understand, the > breakage from not having it (in the original 4.5-era patch) would be > when introducing references to __fpscr_values later, and no longer > having the GOT pointer, but that code is gone now. I have this basically working -- obviously no heavy testing yet, and the specs glue is not sufficient to make it practical for others to try it yet, so it'll be a little longer til I have something to post. One thing I've noticed that's odd is that gcc -mfdpic -fPIC produces different (less efficient) code from just gcc -mfdpic, which seems wrong, but agrees with sh.c which has a number of checks for flag_pic not matched with a TARGET_FDPIC check. I'm thinking all of these should either be flag_pic||TARGET_PIC or flag_pic&&!TARGET_FDPIC, depending on whether the code applies to all PIC or is specific to the non-FDPIC PIC model where r12 is call-saved. Does this sound correct? I think we need spurious -fPIC to work (although it could be handled with spec magic) and not pessimize code, since most library builds will use -fPIC. Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-04 20:22 ` Rich Felker @ 2015-09-04 23:08 ` Segher Boessenkool 2015-09-05 12:37 ` Rich Felker 2015-09-13 17:11 ` Rich Felker 2015-09-11 4:05 ` Rich Felker 1 sibling, 2 replies; 12+ messages in thread From: Segher Boessenkool @ 2015-09-04 23:08 UTC (permalink / raw) To: Rich Felker; +Cc: Joseph Myers, gcc-patches On Fri, Sep 04, 2015 at 04:16:40PM -0400, Rich Felker wrote: > One thing I've noticed that's odd is that gcc -mfdpic -fPIC produces > different (less efficient) code from just gcc -mfdpic, which seems > wrong, but agrees with sh.c which has a number of checks for flag_pic > not matched with a TARGET_FDPIC check. Generic code tests flag_pic in important places as well. > I'm thinking all of these > should either be flag_pic||TARGET_PIC or flag_pic&&!TARGET_FDPIC, > depending on whether the code applies to all PIC or is specific to the > non-FDPIC PIC model where r12 is call-saved. Does this sound correct? > I think we need spurious -fPIC to work (although it could be handled > with spec magic) and not pessimize code, since most library builds > will use -fPIC. If you never want -fPIC (or -fpic) if fdpic is enabled, you can disable it (in sh_option_override)? Segher ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-04 23:08 ` Segher Boessenkool @ 2015-09-05 12:37 ` Rich Felker 2015-09-13 17:11 ` Rich Felker 1 sibling, 0 replies; 12+ messages in thread From: Rich Felker @ 2015-09-05 12:37 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Joseph Myers, gcc-patches On Fri, Sep 04, 2015 at 06:04:15PM -0500, Segher Boessenkool wrote: > On Fri, Sep 04, 2015 at 04:16:40PM -0400, Rich Felker wrote: > > One thing I've noticed that's odd is that gcc -mfdpic -fPIC produces > > different (less efficient) code from just gcc -mfdpic, which seems > > wrong, but agrees with sh.c which has a number of checks for flag_pic > > not matched with a TARGET_FDPIC check. > > Generic code tests flag_pic in important places as well. Hmm, these are probably correct: things like default TLS model, whether external functions defined in the same TU are subject to interposition, etc. So ignoring -fPIC would be incorrect, and in fact the different/less-efficient codegen might be correct. But if -fPIE is generating different code, that's probably a bug (or at least unwanted). > > I'm thinking all of these > > should either be flag_pic||TARGET_PIC or flag_pic&&!TARGET_FDPIC, > > depending on whether the code applies to all PIC or is specific to the > > non-FDPIC PIC model where r12 is call-saved. Does this sound correct? > > I think we need spurious -fPIC to work (although it could be handled > > with spec magic) and not pessimize code, since most library builds > > will use -fPIC. > > If you never want -fPIC (or -fpic) if fdpic is enabled, you can disable > it (in sh_option_override)? Even if it weren't needed for the above reasons, having it generate an error would be very problematic from a standpoint of being able to build packages unmodified. So I probably just need to have all the conditionals in sh.c right (checking either ||TARGET_FDPIC or &&!TARGET_FDPIC). Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-04 23:08 ` Segher Boessenkool 2015-09-05 12:37 ` Rich Felker @ 2015-09-13 17:11 ` Rich Felker 1 sibling, 0 replies; 12+ messages in thread From: Rich Felker @ 2015-09-13 17:11 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Joseph Myers, gcc-patches On Fri, Sep 04, 2015 at 06:04:15PM -0500, Segher Boessenkool wrote: > On Fri, Sep 04, 2015 at 04:16:40PM -0400, Rich Felker wrote: > > One thing I've noticed that's odd is that gcc -mfdpic -fPIC produces > > different (less efficient) code from just gcc -mfdpic, which seems > > wrong, but agrees with sh.c which has a number of checks for flag_pic > > not matched with a TARGET_FDPIC check. > > Generic code tests flag_pic in important places as well. > > > I'm thinking all of these > > should either be flag_pic||TARGET_PIC or flag_pic&&!TARGET_FDPIC, > > depending on whether the code applies to all PIC or is specific to the > > non-FDPIC PIC model where r12 is call-saved. Does this sound correct? > > I think we need spurious -fPIC to work (although it could be handled > > with spec magic) and not pessimize code, since most library builds > > will use -fPIC. > > If you never want -fPIC (or -fpic) if fdpic is enabled, you can disable > it (in sh_option_override)? It turns out that with !flag_pic, gcc is generating broken code and/or ICE, and this happens even after changing all the remaining flag_pic tests in sh.c to flag_pic||TARGET_FDPIC. There are a few more in sh.md I didn't try changing but they did not look relevant; the ICE came via expand_binop in prepare_move_operands. Before I look at it further, is there any reason to expect !flag_pic in the generic code to break things when the target-specific code has PIC-like constraints? For now I just made sh_option_override force flag_pic when TARGET_FDPIC is set. Note that flag_pic by itself is equivalent to -fPIE; -fPIC also sets flag_shlib which affects other things like TLS model and binds_locally interpretation. This seems like a viable solution (and I got rid of the suboptimal codegen by fixing the condition in sh_function_ok_for_sibcall so that flag_pic doesn't preclude sibcalls when TARGET_FDPIC is set) but I'd still like to figure out why gcc is breaking without flag_pic... Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-04 20:22 ` Rich Felker 2015-09-04 23:08 ` Segher Boessenkool @ 2015-09-11 4:05 ` Rich Felker 2015-09-11 8:02 ` Rich Felker 1 sibling, 1 reply; 12+ messages in thread From: Rich Felker @ 2015-09-11 4:05 UTC (permalink / raw) To: Joseph Myers; +Cc: gcc-patches On Fri, Sep 04, 2015 at 04:16:40PM -0400, Rich Felker wrote: > On Thu, Sep 03, 2015 at 11:53:45AM -0400, Rich Felker wrote: > > On Thu, Sep 03, 2015 at 02:58:39PM +0000, Joseph Myers wrote: > > > On Wed, 2 Sep 2015, Rich Felker wrote: > > > > > > > So if __fpscr_values was the only reason for patch 1/3 in the FDPIC > > > > patchset, I think we can safely drop it. And patch 2/3 was already > > > > committed, so 3/3, the one I was originally looking at, seems to be > > > > all we need. It was approved at the time, so I'll proceed with merging > > > > it with 5.2.0. > > > > > > Well, obviously if trying dropping patch 1/3 you need to remove everything > > > related to use_initial_val (the feature added in patch 1/3) from patch > > > 3/3. > > > > As far as I can tell, the only "use" of use_initial_val is defining > > the pseudo-instruction in the md file, which causes the code in patch > > 1/3 to use it. I see no other references to it. As I understand, the > > breakage from not having it (in the original 4.5-era patch) would be > > when introducing references to __fpscr_values later, and no longer > > having the GOT pointer, but that code is gone now. > > I have this basically working -- obviously no heavy testing yet, and > the specs glue is not sufficient to make it practical for others to > try it yet, so it'll be a little longer til I have something to post. I'm running into a problem with crtstuff.c: the references to __TMC_END__, etc. end up using @GOTOFF relocations which the linker then rejects. Is this a linker bug or something I need to fix in the target code to avoid generating @GOTOFF relocations? Since crtbegin.o has no information about what section __TMC_END__ will be in when it's defined, I don't see a lot of options except to avoid using @GOTOFF entirely for symbols that aren't defined in the same TU. Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Reviving SH FDPIC target 2015-09-11 4:05 ` Rich Felker @ 2015-09-11 8:02 ` Rich Felker 0 siblings, 0 replies; 12+ messages in thread From: Rich Felker @ 2015-09-11 8:02 UTC (permalink / raw) To: Joseph Myers; +Cc: gcc-patches On Thu, Sep 10, 2015 at 11:49:19PM -0400, Rich Felker wrote: > On Fri, Sep 04, 2015 at 04:16:40PM -0400, Rich Felker wrote: > > On Thu, Sep 03, 2015 at 11:53:45AM -0400, Rich Felker wrote: > > > On Thu, Sep 03, 2015 at 02:58:39PM +0000, Joseph Myers wrote: > > > > On Wed, 2 Sep 2015, Rich Felker wrote: > > > > > > > > > So if __fpscr_values was the only reason for patch 1/3 in the FDPIC > > > > > patchset, I think we can safely drop it. And patch 2/3 was already > > > > > committed, so 3/3, the one I was originally looking at, seems to be > > > > > all we need. It was approved at the time, so I'll proceed with merging > > > > > it with 5.2.0. > > > > > > > > Well, obviously if trying dropping patch 1/3 you need to remove everything > > > > related to use_initial_val (the feature added in patch 1/3) from patch > > > > 3/3. > > > > > > As far as I can tell, the only "use" of use_initial_val is defining > > > the pseudo-instruction in the md file, which causes the code in patch > > > 1/3 to use it. I see no other references to it. As I understand, the > > > breakage from not having it (in the original 4.5-era patch) would be > > > when introducing references to __fpscr_values later, and no longer > > > having the GOT pointer, but that code is gone now. > > > > I have this basically working -- obviously no heavy testing yet, and > > the specs glue is not sufficient to make it practical for others to > > try it yet, so it'll be a little longer til I have something to post. > > I'm running into a problem with crtstuff.c: the references to > __TMC_END__, etc. end up using @GOTOFF relocations which the linker > then rejects. Is this a linker bug or something I need to fix in the > target code to avoid generating @GOTOFF relocations? Since crtbegin.o > has no information about what section __TMC_END__ will be in when it's > defined, I don't see a lot of options except to avoid using @GOTOFF > entirely for symbols that aren't defined in the same TU. Well, that was easier than I thought, at least for a quick hack: I added: SYMBOL_REF_EXTERNAL_P(orig) || DECL_SECTION_NAME(SYMBOL_REF_DECL(orig)) to the list of conditions for using @GOT instead of @GOTOFF. This may actually be optimal under the constraints we have... Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-09-13 16:53 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-09-02 18:36 Reviving SH FDPIC target Rich Felker 2015-09-02 20:00 ` Joseph Myers 2015-09-02 21:14 ` Rich Felker 2015-09-03 4:33 ` Rich Felker 2015-09-03 14:59 ` Joseph Myers 2015-09-03 15:59 ` Rich Felker 2015-09-04 20:22 ` Rich Felker 2015-09-04 23:08 ` Segher Boessenkool 2015-09-05 12:37 ` Rich Felker 2015-09-13 17:11 ` Rich Felker 2015-09-11 4:05 ` Rich Felker 2015-09-11 8:02 ` Rich Felker
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).