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.

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

Add constprop handling for typeof(T) == typeof(Foo) #84224

Merged
merged 4 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 41 additions & 0 deletions src/coreclr/tools/Common/Compiler/TypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,47 @@ public static bool IsArrayTypeWithoutGenericInterfaces(this TypeDesc type)
return type.IsMdArray || elementType.IsPointer || elementType.IsFunctionPointer;
}

public static bool? CompareTypesForEquality(TypeDesc type1, TypeDesc type2)
{
bool? result = null;

// If neither type is a canonical subtype, type handle comparison suffices
if (!type1.IsCanonicalSubtype(CanonicalFormKind.Any) && !type2.IsCanonicalSubtype(CanonicalFormKind.Any))
{
result = type1 == type2;
}
// If either or both types are canonical subtypes, we can sometimes prove inequality.
else
{
// If either is a value type then the types cannot
// be equal unless the type defs are the same.
if (type1.IsValueType || type2.IsValueType)
{
if (!type1.IsCanonicalDefinitionType(CanonicalFormKind.Universal) && !type2.IsCanonicalDefinitionType(CanonicalFormKind.Universal))
{
if (!type1.HasSameTypeDefinition(type2))
{
result = false;
}
}
}
// If we have two ref types that are not __Canon, then the
// types cannot be equal unless the type defs are the same.
else
{
if (!type1.IsCanonicalDefinitionType(CanonicalFormKind.Any) && !type2.IsCanonicalDefinitionType(CanonicalFormKind.Any))
{
if (!type1.HasSameTypeDefinition(type2))
{
result = false;
}
}
}
}

return result;
}

public static TypeDesc MergeTypesToCommonParent(TypeDesc ta, TypeDesc tb)
{
if (ta == tb)
Expand Down
41 changes: 5 additions & 36 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2658,46 +2658,15 @@ private TypeCompareState compareTypesForCast(CORINFO_CLASS_STRUCT_* fromClass, C

private TypeCompareState compareTypesForEquality(CORINFO_CLASS_STRUCT_* cls1, CORINFO_CLASS_STRUCT_* cls2)
{
TypeCompareState result = TypeCompareState.May;

TypeDesc type1 = HandleToObject(cls1);
TypeDesc type2 = HandleToObject(cls2);

// If neither type is a canonical subtype, type handle comparison suffices
if (!type1.IsCanonicalSubtype(CanonicalFormKind.Any) && !type2.IsCanonicalSubtype(CanonicalFormKind.Any))
{
result = (type1 == type2 ? TypeCompareState.Must : TypeCompareState.MustNot);
}
// If either or both types are canonical subtypes, we can sometimes prove inequality.
else
return TypeExtensions.CompareTypesForEquality(type1, type2) switch
{
// If either is a value type then the types cannot
// be equal unless the type defs are the same.
if (type1.IsValueType || type2.IsValueType)
{
if (!type1.IsCanonicalDefinitionType(CanonicalFormKind.Universal) && !type2.IsCanonicalDefinitionType(CanonicalFormKind.Universal))
{
if (!type1.HasSameTypeDefinition(type2))
{
result = TypeCompareState.MustNot;
}
}
}
// If we have two ref types that are not __Canon, then the
// types cannot be equal unless the type defs are the same.
else
{
if (!type1.IsCanonicalDefinitionType(CanonicalFormKind.Any) && !type2.IsCanonicalDefinitionType(CanonicalFormKind.Any))
{
if (!type1.HasSameTypeDefinition(type2))
{
result = TypeCompareState.MustNot;
}
}
}
}

return result;
true => TypeCompareState.Must,
false => TypeCompareState.MustNot,
_ => TypeCompareState.May,
};
}

private CORINFO_CLASS_STRUCT_* mergeClasses(CORINFO_CLASS_STRUCT_* cls1, CORINFO_CLASS_STRUCT_* cls2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ public override MethodIL GetMethodIL(MethodDesc method)
var resultDef = result.GetMethodILDefinition();
if (resultDef != result)
{
MethodIL newBodyDef = GetMethodILWithInlinedSubstitutions(resultDef);
MethodIL newBodyDef = GetMethodILWithInlinedSubstitutions(result);

// If we didn't rewrite the body, we can keep the existing result.
if (newBodyDef != resultDef)
if (newBodyDef != result)
result = new InstantiatedMethodIL(method, newBodyDef);
}
else
Expand Down Expand Up @@ -150,8 +150,6 @@ public MethodIL GetMethodILWithInlinedSubstitutions(MethodIL method)
// We also attempt to rewrite calls to SR.SomeResourceString accessors with string
// literals looked up from the managed resources.

Debug.Assert(method.GetMethodILDefinition() == method);

// Do not attempt to inline resource strings if we only want to use resource keys.
// The optimizations are not compatible.
bool shouldInlineResourceStrings =
Expand Down Expand Up @@ -542,7 +540,7 @@ public MethodIL GetMethodILWithInlinedSubstitutions(MethodIL method)
}
}

return new SubstitutedMethodIL(method, newBody, newEHRegions.ToArray(), debugInfo, newStrings.ToArray());
return new SubstitutedMethodIL(method.GetMethodILDefinition(), newBody, newEHRegions.ToArray(), debugInfo, newStrings.ToArray());
}

private bool TryGetConstantArgument(MethodIL methodIL, byte[] body, OpcodeFlags[] flags, int offset, int argIndex, out int constant)
Expand Down Expand Up @@ -572,6 +570,13 @@ private bool TryGetConstantArgument(MethodIL methodIL, byte[] body, OpcodeFlags[
constant = (int)substitution.Value;
return true;
}
else if (method.IsIntrinsic && method.Name is "op_Inequality" or "op_Equality"
&& method.OwningType is MetadataType mdType
&& mdType.Name == "Type" && mdType.Namespace == "System" && mdType.Module == mdType.Context.SystemModule
&& TryExpandTypeEquality(methodIL, body, flags, currentOffset, method.Name, out constant))
MichalStrehovsky marked this conversation as resolved.
Show resolved Hide resolved
{
return true;
}
else
{
constant = 0;
Expand Down Expand Up @@ -726,6 +731,87 @@ private string GetResourceStringForAccessor(EcmaMethod method)
return null;
}

private static bool TryExpandTypeEquality(MethodIL methodIL, byte[] body, OpcodeFlags[] flags, int offset, string op, out int constant)
{
// We expect to see a sequence:
// ldtoken Foo
// call GetTypeFromHandle
// ldtoken Bar
// call GetTypeFromHandle
// -> offset points here
constant = 0;
const int SequenceLength = 20;
if (offset < SequenceLength)
return false;

if ((flags[offset - SequenceLength] & OpcodeFlags.InstructionStart) == 0)
return false;

ILReader reader = new ILReader(body, offset - SequenceLength);

TypeDesc type1 = ReadLdToken(ref reader, methodIL, flags);
if (type1 == null)
return false;

if (!ReadGetTypeFromHandle(ref reader, methodIL, flags))
return false;

TypeDesc type2 = ReadLdToken(ref reader, methodIL, flags);
if (type1 == null)
return false;

if (!ReadGetTypeFromHandle(ref reader, methodIL, flags))
return false;

// Dataflow runs on top of uninstantiated IL and we can't answer some questions there.
// Unfortunately this means dataflow will still see code that the rest of the system
// might have optimized away. It should not be a problem in practice.
if (type1.ContainsSignatureVariables() || type2.ContainsSignatureVariables())
return false;

bool? equality = TypeExtensions.CompareTypesForEquality(type1, type2);
if (!equality.HasValue)
return false;

constant = equality.Value ? 1 : 0;

if (op == "op_Inequality")
constant ^= 1;

return true;

static TypeDesc ReadLdToken(ref ILReader reader, MethodIL methodIL, OpcodeFlags[] flags)
{
ILOpcode opcode = reader.ReadILOpcode();
if (opcode != ILOpcode.ldtoken)
return null;

TypeDesc t = (TypeDesc)methodIL.GetObject(reader.ReadILToken());

if ((flags[reader.Offset] & OpcodeFlags.BasicBlockStart) != 0)
return null;

return t;
}

static bool ReadGetTypeFromHandle(ref ILReader reader, MethodIL methodIL, OpcodeFlags[] flags)
{
ILOpcode opcode = reader.ReadILOpcode();
if (opcode != ILOpcode.call)
return false;

MethodDesc method = (MethodDesc)methodIL.GetObject(reader.ReadILToken());

if (!method.IsIntrinsic || method.Name != "GetTypeFromHandle")
return false;

if ((flags[reader.Offset] & OpcodeFlags.BasicBlockStart) != 0)
return false;

return true;
}
}

private sealed class SubstitutedMethodIL : MethodIL
{
private readonly byte[] _body;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,19 +327,34 @@ public static void Run()

class TestBranchesInGenericCodeRemoval
{
class ClassWithUnusedVirtual
{
public virtual string MyUnusedVirtualMethod() => typeof(UnusedFromVirtual).ToString();
public virtual string MyUsedVirtualMethod() => typeof(UsedFromVirtual).ToString();
}

class UnusedFromVirtual { }
class UsedFromVirtual { }

struct Unused { public byte Val; }
struct Used { public byte Val; }

[MethodImpl(MethodImplOptions.NoInlining)]
static T Cast<T>(byte o)
static T Cast<T>(byte o, ClassWithUnusedVirtual inst)
{
if (typeof(T) == typeof(Unused))
{
// Expect this not to be scanned. The virtual slot should not be created.
inst.MyUnusedVirtualMethod();

Unused result = new Unused { Val = o };
return (T)(object)result;
}
else if (typeof(T) == typeof(Used))
{
// This will introduce a virtual slot.
inst.MyUsedVirtualMethod();

Used result = new Used { Val = o };
return (T)(object)result;
}
Expand All @@ -350,13 +365,16 @@ public static void Run()
{
Console.WriteLine("Testing dead branches guarded by typeof in generic code removal");

Cast<Used>(12);
Cast<Used>(12, new ClassWithUnusedVirtual());

// We only expect to be able to get rid of it when optimizing
#if !DEBUG
ThrowIfPresentWithTypeHandle(typeof(TestBranchesInGenericCodeRemoval), nameof(Unused));
ThrowIfPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(Unused));
#endif
ThrowIfNotPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(Used));

ThrowIfPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(UnusedFromVirtual));
ThrowIfNotPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(UsedFromVirtual));
}
}

Expand All @@ -372,30 +390,6 @@ private static void ThrowIfPresent(Type testType, string typeName)
}
}

private static void ThrowIfPresentWithTypeHandle(Type testType, string typeName)
{
Type t = GetTypeSecretly(testType, typeName);
if (t == null)
{
throw new Exception("Not found " + typeName);
}

bool thrown = false;
try
{
_ = t.TypeHandle;
}
catch (NotSupportedException)
{
thrown = true;
}

if (!thrown)
{
throw new Exception(typeName + " has type handle");
}
}

private static void ThrowIfNotPresent(Type testType, string typeName)
{
if (GetTypeSecretly(testType, typeName) == null)
Expand Down