Skip to content

Commit

Permalink
[NativeAOT-LLVM] Fix most warnings for the HelloWasm runtime test (#…
Browse files Browse the repository at this point in the history
…2168)

* Fix tentative instance method target

There are two throw helpers used for tentative methods: "ThrowBodyRemoved"
and "ThrowInstanceBodyRemoved". We were always using the first one.

* Add DAM to silence linker warnings

* Disable implicit SDL headers

* Prevent tests from importing "normal" NuGet props/targets

We import them manually from a shared location.

Usually this is not a problem because batch build (via the build script)
does its own restore thing, but it is a problem in case the project is
restored on its own, such as when it is built with "dotnet build".
  • Loading branch information
SingleAccretion authored Jan 16, 2023
1 parent cf8252a commit 07fd0a3
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,10 @@ The .NET Foundation licenses this file to you under the MIT license.
<EmccArgs>&quot;$(LlvmObject)&quot; -c -o &quot;$(NativeObject)&quot; -s DISABLE_EXCEPTION_CATCHING=0</EmccArgs>
<EmccArgs Condition="'$(NativeDebugSymbols)' != 'true'">$(EmccArgs) -O2</EmccArgs>
<EmccArgs Condition="'$(NativeDebugSymbols)' == 'true'">$(EmccArgs) -g3</EmccArgs>
<!-- TODO-LLVM: remove when we update to Emscripten with https://github.com/emscripten-core/emscripten/pull/18443 -->
<EmccArgs>$(EmccArgs) -s USE_SDL=0</EmccArgs>
</PropertyGroup>

<Exec Command="&quot;$(EMSDK)/upstream/emscripten/emcc.bat&quot; $(EmccArgs)" Condition="'$(EMSDK)' != '' and '$(OS)' == 'Windows_NT'" />
<Exec Command="&quot;$(EMSDK)/upstream/emscripten/emcc&quot; $(EmccArgs)" Condition="'$(EMSDK)' != '' and '$(OS)' != 'Windows_NT'" />

Expand All @@ -346,8 +348,10 @@ The .NET Foundation licenses this file to you under the MIT license.
<EmccArgs>&quot;$(LlvmClrjitObject)&quot; -c -o &quot;$(NativeClrjitObject)&quot; -s DISABLE_EXCEPTION_CATCHING=0</EmccArgs>
<EmccArgs Condition="'$(NativeDebugSymbols)' != 'true'">$(EmccArgs) -O2</EmccArgs>
<EmccArgs Condition="'$(NativeDebugSymbols)' == 'true'">$(EmccArgs) -g3</EmccArgs>
<!-- TODO-LLVM: remove when we update to Emscripten with https://github.com/emscripten-core/emscripten/pull/18443 -->
<EmccArgs>$(EmccArgs) -s USE_SDL=0</EmccArgs>
</PropertyGroup>

<Exec Command="&quot;$(EMSDK)/upstream/emscripten/emcc.bat&quot; $(EmccArgs)" Condition="'$(EMSDK)' != '' and '$(OS)' == 'Windows_NT'" />
<Exec Command="&quot;$(EMSDK)/upstream/emscripten/emcc&quot; $(EmccArgs)" Condition="'$(EMSDK)' != '' and '$(OS)' != 'Windows_NT'" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ public TentativeInstanceMethodNode(IMethodBodyNode methodNode)

public override bool HasConditionalStaticDependencies => true;

protected override ISymbolNode GetTarget(NodeFactory factory)
public override IMethodNode GetTarget(NodeFactory factory)
{
// If the class library doesn't provide this helper, the optimization is disabled.
MethodDesc helper = factory.InstanceMethodRemovedHelper;
return helper == null ? RealBody: factory.MethodEntrypoint(helper);
return helper == null ? RealBody : factory.MethodEntrypoint(helper);
}

public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,13 @@ public TentativeMethodNode(IMethodBodyNode methodNode)
_methodNode = methodNode;
}

protected virtual ISymbolNode GetTarget(NodeFactory factory)
public virtual IMethodNode GetTarget(NodeFactory factory)
{
// If the class library doesn't provide this helper, the optimization is disabled.
MethodDesc helper = factory.TypeSystemContext.GetOptionalHelperEntryPoint("ThrowHelpers", "ThrowBodyRemoved");
return helper == null ? RealBody : factory.MethodEntrypoint(helper);
}

public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
{
if (factory.Target.Architecture == TargetArchitecture.Wasm32)
{
return new ObjectData(null, new Relocation[] { new Relocation(RelocType.IMAGE_REL_BASED_HIGHLOW, 0, GetTarget(factory)) }, 0, null);
}
return base.GetData(factory, relocsOnly);
}

public MethodDesc Method => _methodNode.Method;

protected override string GetName(NodeFactory factory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1269,16 +1269,17 @@ private void GetCodeForReadyToRunHelper(LLVMCodegenCompilation compilation, Read

private void GetCodeForTentativeMethod(LLVMCodegenCompilation compilation, TentativeMethodNode node, NodeFactory factory)
{
IMethodNode helperMethodNode = node.GetTarget(factory);
string helperMangledName = helperMethodNode.GetMangledName(compilation.NameMangler);

LLVMBuilderRef builder = LLVMCodegenCompilation.Module.Context.CreateBuilder();
MethodDesc method = node.Method;
string mangledName = node.GetMangledName(factory.NameMangler);
LLVMValueRef tentativeStub = ILImporter.GetOrCreateLLVMFunction(Module, mangledName, method.Signature, method.RequiresInstArg());

LLVMBasicBlockRef block = tentativeStub.AppendBasicBlock("tentativeStub");
builder.PositionAtEnd(block);
MethodDesc helperMethod = factory.TypeSystemContext.GetOptionalHelperEntryPoint("ThrowHelpers", "ThrowBodyRemoved");
string helperMangledName = compilation.NodeFactory.MethodEntrypoint(helperMethod).GetMangledName(compilation.NameMangler);
LLVMValueRef fn = ILImporter.GetOrCreateLLVMFunction(Module, helperMangledName, helperMethod.Signature, false);
LLVMValueRef fn = ILImporter.GetOrCreateLLVMFunction(Module, helperMangledName, helperMethodNode.Method.Signature, false);
builder.BuildCall(fn, new LLVMValueRef[] { tentativeStub.GetParam(0) }, string.Empty);
builder.BuildUnreachable();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ protected override IMethodNode CreateMethodEntrypointNode(MethodDesc method)
}
if (CompilationModuleGroup.ContainsMethodBody(method, false))
{
// TODO-LLVM: delete when merging https://github.com/dotnet/runtime/pull/80414 (Jan 10, 2023)
// We might be able to optimize the method body away if the owning type was never seen as allocated.
if (method.NotCallableWithoutOwningEEType() && CompilationModuleGroup.AllowInstanceMethodOptimization(method))
return new TentativeInstanceMethodNode(new LlvmMethodBodyNode(method));
Expand Down
4 changes: 4 additions & 0 deletions src/tests/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@
<BuildAsStandalone Condition="'$(BuildAsStandalone)' == ''">true</BuildAsStandalone>
<OutputType Condition="$(BuildAsStandalone)">Exe</OutputType>
<TestFramework>GeneratedRunner</TestFramework>

<!-- Prevent project-specific NuGet props/targets from clashing with the ones we manually import below. -->
<ImportProjectExtensionProps>false</ImportProjectExtensionProps>
<ImportProjectExtensionTargets>false</ImportProjectExtensionTargets>
</PropertyGroup>

<Import Project="$(RepositoryEngineeringDir)testing\tests.props" Condition="'$(IsTestsCommonProject)' != 'true'" />
Expand Down
3 changes: 2 additions & 1 deletion src/tests/nativeaot/SmokeTests/HelloWasm/HelloWasm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Collections;
using System.Reflection;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Collections.Specialized;

#if TARGET_WINDOWS
Expand Down Expand Up @@ -1576,7 +1577,7 @@ private static void TestMetaData()
EndTest(Object.ReferenceEquals(retVal, instance));
}

private static void NewMethod(Type classForMetaTestsType, ClassForMetaTests instance)
private static void NewMethod([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type classForMetaTestsType, ClassForMetaTests instance)
{
StartTest("Class get+invoke simple method via reflection");
var mtd = classForMetaTestsType.GetMethod("ReturnTrueIf1");
Expand Down

0 comments on commit 07fd0a3

Please sign in to comment.