[PATCH] D41617: [AMDGPU] Add HW_REG_SH_MEM_BASES symbolic name for s_getreg_b32

[Artem Tamazov via Phabricator via llvm-commits]

artem.tamazov accepted this revision.
artem.tamazov added a comment.
This revision is now accepted and ready to land.

Some style-related fixes are desirable, otherwise LGTM.



================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:3168
+    if (isSI() || isCI() || isVI())
+      Last = ID_SYMBOLIC_FIRST_GFX9_;
+    for (int i = ID_SYMBOLIC_FIRST_; i < Last; ++i) {
----------------
Just a recommendation:
```
const int Last = (isSI() || isCI() || isVI()) ? ID_SYMBOLIC_LAST_PRIOR_GFX9_
    : ID_SYMBOLIC_LAST_;
```
Not ideal anyway, but `const` at least. Up to you.


================
Comment at: lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp:1259
+  if (AMDGPU::isSI(STI) || AMDGPU::isCI(STI) || AMDGPU::isVI(STI))
+    Last = ID_SYMBOLIC_FIRST_GFX9_;
+  if (ID_SYMBOLIC_FIRST_ <= Id && Id < Last && IdSymbolic[Id]) {
----------------
Ditto.


================
Comment at: lib/Target/AMDGPU/SIDefines.h:277
   ID_MEM_BASES = 15,
+  ID_SYMBOLIC_FIRST_GFX9_ = ID_MEM_BASES,
+  ID_SYMBOLIC_LAST_ = 16,
----------------
Recommendation: I would rename `ID_SYMBOLIC_FIRST_GFX9_` to `ID_SYMBOLIC_LAST_PRIOR_GFX9_` or alike.


================
Comment at: lib/Target/AMDGPU/SIDefines.h:277
   ID_MEM_BASES = 15,
+  ID_SYMBOLIC_FIRST_GFX9_ = ID_MEM_BASES,
+  ID_SYMBOLIC_LAST_ = 16,
----------------
artem.tamazov wrote:
> Recommendation: I would rename `ID_SYMBOLIC_FIRST_GFX9_` to `ID_SYMBOLIC_LAST_PRIOR_GFX9_` or alike.
Value should be set to 8, as it was prior these changes.


https://reviews.llvm.org/D41617