[PATCH] D96469: [AMDGPU] Use single cache policy operand

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

MC part overall looks good with minor reservations:

- The change breaks compatibility with existing code. The following code is no longer accepted:

  buffer_load_dword v5, off, s[8:11], s3 lds dlc   // accepted if lds and dlc are swapped

Is this an issue? (Fortunately I found no other failures.)

- With this change assembler will accept duplicate cache policy modifiers. This can be easily amended in some cases, others are more problematic:

  global_atomic_add v[3:4], v5, off slc slc              # easy to detect
  global_atomic_add v[3:4], v5, off slc noglc noglc      # not so easy to detect

- Some combinations of modifiers will be accepted though they look weird:

  global_atomic_add v[3:4], v5, off slc glc noglc

- There is a case when assembler incorrectly detects an offending token:

  global_atomic_add v[3:4], v5, off slc noglc glc 

- I'd have preferred to enforce cache policy modifiers go in one block (i.e. do not allow them mix with other modifiers). Unfortunately this would result in other incompatibilities with legacy code if implemented.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96469/new/

https://reviews.llvm.org/D96469