[PATCH] D141110: [RS4GC] Remove the hardcoded GC strategy names (v2)

Fri Jan 13 06:34:48 PST 2023

dantrushin added a comment.

In D141110#4048764 <https://reviews.llvm.org/D141110#4048764>, @znix wrote:

> I'll work on making a GCStrategyAnalysisPass then. Given RS4GC supports both the new and legacy pass manager, I assume this new pass needs to support both, too?

I think yes. Even though Legacy PM is deprecated, supporting it is just several extra lines.

> With regards to moving all the static RS4GC functions to a class, should I make that a separate diff, a separate commit within the diff, or something similar? I ask because there will be a lot of mostly-whitespace changes as all the function arguments are indented, which would make the patch quite long.

This usually done as a separate NFC (non-functional change) patch.
Note that such change is not a blocker for this review - we can do it later.

> Apologies for this very basic question, but how do I update a patch using Arcanist? I had a look on the docs page about Phabricator reviews, and it didn't mention how to do them.

Simply do `arc diff` again. As long as your commit message has `Differential Revision` line, arcanist is smart enough to update existing review instead of creating new one.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141110