Thank you @Szelethus for the guidance. Thank you @NoQ for further input. Please see inline:

ping

In D53185#1284536, @Szelethus wrote:

Let's talk about more general topics: Your checker's name is "TimeChecker", not "Y2K38Checker" or something similar, do you have plans on expanding this checker to cover more time.h related functionality?

Incorporated code review feedback; thank you @Szelethus and @george.karpenkov

Re-spun with feedback received into: D53185

Thank you very much for the feedback. I have added a little functionality to demonstrate the working of a prototype checker for a some sample Y2K38 issues. There are some TODO's in the test files indicative of some subsequent checking that we will add.

Thank you, everybody, for the valuable inputs.

In D28075#645429, @aprantl wrote:

There are several options I could think of:
Either surfacing it through a driver option, enabling it at -Og, or benchmarking it and arguing to enable it by default at some optimization settings.

• Added documentation to source code on -mergefunc-preserve-debug-info behaviour and noted the difference from the base -mergefunc
• Elaborated & consolidated comments in regards MergeFunctionsPDI for MergeFunctions::writeThunk()
• Made suggested stylistic changes.

In D28075#644203, @aprantl wrote:

Added some more stylistic changes.

Is this good?

Submitting ticked off items.

Under -mergefunc-preserve-debug-info

• A thunk's call site is preserved to point to the thunk when both occur within the same translation unit.

In D28075#638292, @aprantl wrote:

In D28075#638014, @appcs wrote:

What should -mergefunc-preserve-debug-info do about the thunk's call to the shared implementation?
Mark as tail call or not? The existing -mergefunc behaviour is to mark it as a tail call. We could leave it as is,
unless someone specifically asks for a change under -mergefunc-preserve-debug-info; would that be OK?

I think marking it as a tail call makes most sense.

In D28075#637947, @aprantl wrote:

In D28075#637667, @appcs wrote:

In D28075#637188, @aprantl wrote:

Call sites of the merged function occurring from within the TU of the merged function's definition will point directly to the shared implementation and call sites of the merged function that are external to the TU of the merged function's definition call the thunk for the merged function (which tail call's the shared implementation passing forward the incoming arguments). This is existing behaviour under -mergefunc and remains unchanged under -mergefunc-preserve-debug-info (except for the tail call part).

Is this the right thing to do, though? I would think that for better debugability users would always prefer the version with the thunk even in the current TU. Then again, subsequent optimization passes would likely inline the thunk and thus potentially undo most of the effect. Have you experimented with this variant? Would the debug info from the thunk survive?

Thank you; I agree that this greatly enhances the debug experience. Automatically modifying the call site with a direct call to the shared implementation instead of (what is now) the thunk, when within the TU, is confusing when debugging. Stepping into the thunk from the call site and from within the thunk, into the shared implementation keeps the debug experience uniform for internal and external call sites of functions that become thunks. (If you ask me, this is similar in spirit to not marking the thunk's call to the shared implementation as a tail call to help debugging - the full back trace is the same in both cases and otherwise, there is a certain element of surprise in both cases. Basically, we are saying that under -mergefunc-preserve-debug-info we trade off optimization partly (at -O0) to aid debugability, modifying the transformation slightly, to make the execution flow explicit).

Yes. I think this is the right approach. When using -mergefunc-preserve-debug-info we should always call the thunk even in the same TU.

In D28075#637188, @aprantl wrote:

Call sites of the merged function occurring from within the TU of the merged function's definition will point directly to the shared implementation and call sites of the merged function that are external to the TU of the merged function's definition call the thunk for the merged function (which tail call's the shared implementation passing forward the incoming arguments). This is existing behaviour under -mergefunc and remains unchanged under -mergefunc-preserve-debug-info (except for the tail call part).

Is this the right thing to do, though? I would think that for better debugability users would always prefer the version with the thunk even in the current TU. Then again, subsequent optimization passes would likely inline the thunk and thus potentially undo most of the effect. Have you experimented with this variant? Would the debug info from the thunk survive?

MergeSimilarFunctions demonstrates great improvement on Cisco software, so this is of great value to us. We have some improvements upon the base work to improve the compile time.