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
