[PATCH] D129086: [NFC][AMDGPU] Cleanup the SIOptimizeExecMasking pass.

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:744
+
+  Utils = std::make_unique<SIOptimizeExecMaskingUtils>(MF);
+
----------------
foad wrote:
> I don't understand the value of making this a separate class, instead of part of the main SIOptimizeExecMasking class.
After doing so and adding all necessary members to the SIOptimizeExecMasking class, it felt very cluttered.
I can revert that if you want, but the idea was to have a class that consists of various references to the sub-objects (TII, TRI etc.). These rely on the MachineFunction being passed. So, to ensure that these references are actually set, I created a separate class which can only be constructed by passing a MachineFunction. If we would add these members (references) to SIOptimizeExecMasking, we would be required to initialize them in the constructor AFAIK, which is not possible because of the required MachineFunction which is only passed in runOnMachineFunction. So there is the choice of using a) a separate class, b) using pointers and adding nullptr-checks everywhere or c) just go the unsafe route. I preferred to do a), cleanly abstract the sub-objects and all corresponding logic away, have the member references being initialized in the constructor of the Utils class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129086