More tagging functions as artificial.

rL370219 and rG3c307370c8f8

In D66556#1648591, @hans wrote:

In D66556#1648118, @dexonsmith wrote:

In D66556#1648109, @rnk wrote:

I'm not sure what happens, but I see you added .gitattributes. I'd commit it as is. Buildbots using svn will keep working. You can check that the monorepo has the right line endings afterwards, and try again if not.

SGTM.

This broke users of the monorepo, where the file would show up as having changed locally, and with no way to reset it. I'm guessing that's because it was checked in with LF endings and then because of the .gitattributes file, it changes at checkout. I think the correct solution would be to check in the file with CRLF endings and not set any attributes or stuff. (Though this is all super confusing and I might have got it wrong.)

I've deleted the test file in r370175 to unblock development in the meantime.

In D66556#1647669, @arphaman wrote:

Will the git monorepo handle svn:eol-style correctly?

Other files in the repo already use svn:eol-style. I'm not sure what happens with SVN properties when commits are replicated in git. @jyknight @rnk ? I've added .gitattributes to ensure these files are checked out from git with the proper line ending.

In D66328#1647062, @probinson wrote:

I don't see a test for the __cxx_global_array_dtor case?

Re-opening this because the previous commit broke Chromium.

This failed the build - I've changed unix2dos to svn:eol-style CRLF instead.

In D66431#1645454, @zturner wrote:

Finally got around to trying to commit this.

Creating svn staging directory: (.git\llvm-upstream-svn)
svn staging area ready in '.git\llvm-upstream-svn'
Pushing 1 monorepo commit:
  29ff148f662 [PDB] Fix bug when using multiple PCH header objects with the same name.
`svn update --depth=files lld lld/trunk 'lld/trunk\test' 'lld/trunk\COFF' 'lld/trunk\test/COFF' 'lld/trunk\test/COFF/Inputs' 'lld/trunk\test/COFF/Inputs/precompb' 'lld/trunk\test/COFF/Inputs/precompa'` returned 1
svn: E170013: Unable to connect to a repository at URL 'https://llvm.org/svn/llvm-project'
svn: E175012: Connection timed out

Is git llvm push a thing of the past? Should I be using a new workflow?

Fixed unit tests.

As requested.

Reduced to:

template <class, class = int> class a;
class b {
public:
  b(char *);
  ~b();
};
template <class, class> class a {
public:
  ~a();
};
struct {
  b c;
  b f;
  const a<a<float>> &d;
} e[]{"", "", {}};

Will take a look.

In D66431#1642619, @thakis wrote:

Is this only for cl.exe-created pch files? Those create special obj files iirc. Or does it affect clang-cl-built pch obj files as well? (If it's cl.exe-only, please mention that in the patch description.)

Yes, this code is only for MSVC PCH.OBJ files. Clang does not generate LF_PRECOMP records.

Please see the other reviews I've sent.

Use proper test file.

LGTM, just a missing include (see below).

Thanks! It somehow got lost in translation :-( I am not receiving bots notifications anymore, anything I should do?

Thank you!

In D63648#1593619, @thakis wrote:

• Does fastbuild have something like distcc-pump?

Fastbuild works like plain distcc and unfortunately it does not have pump mode.

As requested.

In D66431#1636198, @zturner wrote:

Ugh nevermind, you can't because of the binary files. Fineeeeee.

Thanks for fixing this Zachary! Our precomp files are named after their project, ie. Engine.pch.obj

LGTM!

In D65906#1622209, @arphaman wrote:

@aganea These are not just any invisible characters that you have, this is the UTF8 BOM. Clang's Lexer skips over them if they're in the beginning of the file (Lexer::InitLexer). The minimizer should do the same thing, so ideally you would factor out the BOM detection and teach the minimizer to skip past it.

In D65906#1620034, @arphaman wrote:

Regarding the invisible characters before #ifdef, are you sure that's the right behavior?

Should we then copy these invisible characters to the minimized output? Believe it or not, these characters are there in our codebase since 2013, and never clang has complained about it :)

LGTM, thank you!

Thanks for the update Alex! Just a few more comments and we should be good to go:

I will take a look next week when I get back!

It totally makes sense, thanks for the explanation Nico! Let's forget about cl compatibility, that wasn't my initial intent.

The problem is that it is not cl.exe's behavior - it always makes paths absolute:

F:\svn\test>cl /c test.cc /showIncludes
Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Thanks for getting back Hans!

In D52193#1589096, @yurybura wrote:

Do you have any plans to make this PR compatible with trunk? Now MSVC with /MP builds much faster than clang-cl (at least 2x faster for our project)...

I'll get back to this after the vacation (somewhere in August)

Ping!

In D64301#1574304, @rnk wrote:

I think @zturner wanted to teach lit to completely remove the Output/ directory for every test so we don't have to deal with stale files from previous tests hanging around. I remember objecting that we can't do that on startup since it's slow and not parallelized. However, I think we could probably add it as an early step to TestRunner.executeShTest so we don't have to add these extra 'rm' commands anymore.

Just a reminder, the DeleteFile API, and thus rm on Windows are async. See this. Occasionally, the tests are failing locally on my PC because of that (maybe 1 out of 50 times).

A bit more detail on what we're seeing on our end (specs in the post above). The 'Count' column represents the number of 1ms samples taken in that function. The 'Weight' column is cumulated times for all cores, for a given process, in ms.