addressed comments(removed variable renaming, removed std::move() usage, used std::remove_if() implementation.

renamed variable.

move usage of createFileError at high level.

replaced createStringError with createFileError, renamed variable.

renamed variables.

addressed comments.

addressed comments.

What's the scenario where you run the DWARF linker with already-linked binaries?

Among all the attributes that can be attached to an argument of a tail call, byval is unusual. It significantly changes the semantics of the call: when the call executes, instead of passing the pointer itself, it makes a copy of the data behind the pointer.

Consider, for example, the following testcase on x86-64:

typedef struct A { long long x[100]; } A;
A global;
int printf(const char*, ...);
void dostuff(A arg, int i) { if (i==100) return; arg.x[5]++; printf("%lld\n", arg.x[i]); dostuff(global, i+1); }
__attribute((optnone)) int main() { dostuff(global, 0); }

dostuff has a byval argument. At the point of the call, the compiled code is supposed to copy that argument onto the stack, so in the callee the stack contains the values of all the array elements. If this is working correctly, the fifth line of the output will be "1": each call copies the value from the global, so we only ever increment the array element once. (With optimizations enabled, clang currently gets this wrong.)

Do we actually have any non-fatal errors in llvm-objcopy etc currently?

yes. there are some. f.e. CopyConfig.cpp::parseObjcopyOptions uses ErrorCallback to decide whether the error would be fatal or non-fatal.
I can leave this callback for places where it is already used and convert interfaces into Expected and Error usage in other places.

fixed several minor mistakes.

@ayermolo Hi Alexander, I rebased on latest but did not check whether current size/performance numbers match with reported previously.

rebased.

@RKSimon @craig.topper Would you mind to take a look at this review, please?

ping.

rebased,
retested(bootstrap with check-all),
added i386/i686 triples into X86/x32-va_start.ll,
regenerated tests with update_llc_test_checks.py.

@efriedma would you mind to take a look at this patch once more, please?

cure tests.

updated with recomendations from the thread.

The link to the llvm-dev with the proposal - http://lists.llvm.org/pipermail/llvm-dev/2020-August/144579.html

ping.

Do we need better i686- triple testing?

ping.

Do we need better i686- triple testing?

That is a fair argument for the "1" approach for .debug_ranges (although I doubt in practice it makes any difference). It doesn't really help .debug_loc though, since the behaviour there is currently broken in BFD land.

However, we can use ANY other single value (1, 2, 123456, -2 etc) except -1 which also has a special meaning, because semantically, those values all result in an empty range (-2, -2)/(1, 1) etc. There is no need for special handling within consumers for any given value in this case (or at least there shouldn't be), and thus the specific BFD-identical value for .debug_ranges shouldn't matter.

I'll wait and rebase after D82085.

Thank you, for the review.

addressed comments:

addressed comments.

@efriedma What do you think about current state of this patch? Is it OK?

rebased.

ping.

ping.

removed early check for TRE candidates from canTRE().

check valid TRE candidate into findTRECandidate()().

removed usages of AllocaDerivedValueTracker from canTRE().

addressed comments:

Already integrated by this commit - https://reviews.llvm.org/rGa04ab2ec08014d36337d686d43ecef7668622f1b

1. deleted code doing more strict tailcall marking.
2. left removal of "AllCallsAreTailCalls".
3. added check for non-capturing calls while tracking alloca.
4. re-titled the patch.

abandoned in favor D81784

It makes sense to teach tail recursion elimination not to depend so heavily on tail markings. But I don't think that implies we want to mess with the markings themselves.

It's OK to set "tail" on any call that satisfies these requirements (from https://llvm.org/docs/LangRef.html#call-instruction): "Both markers [tail and musttail] imply that the callee does not access allocas from the caller. The tail marker additionally implies that the callee does not access varargs from the caller."

I am happy with it. There is a small comment inside. other than that lgtm.

ping.

addressed comments.
(not used yaml, since there is a request to use asm.
added tests for various architectures, not added test for COMDAT
sections yet)

would address all comments and rewrite test case with yaml.

removed backward compatibility option.

It looks like most common opinion is to not to create an option. Will remove it and update the patch.

Case 2 should not occur, I think; the compiler should not produce a range where low/high point to different sections.

Yeah, I just worry then it becomes another source of inconsistency between consumers/producers/etc. Perhaps we could name it in some way that's clear that it might be removed later - to encourage people to either reach out to us to explain their use case, or to invest a bit more in working with the new default behavior.

I'd like to avoid adding a flag at all, if possible.

Do we have agreement on following option syntax;

ping