In D90948#2578843, @kripken wrote:

I think this broke the LLVM roll on Emscripten CI. I don't have time to bisect atm, but this is the only wasm-related commit in the range, and the compiler crash is on a file and function modified in this patch:

We're seeing errors about duplicate gcov symbols in Rust code after this change: https://bugs.llvm.org/show_bug.cgi?id=49226

coff-dwarf.test started failing after this commit: http://lab.llvm.org:8011/#/builders/109/builds/8398

This commit broke the build on aarch64 because OriginAddr() isn't defined on that platform, could you please take a look?

Thank you for the quick followup!

This broke the windows bot: http://lab.llvm.org:8011/#/builders/86/builds/7540/steps/6/logs/stdio

It does feel kind of awkward to me that _this_ is the patch that ends up breaking the builds, versus something at the cmake level that says "you are explicitly unsupported".

If I'm reading git correctly, the change is still present on the 12.x branch. Should it be reverted there too?

Firefox has a build break from this change. In certain Android configurations we use bfd or gold. The statement in the commit message "Android only supports LLD" is news to me, could you point me to any references for this?

Our bots are green at b43c26d036dc. Many thanks @hvdijk for the quick response.

On closer inspection (sorry, I'm juggling too many things this morning) it seems gcc does give a matching int nArgs = 1 under -std=c++17 mode.

The bot that reported this failure was using: clang -cc1 -triple x86_64-pc-windows-msvc19.16.27038 -fms-extensions -fms-compatibility -fms-compatibility-version=19.16.27038 -std=c++17

We have a downstream build break due to this commit. One of our files has some convoluted arg-counting logic that now returns a different count, which does not match gcc: https://godbolt.org/z/W8caMr (Note: At time of writing, the clang trunk on godbolt doesn't yet have this change.)

In D92215#2517633, @sbc100 wrote:

Out of interest how did you notice this breakage? Are you integrating ToT llvm with wasi-libc for a specific reason?

In D92215#2506002, @wingo wrote:

Yeah as @sbc100 says, this is intended -- object files now can have table symbols, and if the compilation unit takes the address of a function or makes an indirect call, it will have a __indirect_function_table symbol. So the expectations need updating; sorry for the bother!

86991d323133 looks good from our side, thank you!

In D93764#2515449, @fhahn wrote:

One issue with the file was that the patch did not account for atomic loads and would add additional atomic loads. I fixed that in 86991d323133. Please let me know if that fixes your issue. Otherwise I'll revert the patch to investigate further.

To give you an early heads up, we're seeing crashes in Firefox after this commit. Unfortunately it's only happening on Mac, which I am not equipped to debug. I may need to try enlisting a colleague to investigate and/or throw more test suites at it and hope we can capture something on Linux or Windows.

In rG4b574008aef5a7235c1f894ab065fe300d26e786#971109, @rsmith wrote:

Should be fixed in 18e093faf726d15f210ab4917142beec51848258. Sorry I
overlooked your comment on the previous landing of this change.

In case the reduced file from the other bug is insufficient, here's the full-size repro:
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/G5HdGFm8Tx-NLTTMnO-2vw/runs/0/artifacts/public/build/Unified_cpp_js_src_wasm2-ff66ce.cpp
and
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/G5HdGFm8Tx-NLTTMnO-2vw/runs/0/artifacts/public/build/Unified_cpp_js_src_wasm2-ff66ce.sh

@zygoloid: we're seeing the same break with this as reported in the first landing: https://reviews.llvm.org/rG9e08e51a20d0d2b1c5724bb17e969d036fced4cd#966941 could you please take a look?

Here's the log: https://firefoxci.taskcluster-artifacts.net/U_L_Cc_QRxiRefZVNDs8KQ/0/public/logs/live_backing.log

Hi, after this commit our builds of wasi-sdk have broken because wasi-libc's makefile [1] finds an undefined symbol for __indirect_function_table that wasn't in its expected list [2].

Confirmed this fixes our original full-size repro, thanks! I'll let those better knowledgeable than me review the change though.

This commit caused a crash on our bots with a null LLVMTy at changeExtendedVectorElementType. Reduced case: https://godbolt.org/z/7ehffx

I tried the updated patch against our build and it was successful. Thanks!

Before the revert, our bots hit the following issue where we only error out when -Wall is given, so there's definitely something strange going on. Also happens with @Quuxplusone's suggested change applied.

I see that this has been reverted, but while it was in tree, we had some errors on our Windows bots with a different symptom than Chrome's.

Confirmed our bots are still OK after the re-landing. Thanks!

Heads up: the tests are failing on the bots http://lab.llvm.org:8011/#/builders/109/builds/4482/steps/6/logs/FAIL__LLVM__calling-conv-sext-zext_ll

A fix for this is pushed now in https://sourceforge.net/p/mingw-w64/mingw-w64/ci/660e09f3cb20f181b6d6435cb623d65a3922a063/.

Thanks a lot! Glad to see it's fine with a value of 1.5. (We have been setting it to 2 in our build as a temporary workaround)

In D91379#2427960, @mstorsjo wrote:

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

In D91379#2426975, @ldionne wrote:

In D91379#2423372, @dmajor wrote:

Our mingw builds are failing with error: no member named '_aligned_malloc' in the global namespace. Should the new header have an #include <malloc.h> somewhere?

That's weird, as we were not including <malloc.h> anywhere before either. Can you confirm that including <malloc.h> fixes your issue? I'm wary of adding non standard includes in our headers, since it causes all kinds of problems.

To connect the dots for posterity: the followup was landed in D92349.

Our mingw builds are failing with error: no member named '_aligned_malloc' in the global namespace. Should the new header have an #include <malloc.h> somewhere?

In D91747#2412346, @alanphipps wrote:

Looks like _LIBCPP_HAS_NO_THREADS is being set for libcxxabi, and the build now fails with this change:

llvm-project/libcxxabi/../libcxx/include/__config:1172:2: error: _LIBCPP_HAS_NO_THREADS cannot be set when STDCPP_THREADS is set

In D81682#2331245, @yamauchi wrote:

It seems that I can't reproduce it. Here's my script. Can you reproduce with it or modify it so it reproduces?

Firefox tests look good as well.

In D91330#2394415, @kadircet wrote:

Looks like it is already green at http://lab.llvm.org:8011/#/builders/109/builds/2693

This broke the bots: http://lab.llvm.org:8011/#/builders/109/builds/2682

Our bots are failing on the new assertion "The number of edge probabilities must match the number of successors." during PGO builds of clang.

I'm very happy to see this land, thank you for writing it!

Oh, I didn't realize that Rui was inactive... trying rnk instead, based on recent commits under lld/COFF/.

After 10f2a0d662d8 our self-host PGO builds are hitting:
llvm::BranchProbability::getBranchProbability(uint64_t, uint64_t): Assertion `Numerator <= Denominator && "Probability cannot be bigger than 1!"' failed.

This commit triggered an assertion failure on our 32-bit bots:

In D89088#2363206, @Carrot wrote:

@dmajor and @aeubanks, could you help to see if the following patch can fix the crash when combined with patch adfb5415010f?

In D89623#2361288, @nikic wrote:

@dmajor If the call slot optimization applies, then that load/store pair should also get dropped as part of the optimization. I don't immediately see how we can end up both changing the call argument and not dropping the load/store, if I understand right what is happening in your case.

I thiiiink I'm starting to see this a little better, and the problem is that the memcpy clobbers the optimized call slot.

Hi, we have a regression that started with this commit. The problem happens when GEP %43 is moved before the call on line 49: https://paste.mozilla.org/gYqBnKes . Can you see anything obviously wrong with that move?

After this commit our bots have a failure on the wasm target, could you please take a look?

In case it helps, I turned on debug logging for this pass on our repro: https://paste.mozilla.org/HBDkeGXO (starts at line 60)

Could you help to check when block bb.0 is moved below bb.2?

Adding @jmorse in case D83046 is related.

Unfortunately I don't have my git account handy either. I'm fine if this waits until tomorrow.

This is causing an assertion failure in our stage2 builds, could you please take a look? https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=319157722&repo=ash&lineNumber=9931 The reproducer artifacts can be obtained by clicking "Show Job Info" in the top bar.

Just a quick status update, I don't reproduce the issue with your script. I'm attempting to adjust the script to match our CI, but I'm distracted by other work.

This caused a regression when calling through function pointers in a certain way as seen in sqlite3: https://bugs.llvm.org/show_bug.cgi?id=47905

Hi, this commit caused an assertion in one of our builds:

/builds/worker/fetches/llvm-project/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp:1669: bool (anonymous namespace)::VarLocBasedLDV::join(llvm::MachineBasicBlock &, (anonymous namespace)::VarLocBasedLDV::VarLocInMBB &, (anonymous namespace)::VarLocBasedLDV::VarLocInMBB &, const (anonymous namespace)::VarLocBasedLDV::VarLocMap &, SmallPtrSet<const llvm::MachineBasicBlock *, 16> &, SmallPtrSetImpl<const llvm::MachineBasicBlock *> &): Assertion `(NumVisited || MBB.pred_empty()) && "Should have processed at least one predecessor"' failed.

It's an LTO+PGO build of Firefox so reproducing it is very time-consuming. I will try to get more info but it may take time and I wanted to give you an advance notice. In the meantime is there anything you can guess by inspection about what went wrong?

It may actually be that -vp-counters-per-site needs adjusting. How do you reproduce?

In D81682#2168134, @yamauchi wrote:

In D81682#2168028, @dmajor wrote:

The re-landing of this patch in 4a539faf74b9b4c25ee3b880e4007564bd5139b0 causes our PGO build of clang to have thousands of

LLVM Profile Warning: Unable to track new values: Running out of static counters.  Consider using option -mllvm -vp-counters-per-site=<n> to allocate more value profile counters at compile time.

Is that expected? Should the cmake set a higher value of -vp-counters-per-site in self-hosted PGO builds?

This has been reverted for now. There was a related issue.

The build bot is happy with Diff 6! \o/

[EDIT: This is specific to the armv7-linux-android target]

More errors: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318436438&repo=ash&lineNumber=15974
(Please note that there are two different files mentioned -- sanitizer_linux.cpp and sanitizer_linux_libcdep.cpp)

Latest errors: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318424891&repo=ash&lineNumber=23010

The v2 patch still fails with

In D85927#2325276, @oontvoo wrote:

Thanks for the report!

In D85927#2325138, @dmajor wrote:

This caused a build break in our compiler-rt build for Android: use of undeclared identifier '__interceptor_memalign'

Are you also building with API 16? (From your second comment)

And later we also get use of undeclared identifier 'dl_iterate_phdr' in (EDIT:) lsan_common_linux.cpp. We compile for API 16, which doesn't implement that.

This caused a build break in our compiler-rt build for Android: use of undeclared identifier '__interceptor_memalign'

This caused a failure on the bots: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/38144

The bots are happy, thanks.

Thanks. I get the impression that libc++ has generally been more aggressive about raising requirements than LLVM overall. My org's crusty and underowned build has been getting caught up in that, so anything that you could codify as an upfront enforcement would be appreciated and probably lead to me bugging you less. =)

In rG3e5f9dacb092a1414f72500111c2b049673e0055#950737, @ldionne wrote:

Can you move to the runtimes build, which builds libc++ with the just-built Clang instead of with whatever compiler happens to be on your system?

This appears to have broken our stage1 build which uses gcc 7:

This change broke the configure step of Firefox mingw builds.

The expensive-checks bots have been red for several days. Could you please take a look or revert?

I threw as many tests as I could find at 9d172c8e9c84, and I don't see any regressions compared to its parent revision.

Thanks, this fixes the specific translation unit I was looking at today. It's going to take some time before I have full build results but I don't expect any problems.

This commit broke Firefox builds on Mac with an error in the SDK headers. Could you please revert if a fix is not readily available?

Thanks for the heads up. I'll schedule a more intensive test run over the weekend while there's less demand for machines.

This broke Firefox builds too, in one of our helper binaries that uses a wWinMain, although I'm having trouble writing a minimal reproducer for it. Simply making a barebones wWinMain program doesn't hit the error.

In D87163#2275896, @asbirlea wrote:

I checked in a fix in https://reviews.llvm.org/rGfc8200633122, but I haven't yet verified it addresses all the failures reported.

Reduced a bit more: https://godbolt.org/z/j59evK (C++) and https://godbolt.org/z/8xG688 (IR) -- the store at line 43 of while.end has been removed.

Here is a reduced-ish repro, I can keep poking at it but maybe this is sufficient for you to notice something: https://godbolt.org/z/endf6d.

In D87163#2272105, @fhahn wrote:

I also pushed a fix for a MemorySSA issue that was exposed through DSE + MemorySSA in c4f1b3144184. This might also fix (some) of the failures you are seeing.

In D87163#2270871, @fhahn wrote:

In D87163#2270444, @thakis wrote:

Heads-up: We're seeing fairly widespread test failures with this in Chromium that go away when we force this flag off. It's possible that it's due to only a small number of issues and they might be previously-benign UB (no idea), but I figured I'd give an early note. We'll send an update once we've reduced a failing test.

Thanks for the heads-up. I just pushed another fix for the issue @uabelho. I'd guess at least some of the failures you are seeing should be related. If not, I think it makes sense to revert this once we have the reproducers and can work in ironing out the remaining issues.

Our builds are back to green with that change. I agree that the root cause is likely elsewhere, but getting to the bottom of it would unfortunately take more time than I am able to commit.

Yeah, I wish I could give a nice test case, but it's a huge build and the affected configuration needs a lot of setup so I can't even replicate it locally at the moment.

Hi, we're seeing an assertion failure in one of the Firefox LTO bots:

char llvm::StringRef::operator[](size_t) const: Assertion `Index < Length && "Invalid index!"' failed.

I'm still working on finishing the bisection but I have a hunch that it may be this change. Unfortunately I can't seem to get a backtrace out of this particular bot.

We're seeing a break in the Firefox build after this change:

WidenVectorResult #0: t29: v2i32 = rotl t25, t28, /builds/worker/checkouts/gecko/gfx/angle/checkout/src/image_util/loadimage.cpp:640