[PATCH] D49037: AMDGPU: Refactor Subtarget classes

[Jan Vesely via Phabricator via llvm-commits]

jvesely accepted this revision.
jvesely added a comment.

Other than the few nits mentioned in the text, LGTM.



================
Comment at: lib/Target/AMDGPU/AMDGPUInstructionSelector.h:42
 class SIRegisterInfo;
-class SISubtarget;
+class GCNSubtarget;
 
----------------
This rename duplicates GCNSubtarget declaration.


================
Comment at: lib/Target/AMDGPU/AMDGPUSubtarget.h:230
     // Gap for R600 generations, so we can do comparisons between
-    // AMDGPUSubtarget and r600Subtarget.
+    // GCNSubtarget and r600Subtarget.
     SOUTHERN_ISLANDS = 4,
----------------
Just out of curiosity what's the benefit of having this enum split between GCNSUbtarget and R600Subtarget, vs merging it in AMDGPUSubtarget?


================
Comment at: lib/Target/AMDGPU/AMDGPUSubtarget.h:981
+};
+#endif
+
----------------
Is this #if 0 code here intentionally? If yes an explanatory comment would be nice.


================
Comment at: lib/Target/AMDGPU/R600RegisterInfo.h:23
 
-class AMDGPUSubtarget;
+class GCNSubtarget;
 
----------------
Why is this necessary in R600 file? Looks like a leftover from the time when AMDGPUSubtarget was shared parent.


================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.h:27
 class MachineRegisterInfo;
-class SISubtarget;
+class GCNSubtarget;
 class SIMachineFunctionInfo;
----------------
This rename duplicates GCNSUbtarget declaration.


Repository:
  rL LLVM

https://reviews.llvm.org/D49037