On 2012/7/6 02:23 PM, Richard Sandiford wrote: > Richard Sandiford <rdsandif...@googlemail.com> writes: >>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit >>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32). >>> >>> As you can see in the original Feb. patch, I had changes to emit a >>> MIPS16 version of these static calls, but with the changes in (2) above, >>> they will not work with the usual situation of a 32-bit MIPS built /lib >>> (.init/.fini will have 32/16-bit code improperly concatenated). >>> >>> The CodeSourcery builds use an independent mips16 sysroot for this, so a >>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think >>> making it 32-bit is the compatible choice. >> >> Yeah, I agree that sounds like the right call. Please do the same >> for the n32/n64 version (i.e. explicitly make it nomips16 rather >> than add the #error). > > BTW, doing this has removed my main concern about having dead code. > The original patch had a separate MIPS16 implementation that (as things > stood) could never be used by stock sources. That would make it difficult > to maintain. > > Now that the MIPS16 library support is purely adding nomips16 attributes > to code that is obviously nomips16, those parts are OK on their own, thanks. > (I.e. the mips.h change, the libgcc change, and the libgomp change.) > Feel free to drop the multilib thing if you don't want to implement > --with-multilib-list.
Thanks, I'll probably commit those parts first, and maybe look a bit at the --with-multilib-list thing later. Something that came to my mind just now, is how this will mesh with micromips later... :P > __builtin_thread_pointer is logically a separate patch anyway. > In case it isn't clear, the reason I'm pushing back about the > target-dependent thing is that you're adding a fair bit of extra > code to the general MIPS built-in infrastructure in order to > handle the set of "__builtin_thread_pointer-like functions". > And my concern is that that set probably contains just one function. Well, some other architectures also have __builtin_set_thread_pointer(), as you probably also noticed. The thing is that, unlike the __builtin_return_address(), __builtin_saveregs() you mentioned, there seems no general concept of the TLS environment exposed by the backend (as opposed to things like frame-pointers, return-addr), so the machine-independent code can't properly do things at arm's length. For a gcc/builtins.c expand function for __builtin_thread_pointer(), it's probably either a target-hook or "sorry not implemented". That's probably still okay, though I'm not sure it's the intended style. > I also notice that the patch isn't marking __builtin_thread_pointer > as nothrow, whereas other targets do. Adding the function > to builtins.def would avoid that kind of accidental difference. > (For avoidance of doubt, the fact that the TP read instruction > can trap to a kernel handler doesn't make the function throwing.) Oh I missed that in this new patch, thanks for catching. FTR, I think I did properly mark it in the older patch :) Thanks, Chung-Lin
