[PATCH] D108158: AMDGPU: Invert AMDGPUAttributor

[Kuter Dinel via Phabricator via llvm-commits]

kuter added a comment.

I think it makes sense to reverse the attributes.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:389
+      return ChangeStatus::CHANGED;
     }
 
----------------
I don't think this should always return `CHANGED` here. I think we should only return `CHANGED`
if we actually changed a bit.

This code as is, will always return `CHANGED`  if NeedsQueuePtr` is true. Which will make the `AbstractAttribute` stuck in a loop. Attributor times out after a set number of iterations, but this will waste compile time.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:416
+      removeAssumedBits(1 << QUEUE_PTR);
+      return ChangeStatus::CHANGED;
     }
----------------
Same here ^^^


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:428
+              removeAssumedBits(1 << QUEUE_PTR);
+              return ChangeStatus::CHANGED;
             }
----------------
Here too ^^


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

https://reviews.llvm.org/D108158