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,

Galina, Diana, any other feedback? Or shall I commit the patch?

For reference, here are testing builds:

• http://ex40-01.tcwglab.linaro.org:8011/builders/clang-arm64-windows-msvc
• http://ex40-01.tcwglab.linaro.org:8011/builders/clang-arm64-windows-msvc-2stage

LGTM.

Hi @mkazantsev ,
Linaro benchmarking CI flagged this patch as increases code-size of SPEC2k6's 401.bzip2 by 3% on ARM (Thumb2 mode) and by 5% on AArch64. This happens at -Os -flto.

Hi Sam,
Linaro benchmarking CI flagged this patch as it regresses SPEC2k6's 462.libquantum by 10% at -O2, -O3 and "-O3 -flto" for aarch64-linux-gnu. Impact on other benchmarks is below 2%, which is within noise.

Quite surprisingly, this patch increases code size by 3-4% on several SPEC CPU2006 benchmarks for arm-linux-gnueabihf at -Oz optimization level:

1. 473.astar,astar_base.default regressed by 103
2. 450.soplex,soplex_base.default regressed by 103
3. 483.xalancbmk,Xalan_base.default regressed by 104

It's runtime failure and occurs on "ref" input. Compilation flags are
-O3 -marm --target=armv7a-linux-gnueabihf --sysroot=/path/to/build/sysroots/arm-linux-gnueabihf
and linking flags are (note that I'm linking with LLD).
-Wl,--build-id -Wl,-dynamic-linker=/path/to/run/sysroot/lib/ld-2.31.9000.so -Wl,-rpath=/path/to/run/sysroot/lib -Wl,-rpath=/path/to/run/sysroot/usr/lib -O3 -marm --target=armv7a-linux-gnueabihf --sysroot=/path/to/build/sysroots/arm-linux-gnueabihf -fuse-ld=lld

@MaskRay , hi. This patch miscompiles 471.omnetpp and 453.povray on arm-linux-gnueabihf. Would you please investigate?

This commit 4698bf145d583e26ed438026ef7fde031ef322b1

Author: Kazu Hirata <kazu@google.com>
Date: Wed Feb 5 08:24:01 2020 -0800

Hi Nikita,

Hi @kazu ,
This patch increases code-size of 444.namd by 1% on arm-linux-gnueabihf:

• when compiling with -Os: from 171653 to 173645
• when compiling with -Os -flto: from 137772 to 139708

Could you take a look if these regressions can be easily avoided?
Let me know if you need help reproducing the regressions. I have pre-processed source and generated assembly handy.
Thanks!

Hi John,

Looks good.