[PATCH] D43414: AMDGPU: Define FP_FAST_FMA{F} macros for amdgcn
Konstantin Zhuravlyov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 23 15:39:07 PST 2018
kzhuravl added inline comments.
================
Comment at: lib/Basic/Targets/AMDGPU.cpp:159
// XXX - What does the member GPU mean if device name string passed here?
if (getTriple().getArch() == llvm::Triple::amdgcn) {
----------------
t-tye wrote:
> What does this mean? Has it now been addressed by this patch?
This class has a member called *GPU*. We are using the *CPU* that is passed as an argument.
> Has it now been addressed by this patch?
No.
================
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},
----------------
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.
================
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},
----------------
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
================
Comment at: lib/Basic/Targets/AMDGPU.h:325
- return GPU.Kind != GK_NONE;
+ return GK_NONE != GPU.Kind;
}
----------------
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.
https://reviews.llvm.org/D43414
More information about the llvm-commits
mailing list