libswift: Start implementing SIL optimizations in Swift #37058

eeckstein · 2021-04-25T16:44:14Z

With libswift it is possible to add SIL optimization passes written in Swift. It allows to gradually migrate the SIL optimizer from C++ to Swift.

libswift is a Swift package, parallel to the lib directory in the repository.

This PR is a very first initial version - a proof of concept.

It contains the basic (and still imcomplete) SIL and optimizer API and some example passes.
To see how how a Swift optimization pass looks like, take a look at SILPrinter, MergeCondFails or SimplifyGlobalValue

To see how all this works, take a look at libswift/README.md. It contains a lot more information about design and implementation.

owenv

Neat! Sorry for the unsolicited review, I just added a couple comments regarding the build setup. Feel free to ignore if they're irrelevant.

I was recently looking into porting some of the DiagnosticConsumers to Swift for better Unicode-awareness and I think the infra here could eventually be a good foundation for that too.

owenv · 2021-04-25T17:23:19Z

CMakeLists.txt

+
+  # Workaround to make lldb happy: we have to explicitly add all libswift modules
+  # to the linker command line.
+  set(LIBSWIFT_MODULES "SIL" "Optimizer")


It might be worth adding a comment to the package's targets list pointing to here for anyone adding a new module

owenv · 2021-04-25T17:35:57Z

libswift/Package.swift

+let package = Package(
+  name: "libswift",
+  platforms: [
+    .macOS("10.9"),


Unless this needs an earlier deployment target, would it make sense to align it with the driver/SwiftPM at 10.15?

yes, I changed it

I need to take this back, because it must match the deployment target in the cmake (which is 10.9)

owenv · 2021-04-25T17:51:06Z

libswift/README.md

+
+Though, there are some small deviations from the C++ SIL design as well. Either due to the nature of the Swift language (e.g. the SIL `Value` is a protocol, not a class), or improvements, which could be done in C++ as well.
+
+In this early stage, the design of _libswift_ is of course not fixed and will probably get some find tuning.


Suggested change

In this early stage, the design of _libswift_ is of course not fixed and will probably get some find tuning.

In this early stage, the design of _libswift_ is of course not fixed and will probably get some fine tuning.

Thanks, but I pushed a new version anyway where I rewrote this section (and added more documentation in general).

tschuett · 2021-04-25T18:57:53Z

Will libswift eventually depend on https://github.com/apple/swift-tools-support-core?

gottesmm · 2021-04-25T19:17:38Z

@eeckstein some initial review on architecture. This is not the end of my review.

I am against invasively placing support for a swiftpm subproject into swift's cmake like this. The build system is already too complex and adding into the custom cmake a sub-buildsystem will make it have even more issues. libswift is not special and should be able to be integrated without putting in conditional control flow into the basic build commands of the cmake (see add_swift_host_library). We need to minimize all non-standard cmake in the tree as much as possible.

Some solutions:

We could instead of using swiftpm to build, just by default use normal cmake swift support. I would prefer this solution. This would get rid of all of the goop in add_swift_host_library. We could leave in a swiftpm project file as well so that one can quickly iterate without needing to build everything.
We could also use an external project (which has been my preferred way to do this in the past). That is how one does this delegating thing in CMake generally. Then you can tell cmake about the outputs that you want and everything is done appropriately. You would be able to just have a file in ./libswift/CMakeLists.txt.
Aren't we able to build libswift against the swift sources/header files and produce a .a/dylib? In such a case, why couldn't we make a separate build-script project. Just pass in the files that are necessary, have the host swiftpm build it and then treat that as an input to Swift's cmake. Then swift's cmake has all of the dependency information that it needs and we are not just hacking it in.

I have more comments, but lets talk about this first.

eeckstein · 2021-04-26T07:25:26Z

Will libswift eventually depend on https://github.com/apple/swift-tools-support-core?

@tschuett I don't think there is a need for that (right now)

eeckstein · 2021-04-26T08:09:05Z

@gottesmm I don't see the problem here. I agree that our build system is very complex. But, given that we have >15K lines of cmake code in the project, this little additional logic doesn't make a big difference.
I tested this build integration quite extensively during my work on libswift and it worked pretty well and stable.

ad 1. swiftpm is the standard build system for swift code. It is better than cmake in every aspect. It would be a step back use cmake instead of swiftpm. We really shouldn't do this.

ad 3. It is important that the tools can be built with the build system (without using build-script). i.e. invoking ninja swift-frontend should build swift-frontend including everything which is needed to do so. I use this a lot (for very good reasons). If we would give this up it will probably break the workflow for a lot of people.

ad 2. This sounds interesting. If this works (e.g. tracking correct build dependencies), then we could use it. I'll let this up to someone who knows this better than me.

gottesmm · 2021-04-26T15:11:27Z

@eeckstein Its an easy problem unless one is maintaining/evolving it. The problem here is that once we let in one thing another thing comes in. We are specifically trying to not grow the custom cmake part of the build system as much as possible. Integrating another build system into it without 2 would be exactly that.

Let me respond in a subsequent comment line by line.

atrick

I don't think we need to verbatim repeat all of the broken types and APIs on the Swift side that we've been wanting to fix for years on the C++ side. We know how to simplify all of these things, but no one was sufficiently motivated to do the big search/replace on the C++ code base.

Here I'm drawing particular attention to

Location
Builder
Argument

atrick · 2021-04-26T15:45:10Z

libswift/Sources/SIL/Location.swift

+
+import SILBridging
+
+public struct Location {


The debug scope is missing from this encapsulation.

You are right, this should be actually mapped to SILDebugLocation.

include/swift/SIL/SILBasicBlock.h

libswift/Sources/SIL/Argument.swift

atrick · 2021-04-26T15:53:49Z

libswift/Sources/Optimizer/PassManager/PassUtils.swift

@@ -49,3 +49,8 @@ struct FunctionPass {
  }
 }

+extension Builder {
+  init(at insertionPoint: Instruction, _ context: FunctionPassContext) {


A Builder should always be created with the (insertionPoint, (DebugScope, DebugLocation))

Is the debug location argument even needed? Isn't this always the location of the insertion point?

Yeah, I think the debug info is usually inferred from the insertion point. Initially, you could just infer it and add overrides later when we need it.

The debug location always needs to be consistent with the insertion point, but I think there are a few places where we override it. We might choose location before or after insertion. IIRC inlining changes the scope. More commonly, we need to decide whether to mark a location as "compiler-generated" vs. inheriting from the surrounding code. We might just want to add some kind of enum to the initializer to make that selection.

Either way, I always wanted the SILLocation + DebugScope to be a single "DebugLocation" type with both information.

atrick · 2021-04-26T15:54:42Z

libswift/Sources/SIL/Builder.swift

+
+  private var bridgedInsPoint: BridgedInstruction { insertionPoint.bridged }
+
+  private func notifyInstructionsChanged() {


Why isn't this interface on a PassContext API instead of calling directly from the Builder into C++?

To minimize the usage of the bridging API. notifyInstructionsChanged() will be called from a lot of places in the builder and here it's a central place where PassContext_notifyChanges is called. It makes it easier in case we change the bridging layer at some time.

Right, the C API needs to be called in a central place. That's why I expect that the Swift code to need a PassContext struct to encapsulate the C API. There are more ways to change instructions than just creating new ones

libswift/Sources/SIL/Builder.swift

atrick · 2021-04-26T16:28:08Z

@eeckstein some initial review on architecture. This is not the end of my review.

I am against invasively placing support for a swiftpm subproject into swift's cmake like this. The build system is already too complex and adding into the custom cmake a sub-buildsystem will make it have even more issues. libswift is not special and should be able to be integrated without putting in conditional control flow into the basic build commands of the cmake (see add_swift_host_library). We need to minimize all non-standard cmake in the tree as much as possible.

I didn't try to review the build changes. I suspect the build script just needs to communicate the location of bridging headers and installed Swift libraries, which should be something we can pass into the libswift project. It seems wrong that adding a Swift project to the build requires custom logic in the build script just to do this. Whatever happens here, I want to be able to rebuild libswift cleanly without running the build script. As a rule, I don't use the build script to build because it totally destroys my workflow. I often need to rebuild one project in isolation regardless of other changes in the tree.

WowbaggersLiquidLunch · 2021-04-27T22:52:31Z

libswift/README.md

@@ -0,0 +1,117 @@
+# Swift implemented in Swift
+
+_Libswift_ is the part of the Swift compiler, which is implemented in Swift.


Should "is the part" be "is a part" or "is the SIL optimizations part"?

WowbaggersLiquidLunch · 2021-04-27T23:00:51Z

libswift/Sources/Optimizer/FunctionPasses/MergeCondFails.swift

+
+import SIL
+
+let mergeCondFailsPass = FunctionPass(name: "merge-condfails", runMergeCondFails)


I think it's more swifty to un-abbreviate "cond" as "condition" in identifiers defined on the Swift side. E.g. mergeCondFailsPass → mergeConditionFailsPass, runMergeCondFails → runMergeConditionFails, condFailToMerge → conditionFailToMerge etc.

In general, I agree. But in this case "CondFail" refers to the "cond_fail" instruction.

WowbaggersLiquidLunch

^ 2 tiny non-functional comments.

karwa · 2021-04-28T15:43:25Z

libswift/Sources/Optimizer/FunctionPasses/MergeCondFails.swift

+  }
+}
+
+/// Try to merge the cond_fail instructions. Returns true if any could


This function doesn't have a return value

Thanks, I'll fix it.

karwa · 2021-04-28T15:48:21Z

libswift/Sources/Optimizer/PassManager/PassUtils.swift

+struct FunctionPass {
+
+  let name: String
+  let runFunction: (Function, FunctionPassContext) -> ()


Given that this, and InstructionPass, just have a name + closure (which I assume will be unique for each pass), would it make sense for it to be a protocol?

This would mean that instead of writing:

let mergeCondFailsPass = FunctionPass(name: "merge-condfails", runMergeCondFails)

You could write:

struct MergeCondFailsPass: FunctionPass { var name: String { "merge-condfails" } /* mutating? In case a pass has state? */ func callAsFunction(_ function: Function, _ context: FunctionPassContext) { // ... } }

A pass should not have any state (that means: stored properties), because the pass object lives forever and it's dangerous to carry over state (like analysis caches) from one run to another. We have this in the C++ optimizer, and it ends up that the run functions of passes always instantiate another "pass" object which does the actual work - a lot of boiler plate.

Providing the run function as a function, makes this clear.

You could also make the run function a static requirement, to enforce that there is no instance and hence no state.

The real benefit of using a protocol is that it gives you the full power of subtyping. So you could create a protocol SpecialFunctionPass: FunctionPass with additional requirements, and which provides additional utility functions, etc.

Well, in the existing optimizer we didn't have a use for different kind of function passes. I'm not a big fan of adding abstractions/features if there is not a good use for them. I think simplicity should come first - and I think that not needing to define a separate nominal type for a pass makes it a bit simpler.

eeckstein · 2021-05-05T15:19:50Z

I pushed a new version with some changes in the build system. Most importantly: I added bootstrapping support in the build.
Also, there is now an option to disable libswift, which is the default for now. For details see libswift/README.md

eeckstein · 2021-05-06T07:31:45Z

@swift-ci test

eeckstein · 2021-05-06T15:45:42Z

@swift-ci smoke test

eeckstein · 2021-05-06T17:29:20Z

@swift-ci smoke test windows

eeckstein · 2021-05-06T18:37:54Z

@swift-ci smoke test windows

eeckstein · 2021-05-06T19:52:46Z

@swift-ci smoke test windows

eeckstein · 2021-05-07T07:18:14Z

@swift-ci smoke test

eeckstein · 2021-06-04T18:01:37Z

@swift-ci test

swift-ci · 2021-06-04T20:00:45Z

Build failed
Swift Test OS X Platform
Git Sha - 5bb9b1b09c07325e43e9edfb07d2382d8c298406

eeckstein · 2021-06-05T17:29:02Z

@swift-ci test

swift-ci · 2021-06-05T18:42:03Z

Build failed
Swift Test Linux Platform
Git Sha - f20d0a932f96fb93c41c9369d1759d3322e517c4

eeckstein · 2021-06-06T07:52:08Z

@swift-ci test linux

gottesmm

A few small things. But overall the cmake isn't that invasive and we should get the fixes so we can use normal cmake relatively soon.

libswift/CMakeLists.txt

gottesmm · 2021-06-07T18:15:10Z

cmake/modules/AddSwift.cmake

+
+    if(${SWIFT_HOST_VARIANT_SDK} IN_LIST SWIFT_APPLE_PLATFORMS)
+      # For some reason, invoking CMAKE_Swift_COMPILER does not work.
+      set(swift_command "xcrun" "swiftc")


Does this really not work still? That is surprising to me.

gottesmm · 2021-06-07T18:16:03Z

cmake/modules/AddSwift.cmake

+        set_property(TARGET ${executable} APPEND_STRING PROPERTY
+                     LINK_FLAGS ${libswift_ast_path_flags})
+
+        # Workaround for a linker crash: rdar://77839981


Can you describe the crash symptons/etc in the comment here (if possible)?

include/swift/Basic/InitializeLibSwift.h

eeckstein · 2021-06-07T20:12:56Z

@gottesmm I pushed a new version

eeckstein · 2021-06-08T10:12:12Z

@swift-ci test

swift-ci · 2021-06-08T11:25:54Z

Build failed
Swift Test Linux Platform
Git Sha - be383d6

gottesmm

I am fine with the cmake. I am a little worried about __has_feature part of the change and I also think that the immediate part is already fixed on main since I changed our tests to always use the just built stdlib. The real solution is to run the JIT out of process but we aren't there yet.

Can you fix my comments and lets find a resolution to my two questions? I am not sure if this should require a whole test cycle though. If you are going to do more changes, feel free to fix them in this PR. Otherwise can you fix in a follow on?

gottesmm · 2021-06-08T18:35:01Z

cmake/modules/AddSwift.cmake

@@ -477,7 +477,9 @@ function(add_swift_host_library name)
  endif()

  add_library(${name} ${libkind} ${ASHL_SOURCES})
-  add_dependencies(${name} ${LLVM_COMMON_DEPENDS})
+  if (LLVM_COMMON_DEPENDS)


Can you add a comment here describing what LLVM_COMMON_DEPENDS is and maybe provide a reference to the large comment in ./lib/CMakeLists.txt where we describe it? I just don't want this to be mysterious.

you already did this. Thanks

gottesmm · 2021-06-08T18:37:02Z

cmake/modules/AddSwift.cmake

@@ -597,6 +599,112 @@ function(add_swift_host_library name)
  endif()
 endfunction()

+# Add a module of libswift


Can you add to all of the comments here that this is a temporary addition to AddSwift.cmake until fixes in the swift-driver are available on macOS host toolchains? That way it is 100% clear in the source that this is going away once what I mentioned happens.

gottesmm · 2021-06-08T18:40:14Z

include/swift/SIL/BridgedSwiftObject.h

+
+#include <stdint.h>
+
+#if !__has_feature(nullability)


I am fine fixing this in a follow up, but isn't __has_feature an extension? @compnerd is my memory right? I think we may need to do something more here.

This is handled in type_traits.h, where __has_feature is defined to 0 if it's not available. Also, it's tested with the windows PR testing - so it works for windows.

gottesmm · 2021-06-08T18:42:15Z

include/swift/SIL/BridgedSwiftObject.h

+
+/// The header of a Swift object.
+///
+/// This must be in sync with HeapObject, which is defined in the runtime lib.


Can you be more specific with this comment? My feeling is that I want it to be clear that we are matching the ABI of HeapObject. In Sync is good, but I think making it clear we are dealing with layout compatibility for correctness is important to make explicit.

gottesmm · 2021-06-08T18:43:35Z

include/swift/SIL/SILModule.h

-class FixedSizeSlab : public llvm::ilist_node<FixedSizeSlab>,
-                       public SILAllocated<FixedSizeSlab> {
+/// The payload for the FixedSizeSlab.
+/// This is a super-class rather than a member of FixedSizeSlab to make briding


briding -> binding.

gottesmm · 2021-06-08T18:44:02Z

include/swift/SIL/SILModule.h

@@ -88,6 +88,16 @@ class FixedSizeSlab : public llvm::ilist_node<FixedSizeSlab>,
  template<typename T> const T *dataFor() const { return (const T *)(&data[0]); }
 };

+/// A fixed size slab of memory, which can be allocated and freed by the
+/// SILModule at (basically) zero cost.


Can you add a comment here explaining in a paragraph below why it is zero cost? Just that way people do not have to look around.

I'll add a "See SILModule::allocSlab()", which has more info

gottesmm · 2021-06-08T18:44:50Z

include/swift/SIL/SILNode.h

@@ -114,7 +115,7 @@ class SILNodePointer {
 ///   subobject.  If the SILNode is actually the base subobject of a
 ///   ValueBase subobject, the cast will yield a corrupted value.
 ///   Always use the LLVM casts (cast<>, dyn_cast<>, etc.) instead.
-class alignas(8) SILNode {
+class alignas(8) SILNode : public SwiftObjectHeader {


Can you add a comment here explaining that SILNode inherits from SwiftObjectHeader to allow for libswift bridging/ to look for information in SwiftObjectHeader.h about how this all works?

I'll mention libswift/README.md, where this is explained

gottesmm · 2021-06-08T18:46:29Z

include/swift/SIL/SwiftObjectHeader.h

+
+/// The C++ version of SwiftObject.
+///
+/// In C++ code, never use BridgedSwiftObject directly. SwiftObjectHeader has


I really would like somewhere a large description of how the bridging works that is centralized in the code base. One of those long in source comments. It IMO should describe how we rely on layout compatibility for correctness and also all of the little important bits like this. That way the whole model/its interesting parts is made explicit in the source.

It's explained in libswift/README.md. I'll mention it here.

gottesmm · 2021-06-08T18:46:55Z

include/swift/SILOptimizer/OptimizerBridging.h

+#ifndef SWIFT_SILOPTIMIZER_OPTIMIZERBRIDGING_H
+#define SWIFT_SILOPTIMIZER_OPTIMIZERBRIDGING_H
+
+#include "../SIL/SILBridging.h"


We generally do not do relative paths like this in the include dir. Can you do the full "swift/SIL/SILBridging.h"?

In this case it's needed for the libswift build.

gottesmm · 2021-06-08T18:48:55Z

lib/SIL/IR/SILBasicBlock.cpp

@@ -26,12 +26,24 @@
 #include "swift/SIL/SILModule.h"
 #include "swift/SIL/SILUndef.h"
 #include "swift/Strings.h"
+extern "C" {
+#include "swift/SIL/SILBridging.h"


Can we just put the extern "C" in swift/SIL/SILBridging.h and #ifdef __cplusplus? The includers of a header in c++ shouldn't need to know to do this. They should just be able to include SILBridging.h.

gottesmm · 2021-06-08T18:54:24Z

Also I think the cmake changes look good! I just want more comments there.

* add the (still empty) libswift package * add build support for libswift in CMake * add libswift to swift-frontend and sil-opt The build can be controlled with the LIBSWIFT_BUILD_MODE cmake variable: by default it’s “DISABLE”, which means that libswift is not built. If it’s “HOSTTOOLS”, libswift is built with a pre-installed toolchain on the host system.

This is the initial version of a buildable SIL definition in libswift. It defines an initial set of SIL classes, like Function, BasicBlock, Instruction, Argument, and a few instruction classes. The interface between C++ and SIL is a bridging layer, implemented in C. It contains all the required bridging data structures used to access various SIL data structures.

This lets tests run conditionally on libswift: // REQUIRES: libswift

…LOptimizer's PassManager With the macro SWIFT_FUNCTION_PASS a new libswift function pass can be defined in Passes.def. The SWIFT_FUNCTION_PASS_WITH_LEGACY is similar, but it allows to keep an original C++ “legacy” implementation of the pass, which is used if the compiler is not built with libswift.

And two example instruction building functions.

StackList is a very efficient data structure for worklist type things. This is a port of the C++ utility with the same name. Compared to Array, it does not require any memory allocations.

SILPrinter prints the SIL of a function. It's and example pass which demonstrates the basic SIL API of libswift.

But keeping the old pass as legacy pass

Instruction passes are basically visit functions in SILCombine for a specific instruction type. With the macro SWIFT_INSTRUCTION_PASS such a pass can be declared in Passes.def. SILCombine then calls the run function of the pass in libswift.

…tion pass in libswift. But keeping the old visit function as legacy

eeckstein · 2021-06-09T09:36:38Z

@swift-ci smoke test

eeckstein · 2021-06-09T11:43:19Z

@swift-ci smoke test macOS

eeckstein · 2021-06-09T14:56:17Z

@swift-ci smoke test macOS

owenv reviewed Apr 25, 2021

View reviewed changes

mackoj mentioned this pull request Apr 25, 2021

[184] Issue #184 - May 6, 2021 SwiftWeekly/swiftweekly.github.io#635

Closed

eeckstein force-pushed the libswift branch from ba683d6 to b35b8c4 Compare April 26, 2021 07:08

atrick reviewed Apr 26, 2021

View reviewed changes

eeckstein force-pushed the libswift branch from b35b8c4 to 3f6a3da Compare April 27, 2021 14:33

WowbaggersLiquidLunch reviewed Apr 27, 2021

View reviewed changes

eeckstein force-pushed the libswift branch from 3f6a3da to 3de3270 Compare April 28, 2021 09:55

karwa reviewed Apr 28, 2021

View reviewed changes

eeckstein force-pushed the libswift branch from 3de3270 to 21c840e Compare May 5, 2021 15:18

eeckstein requested a review from edymtt May 5, 2021 15:20

eeckstein force-pushed the libswift branch from 21c840e to 462d5b8 Compare May 6, 2021 15:42

eeckstein force-pushed the libswift branch from 462d5b8 to 15b6aa5 Compare May 6, 2021 17:27

eeckstein force-pushed the libswift branch from 15b6aa5 to 54f44a2 Compare May 6, 2021 19:52

eeckstein force-pushed the libswift branch from 54f44a2 to 8010475 Compare May 7, 2021 07:17

eeckstein force-pushed the libswift branch from 2a89965 to 5bb9b1b Compare June 4, 2021 18:00

eeckstein force-pushed the libswift branch from 5bb9b1b to f20d0a9 Compare June 5, 2021 17:28

gottesmm reviewed Jun 7, 2021

View reviewed changes

eeckstein force-pushed the libswift branch from f20d0a9 to be383d6 Compare June 7, 2021 20:11

gottesmm approved these changes Jun 8, 2021

View reviewed changes

eeckstein added 10 commits June 9, 2021 11:25

libswift: if libswift is enabled, add ‘libswift’ to the lit features

b82173e

This lets tests run conditionally on libswift: // REQUIRES: libswift

libswift: add the SIL Builder

d49108d

And two example instruction building functions.

libswift: Add the StackList data structure

14422cd

StackList is a very efficient data structure for worklist type things. This is a port of the C++ utility with the same name. Compared to Array, it does not require any memory allocations.

libswift: add an example SILPrinter pass.

9c363fb

SILPrinter prints the SIL of a function. It's and example pass which demonstrates the basic SIL API of libswift.

libswift: implement the MergeCondFail pass in libswift

6fe7196

But keeping the old pass as legacy pass

libswift: implement SILCombine's global_value optimization as instruc…

e8d408d

…tion pass in libswift. But keeping the old visit function as legacy

eeckstein force-pushed the libswift branch from be383d6 to e8d408d Compare June 9, 2021 09:35

eeckstein merged commit 7fd69a0 into apple:main Jun 9, 2021

eeckstein deleted the libswift branch June 9, 2021 18:21


		Though, there are some small deviations from the C++ SIL design as well. Either due to the nature of the Swift language (e.g. the SIL `Value` is a protocol, not a class), or improvements, which could be done in C++ as well.

		In this early stage, the design of _libswift_ is of course not fixed and will probably get some find tuning.


		private var bridgedInsPoint: BridgedInstruction { insertionPoint.bridged }

		private func notifyInstructionsChanged() {

		@@ -0,0 +1,117 @@
		# Swift implemented in Swift

		_Libswift_ is the part of the Swift compiler, which is implemented in Swift.


		import SIL

		let mergeCondFailsPass = FunctionPass(name: "merge-condfails", runMergeCondFails)

libswift: Start implementing SIL optimizations in Swift #37058

libswift: Start implementing SIL optimizations in Swift #37058

Conversation

eeckstein commented Apr 25, 2021 • edited

owenv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eeckstein Apr 26, 2021 • edited

Choose a reason for hiding this comment

tschuett commented Apr 25, 2021

gottesmm commented Apr 25, 2021

eeckstein commented Apr 26, 2021 • edited

eeckstein commented Apr 26, 2021

gottesmm commented Apr 26, 2021 • edited

atrick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atrick commented Apr 26, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WowbaggersLiquidLunch left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karwa Apr 28, 2021 • edited

Choose a reason for hiding this comment

eeckstein Apr 29, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eeckstein commented May 5, 2021 • edited

eeckstein commented May 6, 2021

eeckstein commented May 6, 2021

eeckstein commented May 6, 2021

eeckstein commented May 6, 2021

eeckstein commented May 6, 2021

eeckstein commented May 7, 2021

eeckstein commented Jun 4, 2021

swift-ci commented Jun 4, 2021

eeckstein commented Jun 5, 2021

swift-ci commented Jun 5, 2021

eeckstein commented Jun 6, 2021

gottesmm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eeckstein commented Jun 7, 2021

eeckstein commented Jun 8, 2021

swift-ci commented Jun 8, 2021

gottesmm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eeckstein commented Apr 25, 2021 •

edited

eeckstein Apr 26, 2021 •

edited

eeckstein commented Apr 26, 2021 •

edited

gottesmm commented Apr 26, 2021 •

edited

atrick commented Apr 26, 2021 •

edited

WowbaggersLiquidLunch left a comment •

edited

karwa Apr 28, 2021 •

edited

eeckstein Apr 29, 2021 •

edited

eeckstein commented May 5, 2021 •

edited