On Thu, Nov 16, 2017 at 12:48 PM, Michael Meissner <meiss...@linux.vnet.ibm.com> wrote: > On Thu, Nov 16, 2017 at 04:48:18AM -0600, Segher Boessenkool wrote: >> On Wed, Nov 15, 2017 at 04:56:10PM -0500, Michael Meissner wrote: >> > David tells me that the patch to enable float128 built-in functions to work >> > with the -mabi=ieeelongdouble option broke AIX because on AIX, the float128 >> > insns are disabled, and they all become CODE_FOR_nothing. The switch >> > statement >> > that was added in rs6000.c to map KFmode built-in functions to TFmode >> > breaks >> > under AIX. >> >> It also breaks on Linux with older binutils (no HAVE_AS_POWER9 defined). >> >> > I changed the code to have a separate table, and the first call, I build >> > the >> > table. If the insn was not generated, it will just be CODE_FOR_nothing, >> > and >> > the KF->TF mode conversion will not be done. >> > >> > I have tested this on a little endian power8 system and there were no >> > regressions. Once David verifies that it builds on AIX, can I check this >> > into >> > the trunk? >> >> I don't like this scheme much (huge table, initialisation at runtime, etc.), >> but okay for trunk, to unbreak things there. >> >> Some comments on the patch: >> >> > + if (first_time) >> > + { >> > + first_time = false; >> > + gcc_assert ((int)CODE_FOR_nothing == 0); >> >> No useless cast please. The whole assert is pretty useless fwiw; just >> take it out? >> >> > + for (i = 0; i < ARRAY_SIZE (map); i++) >> > + map_insn_code[(int)map[i].from] = map[i].to; >> > + } >> >> Space after cast. >> >> Only do this for codes that are *not* CODE_FOR_nothing? > > I must admit to not liking the code, and it is overly complicated. > > It occurred to me this morning that a much simpler patch is to just #ifdef out > the switch statement if we don't have the proper assembler. I tried this on > an > old power7 system using the system assembler (which does not support the ISA > 3.0 instructions) and it built fine. I think this will work on AIX. David > can > you check this? > > I will fire off a build, and if it is successful, can I check this patch > instead of the other patch?
This patch will solve the problem. GCC policy prefers runtime tests over #ifdef, but I agree that the runtime approach is overly messy. This seems like a reasonable approach to me. Thanks, David