foad added inline comments.
================ Comment at: llvm/lib/Target/AMDGPU/BUFInstructions.td:2891 +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/FLATInstructions.td:1915 +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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146701/new/ https://reviews.llvm.org/D146701 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits