On Tue, 22 May 2018, Michael Meissner wrote: > I posted this patch at the end of GCC 8 as a RFC. Now that we are in GCC 9, I > would like to repose it. Sorry to spam some of you. It is unclear whom the > reviewers for things like target hooks and basic mode handling are. > > Here is the original patch. > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00764.html > > PowerPC has 3 different 128-bit floating point types (KFmode, IFmode, and > TFmode). We are in the process of migrating long double from IBM extended > double to IEEE 128-bit floating point. > > * IFmode is IBM extended double (__ibm128) > * KFmode is IEEE 128-bit floating point (__float128, _Float128N) > * TFmode is whatever long double maps to > > If we are compiling for a power8 system (which does not have hardware IEEE > 128-bit floating point), the current system works because each of the 3 modes > do not have hardware support. > > If we are compiling for a power9 system and long double is IBM extended > double, > again things are fine. > > However, if we compiling for power9 and we've flipped the default for long > double to be IEEE 128-bit floating point, then the code to support __ibm128 > breaks. The machine independent portions of the mode handling says oh, there > is hardware to support TFmode operations, lets widen the type to TFmode and do > those operations. However, converting IFmode to TFmode is not cheap, it has > to > be done in a function call. > > This patch adds a new target hook, that if it is overriden, the backend can > say > don't automatically widen this type to that type. The PowerPC port defines > the > target hook so that it doesn't automatically convert IBM extended double to > IEEE 128-bit and vice versa. > > This patch goes through all of the places that calls GET_MODE_WIDER_MODE and > then calls the target hook. Now, the PowerPC only needs to block certain > floating point widenings. Several of the changes are to integer widenings, > and > if desired, we could restrict the changes to just floating point types. > However, there might be other ports that need the flexibility for other types. > > I have tried various other approprches to fix this problem, and so far, I have > not been able to come up with a PowerPC back-end only solution that works. > > Alternatively, Segher has suggested that the call to the target hook be in > GET_MODE_WIDER_MODE and GET_MODE_2XWIDER_MODE (plus any places where we access > the mode_wider array direcly). > > I have built little endian PowerPC builds with this patch, and I have verified > that it does work. I have tested the same patch in April on a big endian > PowerPC system and x86_64 and it worked there also. > > Can I check in this patch as is (I will verify x86/PowerPC big endian still > works before checkin). Or would people prefer modifications to the patch?
Just a question for clarification - _is_ KFmode strictly a wider mode than IFmode? That is, can it represent all values that IFmode can? On another note I question the expanders considering wider FP modes somewhat in general. So maybe the hook shouldn't be named default_widening_p but rather mode_covers_p ()? And we can avoid calling the hook for integer modes. That said, I wonder if the construction of mode_wider and friends should be (optionally) made more explicit in the modes .def file so powerpc could avoid any wider relation for IFmode. Thanks, Richard. > [gcc] > 2018-05-22 Michael Meissner <meiss...@linux.vnet.ibm.com> > > PR target/85358 > * target.def (default_widening_p): New target hook to say whether > default widening between modes should be done. > * targhooks.h (hook_bool_mode_mode_bool_true): New declaration. > * targhooks.c (hook_bool_mode_mode_bool_true): New default target > hook. > * optabs.c (expand_binop): Before doing default widening, check > whether the backend allows the widening. > (expand_twoval_unop): Likewise. > (expand_twoval_binop): Likewise. > (widen_leading): Likewise. > (widen_bswap): Likewise. > (expand_unop): Likewise. > * cse.c (cse_insn): Likewise. > * combine.c (simplify_comparison): Likewise. > * var-tracking.c (prepare_call_arguments): Likewise. > * config/rs6000/rs6000.c (TARGET_DEFAULT_WIDENING_P): Define > target hook to prevent IBM extended double and IEEE 128-bit > floating point from being converted to each by default. > (rs6000_default_widening_p): Likewise. > * doc/tm.texi (TARGET_DEFAULT_WIDENING_P): Document the new > default widening hook. > * doc/tm.texi.in (TARGET_DEFAULT_WIDENING_P): Likewise. > > [gcc/testsuite] > 2018-05-22 Michael Meissner <meiss...@linux.vnet.ibm.com> > > PR target/85358 > * gcc.target/powerpc/pr85358.c: New test to make sure __ibm128 > does not widen to __float128 on ISA 3.0 systems. > > In order to start the transition of PowerPC long double to IEEE 128-bit, we > will need this patch or a similar patch to be back ported to GCC 8.2. > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
