Adding a jitstressregs_x86_noavx outerloop job group#24650
Conversation
|
CC. @sandreenko, @CarolEidt, @BruceForstall, @echesakovMSFT |
| - Windows_NT_x86 | ||
| ${{ if eq(variables['Build.DefinitionName'], 'coreclr-outerloop-jitstress-regs-x86') }}: | ||
| platforms: | ||
| - Linux_x64 |
There was a problem hiding this comment.
Unlike with the jitstress-isas jobs, OSX is not particularly interesting for the jitstress-regs jobs as compared to Linux.
| testGroup: outerloop-jitstress-isas-arm | ||
| ${{ if eq(variables['Build.DefinitionName'], 'coreclr-outerloop-jitstress-isas-x86') }}: | ||
| testGroup: outerloop-jitstress-isas-x86 | ||
| ${{ if eq(variables['Build.DefinitionName'], 'coreclr-outerloop-jitstress-regs-x86') }}: |
There was a problem hiding this comment.
I added a separator in the names based on the feedback from @sandreenko in the last PR: #24630 (review)
This does make it not match the outer jitstressregs jobs, however. It would likely be worth making them all consistent, and I don't have a preference either way, but the existing build-definitions can't simply be renamed due to other branches/PRs actively using them.
|
This new build-definition (to be created) along with the new outerloop build definitions from #24630 still need to be inserted into the schedule for automatically running (existing schedule is here: #24630 (comment)). |
BruceForstall
left a comment
BruceForstall
left a comment
There was a problem hiding this comment.
I don't like this. The COMPlus variable is JitStressRegs. I don't think that needs any separator. JitStress is a different variable, meaning something different. I don't think they need to "look" the same, as that seems confusing.
Comments?
|
I agree with @BruceForstall that we shouldn't have a separator between |
|
Will fix the names |
|
I have a few questions:
We do not want to create dozens of pipelines in AzDO because it is hard to maintain them; also, we have limited machine resources. |
No, we did not have this exact mode.
Yes. As called out in the top post, Having AVX enabled vs disabled changes the semantics and encoding of a large number of instructions. For normal code, this is any floating-point operation and for performance oriented code this is most of the Ensuring that we are correctly handling things like extended registers for x86 hardware without AVX support is important, especially as the AVX hardware becomes more common place (and is the default for all current x86 machines in our machine pool). |
|
I agree with @tannergooding on this.
Yes, we have found a number of register allocation or encoding issues that are exposed by differences in the handling of 2-operand vs. 3-operand instructions. |
tannergooding
changed the title
|
Any other feedback now that the names have been fixed, or is this good to merge after the last jobs complete? |
…24650) * Adding a jitstress_regs_x86_noavx outerloop job group * Fixing the names of `jitstress-regs` to `jitstressregs` Commit migrated from dotnet/coreclr@2228810
6 participants
As per #24630 (comment), this adds an outerloop build definition group that runs the register jitstress tests with AVX also disabled.
This, unlike many of the other configurations, is particularly interesting as AVX changes most SIMD instructions from
ins op1, src(whereop1is both a dst and src) toins dst, src, src. This can end up impacting register allocation in interesting ways.