On 6/29/23 4:13 AM, Kewen.Lin wrote: > on 2023/6/19 23:57, Carl Love wrote: >> The following patch changes the return type of the >> __builtin_set_fpscr_rn builtin from void to double. The return value >> is the current value of the various FPSCR fields DRN, VE, OE, UE, ZE, >> XE, NI, RN bit positions when the builtin is called. The builtin then >> updated the RN field with the new value provided as an argument to the >> builtin. The patch adds new testcases to test_fpscr_rn_builtin.c to >> check that the builtin returns the current value of the FPSCR fields >> and then updates the RN field. > > But this patch also introduces one more overloading instance with argument > type double, I had a check on glibc usage of mffscrn/mffscrni, I don't > think it's necessary to add this new instance, as it takes the given > rounding mode as integral type.
I agree with Ke Wen, I don't know why there is an extra overloaded instance. I assumed we'd have just the one we had before, except now its return type is double and not void. That said, we need to announce to users that the return type has changed, so new code compiled with a new GCC can get the new return value, but if that new code is compiled with an old GCC (still has void return type), it knows it needs to use some other method to get the FPSCR value, if it needs it. We normally do that with a predefine macro (#define ...) that the user can test for in their code, ala: #ifdef __SET_FPSCR_RN_RETURNS_FPSCR__ /* Or whatever name. */ ret = __builtin_set_fpscr_rn (......); #else __builtin_set_fpscr_rn (......); ret = <some other method for reading FPSCR>; #endif We add those predefined macros in rs6000-c.cc:rs6000_target_modify_macros() and it should be gated via the same conditions that the built-in itself is enabled. Look at my addition of the __TM_FENCE__ macro that let the user know our HTM insn patterns were fixed to now act as memory barriers. Peter
