Ping! :)

This would make much more sense IMO. Especially for passes with long names. (For example, I'd much rather see "patchable-function" than "Implement the 'patchable-function' attribute" in the JSON)

Fixed comments.

• Added a comment for saving/restoring LR
• Removed LRUOut, since it's not necessary here, and incurs unnecessary calculation
• LRUIn -> LRU

Updated patch to handle the live-in case. Updated the test to be more explicit as well.

Closed in r333952 (https://reviews.llvm.org/rL333952)

In D47553#1119498, @asb wrote:

I think I can disable outlining for an entire function using isFunctionSafeToOutlineFrom

Yeah, that'll work. It's conservative, but it will ensure that the outliner will never touch the function, and thus, won't touch the instructions (or anything between them).

Got rid of the whitespace changes and remembered to pass -U999999 this time.

Cool! LGTM.

Ah, you’re right.

Committed as r332739.

Updated patch to address review.

A few changes here after running this on some more complicated code.

Fixed a typo in initSizeRemarkInfo.

Thanks for finding this. LGTM!

You are entirely correct. LGTM.

Thanks for finding this! LGTM.

A couple more changes:

• Updated the test to use globaldce for the module pass
• Ensured that we don't *ever* get the module's instruction count unless the remark has been requested but the user. This is handled in a new helper function, initSizeRemarkInfo
• Improved the test. It only runs opt once per pass type we want to handle. It now no longer has hard-coded values for sizes, which should make sure it isn't terribly flakey

This is pretty non-invasive, so code LGTM aside from one minor stylistic comment on an assertion. I would wait for comments from another reviewer though, since I'm not super familiar with this particular bit of code.

Updated diff to address review comments.

A bit of cleanup. Updated diff for current pass manager, changed get(IRType)IRCount to a general getIRCount() function.

LGTM

Funnily enough, this version of the patch uncovered a bug in a different pass, which caused its lit tests to fail (D46330).

Thanks! Switched the begin() to end() as suggested.

Updated patch for review. Lots of tests modified this time because in this round the outliner is *always* added to the pass pipleline. It then decides whether or not it should run based off of user input/target default behaviour.

Committed in r329120.

We decided it'd be better to just use function attributes for the outliner, so I'm abandoning this.

We decided it'd be better to just use function attributes for the outliner, so I'm abandoning this.

We decided it'd be better to just use function attributes for the outliner, so I'm abandoning this.

Updated diff to reflect review comments and discussion on the RFC. In the RFC, we decided that it'd be better to base the decision to outline off of function attributes for compatibility with LTO and extensibility of the outliner. This is a far less invasive change, and means that every patch other than this can be abandoned.

(If we want to outline from these, we can probably try performing a separate outlining pass for each section. That way we can enforce that functions outlined from a specific section all come from the same section, and also end up in that *same* section.)

Add context to diff.

Added context to diff.

Added context to diff.

Updated diff to use -U999999 for context

I think (hope) r329723 should fix it…

Currently looking into it.

Committed in r329716.