[PATCH] [AVX-512] - Add FMA instruction with Rounding mode

[Ahmed Bougacha]

On Thu, Jan 15, 2015 at 9:52 AM, Demikhovsky, Elena
<elena.demikhovsky at intel.com> wrote:
>> Say something like:
>>    (AVX512OpWithRounding fadd, op1, op2, <rounding mode, ...>)
>
> We proposed the following, and there is no need to reinvent the wheel

Yes, but both of these have problems, no?

> Option 2:      (AVX512OpWithRounding (fadd, op1, op2,..), <rounding mode>)

Means breaking the SelectionDAG "guarantee" Adam was explaining.  (we
have enough gotchas and corner cases as it is!)

> And less preferable, in my opinion
> Option 1:     (AVX512OpWithRounding_fadd, op1, op2,.. <rounding mode>)

Involves lots of duplication. (otherwise I'm fine with it, if Adam is.)

Folding the operation in the new node (if it works) has none of those
inconvenients, no?  You avoid duplicating the opcodes, and you keep it
all in a single node.  You might lose potential optimisation
opportunities, since the node isn't an fadd/whatever anymore, but
that's not applicable here anyway, right?

-Ahmed

> -  Elena
>
> -----Original Message-----
> From: Ahmed Bougacha [mailto:ahmed.bougacha at gmail.com]
> Sent: Thursday, January 15, 2015 10:41
> To: reviews+D6835+public+db07cccdafa4527c at reviews.llvm.org
> Cc: Badouh, Asaf; Demikhovsky, Elena; anemet at apple.com; Nadav Rotem; Chandler Carruth; halfdan at gmail.com; LLVM Commits
> Subject: Re: [PATCH] [AVX-512] - Add FMA instruction with Rounding mode
>
> On Thu, Jan 15, 2015 at 8:42 AM, Elena Demikhovsky <elena.demikhovsky at intel.com> wrote:
>> Hi Adam,
>>
>> We'd like to submit this code and proceed.
>> The goal is to let setting rounding mode in intrinsics. The operations that we talk about are FP arithmetic - like FADD, FMUL, FSUB, FDIV, FMA - 512 bit vector only.
>> And FP conversions. Nothing more, so there is no correlation with masks.
>>
>> I see 2 options right now.
>>
>> 1. define an additional X86 node type for each intrinsic 2. Wrap the
>> existing node with ROUNDMODE
>>
>> In the patch we are giving solution number 2 and say that it is safe. If ROUNDMODE and FADD will be separated, the compilation will fail with "cannot select".
>> Do you still have any concerns?
>
> FWIW I totally share Adam's concerns, but I'll admit I don't have a better alternative to creating all the specific nodes, and that doesn't seem perfect either.
>
> How about - and this is totally handwavy - adding a single new node, having all of the operation, rounding mode, and mask as operands?  I don't know if there's precedent, but since you say we're late in the SelectionDAG, your only problem would then be to match these, correct?  That might involve some tweaks in tablegen to be able to specify an FP op as an operand, but that doesn't sound too hard.  In the SelectionDAG proper, the operation "operand" would simply be a ConstantSDNode, holding the ISD opcode.
>
> Say something like:
>     (AVX512OpWithRounding fadd, op1, op2, <rounding mode, ...>)
>
> I'm guessing you won't be able to reuse the existing "fadd"/... in tablegen, but adding a few new defs, with a 1-to-1 mapping with the ISD opcode, should do the trick.
>
> Anyway, just my 2 cents.
>
> -Ahmed
>
>> Thank you for reviewing this.
>>
>>
>> REPOSITORY
>>   rL LLVM
>>
>> http://reviews.llvm.org/D6835
>>
>> EMAIL PREFERENCES
>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.