Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Codegen: String.Concat called multiple times for concats of 3 and 4 strings #5560

Closed
auduchinok opened this issue Aug 27, 2018 · 3 comments
Closed
Milestone

Comments

@auduchinok
Copy link
Member

String.Concat is called multiple times in a row for 3 and 4 strings despite overloads taking these numbers of params are available.

3 strings:

let s a b =
    a + "." + b
IL_0000: ldarg.0      // a
IL_0001: ldstr        "."
IL_0006: call         string [System.Runtime]System.String::Concat(string, string)
IL_000b: ldarg.1      // b
IL_000c: call         string [System.Runtime]System.String::Concat(string, string)
IL_0011: ret        

4 strings:

let s1 a b c =
    a + "." + b + c
IL_0000: ldarg.0      // a
IL_0001: ldstr        "."
IL_0006: call         string [System.Runtime]System.String::Concat(string, string)
IL_000b: ldarg.1      // b
IL_000c: call         string [System.Runtime]System.String::Concat(string, string)
IL_0011: ldarg.2      // c
IL_0012: call         string [System.Runtime]System.String::Concat(string, string)
IL_0017: ret     

And the same in C#:

public static string S(string a, string b) => a + "." + b;
IL_0000: ldarg.0      // a
IL_0001: ldstr        "."
IL_0006: ldarg.1      // b
IL_0007: call         string [netstandard]System.String::Concat(string, string, string)
IL_000c: ret          
public static string S2(string a, string b, string c) => a + "." + b + c;
IL_0000: ldarg.0      // a
IL_0001: ldstr        "."
IL_0006: ldarg.1      // b
IL_0007: ldarg.2      // c
IL_0008: call         string [netstandard]System.String::Concat(string, string, string, string)
IL_000d: ret          
@zpodlovics
Copy link

Offending line:
https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/FSharp.Core/prim-types.fs#L3464

In theory the ast could be matched and optimized for this case.

@cartermp cartermp added this to the Unknown milestone Aug 29, 2018
@TIHan
Copy link
Contributor

TIHan commented Aug 29, 2018

I messed around this for a few hours and managed to come up with a solution. Will make a PR (without tests) in a bit.

@cartermp
Copy link
Contributor

cartermp commented Nov 8, 2018

Merged.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants