[PATCH] D43414: AMDGPU: Define FP_FAST_FMA{F} macros for amdgcn

[Tony Tye via Phabricator via llvm-commits]

t-tye added inline comments.


================
Comment at: lib/Basic/Targets/AMDGPU.h:136
+  static constexpr GPUInfo AMDGCNGPUs[30] = {
+    {{"gfx600"},    {"gfx600"},  GK_GFX600,  true, true, true, true, true},
+    {{"tahiti"},    {"gfx600"},  GK_GFX600,  true, true, true, true, true},
----------------
kzhuravl wrote:
> t-tye wrote:
> > Same comment as above.
> > 
> > Also,  I think the fast_fma and fast_fmaf columns are reversed. All gcn has fast_fma, it is fast_fmaf that varies.
> fast_fmaf is the last column. I think those are in the correct order.
Agreed.


================
Comment at: lib/Basic/Targets/AMDGPU.h:147
+    {{"hawaii"},    {"gfx701"},  GK_GFX701,  true, true, true, true, true},
+    {{"gfx702"},    {"gfx702"},  GK_GFX702,  true, true, true, true, false},
+    {{"gfx703"},    {"gfx703"},  GK_GFX703,  true, true, true, true, false},
----------------
kzhuravl wrote:
> t-tye wrote:
> > gfx702 should have true for fast fmaf.
> > 
> > Does the TD file need correcting too?
> Not according to the TD files in our BE: https://github.com/llvm-mirror/llvm/blob/master/lib/Target/AMDGPU/AMDGPU.td#L545
I believe the TD file is incorrect. gfx701 and 702 should both have fast fmaf.


================
Comment at: lib/Basic/Targets/AMDGPU.h:325
 
-    return GPU.Kind != GK_NONE;
+    return GK_NONE != GPU.Kind;
   }
----------------
kzhuravl wrote:
> t-tye wrote:
> > I found the original operand order easier to read:-)
> I was trying to match the style above. But I can change it back.
Leave it in your new order which keeps it consistent as you point out. (For me they are all backwards:-) )


https://reviews.llvm.org/D43414