Add atomics support in the standard library #1637

lucteo · 2024-12-20T20:38:33Z

No description provided.

This way, we know more about what failed, and not get just "exception".

lucteo · 2025-01-03T18:42:45Z

@kyouko-taiga : Do you have any idea why I get a "Sources/FrontEnd/TypeChecking/TypeChecker.swift:6183: Assertion failed: type already inferred" error on many of the tests on Ubuntu builds (see https://github.com/hylo-lang/hylo/actions/runs/12597537109/job/35110729555?pr=1637)?

kyouko-taiga · 2025-01-09T09:17:26Z

@kyouko-taiga : Do you have any idea why I get a "Sources/FrontEnd/TypeChecking/TypeChecker.swift:6183: Assertion failed: type already inferred" error on many of the tests on Ubuntu builds (see https://github.com/hylo-lang/hylo/actions/runs/12597537109/job/35110729555?pr=1637)?

It means something causes the type checker to visit the same tree twice. If you point me at the test cases that are failing I can investigate if you want.

lucteo · 2025-01-09T19:21:09Z

The failures can be seen at https://github.com/hylo-lang/hylo/actions/runs/12597537109/job/35110729555?pr=1637.
It shows me the following failed tests:

	  2 - DriverTests (ILLEGAL)
	  3 - ManglingTests (ILLEGAL)
	  4 - HyloTests (ILLEGAL)
	  5 - HyloTests_TestCases_Lowering_0 (ILLEGAL)
	  6 - HyloTests_TestCases_Lowering_1 (ILLEGAL)
	  7 - HyloTests_TestCases_Lowering_2 (ILLEGAL)
	  8 - HyloTests_TestCases_Lowering_3 (ILLEGAL)
	  9 - HyloTests_TestCases_Parsing_0 (ILLEGAL)
	 10 - HyloTests_TestCases_Parsing_1 (ILLEGAL)
	 11 - HyloTests_TestCases_Parsing_2 (ILLEGAL)
	 12 - HyloTests_TestCases_TypeChecking_0 (ILLEGAL)
	 13 - HyloTests_TestCases_TypeChecking_1 (ILLEGAL)
	 14 - HyloTests_TestCases_TypeChecking_2 (ILLEGAL)
	 15 - HyloTests_TestCases_TypeChecking_3 (ILLEGAL)
	 16 - HyloTests_TestCases_TypeChecking_4 (ILLEGAL)
	 17 - HyloTests_TestCases_TypeChecking_5 (ILLEGAL)
	 18 - HyloTests_TestCases_TypeChecking_6 (ILLEGAL)
	 19 - HyloTests_TestCases_TypeChecking_7 (ILLEGAL)
	 20 - HyloTests_TestCases_TypeChecking_8 (ILLEGAL)
	 21 - HyloTests_TestCases_TypeChecking_9 (ILLEGAL)
	 22 - HyloTests_TestCases_TypeChecking_10 (ILLEGAL)
	 23 - EndToEndTests (ILLEGAL)
	 24 - EndToEndTests_TestCases_0 (ILLEGAL)
	 25 - EndToEndTests_TestCases_1 (ILLEGAL)
	 26 - EndToEndTests_TestCases_2 (ILLEGAL)
	 27 - EndToEndTests_TestCases_3 (ILLEGAL)
	 28 - EndToEndTests_TestCases_4 (ILLEGAL)
	 29 - EndToEndTests_TestCases_Concurrency (ILLEGAL)
	 31 - LibraryTests_TestCases_0 (ILLEGAL)
	 32 - LibraryTests_TestCases_1 (ILLEGAL)

Please note that these tests only fail on Ubuntul, I haven't seen any failures on MacOS.

kyouko-taiga · 2025-01-14T06:07:18Z

If the failure is so widespread I think there's something going wrong with your code generator. It's surprising that the error occurs only on Ubuntu but perhaps that's because you're generating a duplicate declaration only on this platform?

My current theory is that two identical definitions are being fed to the type checker (which currently does not check for duplicate declaration).

codecov · 2025-01-26T18:27:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.75%. Comparing base (890ee11) to head (e1911dc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
+ Coverage   88.33%   88.75%   +0.42%     
==========================================
  Files         382      382              
  Lines       31548    31548              
==========================================
+ Hits        27867    28000     +133     
+ Misses       3681     3548     -133

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

StandardLibrary/Sources/LowLevel/Threading/AtomicRepresentable.hylo

kyouko-taiga

Nice work. See inline comments.

StandardLibrary/Sources/LowLevel/Threading/AtomicRepresentable.hylo

kyouko-taiga · 2025-03-11T15:54:14Z

StandardLibrary/Sources/LowLevel/Threading/PlatformAtomics.hylo

+///////////////////////////////////////////////////////////////////////////////
+// MARK: IntPlatformAtomic


We probably don't need these comments.

They help me navigate in such big files like this. But, I'm fine if you want me to delete them.

kyouko-taiga · 2025-03-11T15:55:29Z

StandardLibrary/Sources/LowLevel/Threading/PlatformAtomics.hylo

+// MARK: IntPlatformAtomic
+
+/// An atomic representation of Int.
+public type IntPlatformAtomic: Regular {


I'm not sure to understand why we need a wrapped. Why can't we simply make Hylo.Int conform to PlatformAtomic?

I can't remember the exact reason why I went this route.
I think one reason is that we might not have all primitive types to be "platform atomics" and, we might have "platform atomics" that are not necessarily primitive integers. But, I guess we can work around that easily.

I will try to use directly the basic integer types.

I remember now the reason for this choice. I don't want to expose the atomic functions on types like Int, Int32, etc. This is why, these primitives are hidden behind other types.

kyouko-taiga · 2025-03-11T15:56:32Z

StandardLibrary/Sources/LowLevel/Threading/PlatformAtomics.hylo

+public conformance IntPlatformAtomic: IntegerPlatformAtomic {
+
+  public fun load(ordering: AtomicLoadOrdering) -> Value {
+    if /*ordering == AtomicLoadOrdering.relaxed*/ ordering.value == 0 {


Should I understand that AtomicLoadOrdering.relaxed has value 0 from this comment? If so why not create a global constant?

I tied this, but the current version of the compiler failed on me. The comment is the code I wanted to write.

StandardLibrary/Sources/LowLevel/Threading/Atomic.hylo

kyouko-taiga · 2025-03-11T16:03:34Z

StandardLibrary/Sources/LowLevel/Threading/PlatformAtomic.hylo

+}
+
+/// A platform-specific atomic integer type.
+trait IntegerPlatformAtomic: PlatformAtomic {


I don't understand the trait hierarchy that's being defined and I'm under the impression that it may be simplified. Some documentation would help me understand the intent and see whether I'm wrong.

I'll try to simplify it.

kyouko-taiga · 2025-03-11T16:05:34Z

StandardLibrary/Sources/LowLevel/Threading/PlatformAtomics.hylo

+}
+
+///////////////////////////////////////////////////////////////////////////////
+// MARK: Int8PlatformAtomic


A specific reason why all these types are declared in a single file? The usual approach has been to declare one type per file so far. Although it is not a strict convention I'd like to understand why it is not followed here.

Co-authored-by: Dimi Racordon <[email protected]>

Add atomics support in the standard library

c5720a9

Base automatically changed from atomics_store_load_intrinsics to main December 24, 2024 12:24

lucteo added 2 commits January 2, 2025 11:04

Merge remote-tracking branch 'origin/main' into atomics_lib

8a387ff

Fix typos

485db41

lucteo requested a review from kyouko-taiga January 2, 2025 09:30

Enable "output on failure" when running tests with ctest.

14be838

This way, we know more about what failed, and not get just "exception".

lucteo added 3 commits January 26, 2025 19:00

Merge remote-tracking branch 'origin/main' into atomics_lib

b5e9837

Move the tests in `Tests/Library Tests/.

1beea4c

Rename XXXAtomicRepresentation to XXXPlatformAtomic

396a0b7

kyouko-taiga reviewed Mar 11, 2025

View reviewed changes

StandardLibrary/Sources/LowLevel/Threading/AtomicRepresentable.hylo Outdated Show resolved Hide resolved

kyouko-taiga requested changes Mar 11, 2025

View reviewed changes

lucteo and others added 3 commits March 11, 2025 23:00

Apply suggestions from code review

745ce6b

Co-authored-by: Dimi Racordon <[email protected]>

Fixes after code-review changes

f65c199

Merge remote-tracking branch 'origin/main' into atomics_lib

e1911dc

		///////////////////////////////////////////////////////////////////////////////
		// MARK: IntPlatformAtomic

Add atomics support in the standard library #1637

Are you sure you want to change the base?

Add atomics support in the standard library #1637

Uh oh!

Conversation

lucteo commented Dec 20, 2024

Uh oh!

lucteo commented Jan 3, 2025

Uh oh!

kyouko-taiga commented Jan 9, 2025

Uh oh!

lucteo commented Jan 9, 2025

Uh oh!

kyouko-taiga commented Jan 14, 2025

Uh oh!

codecov bot commented Jan 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

kyouko-taiga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jan 26, 2025 •

edited

Loading