On Wed, 15 Jun 2016, Michael Meissner wrote: > On Wed, Jun 15, 2016 at 11:01:05AM +0200, Richard Biener wrote: > > On Tue, 14 Jun 2016, Bill Schmidt wrote: > > > > > Hi Richard, > > > > > > As nobody else has replied, let me take a stab at this one. > > > > > > > On Jun 10, 2016, at 2:06 AM, Richard Biener <rguent...@suse.de> wrote: > > > > > > > > On Thu, 9 Jun 2016, Michael Meissner wrote: > > > > > > > >> I'm including the global reviewers on the list. I just want to be > > > >> sure that > > > >> there is no problem installing these patches on the GCC 6.2 branch. > > > >> While it > > > >> is technically an enchancement, it is needed to be able to install the > > > >> glibc > > > >> support that is needed to complete the work to add IEEE 128-bit > > > >> floating point. > > > >> > > > >> The issue being fixed is that when we are creating the complex type, > > > >> we used to > > > >> do a lookup for the size, and that fails on the PowerPC which has 2 > > > >> 128-bit > > > >> floating point types (__ibm128 and __float128, with long double > > > >> currently > > > >> defaulting to __ibm128). > > > > > > > > As this enhancement includes middle-end changes I am hesitant to approve > > > > it for the branch. Why is it desirable to backport this change? > > > > > > It comes down to community requirements and schedules. We are in the > > > process of > > > replacing the incompatible IBM long double type with true IEEE-754 > > > 128-bit floating > > > point (__float128). This is a complex multi-stage process where we will > > > have to > > > maintain the functionality of the existing IBM long double for backward > > > compatibility > > > while the new type is implemented. This impacts multiple packages, > > > starting with > > > gcc and glibc. > > > > > > The glibc maintainers have indicated that work there depends on a certain > > > level of > > > functionality within GCC. Specifically, both the old and new types must > > > be supported, > > > including corresponding complex types. Unfortunately the realization > > > that the complex > > > types had to be supported came late, and this work didn't fully make it > > > into GCC 6.1. > > > > > > (Part of the problem that has made this whole effort difficult is that it > > > is complicated to > > > maintain two floating-point types of the exact same size.) > > > > > > In any case, the glibc maintainers require this work in GCC 6 so that > > > work can begin > > > in glibc 2.24, with completion scheduled in glibc 2.25. We are asking > > > for an exception > > > for this patch in order to allow those schedules to move forward. > > > > > > So that's the history as I understand it... Perhaps others can jump in if > > > I've munged > > > or omitted anything important. > > > > Ok, I see the reason for the backport then. Looking at the patch > > the only fragile thing is the layout_type change give it adds an assert > > and you did need to change frontends (but not all of them?). I'm not > > sure about testsuite coverage for complex type for, say, Go or Ada > > or Java. > > I added the assert after the review from Berndt. It was to make sure there > were no other callers to layout_type to create complex nodes. > https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00077.html > > > And I don't understand the layout_type change either - it looks to me > > it could just have used > > > > SET_TYPE_MODE (type, GET_MODE_COMPLEX_MODE (TYPE_MODE > > (TREE_TYPE (type)))); > > > > and be done with it. To me that looks a lot safer. > > It has been some time since I looked at the code, I will have to investigate > it futher. > > Note, I will be offline for the next 4 days. > > > With now having two complex FP modes with the same size how does > > iteration over MODE_COMPLEX_FLOAT work with GET_MODE_WIDER_MODE? > > Is the outcome random? Or do we visit both modes? That is, could > > GET_MODE_COMPLEX_MODE be implemented with iterating over complex modes > > and in addition to size also match the component mode instead? > > I struggled quite a bit with GET_WIDER_MODE. There are three distinct usage > cases in the compiler. One case uses GET_WIDER_MODE to initialize all of the > types. Here you want to visit all of the types (though we could change the > code, since genmode does sort the types so that all of the types for a given > class are together). > > Another case is the normal case, where given a type, go up to a wider type > that > might implment the code you are looking for. > > A third case is when generating floating point constants in the constant pool, > see if there is a smaller type that maintains the precision. > > Eventually, I decided to punt having to have explicit paths for widening. I > used fractional modes for IFmode (ibm long double format) and KFmode (IEEE > 128-bit format). IFmode widens to KFmode which widens to TFmode. A backend > hook is used to not allow IBM long double to widen to IEEE long double and > vice > versa. At the moment, since there is no wider type than 128-bits, it isn't an > issue.
Ok, using fractional float modes is a lie though as both IFmode and KFmode have no insignificant bits. I checked that if you have two same-size FLOAT_MODEs they still get iterated over with GET_MODE_WIDER_MODE, in order of appearance. So your reason to use fractional modes was to make the order explicit and to avoid bogus handling in for example the constant pool handling which would mistake say IFmode as being smaller than KFmode just because one is GET_MODE_WIDER_MODE of the other (and both have the same precision)? Interestingly for GET_MODE_2XWIDER_MODE it "skips" the duplicate-sized and chooses the "first" one. I guess having to have both float formats usable at the same time is not really supported by our mode machinery. > Note, I do feel the front ends should be modified to allow __complex > __float128 directly rather than having to use an attribute to force the > complex type (and to use mode(TF) on x86 or mode(KF) on PowerPC). It > would clean up both x86 and PowerPC. However, those patches aren't > written yet. Sure, but that's independent of the modes issue. Anyway, I'm fine with the backport if you sort out the issue with the assert in layout_type. Thanks, Richard.
