[CompilerPerf] Fix 5136: LAddrOf is a constant and {x with ...} respects byrefness of x

[dsyme]

1, { x with ... } respecting byrefs
2. In optimization, &x (i.e. LAddrOf) is a constant value even if x is mutable and so can be propagated (so for let y = &x in ... y ... the occurrences of y get replaced by &x and y gets removed)

See #5136, also related to #2688.

For (1), basically

{ x with c = E }

should respect the byrefness of x in the case that x is a byref pointer to a struct which is being implicitly dereferenced. At the moment this elaborates to:

let recdValue = implicit-deref(x) in { a=recdValue.a; b=recdValue.b; c=E }

It should elaborate to:

{ a=&x->a; b=&x->b; c=E }

which removes a struct copy. #2688 covers a similar case for struct unions.

[dsyme]

1. CI is not running, I don't know why

2. I updated this PR to improve the code generated for matching on struct unions (and any byref-to-struct-union) like this:

    member x.Next = let (SingleUnion i) = x in SingleUnion (i+1)

or

    member x.Next = 
        match x with 
        | SingleUnion i -> SingleUnion (i+1)

Code size 24 down to 16 with the extractive copy removed:

AFTER:

.method public hidebysig specialname instance valuetype A/SingleRec 
        get_Next() cil managed
{
  // Code size       16 (0x10)
  .maxstack  4
  .locals init (int32 V_0)
  IL_0000:  ldarg.0
  IL_0001:  ldfld      int32 A/SingleRec::item
  IL_0006:  stloc.0
  IL_0007:  ldloc.0
  IL_0008:  ldc.i4.1
  IL_0009:  add
  IL_000a:  call       valuetype A/SingleRec A/SingleRec::NewSingleUnion(int32)
  IL_000f:  ret
} // end of method SingleRec::get_Next

BEFORE:

.method public hidebysig specialname instance valuetype A/SingleRec 
        get_Next() cil managed
{
  // Code size       24 (0x18)
  .maxstack  4
  .locals init (valuetype A/SingleRec V_0,
           int32 V_1)
  IL_0000:  ldarg.0
  IL_0001:  ldobj      A/SingleRec
  IL_0006:  stloc.0
  IL_0007:  ldloca.s   V_0
  IL_0009:  ldfld      int32 A/SingleRec::item
  IL_000e:  stloc.1
  IL_000f:  ldloc.1
  IL_0010:  ldc.i4.1
  IL_0011:  add
  IL_0012:  call       valuetype A/SingleRec A/SingleRec::NewSingleUnion(int32)
  IL_0017:  ret
} // end of method SingleRec::get_Next

[dsyme]

@dotnet-bot test this please

[dsyme]

@dotnet-bot test this please

[dsyme]

NOTE: CI is still not running for some reason, and needs to be re-run on this PR when it starts working again

[mmitche]

@dotnet-bot test this please

[mmitche]

@dotnet-bot test this please

[thinkbeforecoding]

It seems a test is failing for "StructUnion01.il.bsl" since the IL has actually changed.

[thinkbeforecoding]

👍 this time it should pass.
It's cool because it should now avoid extra copies for struct records and struct unions 😄

[thinkbeforecoding]

And the fix is actually really small

[thinkbeforecoding]

I tried to compile this code:

[<Struct>]
type Context = { Img: int; Pos: nativeint}
module Context =
    let inline move n ctx = { ctx with Pos = ctx.Pos + n } 

but get the following error:

error FS0421: Impossible d'utiliser l'adresse de la variable 'ctx' actuellement

Which translates to "Unable to curently use 'ctx' variable's address"
This seems due to the fact the function is inline...

[thinkbeforecoding]

Actually it not due to inline but to the inference. To following fix the error:

[<Struct>]
type Context = { Img: int; Pos: nativeint}
module Context =
    let inline move n (ctx: Context)  = { ctx with Pos = ctx.Pos + n } 

It seems that has this point the type ety line 1135 of PostInferenceChecks.fs is ?7776 (solved) which lead to a test CheckExprNoByrefs on the Op(LValueOp, LAddrOf true, ctx))) which raises the error.

[thinkbeforecoding]

Something to do with #5146 probably...

[KevinRansom]

@dsyme ---> When do you think this will be ready? Master is cleared for Dev16.0 now.

[dsyme]

Ugh, now getting a CI failure on Linux - feels like a compiler bug though I don't know why this would only affect Mono (it could be we're hitting some size limitation or bug caused by very large binaries and that is triggered on Mono first due to somhow extra code being added there)

2018-09-13T09:32:54.2374763Z FSC : error FS2014: A problem occurred writing the binary
 '/home/vsts/work/1/s/src/fsharp/Fsc-proto/obj/Proto/net40/fsc.exe': EmitZUntaggedIndex: 
too big for small address or simple index [/home/vsts/work/1/s/src/fsharp/Fsc-proto/Fsc-proto.fsproj]

I've just pushed some extra diagnostics to try to diagnose this a little.

[dsyme]

I added diagnostics, and the build completed ok... Trying again with the original commit 771ef66

[dsyme]

@KevinRansom It is ready. Odd CI error before but now it is gone