Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Wasm: add support for ovf op with char type #8257

Merged
merged 2 commits into from
Aug 11, 2020
Merged

Wasm: add support for ovf op with char type #8257

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
008b0b0
add support for ovf op with char type
yowl Aug 7, 2020
10f80de
use stack kind for 32/64 switching and for asserts
yowl Aug 10, 2020
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
4 changes: 2 additions & 2 deletions src/ILCompiler.WebAssembly/src/CodeGen/ILToWebAssemblyImporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3814,8 +3814,8 @@ private void ImportBinaryOperation(ILOpcode opcode)
break;

case ILOpcode.add_ovf:
Debug.Assert(type.Category == TypeFlags.Int32 || type.Category == TypeFlags.Int64);
if (type.Category == TypeFlags.Int32)
Debug.Assert(type.Category == TypeFlags.Int32 || type.Category == TypeFlags.Int64 || type.Category == TypeFlags.Char);
Copy link
Member

@jkotas jkotas Aug 10, 2020

Choose a reason for hiding this comment

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

It does not sound right to have char on the IL stack. Should char be converted to int32 when it gets loaded onto the IL stack instead?

Copy link
Member

@jkotas jkotas Aug 10, 2020

Choose a reason for hiding this comment

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

ILVerify does it in StackValue.CreateFromType method.

Copy link
Contributor Author

@yowl yowl Aug 10, 2020

Choose a reason for hiding this comment

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

Webassembly stack entries keep both the StackValueKind and the TypeDesc . I've changed these asserts and ifs to use thte StackValueKind which I think is more correct and inline with StackValue.CreateFromType

if (type.Category == TypeFlags.Int32 || type.Category == TypeFlags.Char)
{
BuildAddOverflowChecksForSize(ref AddOvf32Function, left, right, LLVMTypeRef.Int32, BuildConstInt32(int.MaxValue), BuildConstInt32(int.MinValue), true);
}
Expand Down
17 changes: 17 additions & 0 deletions tests/src/Simple/HelloWasm/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2272,6 +2272,8 @@ private static unsafe bool CkFinite64(ulong value)

static void TestIntOverflows()
{
TestCharInOvf();

TestSignedIntAddOvf();

TestSignedLongAddOvf();
Expand Down Expand Up @@ -2329,6 +2331,21 @@ private static void TestSignedLongAddOvf()
EndTest(true);
}

private static void TestCharInOvf()
{
// Just checks the compiler can handle the char type
// This was failing for https://github.com/dotnet/corert/blob/f542d97f26e87f633310e67497fb01dad29987a5/src/System.Private.CoreLib/shared/System/Environment.Unix.cs#L111
StartTest("Test char add overflows");
char opChar = '1';
int op32r = 2;
if (checked(opChar + op32r) != 51)
{
FailTest("No overflow for char failed"); // check not always throwing an exception
return;
}
PassTest();
}

private static void TestSignedIntAddOvf()
{
StartTest("Test int add overflows");
Expand Down