Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Implement IL codes for #91, #93, fix #105 #104

Open
wants to merge 46 commits into
base: devel
Choose a base branch
from
Open

Implement IL codes for #91, #93, fix #105 #104

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
7ac3be8
Merge branch 'kekyo:devel' into devel
cyborgyn Oct 9, 2021
a2c5f30
Merge branch 'devel' of git://github.com/kekyo/IL2C into kekyo-devel
cyborgyn Oct 10, 2021
69cf5f0
Merge branch 'kekyo-devel' into devel
cyborgyn Oct 10, 2021
e29d09e
Merge pull request #3 from kekyo/devel
cyborgyn Oct 12, 2021
e7fef2b
Merge pull request #4 from cyborgyn/devel
cyborgyn Oct 12, 2021
69e814f
Fix double addition of the same DLL reference, in case the transpiled…
cyborgyn Oct 12, 2021
663563c
Fix discovery of implemented method in case of explicit interface imp…
cyborgyn Oct 12, 2021
4234755
Add bitwise NOT arithmetic operator
cyborgyn Oct 12, 2021
19c92d9
Implement shr, shl IL codes
cyborgyn Oct 12, 2021
f6bb406
Add starg and star.s IL codes
cyborgyn Oct 12, 2021
9091c5b
Fix for #105, stop endless loop, fix static constructor handling #97,…
cyborgyn Oct 13, 2021
bab32a0
Implement Shr.un and Neg IL codes
cyborgyn Oct 13, 2021
80e54b5
Implement Bgt.un and Bgt.un.s IL codes
cyborgyn Oct 13, 2021
c5ac124
Extend ConditionalConverters to handle float type parameters
cyborgyn Oct 13, 2021
e01dc9a
Fix typo
cyborgyn Oct 13, 2021
4833ba2
Implement Stind.i1-i8, Stind.r4-r8 IL codes
cyborgyn Oct 13, 2021
90bd0a3
Fix StargConverter
cyborgyn Oct 13, 2021
05ececc
Add Enum to int conversion handling to GetRightExpression()
cyborgyn Oct 13, 2021
f98e142
Fix NullReferenceException occurring in MethodSignatureTypeComparerIm…
cyborgyn Oct 13, 2021
df35a87
Fix InternalWriteVTableTypePreDefinitions() to correctly handle empty…
cyborgyn Oct 13, 2021
1a9cfe8
Enable IL2C_RUNTIME_TYPE_BEGIN writer, to handle compilation of Syste…
cyborgyn Oct 13, 2021
2a73440
Enable handling without error such cases, where BaseType == null
cyborgyn Oct 13, 2021
e8029c1
Implement Switch IL code, for #93 and #91 v0.1
cyborgyn Oct 13, 2021
e1b6cc4
Implement Ldobj and Stobj IL codes
cyborgyn Oct 14, 2021
30c0889
Updated supported-opcodes.md by Unit tests, no regression so far
cyborgyn Oct 14, 2021
08ea06a
Add switch IL code tests
cyborgyn Oct 14, 2021
ff75c55
Fix switch test + generator
cyborgyn Oct 14, 2021
5453d39
Ldobj IL code test + related fixes in converters
cyborgyn Oct 14, 2021
8e3bac8
Fix ldobj unit test
cyborgyn Oct 14, 2021
e0d75e0
More ldobj tests
cyborgyn Oct 15, 2021
1153f48
More Ldobj tests
cyborgyn Oct 15, 2021
b549284
Merge branch 'devel' into feature/implement-ilcodes
cyborgyn Oct 16, 2021
e43b30c
Merge pull request #5 from kekyo/devel
cyborgyn Oct 17, 2021
3c3a734
Merge pull request #6 from cyborgyn/devel
cyborgyn Oct 17, 2021
796f113
Add Stobj IL Code unit tests
cyborgyn Oct 17, 2021
dd83236
Add more TypeInitializer unit tests
cyborgyn Oct 17, 2021
73afbfb
Add Shr and Shr.un IL Code unit tests + fix for it's converter to mak…
cyborgyn Oct 18, 2021
a28587a
Updated supported-opcodes.md & supported-runtime-system-features.md
cyborgyn Oct 18, 2021
92ed390
Merge branch 'devel' into feature/implement-ilcodes
cyborgyn Oct 18, 2021
76eeee7
Add NOT ILCode unit tests
cyborgyn Oct 19, 2021
2b7b595
Remove breaking unit tests
cyborgyn Oct 19, 2021
3cf87a5
Merge branch 'feature/implement-ilcodes' of github.com:cyborgyn/IL2C …
cyborgyn Oct 19, 2021
bc1476f
Refactor ArithmeticalConverters/ NotConverter according to guidelines
cyborgyn Oct 19, 2021
9753e44
Tried to add more Shr unit tests, but discovery fails then for all ot…
cyborgyn Oct 19, 2021
4790411
Remove method filter comment also for AssemblyPreparer
cyborgyn Oct 19, 2021
ca5706c
StargConverter simplification
cyborgyn Oct 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions IL2C.Core/Metadata/MetadataContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,12 @@ internal MetadataContext(string assemblyPath, bool readSymbols)

this.MainAssembly = mainAssemblyInformation;
assemblies.Add(mainAssembly, mainAssemblyInformation);
assemblies.Add(resolvedCoreAssembly, resolvedCoreAssemblyInformation);
modules.Add(resolvedCoreModule, resolvedCoreModuleInformation);
assemblyByModule.Add(resolvedCoreModule, resolvedCoreAssembly);
if (!assemblies.ContainsKey(resolvedCoreAssembly))
{
assemblies.Add(resolvedCoreAssembly, resolvedCoreAssemblyInformation);
modules.Add(resolvedCoreModule, resolvedCoreModuleInformation);
assemblyByModule.Add(resolvedCoreModule, resolvedCoreAssembly);
}

this.VoidType = this.GetOrAddType(resolvedCoreModule.TypeSystem.Void);
this.ObjectType = this.GetOrAddType(resolvedCoreModule.TypeSystem.Object);
Expand Down
41 changes: 37 additions & 4 deletions IL2C.Core/Metadata/MetadataUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,27 @@ public MethodSignatureComparerImpl(bool isVirtual)

public int Compare(IMethodInformation x, IMethodInformation y)
{
var rn = x.Name.CompareTo(y.Name);
var xname = x.Name;
var yname = y.Name;

var rn = xname.CompareTo(yname);
if (rn != 0)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyborgyn Is this operator valid negative? (Equals method is same as?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since String implements IComparable, it's not a ReferenceEquals compare anymore defined on Object. It has 3 possible return values of -1, 0, 1, where 0 indicates match. In case the two strings xname and yname don't match, we do a second try, with explicit interface naming match. There were cases observed, where the Equals specifically was used, and even though it should have been match, it didn't get to the point to validate a parameter list, and returned false. The same logic was adapted to the sorting aid, Compare() method.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I look to seem this expression makes short circuit evaluation:

var rn = xname.CompareTo(yname);    // (a)
if (rn != 0)
{
    if (x.DeclaringType.IsInterface)
    {
          xname = $"{x.DeclaringType.FriendlyName}.{xname}";   // (b-1)
    }
    if (y.DeclaringType.IsInterface)
    {
          yname = $"{y.DeclaringType.FriendlyName}.{yname}";  // (b-2)
    }
    rn = xname.CompareTo(yname);   // (c)
    if (rn != 0)
    {
        // ...

At (a) xname value and yname value is different (not 0), and contains string of last part at (b-1) and (b-2), so final result at (c) is always different...? (always not 0)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think about the following scenario:

namespace NS {
  interface Ib {
    void Method();
  }
  class A : Ib {
    void NS.Ib.Method() { ... }
  }
}

The trouble was, for the class MethodInformation.Name was "NS.Ib.Method()", but in the interface definition it was just "Method()". So when searching for interface implementation on the class, it didn't find it. One of them's DeclaringType is the class, the other's is the interface. So, the Equals() always returned false, and in the CallConverters.cs, at line 100 it throw Exception at this place:

                    var implementationMethod = allDeclaredMethods.First(
                        dm => MetadataUtilities.VirtualMethodSignatureComparer.Equals(dm, m));

So, what this thing does, is just give an other chance, putting the full "namespace.interfaceTypeName." before the appropriate method name (x, or y), and does an other compare. A side effect in the case for ordering, is that all interface implementation would be packed together after each other, I guess, but it is not used for things like this, as I am aware. In the case of Equals() there is no side effect.

Copy link
Owner

@kekyo kekyo Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was insight it and finally understood what is problem for ;)

See branch feature/implement-ilcodes-r1 / 19bb2ea

There was nothing regression test for this code fragment, it's my badness....

The callvirt opcode devirtualization function try it a few conditions:

// Invoke interface method with the class-type instance and
// IL2C detected here for can cast statically.
else if (method.DeclaringType.IsInterface &&
     arg0.TargetType.IsClass &&                // (a)
     method.DeclaringType.IsAssignableFrom(arg0.TargetType))

At (a), this expression will test arg0 (this) is class type (analysis by IL2C), but this case couldn't be realize when using C# compiler. (Maybe) C# compiler never generate these conditions.

So I added some test written IL (b159b49, 65ebbbe). These test will cause making (a) condition with true and false. And I understood your explain.

Then I thought of a way to make your method better. In this case, we can only check for method matches by symbolic name, only methods on class/value type.

For example, if the F# compiler generate an explicit interface implementation method, the method names will not necessarily match. To make sure that the methods match exactly, check if they are included in IMethodInformation.Overrides property.

So, I made the following changes to make sure the unit test passes.

// Try strictly matching when both class/value type member method
// and explicitly implemented interface method are same.
if (!x.DeclaringType.IsInterface && y.DeclaringType.IsInterface)
{
    if (x.Overrides.Contains(y))
    {
        return true;
    }
}
if (x.DeclaringType.IsInterface && !y.DeclaringType.IsInterface)
{
    if (y.Overrides.Contains(x))
    {
        return true;
    }
}

{
return rn;
// If it is an explicit interface implementation, we need to compare with it's full name
if (x.DeclaringType.IsInterface)
{
xname = $"{x.DeclaringType.FriendlyName}.{xname}";
}
if (y.DeclaringType.IsInterface)
{
yname = $"{y.DeclaringType.FriendlyName}.{yname}";
}

rn = xname.CompareTo(yname);
if (rn != 0)
{
return rn;
}
}

var xps = x.Parameters;
Expand All @@ -449,9 +466,25 @@ public int Compare(IMethodInformation x, IMethodInformation y)

public bool Equals(IMethodInformation x, IMethodInformation y)
{
if (x.Name != y.Name)
var xname = x.Name;
var yname = y.Name;

if (xname != yname)
{
return false;
// If it is an explicit interface implementation, we need to compare with it's full name
if (x.DeclaringType.IsInterface)
{
xname = $"{x.DeclaringType.FriendlyName}.{xname}";
}
if (y.DeclaringType.IsInterface)
{
yname = $"{y.DeclaringType.FriendlyName}.{yname}";
}

if (xname != yname)
{
return false;
}
}

var xps = x.Parameters;
Expand Down