From: Carol Eidt Date: Fri, 29 Mar 2019 23:35:09 +0000 (-0700) Subject: Fix HW intrinsic containment bugs (#23558) X-Git-Tag: accepted/tizen/unified/20190813.215958~54^2~27 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1df87c785e0e43392abf4bcba56e2bf4d9249fd4;p=platform%2Fupstream%2Fcoreclr.git Fix HW intrinsic containment bugs (#23558) * Fix HW intrinsic containment bugs For the Fma case (#23430), fix the handling of contained 3-operand HW intrinsic nodes. For the Bmi case (#23534), fix a bad assert placement, and re-enable the Bmi tests. Fix #23530 Fix #23534 * Add guard for Fma test --- diff --git a/src/jit/hwintrinsiccodegenxarch.cpp b/src/jit/hwintrinsiccodegenxarch.cpp index 4d63bef..37d551f 100644 --- a/src/jit/hwintrinsiccodegenxarch.cpp +++ b/src/jit/hwintrinsiccodegenxarch.cpp @@ -1045,39 +1045,47 @@ void CodeGen::genHWIntrinsic_R_R_R_RM( regSet.tmpRlsTemp(tmpDsc); } - else if (op3->OperIsHWIntrinsic()) + else if (op3->isIndir() || op3->OperIsHWIntrinsic()) { - emit->emitIns_SIMD_R_R_R_AR(ins, attr, targetReg, op1Reg, op2Reg, op3->gtGetOp1()->gtRegNum); - return; - } - else if (op3->isIndir()) - { - GenTreeIndir* memIndir = op3->AsIndir(); - GenTree* memBase = memIndir->gtOp1; + GenTree* addr; + GenTreeIndir* memIndir = nullptr; + if (op3->isIndir()) + { + memIndir = op3->AsIndir(); + addr = memIndir->Addr(); + } + else + { + assert(op3->AsHWIntrinsic()->OperIsMemoryLoad()); + assert(HWIntrinsicInfo::lookupNumArgs(op3->AsHWIntrinsic()) == 1); + addr = op3->gtGetOp1(); + } - switch (memBase->OperGet()) + switch (addr->OperGet()) { case GT_LCL_VAR_ADDR: { - varNum = memBase->AsLclVarCommon()->GetLclNum(); + varNum = addr->AsLclVarCommon()->GetLclNum(); offset = 0; - - // Ensure that all the GenTreeIndir values are set to their defaults. - assert(!memIndir->HasIndex()); - assert(memIndir->Scale() == 1); - assert(memIndir->Offset() == 0); - break; } case GT_CLS_VAR_ADDR: { - emit->emitIns_SIMD_R_R_R_C(ins, attr, targetReg, op1Reg, op2Reg, memBase->gtClsVar.gtClsVarHnd, 0); + emit->emitIns_SIMD_R_R_R_C(ins, attr, targetReg, op1Reg, op2Reg, addr->gtClsVar.gtClsVarHnd, 0); return; } default: { + if (memIndir == nullptr) + { + // This is the HW intrinsic load case. + // Until we improve the handling of addressing modes in the emitter, we'll create a + // temporary GT_IND to generate code with. + GenTreeIndir load = indirForm(op3->TypeGet(), addr); + memIndir = &load; + } emit->emitIns_SIMD_R_R_R_A(ins, attr, targetReg, op1Reg, op2Reg, memIndir); return; } @@ -2132,8 +2140,6 @@ void CodeGen::genBMI1OrBMI2Intrinsic(GenTreeHWIntrinsic* node) case NI_BMI2_MultiplyNoFlags: case NI_BMI2_X64_MultiplyNoFlags: { - // These do not support containment - assert(!op2->isContained()); int numArgs = HWIntrinsicInfo::lookupNumArgs(node); assert(numArgs == 2 || numArgs == 3); @@ -2168,6 +2174,8 @@ void CodeGen::genBMI1OrBMI2Intrinsic(GenTreeHWIntrinsic* node) assert(lowReg != targetReg); } + // These do not support containment + assert(!op2->isContained()); emitAttr attr = emitTypeSize(targetType); // mov the first operand into implicit source operand EDX/RDX if (op1Reg != REG_EDX) diff --git a/tests/issues.targets b/tests/issues.targets index d0bab00..0e51874 100644 --- a/tests/issues.targets +++ b/tests/issues.targets @@ -74,18 +74,6 @@ 23224, often fails with timeout in release - - 23534 - - - 23534 - - - 23534 - - - 23545 - @@ -558,15 +546,6 @@ Needs Triage - - 23534 - - - 23534 - - - 23534 - diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23530/GitHub_23530.cs b/tests/src/JIT/Regression/JitBlue/GitHub_23530/GitHub_23530.cs new file mode 100644 index 0000000..0131eeb --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_23530/GitHub_23530.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Numerics; +using System.Runtime.CompilerServices; +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.X86; + +namespace GitHub_23530 +{ + class Program + { + struct vec + { + public float f1; + public float f2; + public float f3; + public float f4; + } + + static unsafe float fmaTest() + { + vec a; + var b = Vector128.Create(1f); + var c = Vector128.Create(2f); + var d = Vector128.Create(3f); + + c = Fma.MultiplyAdd(Sse.LoadVector128((float*)&a), b, c); + + return Sse.Add(c, d).ToScalar(); + } + + static int Main(string[] args) + { + if (Fma.IsSupported) + { + float result = fmaTest(); + if (Math.Abs(result - 5.0F) > System.Single.Epsilon) + { + return -1; + } + } + return 100; + } + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23530/GitHub_23530.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_23530/GitHub_23530.csproj new file mode 100644 index 0000000..fb6ae36 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_23530/GitHub_23530.csproj @@ -0,0 +1,18 @@ + + + + + Release + AnyCPU + $(MSBuildProjectName) + Exe + + True + True + + + + + + +