Dsyme tuple spike #6

KevinRansom · 2016-07-11T14:09:32Z

Don,

I tried to emit the right code for reading from a struct tuple field however, it is not correct. Can you take a look and let me know what I did wrong. I think the issue is at tastops.fs around line 7830.

let struct (one,_) = struct (2, 4)
0

This code:
when compiled generates this error

c:\projects\structtuples>basictuple.exe

Unhandled Exception: System.TypeLoadException: Could not load type 'System.ValueTuple`2' from assembly 'basictuple, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
   at <StartupCode$basictuple>.$Basictuple.main@()

c:\projects\structtuples>

Note the System.ValueTuple dll exists in the execution directory

dsyme · 2016-07-11T14:41:11Z

src/fsharp/TastOps.fs

+
+let mkGetTupleItemN g m n (typ:ILType) isStruct te retty =
+    if isStruct then
+        mkAsmExpr([mkNormalLdfld  (mkILFieldSpecForTupleItem typ n)   ],[],[],[retty],m)


You're not using te (the expression for the object from which you're loading the field) in the first branch

te doesn't make a difference.

dsyme · 2016-07-11T14:44:53Z

@KevinRansom I left one comment, though I'm not sure that explains the error message above. Run peverify.exe on the generated code?

KevinRansom · 2016-07-11T14:46:57Z

c:\projects\structtuples>"c:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.6 Tools\PEVerify.exe" basictuple.exe

[IL]: Error: [c:\projects\structtuples\basictuple.exe : <StartupCode$basictuple>.$Basictuple::main@][offset 0x00000021] Unable to resolve token.
1 Error(s) Verifying basictuple.exe

c:\projects\structtuples>

dsyme · 2016-07-11T14:53:27Z

Use ILDASM /OUT=out.il and paste the IL in a gist, maybe we'll spot something

KevinRansom · 2016-07-11T14:58:04Z

https://gist.github.com/KevinRansom/014a53b4f2f4426f4ae46f78545d7062

dsyme · 2016-07-11T15:05:23Z

This ldfld instruction looks like it is missing a generic instantiation on the ValueTuple type. Looking at the generating code I didn't immediately spot why

https://gist.github.com/KevinRansom/014a53b4f2f4426f4ae46f78545d7062#file-basictuple-il-L153

KevinRansom · 2016-07-11T15:05:48Z

https://gist.github.com/KevinRansom/014a53b4f2f4426f4ae46f78545d7062

From: Don Syme [mailto:notifications@github.com]
Sent: Monday, July 11, 2016 7:53 AM
To: dsyme/visualfsharp visualfsharp@noreply.github.com
Cc: Kevin Ransom Kevin.Ransom@microsoft.com; Mention mention@noreply.github.com
Subject: Re: [dsyme/visualfsharp] Dsyme tuple spike (#6)

Use ILDASM /OUT=out.il and paste the IL in a gist, maybe we'll spot something

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/6#issuecomment-231758674, or mute the threadhttps://github.com/notifications/unsubscribe/AE76FjkIxD9WmhbUL-CF_EmTHsxlNQh1ks5qUljngaJpZM4JJZyI.

KevinRansom · 2016-07-11T15:08:09Z

Yes … that was my suspicion too and I can’t figure out the right Voodoo to make it work.

From: Don Syme [mailto:notifications@github.com]
Sent: Monday, July 11, 2016 8:05 AM
To: dsyme/visualfsharp visualfsharp@noreply.github.com
Cc: Kevin Ransom Kevin.Ransom@microsoft.com; Mention mention@noreply.github.com
Subject: Re: [dsyme/visualfsharp] Dsyme tuple spike (#6)

This ldfld instruction looks like it is missing a generic instantiation on the ValueTuple type. Looking at the generating code I didn't immediately spot why

https://gist.github.com/KevinRansom/014a53b4f2f4426f4ae46f78545d7062#file-basictuple-il-L153https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgist.github.com%2fKevinRansom%2f014a53b4f2f4426f4ae46f78545d7062%23file-basictuple-il-L153&data=01%7c01%7cKevin.Ransom%40microsoft.com%7c3970a3c5b6ae4e05ea3e08d3a99ce963%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=0NSi7iuTYddA1oxRVXOC9s56J1ZBj%2bbTDDjbWub%2fKDM%3d

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/6#issuecomment-231762365, or mute the threadhttps://github.com/notifications/unsubscribe/AE76Fn5lx5Wyp_S3geufCrIhkV4E7RJtks5qUluzgaJpZM4JJZyI.

dsyme · 2016-07-11T15:21:21Z

It feels like the lack of a type instantiation must be flowing in from somewhere earlier (I checked we can emit references to IL fields in generic IL classes ok).

At this stage I'd probably use a debug build with devenv /debugexe fsc.exe test.fs and put a breakpoint in the struct cases of the code generation and double check the instantiation is missing. If so, then try to track backwards for where it got lost.

KevinRansom · 2016-07-11T15:22:15Z

Okay I will give that a try.

Thanks Don

From: Don Syme [mailto:notifications@github.com]
Sent: Monday, July 11, 2016 8:21 AM
To: dsyme/visualfsharp visualfsharp@noreply.github.com
Cc: Kevin Ransom Kevin.Ransom@microsoft.com; Mention mention@noreply.github.com
Subject: Re: [dsyme/visualfsharp] Dsyme tuple spike (#6)

It feels like the lack of a type instantiation must be flowing in from somewhere earlier (I checked we can emit references to IL fields in generic IL classes ok).

At this stage I'd probably use a debug build with devenv /debugexe fsc.exe test.fs and put a breakpoint in the struct cases of the code generation and double check the instantiation is missing. If so, then try to track backwards for where it got lost.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/6#issuecomment-231767498, or mute the threadhttps://github.com/notifications/unsubscribe/AE76FnPPKDYq28GnlY4rr7Ods7mG1hULks5qUl9xgaJpZM4JJZyI.

KevinRansom · 2016-07-11T17:51:00Z

I think the generic information is there:

let mkILFieldSpec (tref:ILFieldRef,ty) =
    printfn "tref %A" (tref.EnclosingTypeRef.BasicQualifiedName)
    match tref.Type with 
    | ILType.Value tr -> 
        printfn "tr %A" (tr.BasicQualifiedName)
        tr.tspecInst |> Seq.iter(fun t ->printfn "   t: %A" (t.BasicQualifiedName))
    | _ -> ()
    { FieldRef= tref; EnclosingType=ty }

let mkILFieldSpecInTy (typ:ILType,nm,fty) = 
    match typ with 
    | ILType.Value tr -> 
        printfn "tr %A" (tr.BasicQualifiedName)
        tr.tspecInst |> Seq.iter(fun t ->printfn "   t: %A" (t.BasicQualifiedName))
    | _ -> ()
    mkILFieldSpec (mkILFieldRef (typ.TypeRef,nm,fty), typ)

c:\projects\structtuples>\KevinRansom\visualfsharp\release\net40\bin\fsc basictuple.fs /debug+ /optimize- /debug:full /r:System.ValueTuple.dll
Microsoft (R) F# Compiler version (private)
Copyright (c) Microsoft Corporation. All Rights Reserved.

basictuple.fs(4,1): warning FS0020: The result of this expression is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.
tref "<StartupCode$basictuple>.$Basictuple"
tr "System.ValueTuple`2[[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]"
   t: "System.Int32"
   t: "System.Int32"
tref "<StartupCode$basictuple>.$Basictuple"
tr "System.Int32"
tr "System.ValueTuple`2[[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]"
   t: "System.Int32"
   t: "System.Int32"
tref "System.ValueTuple`2"
tref "<StartupCode$basictuple>.$Basictuple"
tr "System.Int32"
tref "<StartupCode$basictuple>.$basictuple"
tr "System.Int32"

…otnet#8063) * # This is a combination of 9 commits. # This is the 1st commit message: ref -> mutable in more places in the compiler # The commit message #2 will be skipped: # Update dependencies from https://github.com/dotnet/arcade build 20191229.1 # # - Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.19629.1 # The commit message #3 will be skipped: # Update dependencies from https://github.com/dotnet/arcade build 20191230.1 # # - Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.19630.1 # The commit message #4 will be skipped: # Update dependencies from https://github.com/dotnet/arcade build 20191231.1 # # - Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.19631.1 # The commit message #5 will be skipped: # Update dependencies from https://github.com/dotnet/arcade build 20200101.1 # # - Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.20051.1 # The commit message #6 will be skipped: # Update dependencies from https://github.com/dotnet/arcade build 20191216.5 (dotnet#8079) # # - Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19616.5 # The commit message #7 will be skipped: # dispose fsi at the end of a scripting session (dotnet#8084) # # The commit message #8 will be skipped: # Added static link tests and extended CompilerAssert (dotnet#8101) # # * Changed CompilerAssert to static class. Added Compile/Execute methods that take a Compilation description. Added static link tests # # * Hiding compilation description internals # # * Added another test to check for sanity # # * Making a few optional parameters # # * Hiding internals of CompilationReference # The commit message #9 will be skipped: # Parameterize product version (dotnet#8031) # # * Parameterize Product details # # * fcs # # * Repack pkgdef * no ilread

KevinRansom added 3 commits July 7, 2016 13:02

Teach fsharp that System.ValueTuple is a framework assembly

754284f

Update reflect.fs to use fields for ValueTuples

09a1135

Attempt to emit field access rather than property access.

8efcf96

dsyme reviewed Jul 11, 2016
View reviewed changes

KevinRansom added 3 commits July 18, 2016 08:37

Diagnostic junk

1239cd0

merge from dsyme-tuplespike

f03a6b5

Remove printfn tracingdiagnostics

6db1b99

dsyme merged commit e68c44e into dsyme:tuple-spike Jul 19, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dsyme tuple spike #6

Dsyme tuple spike #6

KevinRansom commented Jul 11, 2016

dsyme Jul 11, 2016

KevinRansom Jul 11, 2016

dsyme commented Jul 11, 2016 •

edited

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

Dsyme tuple spike #6

Dsyme tuple spike #6

Conversation

KevinRansom commented Jul 11, 2016

dsyme Jul 11, 2016

Choose a reason for hiding this comment

KevinRansom Jul 11, 2016

Choose a reason for hiding this comment

dsyme commented Jul 11, 2016 • edited

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

KevinRansom commented Jul 11, 2016

dsyme commented Jul 11, 2016 •

edited