Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for LLVM 13 & bump LDC-LLVM to v13.0.1 #3842

Merged
merged 9 commits into from
Feb 4, 2022

Conversation

kinke
Copy link
Member

@kinke kinke commented Oct 3, 2021

No description provided.

@kinke kinke force-pushed the llvm13 branch 3 times, most recently from 797d1c7 to 2eee309 Compare October 6, 2021 17:54
@kinke kinke force-pushed the llvm13 branch 3 times, most recently from a2303d5 to e16fce8 Compare October 11, 2021 13:37
@kinke
Copy link
Member Author

kinke commented Oct 11, 2021

Issue 1 - atomic lib calls (edit: fixed)

Some atomic ops on x86 seem not to be lowered to instructions anymore, but to library function calls. At least on Linux i686 and MSVC x64. E.g., this:

void main()
{
    import core.atomic;
    shared long* ptr;
    cas(ptr, 0L, 1L);
}

newly fails to link on Linux without libatomic using ldc2 bla.d -mtriple=i686-linux-gnu, even with an explicit -mattr=+cx8.

Issue 2 - cloning debuginfos (edit: fixed)

New assertions when cloning a function with debuginfos for the MSVC @weak emulation. The old clear-subprogram workaround isn't sufficient anymore:

ldc/gen/functions.cpp

Lines 990 to 991 in 3902850

// work around LLVM assertion when cloning a function's debuginfos
func->setSubprogram(nullptr);

I've tried the built-in LLVM weak functionality (I don't know any details, but there seems to be some support now), but our codegen/attr_weak.d then fails (weak function not overridden by strong one in explicitly linked object file).

Issue 3 - lit-test regressions

  • PGO, e.g., PGO/indirect_calls.d, possibly more on Windows
  • compilable/no-integrated-as.d on Linux, possibly depending on a more recent as version for CI

@kinke kinke force-pushed the llvm13 branch 2 times, most recently from a8795d5 to 73fc56e Compare October 11, 2021 19:26
@kinke
Copy link
Member Author

kinke commented Oct 11, 2021

[Preliminary support extracted to #3846.]

@JohanEngelen
Copy link
Member

JohanEngelen commented Oct 12, 2021

The atomic lib calls is perhaps caused by not satisfying alignment requirements (we should explicitly specify them?) ?
https://clang.llvm.org/docs/Toolchain.html#atomics-library

Some additional info about alignment here: https://reviews.llvm.org/D47589

@kinke
Copy link
Member Author

kinke commented Oct 22, 2021

I'd need to test it, but according to the v13 docs, the default alignment is the pointee size; from https://releases.llvm.org/13.0.0/docs/LangRef.html#cmpxchg-instruction:

The instruction can take an optional align attribute. The alignment must be a power of two greater or equal to the size of the <value> type. If unspecified, the alignment is assumed to be equal to the size of the <value> type. Note that this default alignment assumption is different from the alignment used for the load/store instructions when align isn’t specified.

gen/tocall.cpp Outdated
@@ -511,7 +511,7 @@ bool DtoLowerMagicIntrinsic(IRState *p, FuncDeclaration *fndecl, CallExp *e,
auto ret =
p->ir->CreateAtomicCmpXchg(ptr, cmp, val,
#if LDC_LLVM_VER >= 1300
LLMaybeAlign(getABITypeAlign(val->getType())),
llvm::MaybeAlign(), // default alignment
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the alignment regression - e.g., long.alignof is 4 for 32-bit x86...

@kinke kinke force-pushed the llvm13 branch 3 times, most recently from cbea0c8 to 80ecb88 Compare November 13, 2021 02:38
@kinke kinke force-pushed the llvm13 branch 2 times, most recently from 3220ddf to 8a86ce5 Compare November 28, 2021 17:51
@kinke
Copy link
Member Author

kinke commented Dec 19, 2021

The remaining failures on Linux are fixed by switching from ld.gold to lld v13, on my box at least. So this seems to be the inverse of #3861 - LLVM 13 causing ModuleInfo-related troubles with non-lld linkers. I hope this will be fixed by #3850 too.

@kinke
Copy link
Member Author

kinke commented Dec 19, 2021

Seems like it, I've got a single std.experimental.allocator.building_blocks.allocator_list-shared_32 failure with ld.gold when merging #3850.

@kinke kinke force-pushed the llvm13 branch 2 times, most recently from 6971184 to 1de7d99 Compare December 21, 2021 04:53
@kinke
Copy link
Member Author

kinke commented Jan 13, 2022

Rebased; still blocked

  • on Win32 by hitting an LLD 13 assertion (bounds check) when trying to link the LDC unittest executable (with LTO IIRC) with the LDC bootstrap compiler
  • on Linux with ld.gold (lld is fine); apparently much worse with Ubuntu18's gold than with my Ubuntu20's.

@kinke
Copy link
Member Author

kinke commented Jan 15, 2022

I could reproduce strange dmd-testsuite failures with gold on Ubuntu20. Merged current LLVM release/13.x head (13.0.1-rc2+), and 32/64-bit dmd-testsuite is green, so looks like this apparently serious LLVM 13 issue was fixed in the meantime.

@kinke
Copy link
Member Author

kinke commented Jan 15, 2022

Hmm, the vanilla 13.0.0 GH Actions job now seems to pass consistently with its gold linker. The CI image version hasn't changed, the runner has - no idea.

Edit: Oh and the Win32 lld regression was apparently fixed by the latest bump too.

@kinke kinke marked this pull request as ready for review January 15, 2022 19:09
@kinke kinke changed the title Add support for LLVM 13 & bump LDC-LLVM to v13.0.0 Add support for LLVM 13 & bump LDC-LLVM to v13.0.1 Feb 4, 2022
To work around the inconsistent CI results with vanilla LLVM v13.0.0
and the ld.gold linker. There is no vanilla v13.0.1 package yet.
@kinke
Copy link
Member Author

kinke commented Feb 4, 2022

The GH Actions LLVM 13 job with old ld.gold is driving me somewhat nuts. Yesterday, it went from green to red inside an hour without any real change. I've then switched from vanilla v13.0.0 to just-released LDC-LLVM v13.0.1, and the first attempt led to some (different) failures, incl. something hanging for a long time. Retried today, green. So these failures seem totally indeterministic.

I'll go ahead and merge this now that v13.0.1 is available; if the job failure is frequent, I'll do something about it.

@kinke kinke merged commit 93b1b8e into ldc-developers:master Feb 4, 2022
@kinke kinke deleted the llvm13 branch February 4, 2022 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants