AMDGPU: Invert AMDGPUAttributor

[arsenm]

Summary

Switch to using BitIntegerState for each of the inputs, and invert
their meanings.

This now diverges more from the old AMDGPUAnnotateKernelFeatures, but
this isn't used yet anyway.

[kuter]

I think it makes sense to reverse the attributes.

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

381

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.

408

Same here ^^^

423

Here too ^^

[kuter]

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

364

Rather then doing this we can use BitIntegerState::joinAND()or rahter IntegerBaseState::operator&=() which calls joinAND() internally.

[jdoerfert]

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

35

Again, I would suggest an array of structs but I guess it works like this too. Benefit of tying them together is that we don't need the 1 << I trick at the use side but can just use the bit (=enum) itself.

339–340

And this would become a range loop over the array of structs instead.

381

I recommend to take the state before and return CHANGED if the final state is not the initial one, it's an int we copy and compare so there is little cost.

[jdoerfert]

llvm/docs/AMDGPUUsage.rst

914

Btw. this is great. Was missing before and a pain for @kuter .

[arsenm]

Use pair/mask. Return state changed, which also fixes recursive function handling

[jdoerfert]

All but one comment are nits. That one we should probably clarify but generally this is otherwise good to go.

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

341

This I don't get.
If F has no-xyz, why do we remove the bit? Should we not do addKnownBits(Attr.first) instead?

373

Nit: I'd call it AttrMask, not Name, but no strong feelings.

432

Nit: consider a lambda for this check, we probably should have a helper in the AA namespace so you can do return AA::getChangeStatus(OrigAssumed, getAssumed()); It is really common by now.

[arsenm]

Fix handling of existing attributes on declarations

[jdoerfert]

LG

[arsenm]

088cc63640f4593a0af1fc81859853d5ee0d00ef

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

432

A lamdba is way too heavy for this, especially since it's just to return a glorified bool