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

add test with deep execution tree without stores. #10835

Merged
merged 4 commits into from
Apr 10, 2017

Conversation

sandreenko
Copy link

It will not fail if the compiler spills trees from eeStack from time to time or all recursive walkers are replaced by non-recursive algorithms.

Copy link

@JosephTremoulet JosephTremoulet left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding this.

@pgavlin
Copy link

pgavlin commented Apr 10, 2017

Is there a reason the existing https://github.com/dotnet/coreclr/blob/master/tests/src/JIT/jit64/opt/cse/hugeexpr1.cs is insufficient for this scenario?

@sandreenko
Copy link
Author

I didn't see any failures in ci when I disabled stack spilling and send PR.
I think hugeexpt1.cs doesn't have very deep tree, the biggest one has deep near 600 as I see.

@pgavlin
Copy link

pgavlin commented Apr 10, 2017

Hm. I am pretty sure that we're already covering this somewhere... LGTM if we aren't, in any case.

@sandreenko
Copy link
Author

I want to double check.

@sandreenko
Copy link
Author

sandreenko commented Apr 10, 2017

It is pretty interesting that we see this failure only in Windows_NT x64 Debug Build and Test. It is what I tested locally.

And as you see there are no failures on existing tests.
I think I should try to make it fails in Release and Checked. Maybe just to increase the depth of the tree.

Sergey Andreenko added 2 commits April 10, 2017 13:22
now it fails in Checked and Release.
@sandreenko
Copy link
Author

Ok, now it looks fine. It breaks in all modes (x64/x86 Debug/Release/Checked).
So I want to merge it as soon as testing will be finished.

@sandreenko sandreenko merged commit 41933db into dotnet:master Apr 10, 2017
@sandreenko sandreenko deleted the add_test branch April 10, 2017 22:42
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
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.

5 participants