[AVR] standard library support for AVR #75127

carlos4242 · 2024-07-10T15:23:42Z

Standard library support for AVR

when compiling embedded cross compile target standard libraries, include AVR
add 16-bit pointer as a conditional compilation condition and get the void pointer size right for gyb sources

@kubamracek this is the first attempt to get the standard library compiling

kubamracek · 2024-07-10T15:34:56Z

@swift-ci please smoke test

test/Parse/ConditionalCompilation/avrTargetWithStdlib.swift

lib/Basic/LangOptions.cpp

stdlib/public/Synchronization/Atomics/AtomicFloats.swift

kubamracek · 2024-07-15T15:58:33Z

@swift-ci please smoke test

carlos4242 · 2024-07-15T22:59:40Z

OK @kubamracek, I addressed a couple of your suggestions, they were all good ideas. I also sent up a small patch on CTypes.swift. Without this, the clang importer imports some int types incorrectly on 16-bit. It's the old thing of what an unsigned int is (16-bit on 16-bit platforms). I'm happy to put this into another PR if you'd prefer.

The last couple of days I've been flexing the muscles of the compiler with this patched standard library and trying to build our core hardware library, which is a good test! This fix came out of that work, along with a load of other cherry-picks from other compiler work I've needed in the past... none of which are suitable for this PR! They will come later, if required.

There are question marks around the synchronisation stuff here. that might be a bit of a hack to get things compiling and I'm not sure it's right.

The big question is around what to do with String. As discussed elsewhere!

carlos4242 · 2024-07-15T23:34:49Z

DoneOn 15 Jul 2024, at 23:06, Kuba (Brecka) Mracek ***@***.***> wrote: @kubamracek commented on this pull request. In stdlib/public/Synchronization/Atomics/AtomicFloats.swift:

@@ -118,7 +118,7 @@ extension Float: AtomicRepresentable {

// Double AtomicRepresentable conformance //===----------------------------------------------------------------------===// -#if (_pointerBitWidth(_32) && _hasAtomicBitWidth(_64)) || _pointerBitWidth(_64) +#if ((_pointerBitWidth(_32) || _pointerBitWidth(_16)) && _hasAtomicBitWidth(_64)) || _pointerBitWidth(_64) Hmm, now I wonder if we could just for the sake of keeping the size of this PR sane, postpone the porting of the Synchronization library (by not building it for AVR with the same CMake logic you use in the Unicode library). Would you mind doing that and then we can follow up on this after we get the stdlib to build? Thanks :) —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

lib/Basic/LangOptions.cpp

test/embedded/avr/testStdlibFunctioning.swift

stdlib/public/core/StringObject.swift

stdlib/public/core/StringGuts.swift

kubamracek · 2024-07-16T04:13:29Z

@swift-ci please test

kubamracek · 2024-07-16T04:15:36Z

I've reviewed the whole PR, I don't have any more comments besides the ones I just added, with those addressed, I think this PR is ready to go, assuming it passes tests. But let's please also give @phausler a chance to review. Great work, @carlos4242 !

- when compiling embedded cross compile target standard libraries, include AVR - add 16-bit pointer as a conditional compilation condition and get the void pointer size right for gyb sources - attempt to fix clang importer not importing __swift_intptr_t correctly on 16 bit platforms - changed the unit test target to avr-none-none-elf to match the cmake build [AVR] got the standard library compiling in a somewhat restricted form: General - updated the Embedded Runtime - tweaked CTypes.swift to fix clang import on 16 bit platforms Strings - as discussed in https://forums.swift.org/t/stringguts-stringobject-internals-how-to-layout-on-16-bit-platforms/73130, I went for just using the same basic layout in 16 bit as 32 bit but with 16 bit pointers/ints... the conversation is ongoing, I think something more efficient is possible but at least this compiles and will probably work (inefficiently) Unicode - the huge arrays of unicode stuff in UnicodeStubs would not compile, so I skipped it for AVR for now. Synchronization - disabled building the Synchronization library on AVR for now. It's arguable if it adds value on this platform anyway.

carlos4242 · 2024-07-16T13:42:07Z

Hi @kubamracek @rauhul, the test currently failing is abi/macOS/arm64/stdlib-asserts.swift. It looks like this test does something like check the standard library symbols to make sure no unexpected new ones have appeared and no old ones have disappeared?

The error output from lit is...

--- /Users/carl/Documents/Code/swift/swift/test/abi/macOS/arm64/../../Inputs/macOS/arm64/stdlib/baseline-asserts	2024-07-16 12:30:02
+++ /Users/carl/Documents/Code/swift/build/Ninja-ReleaseAssert/swift-macosx-arm64/test-macosx-arm64/abi/macOS/arm64/Output/stdlib-asserts.swift.tmp/symbols	2024-07-16 13:17:21
@@ -9387,6 +9387,7 @@
 _$ss32_diagnoseUnexpectedEnumCaseValue4type03rawE0s5NeverOxm_q_tr0_lF
 _$ss32_makeSwiftNSFastEnumerationStateSo01_bcdE0ayF
 _$ss32_stdlib_binary_CFStringGetLengthySiyXlF
+_$ss32_swift_stdlib_atomicFetchOrInt166object7operands0F0VSpyAEG_AEtF
 _$ss32_swift_stdlib_atomicFetchOrInt326object7operands0F0VSpyAEG_AEtF
 _$ss32_swift_stdlib_atomicFetchOrInt646object7operands0F0VSpyAEG_AEtF
 _$ss33ExpressibleByUnicodeScalarLiteralMp
@@ -9400,10 +9401,13 @@
 _$ss33_ExpressibleByBuiltinFloatLiteralTL
 _$ss33_isBridgedNonVerbatimToObjectiveCySbxmlF
 _$ss33_stdlib_binary_CFStringCreateCopyyyXlyXlF
+_$ss33_swift_stdlib_atomicFetchAddInt166object7operands0F0VSpyAEG_AEtF
 _$ss33_swift_stdlib_atomicFetchAddInt326object7operands0F0VSpyAEG_AEtF
 _$ss33_swift_stdlib_atomicFetchAddInt646object7operands0F0VSpyAEG_AEtF
+_$ss33_swift_stdlib_atomicFetchAndInt166object7operands0F0VSpyAEG_AEtF
 _$ss33_swift_stdlib_atomicFetchAndInt326object7operands0F0VSpyAEG_AEtF
 _$ss33_swift_stdlib_atomicFetchAndInt646object7operands0F0VSpyAEG_AEtF
+_$ss33_swift_stdlib_atomicFetchXorInt166object7operands0F0VSpyAEG_AEtF
 _$ss33_swift_stdlib_atomicFetchXorInt326object7operands0F0VSpyAEG_AEtF
 _$ss33_swift_stdlib_atomicFetchXorInt646object7operands0F0VSpyAEG_AEtF
 _$ss34CustomPlaygroundDisplayConvertibleMp

I'm thinking, perhaps it makes sense that these not be present on the standard library on MacOS?

I think the change that broke this is probably adding 16 bits to this section in AtomicInt.swift.gyb...

%   for bits in [ 16, 32, 64 ]:

// Warning: no overflow checking.
@usableFromInline // used by SwiftPrivate._stdlib_AtomicInt
internal func _swift_stdlib_atomicFetch${operation}Int${bits}(
  object target: UnsafeMutablePointer<Int${bits}>,
  operand: Int${bits}) -> Int${bits} {

  let value = Builtin.atomicrmw_${operation.lower()}_seqcst_Int${bits}(
    target._rawValue, operand._value)

  return Int${bits}(value)
}

%   end

There's two or three approaches we can take. Firstly, simplest, I can just revert this change? I've already removed all the other work on atomics and the synchronisation library from this PR.

Secondly, I can change the above so that it only runs for a number of bits suitable for the platform, so that 16 bit is only emitted for the AVR standard library?

Or lastly, if this should be present, presumably I can change the test to pass on MacOS and add ss32_swift_stdlib_atomicFetchOrInt166object7operands0F0VSpyAEG_AEtF etc. to the expected symbols from the standard library.

What do you think is the best approach? If you have no opinion, I'll probably just revert the original change that broke this test, as it's the simplest approach.

kubamracek · 2024-07-16T18:41:26Z

This ABI/API addition seems pretty reasonable to me, but neither me nor @rauhul can sign off on that, that's more of a question for @lorentey and maybe @phausler. In any case, I think it should go in as a separate change, so what I'd suggest is to drop that change for now, and let's see what @lorentey and @phausler say -- if the change is useful then let's have a separate PR for it.

kubamracek · 2024-07-16T22:03:04Z

@swift-ci please test

kubamracek · 2024-07-17T03:21:37Z

FAIL: Swift(macosx-x86_64) :: embedded/avr/testStdlibFunctioning.swift (9815 of 18049)
******************** TEST 'Swift(macosx-x86_64) :: embedded/avr/testStdlibFunctioning.swift' FAILED ********************
Script:
--
: 'RUN: at line 1';   ...
--
Exit Code: 134

Command Output (stderr):
--
<unknown>:0: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=<mcu name>
Assertion failed: (castIsValid(op, S, Ty) && "Invalid cast!"), function Create, file Instructions.cpp, line 3427.
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the crash backtrace.
Stack dump:
0.	Program arguments: ...
1.	Apple Swift version 6.0-dev (LLVM 4ee06774aa7ace9, Swift 42aa1164f579cce)
2.	Compiling with effective version 4.1.50
3.	While evaluating request IRGenRequest(IR Generation for module testStdlibFunctioning)
4.	While emitting IR SIL function "@swift_once".

I suggest we change the test back to only do -typecheck instead of -emit-ir. And then work on fixing this separately.

carlos4242 · 2024-07-17T05:23:17Z

Alright. I’ve seen a lot of these cast fails over the years. They’re nearly always address space issues. The front end doesn’t take account of program address space. To be fair it’s a fairly new part of llvm. It was only added to support the AVR platform a few years ago!On 17 Jul 2024, at 04:22, Kuba (Brecka) Mracek ***@***.***> wrote: FAIL: Swift(macosx-x86_64) :: embedded/avr/testStdlibFunctioning.swift (9815 of 18049) ******************** TEST 'Swift(macosx-x86_64) :: embedded/avr/testStdlibFunctioning.swift' FAILED ******************** Script: -- : 'RUN: at line 1'; ... -- Exit Code: 134 Command Output (stderr): -- <unknown>:0: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=<mcu name> Assertion failed: (castIsValid(op, S, Ty) && "Invalid cast!"), function Create, file Instructions.cpp, line 3427. Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the crash backtrace. Stack dump: 0. Program arguments: ... 1. Apple Swift version 6.0-dev (LLVM 4ee06774aa7ace9, Swift 42aa116) 2. Compiling with effective version 4.1.50 3. While evaluating request IRGenRequest(IR Generation for module testStdlibFunctioning) 4. While emitting IR SIL function ***@***.***_once". I suggest we change the test back to only do -typecheck instead of -emit-ir. And then work on fixing this separately. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

carlos4242 · 2024-07-17T10:09:52Z

FAIL: Swift(macosx-x86_64) :: embedded/avr/testStdlibFunctioning.swift (9815 of 18049)
******************** TEST 'Swift(macosx-x86_64) :: embedded/avr/testStdlibFunctioning.swift' FAILED ********************
Script:
--
: 'RUN: at line 1';   ...
--
Exit Code: 134

Command Output (stderr):
--
<unknown>:0: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=<mcu name>
Assertion failed: (castIsValid(op, S, Ty) && "Invalid cast!"), function Create, file Instructions.cpp, line 3427.
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the crash backtrace.
Stack dump:
0.	Program arguments: ...
1.	Apple Swift version 6.0-dev (LLVM 4ee06774aa7ace9, Swift 42aa1164f579cce)
2.	Compiling with effective version 4.1.50
3.	While evaluating request IRGenRequest(IR Generation for module testStdlibFunctioning)
4.	While emitting IR SIL function "@swift_once".

I suggest we change the test back to only do -typecheck instead of -emit-ir. And then work on fixing this separately.

OK done.

kubamracek · 2024-07-17T14:12:43Z

@swift-ci please test

phausler

Overall the standard library side looks pretty good to me. The one place where I am not sure 100% is the string side of things - I will try to drudge up some additional reviews for ya

kubamracek · 2024-07-17T18:18:38Z

@swift-ci please test Linux platform

carlos4242 · 2024-07-17T22:12:34Z

@swift-ci please test Linux platform

I must admit I can't understand the logs from the linux test run failure. Are they real test failures do you think?

phausler · 2024-07-17T22:12:35Z

@swift-ci please test linux

phausler · 2024-07-17T22:13:33Z

The linux failures don't seem to be something that would be caused by this set of changes from what I can tell. It did not introduce a --parallel option, nor did it remove that.

carlos4242 · 2024-07-17T22:16:00Z

OkHopefully rerunning it will fix them. Thanks PhilippeOn 17 Jul 2024, at 23:14, Philippe Hausler ***@***.***> wrote: The linux failures don't seem to be something that would be caused by this set of changes from what I can tell. It did not introduce a --parallel option, nor did it remove that. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

kubamracek · 2024-07-18T00:09:59Z

@swift-ci please test Linux platform

DougGregor

This looks great to me.

DougGregor · 2024-07-18T05:17:39Z

Lots of green checks, I'll go ahead and merge

rauhul mentioned this pull request Jul 10, 2024

Add support for AVR #74899

Open

9 tasks

rauhul reviewed Jul 10, 2024

View reviewed changes

test/Parse/ConditionalCompilation/avrTargetWithStdlib.swift Outdated Show resolved Hide resolved

carlos4242 mentioned this pull request Jul 10, 2024

[Standard Library] [16-bit] StringObject not compiling on 16 bit platforms #75143

Open

kubamracek reviewed Jul 15, 2024

View reviewed changes

test/Parse/ConditionalCompilation/avrTargetWithStdlib.swift Outdated Show resolved Hide resolved

kubamracek reviewed Jul 15, 2024

View reviewed changes

lib/Basic/LangOptions.cpp Outdated Show resolved Hide resolved

kubamracek reviewed Jul 15, 2024

View reviewed changes

stdlib/public/Synchronization/Atomics/AtomicFloats.swift Outdated Show resolved Hide resolved

kubamracek requested review from phausler and lorentey July 15, 2024 15:58

carlos4242 requested review from kubamracek and rauhul July 15, 2024 23:32

kubamracek reviewed Jul 16, 2024

View reviewed changes

lib/Basic/LangOptions.cpp Outdated Show resolved Hide resolved

kubamracek reviewed Jul 16, 2024

View reviewed changes

test/embedded/avr/testStdlibFunctioning.swift Outdated Show resolved Hide resolved

kubamracek reviewed Jul 16, 2024

View reviewed changes

stdlib/public/core/StringObject.swift Show resolved Hide resolved

kubamracek reviewed Jul 16, 2024

View reviewed changes

stdlib/public/core/StringGuts.swift Show resolved Hide resolved

carlos4242 force-pushed the embedded-avr-cross-compile-stdlib branch from 58c09af to 3689427 Compare July 16, 2024 11:30

carlos4242 requested a review from kubamracek July 16, 2024 11:32

Carl Peto added 2 commits July 16, 2024 22:30

fix test

3c65c79

revert atomics for now

de449c9

kubamracek marked this pull request as ready for review July 17, 2024 04:51

kubamracek requested review from ahoppen, bnbarham, CodaFi, DougGregor, hamishknight, rintaro, a team, zoecarver, hyp, egorzhdan, beccadax and ian-twilightcoder as code owners July 17, 2024 04:51

simplify standard library unit test to just check basic usage

39adecb

phausler approved these changes Jul 17, 2024

View reviewed changes

kubamracek approved these changes Jul 17, 2024

View reviewed changes

ahoppen removed their request for review July 17, 2024 19:42

rauhul approved these changes Jul 18, 2024

View reviewed changes

DougGregor approved these changes Jul 18, 2024

View reviewed changes

DougGregor merged commit ac0f574 into swiftlang:main Jul 18, 2024
5 checks passed

[AVR] standard library support for AVR #75127

[AVR] standard library support for AVR #75127

Uh oh!

Conversation

carlos4242 commented Jul 10, 2024

Uh oh!

kubamracek commented Jul 10, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kubamracek commented Jul 15, 2024

Uh oh!

carlos4242 commented Jul 15, 2024

Uh oh!

carlos4242 commented Jul 15, 2024 via email

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kubamracek commented Jul 16, 2024

Uh oh!

kubamracek commented Jul 16, 2024

Uh oh!

carlos4242 commented Jul 16, 2024

Uh oh!

kubamracek commented Jul 16, 2024

Uh oh!

kubamracek commented Jul 16, 2024

Uh oh!

kubamracek commented Jul 17, 2024

Uh oh!

carlos4242 commented Jul 17, 2024 via email

Uh oh!

carlos4242 commented Jul 17, 2024

Uh oh!

kubamracek commented Jul 17, 2024

Uh oh!

phausler left a comment

Choose a reason for hiding this comment

Uh oh!

kubamracek commented Jul 17, 2024

Uh oh!

carlos4242 commented Jul 17, 2024

Uh oh!

phausler commented Jul 17, 2024

Uh oh!

phausler commented Jul 17, 2024

Uh oh!

carlos4242 commented Jul 17, 2024 via email

Uh oh!

kubamracek commented Jul 18, 2024

Uh oh!

DougGregor left a comment

Choose a reason for hiding this comment

Uh oh!

DougGregor commented Jul 18, 2024

Uh oh!

Uh oh!

Uh oh!