Skip to content

Conversation

@bnbarham
Copy link
Contributor

@bnbarham bnbarham commented Apr 1, 2024

This set of changes enables the compiler to build, but currently asserts when building the stdlib while creating intrinsics and also when running LLVM's verifier pass.

Verifier failure:

Function debug format should match parent module
ptr @"$s6Darwin7FLT_MAXSfvgTm"
0
; ModuleID = '.../stdlib/swift-macosx-arm64/stdlib/public/ClangOverlays/IOS/arm64e/_Builtin_float.o'
1

ie. the module has IsNewDbgInfoFormat set but the function does not.

Intrinsic assertion:

Assertion failed: (Index < Length && "Invalid index!"), function operator[], file ArrayRef.h, line 257.
...
3.	While evaluating request IRGenRequest(IR Generation for module Synchronization)
4.	While emitting IR SIL function "@$s15Synchronization19AtomicLazyReferenceVfD".
 for 'deinit' (at .../swift/stdlib/public/Synchronization/AtomicLazyReference.swift:31:3)
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
...
5  libsystem_c.dylib        0x0000000183dc9a40 abort + 180
6  libsystem_c.dylib        0x0000000183dc8d30 err + 0
7  swift-frontend           0x00000001074b942c DecodeFixedType(llvm::ArrayRef<llvm::Intrinsic::IITDescriptor>&, llvm::ArrayRef<llvm::Type*>, llvm::LLVMContext&) (.cold.4) + 0
8  swift-frontend           0x0000000105ee1784 DecodeFixedType(llvm::ArrayRef<llvm::Intrinsic::IITDescriptor>&, llvm::ArrayRef<llvm::Type*>, llvm::LLVMContext&) + 744
9  swift-frontend           0x0000000105ee1340 llvm::Intrinsic::getType(llvm::LLVMContext&, unsigned int, llvm::ArrayRef<llvm::Type*>) + 116
10 swift-frontend           0x0000000105ee33c8 llvm::Intrinsic::getDeclaration(llvm::Module*, unsigned int, llvm::ArrayRef<llvm::Type*>) + 48
11 swift-frontend           0x000000010088b470 swift::irgen::IRBuilder::CreateIntrinsicCall(unsigned int, llvm::ArrayRef<llvm::Value*>, llvm::Twine const&) + 84
...

Possibly related to the above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felipepiovezan I assume we probably want to have an iterator bit here?

Comment on lines 3357 to 3368
Copy link
Contributor Author

@bnbarham bnbarham Apr 1, 2024

Choose a reason for hiding this comment

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

@felipepiovezan I assume this is the underlying cause of the crashes and that we should instead use the new (old) intrinsic functions (https://github.com/llvm/llvm-project/blob/main/llvm/docs/RemoveDIsDebugInfo.md):

LLVMDIBuilderInsertDeclareIntrinsicBefore   # Insert a debug intrinsic (old debug info format).
LLVMDIBuilderInsertDeclareIntrinsicAtEnd    # Same as above.
LLVMDIBuilderInsertDbgValueIntrinsicBefore  # Same as above.
LLVMDIBuilderInsertDbgValueIntrinsicAtEnd   # Same as above.

EDIT: No, that's just the C API. It calls the same underlying functions as here but with an extra assertion on top.

@bnbarham bnbarham force-pushed the lets-get-building branch 2 times, most recently from 81f619a to 72f4e00 Compare April 3, 2024 04:38
@bnbarham
Copy link
Contributor Author

bnbarham commented Apr 3, 2024

@beccadax / @egorzhdan I'd appreciate a review on the ClangImporter changes. @aschwaighofer could you take a look over the IRGen ones? And @eeckstein for the SIL/Opt changes. They're all generally pretty small.

@jansvoboda11 I don't think there was anything particularly crazy with the FileEntryRef changes, but a second pair of eyes would be nice. Also the OptTable macros, but again, that seemed pretty straight forward (also, thank you, that is so much better now).

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

ClangImporter changes LGTM, thanks!

Copy link
Contributor

@jansvoboda11 jansvoboda11 left a comment

Choose a reason for hiding this comment

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

The FileEntry to FileEntryRef changes LGTM. So does usage of the new LLVM_*_OPT_* macros, but note that you might need to ensure you use the /Zc:preprocessor flag with MSVC (and maybe bump the minimal supported version): https://reviews.llvm.org/D135128

@bnbarham
Copy link
Contributor Author

bnbarham commented Apr 5, 2024

but note that you might need to ensure you use the /Zc:preprocessor flag with MSVC

Ooo thanks, good to know!

bnbarham added 11 commits April 8, 2024 08:58
`OptTable` was a source of consistent churn due to new arguments to the
`OPTION` macro. LLVM 3f092f37b7362447cbb13f5502dae4bdd5762afd extracted
the handling of the common option parts (eg. an ID and an info) out into
separate macros to reduce this - use those here (since unsurprisingly,
more arguments were added).
LLVM is gearing up to move to `std::endianness` and as part of that has
moved `llvm::support::endianness` to `llvm::endianness`
(bbdbcd83e6702f314d147a680247058a899ba261). Rename our uses.
A bunch of enums were moved to enum classes and slightly renamed. Fix up
their references.
`ThreadPool` was split up in LLVM
6594f428de91e333c1cbea4f55e79b18d31024c4.
`erase_value` is now deprecated.
Ananas/CloudABI/Contiki/Minix have all been removed from LLVM (in
various commits).

Also adds minimal (ie. necessary for compiling) support for the new
triples.
`CommandSetVector::takeVector` returns a `SmallVector` now.
The varargs method was removed in
af74f06322410e867294ea3587e5884342564e5c (claimed NFC) and the
`AddAllArgs` taking an `ArrayRef` was renamed to `addAllArgs` in
4eecfda50a4e7a05f448a59885d2572d0ea2f4a1. But the other `AddAllArgs` was
left as is.
Two new unhandled types: `PackIndexingType` and `CountAttributedType`.
LLVM 9c89b29555a7ccfc3942340f558c3bbea8d10532 changed `mangleTypeName`
to instead mangle the canonical type name and thus also changed the
method name.
bnbarham added 21 commits April 8, 2024 08:58
Presumably this was previously transitively included, who knows from
where.
The constructor takes a new arg which `Create` now handles. Use it
instead.
Removed in LLVM 3a3b84b180278207731451dfac24f47d02b50e84.
142f270c279f2576e4618fc0d1121181c7531fdf fixed a memory leak involving
`APSInt`. It is now returned by value in `getInitVal`.
Renamed in LLVM e90e43fb9cd1d305f7196cd526aa503374e0f616.
Moved from `Sema` to `Token` in LLVM
a8279a8bc541b6027087dd497daa63b6602b7f4b.
…nAttr`

Renamed in our LLVM fork 628ee3b842b1fcc93afdebc646220e8ae6302ed6.
Add corresponding `readTypeCoupledDeclRefInfo` and `writeTypeCoupledDeclRefInfo`.
This was refactored in LLVM 90893a3b3f71524947cb041b2a25d0a02a8956d7,
with `InstrProfiling` becoming implementation details.
LLVM did a bunch of opaque pointer cleanup. Migrate IRGen off the
removed APIs.
Typed pointers are slowly being removed. There's a lot more cleanup to
do here, since really all `IRGenModule::.*PtrTy` should just be `PtrTy`,
but this at least gets us compiling for now.
The various `DIBuilder` inserts now return a `DbgInsertPtr`, update
`DbgIntrinsicEmitter` to as well.
Renamed in LLVM 2d9d9a1a556a5f8845a7a9e19dc52346b825989e.
Added in 9434c083475e42f47383f3067fe2a155db5c6a30, seems to be HLSL
specific.
There was a type overload added to these in LLVM
25bc999d1fb2efccc3ece398550af738aea7d310.
These were consolidated in LLVM in 64da0be1fc06ee2199bd27c980736986e0eccd9d.
This would be done automatically if the module was passed into
`Function::Create`, but we're quite specific about its insert location,
so just set it manually instead.
… list

Originally added in LLVM in 088d272e83259a5d8e577a3d2e62012c42a9f9db
behind a flag, but then the flag was removed in
92eaf036bf22ecc276146cd073208e6a867af8d4.
`BlockT *LoopBase<BlockT, LoopT>::getLoopPreheader()` was changed in
LLVM 7243607867393a2b8ccd477e95e6f62d00f3206f to use `llvm::size` rather
than the checking that `child_begin() + 1 == child_end()`. `llvm::size`
requires that `std::distance` be O(1) and hence `TransformIterator`
(`swift::SILBasicBlock::succblock_iterator` defined as the child
iterator type in the graph traits for `SILBasicBlock *`) support random
access.

The change to `llvm::size` seems rather pointless, so we could probably
also revert that if need be.
@bnbarham bnbarham force-pushed the lets-get-building branch from a4c2c42 to 0950285 Compare April 9, 2024 01:29
@bnbarham bnbarham changed the base branch from next to rebranch April 9, 2024 01:30
@bnbarham bnbarham changed the title Collection of next changes required for the compiler to build Collection of rebranch changes required for the compiler to build Apr 9, 2024
@bnbarham
Copy link
Contributor Author

bnbarham commented Apr 9, 2024

@swift-ci please test

@bnbarham
Copy link
Contributor Author

Going to go ahead and merge to continue along. Thanks for reviewing @jansvoboda11 and @egorzhdan!

@aschwaighofer / @eeckstein when you have the time I'd really appreciate a quick look over 🙏

@bnbarham bnbarham merged commit 7413ecc into swiftlang:rebranch Apr 11, 2024
@bnbarham bnbarham deleted the lets-get-building branch April 11, 2024 19:38
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.

3 participants