[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:233
+private:
+  Optional<bool> UniformWorkGroupSize = llvm::None;
+};
----------------
kuter wrote:
> 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.
Also @jdoerfert There is now several attributes that use `BooleanState` as a "placeholder" state.
I think it would be great if we had something like a `VoidState` to avoid confusion.
even if it was as simple as `using VoidState = BooleanState;` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104997