[PATCH] D127994: [AMDGPU] Expose CLI controls for IGroup ordering

[Jeffrey Byrnes via Phabricator via llvm-commits]

jrbyrnes added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:210
+
+  bool parse(cl::Option &O, StringRef ArgName, StringRef Arg,
+             std::string &Value) {
----------------
kerbowa wrote:
> So this parser is just validating the input? Is there some way to avoid parsing twice?
I can rethink the design a bit.  Would you prefer the CLI parser to create the PipelineOrderGroups (e.g. Pipeline used in adding edges)? I can probably implement this here using enum and array of constructors. Thanks for comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:254
+
 static cl::opt<Optional<unsigned>>
     VMEMGroupMaxSize("amdgpu-igrouplp-vmem-group-size", cl::init(None),
----------------
kerbowa wrote:
> Can these be removed now since the size is embedded into amdgpu-igrouplp-order?
The idea was to default to the prior iteration's style if no amdgpu-igrouplp-order was specified, but I see how these options may be confusing. I'll remove them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127994