-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
@AndyAyersMS PTAL |
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.
ugh "Code went through Microsoft internal process to meet open-source requirements." isn't a bunch better. tag @richlander |
@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. |
Well, glad that was all merged without any further discussion. Tag @richlander |
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.
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? |
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.