[PATCH] D85882: [AMDGPU] Update subtarget features for new target ID support

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:169-172
+  LLVM_DEBUG(dbgs() << "xnack setting for subtarget: "
+                    << TargetID.getXnackSetting() << '\n');
+  LLVM_DEBUG(dbgs() << "sramecc setting for subtarget: "
+                    << TargetID.getSramEccSetting() << '\n');
----------------
kerbowa wrote:
> arsenm wrote:
> > I think having debug printing in the subtarget constructor would be a net annoyance
> We already had debug printouts in the constructor from ParseSubtargetFeatures. Do you think they should all be removed? Right now the output with '-debug-only=amdgpu-subtarget' looks like this, which seems helpful to me.
> 
> ```
> Features:+promote-alloca,+load-store-opt,+enable-ds128,+enable-prt-strict-null,
> CPU:gfx900
> TuneCPU:gfx900
> 
> xnack setting for subtarget: Any
> sramecc setting for subtarget: Unsupported
> ```
I forgot about those, so I guess it doesn't matter


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:296
+
+  if (SramEccRequested.hasValue()) {
+    if (SramEccSupported) {
----------------
Don't need .hasValue()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85882