Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@parjong
Copy link

@parjong parjong commented Apr 6, 2017

This commit revises x86/Linux unwinder to use GCInfo(GetPushedArgSize) for funclet unwinding.

This commit includes the following additional changes:

  • Enforce fully interruptible codegen for functions with EH funclet
  • Enforce full arg info emission even for EBP frames

@parjong
Copy link
Author

parjong commented Apr 7, 2017

@dotnet-bot test Ubuntu x64 Formatting please

@parjong parjong changed the title [WIP][x86/Linux] Use GCInfo for funclet unwinding [x86/Linux] Use GCInfo for funclet unwinding Apr 7, 2017
@parjong
Copy link
Author

parjong commented Apr 7, 2017

@dotnet-bot test Ubuntu arm Cross Release Build please

@danmoseley
Copy link
Member

@dotnet-bot Test OSX10.12 x64 Checked Build and Test (build break fix)

_ASSERTE(*castto(table, unsigned short *)++ == 0xBABE);
#endif

bool isPartialArgInfo;
Copy link
Member

Choose a reason for hiding this comment

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

A nit, could you please name it hasPartialArgInfo instead (like in "the current frame has partial arg info")?

@janvorli
Copy link
Member

@jkotas, @BruceForstall can you please share your opinion on making the funclets fully interruptible and generating full arg info for them? I would like to make sure there are no downsides I don't see.

@jkotas
Copy link
Member

jkotas commented Apr 10, 2017

The downsides are bigger GC info and lower JIT throughput. Each of these hacks make FEATURE_FIXED_OUT_ARGS more appealing.

@BruceForstall BruceForstall self-requested a review April 10, 2017 20:57
genInterruptible = true;
#endif // UNIX_X86_ABI
}

Choose a reason for hiding this comment

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

Please don't insert the code here; I don't want to dilute or confuse the comment here. Instead, add code below the #endif // _TARGET_X86_ below, e.g.:

#ifdef UNIX_X86_ABI
if (info.compXcptnsCount > 0)
{
assert(!codeGen->isGCTypeFixed());
// Enforce fully interruptible codegen for funclet unwinding
genInterruptible = true;
}
#endif // UNIX_X86_ABI

@BruceForstall
Copy link

As a data point, can you compute the increase in GC info size on System.Private.CoreLib.ni.dll? If you can use jit-diff (from jitutils), the jit-diff diff command takes a --gcinfo argument that might help, although I'm not sure if it automatically adds up the size of all GC info and compares it base vs. diff.

@parjong
Copy link
Author

parjong commented Apr 10, 2017

@BruceForstall jit-diff is still unavailable on x86/Linux. It seems that jit-diff has a strict dependency on dotnet host which is unavailable, yet:

runtime/corerun jitutils/bin/jit-diff.dll diff
Environment variable JIT_UTILS_ROOT not found - no configuration loaded.

Unhandled Exception: System.InvalidOperationException: Unable to locate dotnet multiplexer
   at Microsoft.DotNet.Cli.Utils.Muxer.get_MuxerPath()
   at Microsoft.DotNet.Cli.Utils.MuxerCommandResolver.Resolve(CommandResolverArguments commandResolverArguments)
   at Microsoft.DotNet.Cli.Utils.CompositeCommandResolver.Resolve(CommandResolverArguments commandResolverArguments)
   at Microsoft.DotNet.Cli.Utils.CommandResolver.TryResolveCommandSpec(ICommandResolverPolicy commandResolverPolicy, String commandName, IEnumerable`1 args, NuGetFramework framework, String configuration, String outputPath, String applicationName)
   at Microsoft.DotNet.Cli.Utils.Command.Create(ICommandResolverPolicy commandResolverPolicy, String commandName, IEnumerable`1 args, NuGetFramework framework, String configuration, String outputPath, String applicationName)
   at ManagedCodeGen.jitdiff.TryCommand(String name, IEnumerable`1 commandArgs, Boolean capture)
   at ManagedCodeGen.jitdiff.Config.SetRID()
   at ManagedCodeGen.jitdiff.Config..ctor(String[] args)
   at ManagedCodeGen.jitdiff.Main(String[] args)
Aborted (core dumped)

@BruceForstall
Copy link

@parjong Have you tried jit-diff on linux/x64? You could do Linux/x86 asm diffs there using the altjit.

@parjong
Copy link
Author

parjong commented Apr 11, 2017

@BruceForstall I tried, but x86/Linux JIT cannot be loaded from x64/Linux crossgen due to dynamic library issue. x86/Linux JIT is linked to x86/Linux libunwind which does not exist in x64/Linux.

@BruceForstall
Copy link

@parjong ok. you could do it from x86 Windows.

@BruceForstall BruceForstall merged commit 7e5e9c0 into dotnet:master Apr 11, 2017
@parjong parjong deleted the fix/x86_funclet_unwinding branch April 12, 2017 00:11
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
CarolEidt added a commit to CarolEidt/coreclr that referenced this pull request Apr 5, 2018
This assert was formerly enabled only for the "full arg info case", i.e. when there was not a frame on x86. WIth PR dotnet#10782 the sense of the first condition was inadvertently changed. With PR dotnet#17363 the tracking of `argCnt` vs. `argHigh` was fixed so that the assert is correct for the "partial arg info" case.
The first condition should be removed to make it cover the "full argo info case" as well (as before dotnet#10782).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants