From 63f7c2f152443acef349234e6c6dd65eedf38d71 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Thu, 7 Jul 2016 17:55:27 -0700 Subject: [PATCH] Fix RyuJIT/x86 fgMorphMultiregStructArg NYI The code leading to the fgMorphMultiregStructArg NYI shouldn't have been taken for x86. In fgMorphArgs, I put the setting of `hasMultiregStructArgs = true` in one case under `#ifdef`. Disable tests failing with a newly exposed assert, `'isPhiDefn || type != TYP_STRUCT'`, tracked by dotnet/coreclr#6180. I also fixed a number of asserts on strings that would never fail the way they were written -- `assert("string")` instead of `assert(!"string")` -- and fixed some typos/grammar. Commit migrated from https://github.com/dotnet/coreclr/commit/5cd421355052cbd96dc841aa845f95858909e014 --- src/coreclr/src/jit/morph.cpp | 28 ++++++++------- src/coreclr/src/jit/simdcodegenxarch.cpp | 4 +-- src/coreclr/tests/issues.targets | 60 ++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 99cf633..07c55b4 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -3193,7 +3193,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode) } } // Note that there are some additional rules for multireg structs. - // (i.e they cannot be split betwen registers and the stack) + // (i.e they cannot be split between registers and the stack) } else { @@ -3285,15 +3285,15 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode) bool passStructByRef = false; #endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING - // The following if-then-else needs to be carefully refactored - // Basically the else portion wants to turn a struct load (a GT_OBJ)' + // The following if-then-else needs to be carefully refactored. + // Basically the else portion wants to turn a struct load (a GT_OBJ) // into a GT_IND of the appropriate size. - // It can do this with structs sizes that are 1,2,4, or 8 bytes + // It can do this with structs sizes that are 1, 2, 4, or 8 bytes. // It can't do this when FEATURE_UNIX_AMD64_STRUCT_PASSING is defined (Why?) - // TODO-Cleanup: Remove the #ifndef FEATURE_UNIX_AMD64_STRUCT_PASSING below + // TODO-Cleanup: Remove the #ifndef FEATURE_UNIX_AMD64_STRUCT_PASSING below. // It also can't do this if we have a HFA arg, - // unless we have a 1-elem HFA in which case we want to do the optization - // + // unless we have a 1-elem HFA in which case we want to do the optimization. + #ifndef _TARGET_X86_ #ifndef FEATURE_UNIX_AMD64_STRUCT_PASSING // Check for struct argument with size 1, 2, 4 or 8 bytes @@ -3506,10 +3506,14 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode) } } } + +#ifndef _TARGET_X86_ + // TODO-Arm: Does this apply for _TARGET_ARM_, where structs passed by value can be split between registers and stack? if (size > 1) { hasMultiregStructArgs = true; } +#endif // !_TARGET_X86_ } // The 'size' value has now must have been set. (the original value of zero is an invalid value) @@ -4250,14 +4254,14 @@ void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call) NYI_ARM("fgMorphMultiregStructArgs"); #endif #ifdef _TARGET_X86_ - assert("Logic error: no MultiregStructArgs for X86"); + assert(!"Logic error: no MultiregStructArgs for X86"); #endif #ifdef _TARGET_AMD64_ #if defined(UNIX_AMD64_ABI) NYI_AMD64("fgMorphMultiregStructArgs (UNIX ABI)"); -#else -#endif - assert("Logic error: no MultiregStructArgs for Windows X64 ABI"); +#else // !UNIX_AMD64_ABI + assert(!"Logic error: no MultiregStructArgs for Windows X64 ABI"); +#endif // !UNIX_AMD64_ABI #endif for (args = call->gtCallArgs; args != nullptr; args = args->gtOp.gtOp2) @@ -9240,7 +9244,7 @@ GenTreePtr Compiler::fgMorphFieldAssignToSIMDIntrinsicSet(GenTreePtr tree) simdIntrinsicID = SIMDIntrinsicSetW; break; default: - noway_assert("There is no set intrinsic for index bigger than 3"); + noway_assert(!"There is no set intrinsic for index bigger than 3"); } diff --git a/src/coreclr/src/jit/simdcodegenxarch.cpp b/src/coreclr/src/jit/simdcodegenxarch.cpp index f512b46..f6218bd 100644 --- a/src/coreclr/src/jit/simdcodegenxarch.cpp +++ b/src/coreclr/src/jit/simdcodegenxarch.cpp @@ -1233,7 +1233,7 @@ CodeGen::genSIMDIntrinsicRelOp(GenTreeSIMD* simdNode) break; default: - noway_assert("Unimplemented SIMD relational operation."); + noway_assert(!"Unimplemented SIMD relational operation."); unreached(); } @@ -2147,7 +2147,7 @@ CodeGen::genSIMDIntrinsic(GenTreeSIMD* simdNode) break; default: - noway_assert("Unimplemented SIMD intrinsic."); + noway_assert(!"Unimplemented SIMD intrinsic."); unreached(); } } diff --git a/src/coreclr/tests/issues.targets b/src/coreclr/tests/issues.targets index dd4cff6..c67941e 100644 --- a/src/coreclr/tests/issues.targets +++ b/src/coreclr/tests/issues.targets @@ -308,6 +308,66 @@ 6097 + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + + + 6180 + -- 2.7.4