[sil] [irgen] make object instruction valid

[zoecarver]

This patch allows the object instruction to be used everywhere.

Why the change?

Why is this change beneficial? It may appear that it generates the same code as an alloc_ref on the stack. That is mostly true.

The problem with alloc_ref is that it behaves very differently from most other sil instructions. It pretends to be a value but it is actually a pointer. It makes it hard for both the sil optimizer and the llvm optimizer to reason about what's going on. This change allows objects (classes) to be created in a way that is much more semantically similar to structs, enums, and tuples which will allow our current sil otpimization passes to optimize class objects with little to no changes.

For example, mem2reg cannot optimize class objects at all. Adding support would mean re-writing almost the entire pass. However, if objects are created using the object instruction, it can be updated to support objects with only two lines changed in general utility files. That's great.

Additionally, it seems to make it easier for llvm to reason about what's going on as well. In another patch I added support for promoting alloc_refs to objects. And, after making no other change to the sil optimizer (i.e. the sil optimizer still didn't know that objects existed) the llvm optimizer was able to reason about what was going on much more easily.

For example

class Foo {
    var x : Int = 0
}

func a( _ f : Foo) {
    f.x += 1
}

func b( _ f : Foo) -> Int {
    return f.x + 2
}

public func foouser(_ check : Bool) -> Int {
    let f = Foo()
    if check {
        a(f)
    }
    return b(f)
}

Before this patch:

define swiftcc i64 @"$s3run7foouserySiSbF"(i1 %0) #1 {
entry:
  %1 = alloca %T3run3FooC, align 8
  %access-scratch = alloca [24 x i8], align 8
  %access-scratch2 = alloca [24 x i8], align 8
  %2 = tail call swiftcc %swift.metadata_response @"$s3run3FooCMa"(i64 undef) #10
  %3 = extractvalue %swift.metadata_response %2, 0
  %4 = getelementptr inbounds %T3run3FooC, %T3run3FooC* %1, i64 0, i32 0
  %reference.new = call %swift.refcounted* @swift_initStackObject(%swift.type* %3, %swift.refcounted* nonnull %4) #3
  %5 = getelementptr inbounds %swift.refcounted, %swift.refcounted* %reference.new, i64 1
  %._value = bitcast %swift.refcounted* %5 to i64*
  store i64 0, i64* %._value, align 8
  br i1 %0, label %6, label %entry._crit_edge

entry._crit_edge:                                 ; preds = %entry
  %.pre = bitcast %swift.refcounted* %5 to i8*
  br label %14

6:                                                ; preds = %entry
  %7 = getelementptr inbounds [24 x i8], [24 x i8]* %access-scratch2, i64 0, i64 0
  call void @llvm.lifetime.start.p0i8(i64 -1, i8* nonnull %7)
  %8 = bitcast %swift.refcounted* %5 to i8*
  call void @swift_beginAccess(i8* nonnull %8, [24 x i8]* nonnull %access-scratch2, i64 1, i8* null) #3
  %9 = load i64, i64* %._value, align 8
  %10 = call { i64, i1 } @llvm.sadd.with.overflow.i64(i64 %9, i64 1)
  %11 = extractvalue { i64, i1 } %10, 1
  br i1 %11, label %22, label %12, !prof !21, !misexpect !22

12:                                               ; preds = %6
  %13 = extractvalue { i64, i1 } %10, 0
  store i64 %13, i64* %._value, align 8
  br label %14

14:                                               ; preds = %entry._crit_edge, %12
  %.pre-phi = phi i8* [ %.pre, %entry._crit_edge ], [ %8, %12 ]
  %15 = getelementptr inbounds [24 x i8], [24 x i8]* %access-scratch, i64 0, i64 0
  call void @llvm.lifetime.start.p0i8(i64 -1, i8* nonnull %15)
  call void @swift_beginAccess(i8* nonnull %.pre-phi, [24 x i8]* nonnull %access-scratch, i64 0, i8* null) #3
  %16 = load i64, i64* %._value, align 8
  %17 = call { i64, i1 } @llvm.sadd.with.overflow.i64(i64 %16, i64 2)
  %18 = extractvalue { i64, i1 } %17, 1
  br i1 %18, label %21, label %19, !prof !21, !misexpect !22

19:                                               ; preds = %14
  %20 = extractvalue { i64, i1 } %17, 0
  ret i64 %20

21:                                               ; preds = %14
  call void asm sideeffect "", "n"(i32 0) #3
  call void @llvm.trap()
  unreachable

22:                                               ; preds = %6
  call void asm sideeffect "", "n"(i32 1) #3
  call void @llvm.trap()
  unreachable
}

After this patch:

define swiftcc i64 @"$s3run7foouserySiSbF"(i1 %0) {
entry:
  %1 = alloca %T3run3FooC, align 8
  %2 = tail call swiftcc %swift.metadata_response @"$s3run3FooCMa"(i64 undef) #4
  %3 = extractvalue %swift.metadata_response %2, 0
  %4 = getelementptr inbounds %T3run3FooC, %T3run3FooC* %1, i64 0, i32 0
  %reference.new = call %swift.refcounted* @swift_initStackObject(%swift.type* %3, %swift.refcounted* nonnull %4) #3
  %5 = getelementptr inbounds %swift.refcounted, %swift.refcounted* %reference.new, i64 1
  %._value = bitcast %swift.refcounted* %5 to i64*
  %spec.store.select = zext i1 %0 to i64
  store i64 %spec.store.select, i64* %._value, align 8
  %6 = select i1 %0, i64 3, i64 2
  ret i64 %6
}

Why stack?

I spent a few days trying to get this to work when visitObjectInst just emitted an Explosion. In the future, I think this is a thing we can (and should) do but, right now it would mean updating almost every use of a class to handle two paths: an alloc_ref object and an object object. This is because alloc_ref pretends to be a value when it's actually a pointer. The simplest solution was to make object essentially a stack version of alloc_ref and have it pretend to be a pointer as well.

Future plans

I have two more patches that I'll put up for review today. One updates StackPromotion to promote stack alloc_refs to objects and the other adds some basic support for object instructions across the sil optimizer.

Sorry for all the words here :P TL;DR: this patch lays the groundwork for extending struct optimizations to classes as well.

[eeckstein]

@zoecarver I'm not sure if this makes sense.

I would not say that alloc_refs behaves differently than other instructions. What you call 'to pretend to be a pointer' is a feature of the swift/SIL type system: that's what we call a 'reference type'. It's true that the implementation of a reference is done with a pointer, but that's just how it is implemented. In SIL, a reference type has a very well defined semantic and it can be copied ( = increase ref count), destroyed (= decrease ref count), etc.

If you want to represent the contents of a class as a value-type in SIL, then this would need a change in the type system rather than just using a different allocation instruction. In your proposal, the result of 'object' is still a reference type (a ref_element_addr of an object - what does this even mean?)

A straw man proposal: we could introduce a new SIL type, kind of a wrapper-type, _class_content(ClassType), which is a struct containing all the stored properties of the class ClassType.
Instead of

  %3 = object $Foo (%2 : $Int)

we can just use a struct instruction to construct a "value-type" class:

  %3 = struct  $_class_content(Foo) (%2 : $Int)

It also has the advantage that all SIL optimization would work with this out of the box.

[zoecarver]

@eeckstein I completely agree with your points.

I would not say that alloc_refs behaves differently than other instructions.

The problem I am trying to articulate is this (and I probably didn't do a great job articulating it): if we generalize some of the current passes to support alloc_ref instructions, it is likely that we will, for example, get a copy_value instruction where we want a copy_addr because the pass checked getType().isAddress(). As you pointed out, this patch doesn't fix that issue (in fact, it may make it worse).

My original solution to this problem was to emit an Explosion (what we do for struct instructions). That can work as an implementation for reference types by creating a stack_alloc and letting mem2reg do the heavy lifting.

Anyway, I love your proposal! It solves a lot of problems with this patch (doesn't require stack allocation, fixes the above problem, doesn't require any optimizer changes, etc.). What do you think the next steps should be? Do any wrapper types like that exist? Or would this be a new kind of thing? #30743 could probably be updated with minimal changes to create a _class_content struct and emit a store.

[eeckstein]

Thinking about it again: why not just use a tuple which contains all the class properties?

[zoecarver]

I suppose that would probably work. It would also remove the wrapper which would be nice. I think the best way to do this will be to create an alloc_stack and replace all stores and ref_element_addrs with copy_addrs and tuple_element_addrs respectively. Then we can just ref cast and copy_addr the tuple to the alloc_ref.

I really appreciate your suggestions! This will now be a much better implementation. Let me know if you have any other ideas.

[eeckstein]

Adding a new type in SIL optimizations might be a problem, because AFAIK, the AST is considered immutable after the frontend. @slavapestov or @DougGregor can comment on this.

An alternative would be to add this kind of "wrapper-type" just in SIL: add an API layer for struct types in SILType, which either redirect to the AST struct (in most cases) or map the class properties for the wrapper-type case.

[zoecarver]

I think your tuple idea will work well. I've been playing around with it and looking at the IR generated and it seems to have all the benefits of this patch without the drawbacks. In order to efficiently copy the tuple pointer to the class ref I've added a new sil instruction.

We don't necessarily need another sil instruction but it allows us to copy the whole object with a memcpy which I'm not sure we could otherwise do. I'll put it up for review later today and we can decide whether adding another instruction is worth the cost.