In rG1e353f092288309d74d380367aa50bbd383780ed#1058093, @lebedev.ri wrote:

In rG1e353f092288309d74d380367aa50bbd383780ed#1058048, @maxim-kuvyrkov wrote:

Hi Roman,

FYI, your patch series “[SimplifyCFG] Start redesigning FoldTwoEntryPHINode()” improves performance of 470.lbm at -O3 by 20% — awesome job!

It would be great if this speed up would be preserved in your reworked patch.

Apologies, that was not intentional.

Hi Roman,

Looks good to me. Thanks!

Could you elaborate in the commit log about the false positives?

Update commit log message

Add 2-stage bot as well. These are running as expected at
http://ex40-01.tcwglab.linaro.org:8013/#/workers .

Please ignore this. I squashed 3 separate commits into 1.

@mstorsjo
Hi Martin,

In D99573#2658248, @awarzynski wrote:

With this change the corresponding configurations will depend on the worker's internal set-up to use the desired compiler. IMO that's totally fine, provided that:

• linaro-aarch64-flang-latest-clang picks at least clang-11, and
• linaro-aarch64-flang-latest-gcc picks at least gcc-10.

But it is hard for me to review/check this without knowing the configuration of your workers. I'll defer to @maxim-kuvyrkov .

Hi Tom,

This was reviewed in https://reviews.llvm.org/D98493 and https://reviews.llvm.org/D98862.

NFC: Make clang-format happy

This is now squashed into https://reviews.llvm.org/D98935

Hi @amyk , thanks for testing -- I've updated two more places, which I missed on the first go. Would you please test the new version?

In D98493#2636616, @phosek wrote:

They're not part of buildbot but they're not internal, you can see the log here: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8852352747956966400/+/u/clang/test/stdout.

I noticed D98862, but it still hasn't landed and given that it's been 24 hours since your change landed and anyone who is using CLANG_DEFAULT_LINKER is currently broken (I for example use this option in my local build and tests have been failing for me locally as well), I think it's it better to revert this change and then reland it once your fix is ready.

Added more fixes to OpenMP/linking.c and Driver/msvc-link.c .

The only upstream bot that was broken by this change was clang-ppc64le-rhel, and I have notified the bot owner of the problem and said that I'm working on this within 15 minutes of the breakage. I posted a tentative fix for the testsuite several hours later.

This is merged to release/12.x branch.

@SureYeaah , does this fix your internal buildbot?

In D98493#2634107, @SureYeaah wrote:

We are seeing some internal breakage in google because CLANG_DEFAULT_LINKER is set to ld. It would be nice to make the tests more tolerant.

Can we revert this patch until then?

In D98493#2634089, @SureYeaah wrote:

What if CLANG_DEFAULT_LINKER=ld?

This patch broke clang-ppc64le-rhel builder. The patch makes MSVC-compatible driver honour CLANG_DEFAULT_LINKER setting, which this builder sets to “lld”. Therefore there’s an expected change in the output now that CLANG_DEFAULT_LINKER can override link.exe.

The patch was wrong. We use "const Arg *A" at the end of GetLinkerPath, so can't remove it. I thought I tested this, but, apparently, not.

@tstellar , OK to backport to release/12.x if buildbots are happy with this?
I've filed https://bugs.llvm.org/show_bug.cgi?id=49624 with background info.

Fixed thinko: GetLastArg -> GetLastArgValue

Added missing header to pull in definition of CLANG_DEFAULT_LINKER.

I've merged this on Stevan's behalf. Thanks, Stevan!

I've merged this on Stevan's behalf. Thanks, Stevan!

@tstellar
OK to cherry-pick to release/12.x after a couple of days?

Thanks for the reviews

Updated patch to use canonical MSVC syntax

In D96498#2556784, @tstellar wrote:

...

Can you file a bug for this?

Hi Tom,

Looks good to me. It might be prudent to switch comment string from ";" to "//" for 32-bit ARM COFF as well, but that should be a separate change anyway.

@compnerd , fyi, since you requested GNU specialization in https://reviews.llvm.org/D36366 .

In rGb2851aea80e5a8f0cfd6c3c5a56a6b00fb28c6b6#975229, @mstorsjo wrote:

...

Yes, for the aarch64-windows-msvc target, currently semicolon is set as both comment and separator, making no actual char available as separator symbol. I'd suggest changing the comment char to '//' for aarch64-windows-msvc, like it is for aarch64-windows-gnu and ELF targets. I initially tried to do this, but was suggested to only change the -gnu target in the review I linked before. At the time I shrugged and just went with the suggestion, but at the current state I'd suggest revisiting this decision, as there's a clear benefit for making it match the other platforms, and there is no reference for what GAS assembly for aarch64 msvc targets should look like anyway.

And, looking at AArch64MCAsmInfoMicrosoftCOFF, I think we need to do the same for MSVC COFF as for GNU COFF in https://reviews.llvm.org/D36366 -- use "//" for comment, which should return ";" to being statement separator.

In rGb2851aea80e5a8f0cfd6c3c5a56a6b00fb28c6b6#975091, @mstorsjo wrote:

In rGb2851aea80e5a8f0cfd6c3c5a56a6b00fb28c6b6#974873, @maxim-kuvyrkov wrote:

when building LSE builtins. It seems that clang-cl.exe, which, presumably, uses masm parser

A few clarifications here. When clang-cl (for x86) parses assembly, it parses GAS-style .S assembly, including support for GAS macros and all that. MASM (ml.exe/ml64.exe) style assembly is something entirely different, and that's only handled by llvm-ml (which is a work in progress). So there's no real reference for what the MSVC-dialect (triggered by x86*-windows-msvc triples) of GAS assembly would be, as no such thing really exists, so in practice I guess it's just mostly equivalent to the corresponding x86*-windows-gnu ones.

In rGb2851aea80e5a8f0cfd6c3c5a56a6b00fb28c6b6#975095, @tambre wrote:

...

So with that in mind, my preference would be to just make the aarch64-windows-msvc targets behave just like the -gnu ones in this aspect. That was my initial intent when I fixed the comment char for aarch64 windows targets in D36366 (to make semicolon work as separator), but in review it was requested to only make that change for -gnu triples, see https://reviews.llvm.org/D36366#833185. Maybe we should try to revisit that discussion?

Yes. We should backport this to 12.0 before release if possible.
I opened bug 49075 to track this as a release blocker. Would you be willing to take it?

In rGb2851aea80e5a8f0cfd6c3c5a56a6b00fb28c6b6#974874, @tambre wrote:

...

The issue turned out to not be parser bug, but rather the incorrect statement separator being used by compiler-rt on certain platforms.
This workaround was incorrect. While it made the error go away, some of the assembly would stay as comments due to the wrong separator and the code would not work or compile under certain conditions.

Could you provide the full compile command that throws an error during your build?
I should have time to investigate during the weekend.

@tambre
Hi Raul,

Hi Galina,

Thanks!

Any comments?

I've updated buildbot.tac. Thanks, Galina.

@gkistanova
Hi Galina,