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

[Demikhovsky, Elena]

> Well, VSELECT can be translated to an instruction, so I think that’s different.  If the VSELECT and the op get separated we generate less efficient code so that’s always OK.
It may cause FP exception.
> In this case I am not convinced that we can’t generate the wrong result.
If ROUNDMODE and instruction will be separated somehow, the compilation will fail.
I say “somehow” because we are talking about intrinsic that mapped directly to instruction - everything is legal inside.
Can you give an example?
The alternative is to invent one more node per instruction that needs rounding mode.

> It’s an important enough design decision for the AVX512 SDAG IR to warrant some scrutiny.
> I suggest to properly comment this node and provide explanation in the review itself and CC a few more people (Nadav, Chandler, Hal, Jim, etc.)
I see this as a small patch which resolves FP intrinsics rounding mode, but no problem, we can widen the list of reviewers.

-           Elena

From: Adam Nemet [mailto:anemet at apple.com]
Sent: Wednesday, January 07, 2015 20:09
To: Demikhovsky, Elena
Cc: reviews+D6835+public+db07cccdafa4527c at reviews.llvm.org; Badouh, Asaf; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] [AVX-512] - Add FMA instruction with Rounding mode


On Jan 6, 2015, at 11:12 PM, Demikhovsky, Elena <elena.demikhovsky at intel.com<mailto:elena.demikhovsky at intel.com>> wrote:



-  Elena

-----Original Message-----
From: Adam Nemet [mailto:anemet at apple.com]
Sent: Wednesday, January 07, 2015 03:04
To: Badouh, Asaf; Demikhovsky, Elena; anemet at apple.com<mailto:anemet at apple.com>
Cc: cameron.mcinally at nyu.edu<mailto:cameron.mcinally at nyu.edu>; llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: Re: [PATCH] [AVX-512] - Add FMA instruction with Rounding mode

REPOSITORY
 rL LLVM

================
Comment at: lib/Target/X86/X86ISelLowering.h:374
@@ -373,1 +373,3 @@

+      ROUNDMODE,
+
----------------
Hmm, has this been completely thought through?  I may have missed the discussion...

Looks like you're introducing a new node that if it surrounds an FMA op it changes its rounding mode?

What happens if the two nodes get separated by some transformation?
[Demikhovsky, Elena] We surround with VSELECT to set mask for intrinsic. The idea here is the same, even less  risky. The stage is late, all transformations are behind.
We plan to use this node for all FP arithmetic and conversion intrinsics.

Well, VSELECT can be translated to an instruction, so I think that’s different.  If the VSELECT and the op get separated we generate less efficient code so that’s always OK.  In this case I am not convinced that we can’t generate the wrong result.

My main point is that unless this was discussed somewhere else, it shouldn’t be hidden in a patch like this.  It’s an important enough design decision for the AVX512 SDAG IR to warrant some scrutiny.

I suggest to properly comment this node and provide explanation in the review itself and CC a few more people (Nadav, Chandler, Hal, Jim, etc.)

Thanks,
Adam



================
Comment at: lib/Target/X86/X86InstrAVX512.td:3541-3542
@@ +3540,4 @@
+                              X86VectorVTInfo VTI, SDPatternOperator
+ OpNode> {  defm v213r : avx512_fma3_round_rrb<opc213, !strconcat(OpcodeStr, "213", VTI.Suffix),
+                              VTI, OpNode>, EVEX_CD8<VTI.EltSize,
+ CD8VF>;
+
----------------
I think that the EVEX_CD8 thing can be written as VTI.CD8TupleForm.

Same later.

http://reviews.llvm.org/D6835

EMAIL PREFERENCES
 http://reviews.llvm.org/settings/panel/emailpreferences/


---------------------------------------------------------------------
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.

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto: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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150107/57f7b38a/attachment.html>