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

Add JIT CSE tests #738

Merged
merged 1 commit into from
Apr 18, 2015
Merged

Add JIT CSE tests #738

merged 1 commit into from
Apr 18, 2015

Conversation

libengu
Copy link

@libengu libengu commented Apr 18, 2015

Add JIT CSE (common sub-expression elimination) optimization tests.
Related proj, config, and sln files added and updated. Passed build,
buildtest, runtest. Code went through Microsoft internal process
to meet open-source requirements.

@libengu
Copy link
Author

libengu commented Apr 18, 2015

@AndyAyersMS PTAL

@kangaroo
Copy link

Hi guys! Awesome, love seeing more tests. Few questions:

#1: Whats CSE in this context?
#2: Whats SSS in this context?
#3: Whats ToF in this context?

@libengu
Copy link
Author

libengu commented Apr 18, 2015

CSE is common sub-expression elimination. Others are Microsoft internal terms which I should have avoided to use. I will update the description in a more general way so that everyone can understand. Sorry for the confusion.

Add JIT CSE (common sub-expression elimination) optimization tests.
Related proj, config, and sln files added and updated. Passed build,
buildtest, runtest. Code went through Microsoft internal process
to meet open-source requirements.
@kangaroo
Copy link

ugh "Code went through Microsoft internal process to meet open-source requirements." isn't a bunch better. tag @richlander

@AndyAyersMS
Copy link
Member

@libengu LGTM.

@kangaroo Let us know what sort of detail you think is appropriate for these sorts of PRs. We have several thousand more tests coming.

@kangaroo
Copy link

@AndyAyersMS This one is hard to say. You guys are opening tests: "AWESOME". You're only opening them under opaque acronym criteria: "NOT AWESOME". I spoke at length with @richlander about this recently in person. Hoping my tag will incite some internal conversations.

libengu pushed a commit that referenced this pull request Apr 18, 2015
@libengu libengu merged commit 7fb0805 into dotnet:master Apr 18, 2015
@libengu libengu deleted the AddJITCSETests branch April 18, 2015 06:16
@kangaroo
Copy link

Well, glad that was all merged without any further discussion. Tag @richlander

libengu pushed a commit to libengu/llilc that referenced this pull request Apr 19, 2015
25 JIT CSE test cases were added in CoreCLR repo in dotnet/coreclr#738.
12 of them have issues if run with LLILC. Exclude them
in both powershell and python scripts. Issue dotnet#462
is created for investigation of the causes.
@richlander
Copy link
Member

There are a few pieces here that can pull apart. This one might be a red-herring.

With all the code that we release, we have to process it first to make sure that it meets a minimum bar. The biggest part of this is code comments. We have a tool called "SSS" (no idea what it stands for, but I assume at least one of the S's stands for "sanitize"). It removes bad language and other globally sensitive phrases/terms. Developers have been working on this codebase for nearly two decades under the assumption that the comments were for a very small and internal audience. A minority of those developers were sometimes creative with their approach to commenting the logic. We also removed internal TFS bug numbers that are not relevant to an external audience.

"ToF" is a TFS-resident test tree; nothing more exciting. It stands for "Tree of the Future". It's relatively new and a systematic approach to pulling the most relevant tests from our old ultra-massive test tree to test .NET Core. Our old test tree was typically larger than our product tree, which isn't necessarily a good thing.

We gave been avoiding burdening external folks with these terms and knowledge. My expectation was that people would expect that we had to do something to the code prior to releasing it, so the lack of talking about this isn't hiding anything, just not going into the boring details. We're not rewriting the code.

CSE isn't a term that I'm familiar with. Sounds like pure jargon that has now been clarified. Probably best to avoid that, w/o clarifying as parts of its use.

That all said, I have been encouraging teams to push to a feature branch as soon as the open source release management work has been done. There is some engineering that is going on before the code hits a PR. I'll continue to push on that (I've got an idea for that).

Does that help?

As a model, I took a very different approach (while not code) with my .NET Primer. I posted it as a PR in a grossly unfinished state, with the "[WIP]" marker. That's good, right?

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