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

Conversation

AndyAyersMS
Copy link
Member

Follow-up from #21270 and #21414.

Block general cloning from inadvertently cloning a candidate call.

Add a separate path for cloning calls that are inline and guarded
devirtualization candidates for use by guarded devirtualization.
Callers need to take extra steps after cloning one of these calls
to properly fix everything up.

Also fix up some issues in the large comment for the fat calli
transformation.

Follow-up from dotnet#21270 and dotnet#21414.

Block general cloning from inadvertently cloning a candidate call.

Add a separate path for cloning calls that are inline and guarded
devirtualization candidates for use by guarded devirtualization.
Callers need to take extra steps after cloning one of these calls
to properly fix everything up.

Also fix up some issues in the large comment for the fat calli
transformation.
@AndyAyersMS
Copy link
Member Author

@fiigii @CarolEidt PTAL
cc @dotnet/jit-contrib

Verified jitstress=2 passes on some local tests with and without guarded devirt enabled (it is still off by default).

@AndyAyersMS
Copy link
Member Author

x64 innerloop network issue:

13:32:11 -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/Enterprise/VC/Tools/MSVC/14.15.26726/bin/Hostx86/x64/cl.exe
13:36:25 FATAL: command execution failed
13:36:25 java.nio.channels.ClosedChannelException
13:36:25 	at org.jenkinsci.remoting.protocol.i

@dotnet-bot retest Windows_NT x64 Checked Innerloop Build and Test
@dotnet-bot test Windows_NT x64 Checked jitstress2

@fiigii
Copy link

fiigii commented Dec 6, 2018

Question. gentree.cpp has a few functions (like GenTree::Compare) similar to gtCloneExpr, which need to check every field of the GenTree nodes. Shall we consider them as well?

Copy link

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

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

LGTM - I like factoring this out. Just a couple of comment-comments and a question.

Copy link

@fiigii fiigii left a comment

Choose a reason for hiding this comment

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

Thank you for the work.

@AndyAyersMS
Copy link
Member Author

@fiigii I suppose for completeness sake, we indeed should look at operations like Compare.

@AndyAyersMS
Copy link
Member Author

Arm test succeeded but the parent leg network failed before it could be reported back.

18:40:16                                                 GRAND TOTAL: 2547          0          0           0        7487.579s (555.231s)

18:40:49 Parsing test results from (C:\j\workspace\arm_cross_che---c874ca48\bin\Logs\TestRunResults_Windows_NT_arm_Checked)
18:40:49 
18:40:49 Total tests run    : 2547
18:40:49 Total passing tests: 2547
18:40:49 Total failed tests : 0
18:40:49 Total skipped tests: 0
```

@AndyAyersMS AndyAyersMS merged commit 08be98c into dotnet:master Dec 7, 2018
@AndyAyersMS AndyAyersMS deleted the SpecializeCloningOfCandidateCalls branch December 7, 2018 08:12
morganbr pushed a commit that referenced this pull request Dec 14, 2018
Follow-up from #21270 and #21414.

Block general cloning from inadvertently cloning a candidate call.

Add a separate path for cloning calls that are inline and guarded
devirtualization candidates for use by guarded devirtualization.
Callers need to take extra steps after cloning one of these calls
to properly fix everything up.

Also fix up some issues in the large comment for the fat calli
transformation.
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Follow-up from dotnet/coreclr#21270 and dotnet/coreclr#21414.

Block general cloning from inadvertently cloning a candidate call.

Add a separate path for cloning calls that are inline and guarded
devirtualization candidates for use by guarded devirtualization.
Callers need to take extra steps after cloning one of these calls
to properly fix everything up.

Also fix up some issues in the large comment for the fat calli
transformation.


Commit migrated from dotnet/coreclr@08be98c
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.

3 participants