mariusz-sikora-at-amd added inline comments.

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>;
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 

  list<Predicate> Predicates = PredConcat<

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 = 

I will revert these changes and move definitions back under the 

  rG LLVM Github Monorepo


cfe-commits mailing list

Reply via email to