Lgtm

rebased

Usually when you have these kinds of things they're called include and exclude.
I don't think we have any known use cases for include, just for exclude so it would be sufficient to start with just that feature.

LGTM but Haowei should give the final say.

lgtm except for possible susceptibility to invalid DT_STRSZ values.
Perhaps there should be a test case for a corrupted PT_DYNAMIC region.

lgtm

I think in fact the proper thing for this case is to just use a single operand "+t" to directly indicate that the one operand is both input and output. I'm not sure that "=&t" and "0" will behave any different, but this seems to be exactly what "+" is for.

In D118099#3269740, @abrachet wrote:

In D118099#3268320, @sivachandra wrote:

That a compiler unnecessarily requires the "cc" clobber specification is OK but GCC requiring the input operand to have the "f" modifier instead of the "t" modifier seems not so correct. Should we really care about the GCC problem now?

Ok I've committed without the changes to sqrtl

In D117933#3267443, @MaskRay wrote:

Currently a deplib name (${name}) indicates either the literal ${name} or the -l style lib${name}.{a,so}.
We have a conflict if ${name} is foo and there is a directory named foo.

What if we do

if (name.endswith(".a") || name.endswith(".so"))
  findFromSearchPaths
else
  searchLibraryBaseName

?

That should avoid the conflict, too. A directory is unlikely named .so or .a.

lgtm

Please understand that InstrProfData.inc is now a public installed header file in the compiler-rt installation, at <profile/InstrProfData.inc>. So *any* changes to this file have a quite high standard of compatibility concern.

lgtm

Ping. This is very similar to the other change I landed recently. It needs approval from a libc++ maintainer.
Thanks.

LGTM modulo the fallback behaviors as mentioned below. But I'll leave the approval to those who have done the review for this specific codebase more generally, since I'm not sure on all the points of LLVM style or what library features are available or best to use.

Just the fact that any follow-up changes were required is clear testament to the fact that this was never a change that was appropriate to commit without community review and approval as all LLVM changes require. Regardless of the technical details, nontrivial changes, especially ones affecting the user-visible semantics, simply should not be committed without proper review. Resistance to reverting unreviewed changes is simply hostile to the community's established standards and code of conduct and frankly does not merit further technical discussion. The appropriate forum for technical discussion is in review of changes such as these *before* they are committed. The appropriate action right now is to revert all the changes that were not properly reviewed and changed behavior in ways that reviewers have not yet had the opportunity to discuss. Once the damage done has been recovered, then it's appropriate to propose the changes you wanted made and allow proper review and discussion about the semantics being changed.

This feature seems like a good addition. I'm wondering about the interactions with other switches and if it might make sense to rearchitect some of the CLI API here for the more arcane corners. Maybe it's already orthogonal enough, but I'm not sure. I'm thinking in particular about the various --strip-* options. Perhaps it's the case that all such variations apply only to text IFS output? I'm also wondering if there might be use cases for multiple different kinds of IFS output (i.e. different sets of stripping / arch options). Come to think of it, we could even have uses for multiple ELF stub outputs from the same IFS input with different switches (e.g. arch). So perhaps it makes sense to go in a direction where each output file is separately described with both its format (IFS vs ELF) and its format options (stripping, arch, etc), and there can be any number of such output files in whatever combination. I'm not sure exactly what a CLI syntax for that would look like. Another possibility that might fit well for build system integration is to accept a complex output specification in JSON either as a command-line switch value or from a file.

lgtm, but we should be sure to advise users that their debugging tools may need build adjustments to ensure they support the compressed formats.

lgtm, but we should be sure to advise users who may need to use -gdwarf-4 explicitly if their debugging tools become unhappy.

renamed callback

In D114383#3160468, @phosek wrote:

An alternative I thought of would be to always use a call and generate a (weak) implementation of __llvm_profile_counter_dynamic_bias inside each translation unit. The default generated function would simply return the variable. I'm not sure what the impact on performance would be though.

second copy of InstrProfData.inc

Use a DenseMap rather than instruction name to match bias load.

Reordered the switches.

reordered switches

rebased

add more casts

The intent is to sanity-check for garbage header fields. The uncompressed size field is one that you can only warn about heuristically like this I guess. But you can do a definitive test on the CompressedSize field to ensure that it isn't larger than the actual size of the data available. There's no need to even worry about whether UncompressedSize is valid if CompressedSize is clearly invalid.

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm