foad added inline comments.

Comment at: llvm/lib/Target/AMDGPU/
+let SubtargetPredicate = HasAtomicFaddNoRtnInsts in {
+defm BUFFER_ATOMIC_ADD_F32    : MUBUF_Real_Atomic_vi <0x4d>;
mariusz-sikora-at-amd wrote:
> foad wrote:
> > Is this still required?
> No. We can remove this. But I wanted to limit this change only to atomic 
> f16/bf16 and not going deeper 
OK. I thought this was something you added in this patch, but now I see it is 
just moved around.

Comment at: llvm/lib/Target/AMDGPU/
+defm GLOBAL_ATOMIC_PK_ADD_F16 : FLAT_Global_Real_Atomics_vi <0x04e, 0>;
+defm GLOBAL_ATOMIC_PK_ADD_BF16 : FLAT_Global_Real_Atomics_vi<0x52>;
mariusz-sikora-at-amd wrote:
> foad wrote:
> > Are these changes (from here to the end of the file) still required?
> Not sure if I understand what you mean. Could you please elaborate more ? Are 
> you referring to the fact that both flat_atomic and global_atomic have FLAT 
> encoding and could be unified ?
> I thought this is required, but now you got me thinking ...
I don't understand why the changes from here to the end of the file are 
required. It looks like you have just moved some definitions around, so that 
they no longer have a SubtargetPredicate applied. Is that correct? Why?

  rG LLVM Github Monorepo


cfe-commits mailing list

Reply via email to