Hi Richard, Sorry for the delay to respond, and thanks for revising and committing the changes to trunk. The revised changes look much cleaner :)
About the other concerns: > (2) is interesting if there is also a way to build those MIPS16 libraries > out of the box. I'd like such a mechanism to be added at the same time, > so that the new support is easy to test. This is still a 4.7 candidate > if it can be done in time, although it's probably a little tight. I assume you mean the libgomp patch? That's mostly a potential bug fix; as for MIP16 multilib additions, so far I haven't added any to the default mips*-linux* configs, as I guess this should be vendor-stuff (though the current additions should clear away any obstacles) > You've given the built-in function the generic name __builtin_thread_pointer. > I agree that's a good idea, but I think we should also _treat_ it as a generic > function, and make it available on all targets. The actual implementation > can be delegated to a target hook. That makes (3) 4.8 material. Actually, I named and added __builtin_thread_pointer() only because it was needed for building glibc (to avoid using 32-bit asm in MIPS16 mode), and because some other targets also have it (I'm sure ARM also has it, and at least one other) > (Incidentally, I don't think it's correct to define the builtin if TLS > isn't available, so if we did keep it as a MIPS-specific function, > d->avail would need to be nonnull. There would need to be some other > way of indicating that MIPS16 was OK.) The function is essentially a wrapper for mips_get_tp(), which I don't see any guarding for the no TLS case? FWIW, TARGET_HAVE_TLS is just defined as HAVE_AS_TLS for MIPS... >> The __mips16_rdhwr() function is not intended for general use, just >> tightly coupled compiler runtime support; therefore, it is only linked >> statically from libgcc.a, not exported from shared libgcc. > > Well, a fair bit of libgcc is tightly-coupled compiler runtime support. > I don't really see any need to handle the new function differently from > the other __mips16 wrappers. It's not like we're gaining any benefit in > the PIC call overhead: we can't turn JALRs into branches like we can for > nearby non-MIPS16-to-non-MIPS16 calls, so PIC calls will still go via > the GOT. And we're not gaining any benefit in terms of ABI baggage either. > Future libgcc(.a)s would need to continue providing the function as > specified now, in order for future gccs to be able to link library > archives built with 4.7. > > By making the function hidden, we lose the important ability to replace > all instances of it. E.g. it isn't inconceivable that someone will find > a more efficient way to implement the function in cases where the RDHWR > is emulated (perhaps on a kernel other than Linux -- this support isn't > specific to *-linux-gnu). Being able to replace all instances of a > function is useful for other things, such as profiling, debugging, > or working around processor errata. We touched this internally as well, but interestingly came to the opposite conclusion :) We're not sure the __mips16_rdhwr() is ultimately the best choice of API, plus the non-standardness of the calling convention, hence simply decided to hide it from libgcc_s.so, in case we need to change the MIPS16 TLS interface. On the issue of hiding stuff in mips16.S, it may be suitable to raise this issue here: there is a problem when MIPS16 hard-float calls go through PLTs, because the hard-float stub functions in mips16.S use v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated by BFD. (I think I described the scenario mostly correct, CCing Maciej here who worked on this issue) I think the solution was to mark a large portion of the mips16.S functions as all hidden. >> 5) The libgomp MIPS futex.h header has been adjusted; sys_futex0() has >> been modified to be static, non-inlined, nomips16 under MIPS16 mode (for >> sake of using 'syscall'). The inline assembly has also been fixed, as >> Maciej noticed a possible violation of the MIPS syscall restart >> convention; the 'li $2, #syscall_number' must be right before the >> syscall insn. > > This change is OK as part of (2). Okay thanks, I commit this part soon. Thanks, Chung-Lin
