I was wondering how much better it is to use TypeFor (a static operation) versus TypeOf, when the type is known statically, and the answer is: it's worse, because TypeFor is implemented as a subroutine around TypeOf. In particular it has to handle a theoretical but impossible nil case when casting PtrType.Elem.

Presumably TypeFor could be intrinsified to use the same logic in the compiler that constructs the rtype during type erasure of interface conversions, and then it would return the type:*int symbol directly.

Compare:

func f() reflect.Type {
    return reflect.TypeOf((*int)(nil))
}

main.f STEXT size=32 args=0x0 locals=0x0 funcid=0x0 align=0x0 leaf
...
    0x0000 00000    L0031   HINT    $0
    0x0004 00004    L1312   HINT    $0
    0x0008 00008    L0032   MOVD    $go:itab.*reflect.rtype,reflect.Type(SB), R0
    0x0010 00016    L0032   MOVD    $type:*int(SB), R1
    0x0018 00024    L2733   RET (R30)

And:

func f() reflect.Type {
    return reflect.TypeFor[*int]()
}

main.f STEXT size=48 args=0x0 locals=0x0 funcid=0x0 align=0x0 leaf
...
    0x0000 00000    L0031   HINT    $0
    0x0004 00004    L0031   PCDATA  $0, $-3
    0x0004 00004    L1317   MOVD    reflect..dict.TypeFor[*int](SB), R2
    0x000c 00012    L1317   PCDATA  $0, $-1
    0x000c 00012    L0193   MOVD    (R2), R2
    0x0010 00016    L0193   MOVD    48(R2), R2    // load PtrType.Elem
    0x0014 00020            NOP
    0x0014 00020            NOP
    0x0014 00020            NOP
    0x0014 00020    L2733   CMP $0, R2
    0x0018 00024    L0033   MOVD    $go:itab.*reflect.rtype,reflect.Type(SB), R3
    0x0020 00032    L0033   CSEL    NE, R3, ZR, R0
    0x0024 00036    L0033   CSEL    NE, R2, ZR, R1
    0x0028 00040    L2733   RET (R30)

Comment From: gabyhelp

Related Issues

Related Code Changes

Related Discussions

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

Comment From: jakebailey

Funnily, I actually have this on a branch, I just wasn't sure if there was interest in intrinsifying it in the compiler itself.

Absolutely happy to send that over.

Comment From: gopherbot

Change https://go.dev/cl/700136 mentions this issue: cmd/compile/internal/ssa: handle reflect.TypeFor in SSA

Comment From: mateusz834

I just wasn't sure if there was interest in intrinsifying it in the compiler itself.

Have you tried doing this without intrinsifying specifically the reflect.TypeFor?

I remember once thinking about that, and my idea was to create something similar to:

https://github.com/golang/go/blob/8bcda6c79d40f49363cd04cdaf5bd7909de8f94f/src/cmd/compile/internal/ssa/_gen/generic.rules#L2760-L2774

I have not tried doing it since, so it might be more complex though.

Comment From: jakebailey

That seems like a reasonable idea.

My CL doesn't seem to work anymore, unfortunately; I guess enough has changed in the past 9 months to make what I have no longer work. Probably improved inlining causing the call to go away and so my rewrite never hits.

Comment From: mateusz834

Probably improved inlining causing the call to go away and so my rewrite never hits.

That's me! 😄 CL 648395

Comment From: jakebailey

Bah, I remember seeing that and thinking it'd mess up my CL 😄

That's what I get for trying to hold back on my CLs until typescript-go was public...

Comment From: jakebailey

It turns out that half the problem is that the rewrite rules mentioned in https://github.com/golang/go/issues/75203#issuecomment-3239446773 are missing all of the "no offset" cases. I need to test to see what else that changes.

The other half is unfortunate, though. Similar to #37753, the phielim/copyelim stage merges multiple Copy values that point to the same place, but does so without consindering their types. This means that the SSA "forgets" that it's trying to OffPtr a *abi.PtrType, and so I can't write a rewrite rule that targets reads from Elem, because all I have is *abi.Type, so have no idea the original Load was from *abi.PtrType.

The other way I'd handle this is to instead stop special casing reads from type:... and instead have their structure like other syms, but I can't figure out where that should go, or if that info is even available. It seems like things like type:*int are totally opaque, having type Sxxx, not even SRODATA.

Comment From: fengyoulin

I have a CL that optimizes away the dictionaries. I'm still working on it.

Comment From: gopherbot

Change https://go.dev/cl/700336 mentions this issue: cmd/compile/internal/ssa: load constant values from dictionaries

Comment From: jakebailey

That CL is the first part I described, yes (though it can be done for all of the rules).

I'm also working on a solution to the second problem. I don't like how it's turning out, but is technically in line with the existing special case.

Comment From: fengyoulin

Done.

main.f STEXT size=32 args=0x0 locals=0x0 funcid=0x0 align=0x0 leaf
...
        0x0000 00000 L0007  HINT    $0
        0x0004 00004 L1317  HINT    $0
        0x0008 00008 L1317  HINT    $0
        0x000c 00012 L0008  MOVD    $go:itab.*reflect.rtype,reflect.Type(SB), R0
        0x0014 00020 L0008  MOVD    $type:int(SB), R1
        0x001c 00028 L2735  RET     (R30)

Comment From: prattmic

cc @golang/compiler