[PATCH] D104997: [WIP][AMDGPU] Deduce attributes with the Attributor

[Kuter Dinel via Phabricator via llvm-commits]

kuter added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:170-175
+      if (!F->hasExactDefinition()) {
+        LLVM_DEBUG(dbgs() << "[AMDWorkGroupSize] Giving up: " << F->getName() << "\n");
+        NewUniformWorkGroupSize = false;
+        return true;
+      }
+
----------------
jdoerfert wrote:
> As I said before, this is not needed.
`uniform-work-group-propagate-attribute.ll` specifically checks for this the `@weak_func`


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:233
+private:
+  Optional<bool> UniformWorkGroupSize = llvm::None;
+};
----------------
jdoerfert wrote:
> Maybe you just use the boolean state instead? Having the additional boolean here doesn't seem to bring any benefit. You can probably just expose `clampStateAndIndicateChange` in the Attributor.h (`AA` namespace) and then use it in the `CheckCallSite` callback. There should not be anything to do but ask for the callee AA and clamp. This should remove 20 or so lines here.
I don't think this would work. In some cases we need to be able turn `uniform-work-group-size` attribute to false even if it is initialized with true.

in `uniform-work-group-attribute-missing.ll` `@foo` get's initialized with `uniform-work-group-size` true. But we turn it to false because the kernel doesn't have the attribute (for kernels we assume it is false if it is not present).

If we where to use the BooleanState, `@foo` would automatically be at a fixpoint on initialization (since it is set to true).
and we wouldn't be able to turn it to false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104997