mariusz-sikora-at-amd added inline comments.
================ 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>; ---------------- foad wrote: > 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? Yes, I moved them out from under the SubtargetPredicates. All tests are passing. Even 'negative' for unsupported generations, but after deeper looking into 'gen-instr-info' from TableGen I see that there is a difference for GLOBAL_ATOMIC_PK_ADD_F16_*_vi definitions of Real Instructions. 1) I'm still convinced that SubtargetPredicate = isGFX940Plus is not needed for definitions which inherit from FLAT_Global_Real_Atomics_gfx940. Multiclass sets AssemblerPredicate = isGFX940Plus and when looking how Predicates list is created we can see that AssemblerPredicate is used in creation of final list of Predicates. list<Predicate> Predicates = PredConcat< PredConcat<PredConcat<OtherPredicates, SubtargetPredicate>.ret, AssemblerPredicate>.ret, WaveSizePredicate>.ret; 2) In case of FLAT_Global_Real_Atomics_vi I see that SubtargetPredicate = isGFX8GFX9NotGFX940 will be required, because AssemblerPredicate is isGFX8GFX9; I wonder why everything works event when not having these SubtargetPredicates = isGFX8GFX9NotGFX940 I will revert these changes and move definitions back under the SubtargetPredicates. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146701/new/ https://reviews.llvm.org/D146701 _______________________________________________ cfe-commits mailing list email@example.com https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits