From e487fe3aefac98b3583ad29e10cfb08be7027fb2 Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Thu, 3 Dec 2020 13:50:45 -0800 Subject: [PATCH 01/16] GT_NEG optimization for multiplication and division --- src/coreclr/src/jit/morph.cpp | 68 +++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 71f905a79cc44..411d4209cbda2 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -11845,6 +11845,21 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_MUL: + //if (opts.OptimizationEnabled() && fgGlobalMorph && + // (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) + //{ + // // -a * C => a * -C, where C is constant + // // MUL(NEG(a), C) => MUL(a, NEG(C)) + // if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) + // { + // // tree: MUL + // // op1: a + // // op2: NEG + // // op2Child: C + // // tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; // a + // // op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue()); // -C + // } + //} #ifndef TARGET_64BIT if (typ == TYP_LONG) { @@ -12011,6 +12026,28 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) { op2 = gtFoldExprConst(op2); } + + if (opts.OptimizationEnabled() && fgGlobalMorph && + (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) + { + // -a / C => a / -C, where C is constant + // DIV(NEG(a), C) => DIV(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) + { + // tree: DIV + // op1: a + // op2: NEG + // op2Child: C + GenTree* op1Child = op1->AsOp()->gtOp1; // a + GenTree* newOp2 = gtNewOperNode(GT_NEG, op2->TypeGet(), op2); // -C + + DEBUG_DESTROY_NODE(op1); + DEBUG_DESTROY_NODE(op2); + + op1 = op1Child; + op2 = newOp2; + } + } break; case GT_UDIV: @@ -13366,6 +13403,21 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) break; case GT_MUL: + if (opts.OptimizationEnabled() && fgGlobalMorph && + (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) + { + // -a * C => a * -C, where C is constant + // MUL(NEG(a), C) => MUL(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) + { + // tree: MUL + // op1: a + // op2: NEG + // op2Child: C + // tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; // a + // op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue()); // -C + } + } #ifndef TARGET_64BIT if (typ == TYP_LONG) @@ -14921,6 +14973,22 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree) } } + if (opts.OptimizationEnabled() && fgGlobalMorph && + (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) + { + // -a * C => a * -C, where C is constant + // MUL(NEG(a), C) => MUL(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) + { + // tree: MUL + // op1: a + // op2: NEG + // op2Child: C + // tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; // a + // op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue()); // -C + } + } + break; case GT_DIV: From 7f04f5584f7cffd751141fdb77100f1c7784e670 Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Thu, 3 Dec 2020 14:24:18 -0800 Subject: [PATCH 02/16] Distribute negation over parenthetical multiplication or division. --- src/coreclr/src/jit/morph.cpp | 100 ++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 411d4209cbda2..40b51bf8b41c9 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -11843,23 +11843,23 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_CAST: return fgMorphCast(tree); - case GT_MUL: + case GT_MUL: + // -a * C => a * -C, where C is constant + // MUL(NEG(a), C) => MUL(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->IsCnsIntOrI() && op2->IsCnsIntOrI()) + { + // tree: MUL + // op1: a + // op2: NEG + // op2Child: C + op1 = op1->AsOp()->gtOp1; // a + op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C + + tree->AsOp()->gtOp1 = op1; + tree->AsOp()->gtOp2 = op2; + return tree; + } - //if (opts.OptimizationEnabled() && fgGlobalMorph && - // (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) - //{ - // // -a * C => a * -C, where C is constant - // // MUL(NEG(a), C) => MUL(a, NEG(C)) - // if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) - // { - // // tree: MUL - // // op1: a - // // op2: NEG - // // op2Child: C - // // tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; // a - // // op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue()); // -C - // } - //} #ifndef TARGET_64BIT if (typ == TYP_LONG) { @@ -12006,6 +12006,22 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return fgMorphSmpOp(tree, mac); } + // -a / C => a / -C, where C is constant + // DIV(NEG(a), C) => DIV(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->IsCnsIntOrI() && op2->IsCnsIntOrI()) + { + // tree: DIV + // op1: a + // op2: NEG + // op2Child: C + op1 = op1->AsOp()->gtOp1; // a + op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C + + tree->AsOp()->gtOp1 = op1; + tree->AsOp()->gtOp2 = op2; + return tree; + } + #ifndef TARGET_64BIT if (typ == TYP_LONG) { @@ -13403,21 +13419,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) break; case GT_MUL: - if (opts.OptimizationEnabled() && fgGlobalMorph && - (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) - { - // -a * C => a * -C, where C is constant - // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) - { - // tree: MUL - // op1: a - // op2: NEG - // op2Child: C - // tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; // a - // op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue()); // -C - } - } + #ifndef TARGET_64BIT if (typ == TYP_LONG) @@ -13907,6 +13909,26 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return child; } + // Distribute negation over simple multiplication/division expressions + if (op1->OperIs(GT_MUL) || op1->OperIs(GT_DIV)) { + if (!op1->AsOp()->gtOp1->IsCnsIntOrI() && op1->AsOp()->gtOp2->IsCnsIntOrI()) + { + // -(a * C) => a * -C + // -(a / C) => a / -C + oper = op1->gtOper; + tree->SetOper(op1->gtOper); + GenTree* newOp1 = op1->AsOp()->gtOp1; // a + GenTree* newOp2 = op1->AsOp()->gtOp2; // C + newOp2->AsIntCon()->SetIconValue(-newOp2->AsIntCon()->IconValue()); + + // Only need to destroy op1 since op2 will be null already + DEBUG_DESTROY_NODE(op1); + + tree->AsOp()->gtOp1 = op1 = newOp1; + tree->AsOp()->gtOp2 = op2 = newOp2; + } + } + /* Any constant cases should have been folded earlier */ noway_assert(!op1->OperIsConst() || !opts.OptEnabled(CLFLG_CONSTANTFOLD) || optValnumCSE_phase); break; @@ -14973,22 +14995,6 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree) } } - if (opts.OptimizationEnabled() && fgGlobalMorph && - (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) - { - // -a * C => a * -C, where C is constant - // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) - { - // tree: MUL - // op1: a - // op2: NEG - // op2Child: C - // tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; // a - // op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue()); // -C - } - } - break; case GT_DIV: From bbaf21f09ab464f831887b4505f2ab3ceda60ea1 Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Fri, 4 Dec 2020 11:00:35 -0800 Subject: [PATCH 03/16] Removing duplicate logic that I had put in accidently. --- src/coreclr/src/jit/morph.cpp | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 40b51bf8b41c9..3addfe0a8c8be 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -12043,27 +12043,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) op2 = gtFoldExprConst(op2); } - if (opts.OptimizationEnabled() && fgGlobalMorph && - (((op1->gtFlags & GTF_EXCEPT) == 0) || ((op2->gtFlags & GTF_EXCEPT) == 0))) - { - // -a / C => a / -C, where C is constant - // DIV(NEG(a), C) => DIV(a, NEG(C)) - if (op1->OperIs(GT_NEG) && !op1->IsConstInitVal() && op2->IsConstInitVal()) - { - // tree: DIV - // op1: a - // op2: NEG - // op2Child: C - GenTree* op1Child = op1->AsOp()->gtOp1; // a - GenTree* newOp2 = gtNewOperNode(GT_NEG, op2->TypeGet(), op2); // -C - - DEBUG_DESTROY_NODE(op1); - DEBUG_DESTROY_NODE(op2); - - op1 = op1Child; - op2 = newOp2; - } - } break; case GT_UDIV: From bc91dcce9a0ad03d0b480ad3aa594d381160e53e Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Mon, 7 Dec 2020 15:52:59 -0800 Subject: [PATCH 04/16] Check overflow and other conditions before performing morph --- src/coreclr/src/jit/morph.cpp | 205 +++++++++++++++++----------------- 1 file changed, 105 insertions(+), 100 deletions(-) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 3addfe0a8c8be..deebaed8d1921 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -175,7 +175,7 @@ GenTree* Compiler::fgMorphCast(GenTree* tree) // x86: src = float, dst = uint32/int64/uint64 or overflow conversion. && (tree->gtOverflow() || varTypeIsLong(dstType) || (dstType == TYP_UINT)) #endif - ) + ) { oper = gtNewCastNode(TYP_DOUBLE, oper, false, TYP_DOUBLE); } @@ -1309,7 +1309,7 @@ void fgArgInfo::ArgsComplete() || curArgTabEntry->isTmp // I protect this by "FEATURE_FIXED_OUT_ARGS" to preserve the property // that we only have late non-register args when that feature is on. #endif // FEATURE_FIXED_OUT_ARGS - ) + ) { curArgTabEntry->needTmp = true; needsTemps = true; @@ -1575,7 +1575,7 @@ void fgArgInfo::ArgsComplete() if (compiler->fgWalkTreePre(&argx, Compiler::fgChkLocAllocCB) == Compiler::WALK_ABORT) { curArgTabEntry->needTmp = true; - needsTemps = true; + needsTemps = true; continue; } } @@ -2718,8 +2718,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) } *insertionPoint = gtNewCallArgs(arg); #else // !defined(TARGET_X86) - // All other architectures pass the cookie in a register. - call->gtCallArgs = gtPrependNewCallArg(arg, call->gtCallArgs); + // All other architectures pass the cookie in a register. + call->gtCallArgs = gtPrependNewCallArg(arg, call->gtCallArgs); #endif // defined(TARGET_X86) nonStandardArgs.Add(arg, REG_PINVOKE_COOKIE_PARAM); @@ -2786,8 +2786,9 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // This is a register argument - put it in the table. call->fgArgInfo->AddRegArg(argIndex, argx, call->gtCallThisArg, regNum, numRegs, byteSize, byteAlignment, - isStruct, callIsVararg UNIX_AMD64_ABI_ONLY_ARG(REG_STK) UNIX_AMD64_ABI_ONLY_ARG(0) - UNIX_AMD64_ABI_ONLY_ARG(0) UNIX_AMD64_ABI_ONLY_ARG(nullptr)); + isStruct, + callIsVararg UNIX_AMD64_ABI_ONLY_ARG(REG_STK) UNIX_AMD64_ABI_ONLY_ARG(0) + UNIX_AMD64_ABI_ONLY_ARG(0) UNIX_AMD64_ABI_ONLY_ARG(nullptr)); intArgRegNum++; #ifdef WINDOWS_AMD64_ABI @@ -2822,8 +2823,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) { noway_assert(call->gtCallArgs->GetNode()->TypeGet() == TYP_I_IMPL || call->gtCallArgs->GetNode()->TypeGet() == TYP_BYREF || - call->gtCallArgs->GetNode()->gtOper == - GT_NOP); // the arg was already morphed to a register (fgMorph called twice) + call->gtCallArgs->GetNode()->gtOper == GT_NOP); // the arg was already morphed to a register + // (fgMorph called twice) maxRegArgs = 1; } else @@ -3118,8 +3119,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // HFA structs are passed by value in multiple registers. // The "size" in registers may differ the size in pointer-sized units. CORINFO_CLASS_HANDLE structHnd = gtGetStructHandle(argx); - size = GetHfaCount(structHnd); - byteSize = info.compCompHnd->getClassSize(structHnd); + size = GetHfaCount(structHnd); + byteSize = info.compCompHnd->getClassSize(structHnd); } else { @@ -3128,7 +3129,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // if sufficient registers are available. // Structs that are larger than 2 pointers (except for HFAs) are passed by // reference (to a copy) - size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; + size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; byteSize = structSize; if (size > 2) { @@ -3140,20 +3141,20 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) } else { - size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot' + size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot' byteSize = genTypeSize(argx); } #elif defined(TARGET_ARM) || defined(TARGET_X86) if (isStructArg) { - size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; + size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; byteSize = structSize; } else { // The typical case. // Long/double type argument(s) will be modified as needed in Lowering. - size = genTypeStSz(argx->gtType); + size = genTypeStSz(argx->gtType); byteSize = genTypeSize(argx); } #else @@ -3219,7 +3220,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) #ifdef UNIX_AMD64_ABI && (!isStructArg || structDesc.passedInRegisters) #endif - ) + ) { #ifdef TARGET_ARM if (passUsingFloatRegs) @@ -3264,7 +3265,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // registers // unsigned roundupSize = (unsigned)roundUp(structSize, TARGET_POINTER_SIZE); - size = roundupSize / TARGET_POINTER_SIZE; + size = roundupSize / TARGET_POINTER_SIZE; // We also must update fltArgRegNum so that we no longer try to // allocate any new floating point registers for args @@ -3311,7 +3312,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) if (isStructArg) { unsigned int structFloatRegs = 0; - unsigned int structIntRegs = 0; + unsigned int structIntRegs = 0; for (unsigned int i = 0; i < structDesc.eightByteCount; i++) { if (structDesc.IsIntegralSlot(i)) @@ -3452,11 +3453,12 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) #endif // This is a register argument - put it in the table - newArgEntry = call->fgArgInfo->AddRegArg(argIndex, argx, args, nextRegNum, size, byteSize, argAlignBytes, - isStructArg, callIsVararg UNIX_AMD64_ABI_ONLY_ARG(nextOtherRegNum) - UNIX_AMD64_ABI_ONLY_ARG(structIntRegs) - UNIX_AMD64_ABI_ONLY_ARG(structFloatRegs) - UNIX_AMD64_ABI_ONLY_ARG(&structDesc)); + newArgEntry = + call->fgArgInfo->AddRegArg(argIndex, argx, args, nextRegNum, size, byteSize, argAlignBytes, isStructArg, + callIsVararg UNIX_AMD64_ABI_ONLY_ARG(nextOtherRegNum) + UNIX_AMD64_ABI_ONLY_ARG(structIntRegs) + UNIX_AMD64_ABI_ONLY_ARG(structFloatRegs) + UNIX_AMD64_ABI_ONLY_ARG(&structDesc)); newArgEntry->SetIsBackFilled(isBackFilled); newArgEntry->isNonStandard = isNonStandard; @@ -3498,7 +3500,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // we skip the corresponding floating point register argument intArgRegNum = min(intArgRegNum + size, MAX_REG_ARG); #endif // WINDOWS_AMD64_ABI - // No supported architecture supports partial structs using float registers. + // No supported architecture supports partial structs using float registers. assert(fltArgRegNum <= MAX_FLOAT_REG_ARG); } else @@ -3793,12 +3795,12 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) assert(size == 1); copyBlkClass = objClass; #else // UNIX_AMD64_ABI - // On Unix, structs are always passed by value. - // We only need a copy if we have one of the following: - // - The sizes don't match for a non-lclVar argument. - // - We have a known struct type (e.g. SIMD) that requires multiple registers. - // TODO-Amd64-Unix-Throughput: We don't need to keep the structDesc in the argEntry if it's not - // actually passed in registers. + // On Unix, structs are always passed by value. + // We only need a copy if we have one of the following: + // - The sizes don't match for a non-lclVar argument. + // - We have a known struct type (e.g. SIMD) that requires multiple registers. + // TODO-Amd64-Unix-Throughput: We don't need to keep the structDesc in the argEntry if it's not + // actually passed in registers. if (argEntry->isPassedInRegisters()) { if (argObj->OperIs(GT_OBJ)) @@ -4008,16 +4010,16 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // Build the mkrefany as a comma node: // (tmp.ptr=argx),(tmp.type=handle) - GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); + GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); GenTreeLclFld* destTypeSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__type); destPtrSlot->SetFieldSeq(GetFieldSeqStore()->CreateSingleton(GetRefanyDataField())); destPtrSlot->gtFlags |= GTF_VAR_DEF; destTypeSlot->SetFieldSeq(GetFieldSeqStore()->CreateSingleton(GetRefanyTypeField())); destTypeSlot->gtFlags |= GTF_VAR_DEF; - GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); + GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); GenTree* asgTypeSlot = gtNewAssignNode(destTypeSlot, argx->AsOp()->gtOp2); - GenTree* asg = gtNewOperNode(GT_COMMA, TYP_VOID, asgPtrSlot, asgTypeSlot); + GenTree* asg = gtNewOperNode(GT_COMMA, TYP_VOID, asgPtrSlot, asgTypeSlot); // Change the expression to "(tmp=val)" args->SetNode(asg); @@ -4454,7 +4456,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if !defined(HOST_UNIX) && defined(TARGET_ARM64) && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - ) + ) { elemType = hfaType; elemSize = genTypeSize(elemType); @@ -4566,7 +4568,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if !defined(HOST_UNIX) && defined(TARGET_ARM64) && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - ) + ) { // We have a HFA struct. noway_assert(elemType == varDsc->GetHfaType()); @@ -4612,11 +4614,12 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if defined(TARGET_ARM64) || defined(UNIX_AMD64_ABI) // Is this LclVar a promoted struct with exactly 2 fields? // TODO-ARM64-CQ: Support struct promoted HFA types here - if (varDsc->lvPromoted && (varDsc->lvFieldCnt == 2) && (!varDsc->lvIsHfa() + if (varDsc->lvPromoted && (varDsc->lvFieldCnt == 2) && + (!varDsc->lvIsHfa() #if !defined(HOST_UNIX) && defined(TARGET_ARM64) - && !fgEntryPtr->IsVararg() + && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - )) + )) { // See if we have two promoted fields that start at offset 0 and 8? unsigned loVarNum = lvaGetFieldLocal(varDsc, 0); @@ -5049,7 +5052,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, // Structs are always on the stack, and thus never need temps // so we have to put the copy and temp all into one expression. argEntry->tmpNum = tmp; - GenTree* arg = fgMakeTmpArgNode(argEntry); + GenTree* arg = fgMakeTmpArgNode(argEntry); // Change the expression to "(tmp=val),tmp" arg = gtNewOperNode(GT_COMMA, arg->TypeGet(), copyBlk, arg); @@ -6295,7 +6298,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) // Create "comma2" node and link it to "tree". // GenTree* comma2; - comma2 = gtNewOperNode(GT_COMMA, + comma2 = gtNewOperNode(GT_COMMA, addr->TypeGet(), // The type of "comma2" node is the same as the type of "addr" node. comma, addr); tree->AsOp()->gtOp1 = comma2; @@ -9025,8 +9028,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) // copy-back). unsigned retValTmpNum = BAD_VAR_NUM; CORINFO_CLASS_HANDLE structHnd = nullptr; - if (call->HasRetBufArg() && - call->gtCallLateArgs == nullptr) // Don't do this if we're re-morphing (which will make late args non-null). + if (call->HasRetBufArg() && call->gtCallLateArgs == nullptr) // Don't do this if we're re-morphing (which will make + // late args non-null). { // We're enforcing the invariant that return buffers pointers (at least for // struct return types containing GC pointers) are never pointers into the heap. @@ -11540,16 +11543,16 @@ GenTree* Compiler::getSIMDStructFromField(GenTree* tree, } /***************************************************************************** -* If a read operation tries to access simd struct field, then transform the -* operation to the SIMD intrinsic SIMDIntrinsicGetItem, and return the new tree. -* Otherwise, return the old tree. -* Argument: -* tree - GenTree*. If this pointer points to simd struct which is used for simd -* intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. -* Return: -* A GenTree* which points to the new tree. If the tree is not for simd intrinsic, -* return nullptr. -*/ + * If a read operation tries to access simd struct field, then transform the + * operation to the SIMD intrinsic SIMDIntrinsicGetItem, and return the new tree. + * Otherwise, return the old tree. + * Argument: + * tree - GenTree*. If this pointer points to simd struct which is used for simd + * intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. + * Return: + * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, + * return nullptr. + */ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) { @@ -11570,16 +11573,16 @@ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) } /***************************************************************************** -* Transform an assignment of a SIMD struct field to SIMD intrinsic -* SIMDIntrinsicSet*, and return a new tree. If it is not such an assignment, -* then return the old tree. -* Argument: -* tree - GenTree*. If this pointer points to simd struct which is used for simd -* intrinsic, we will morph it as simd intrinsic set. -* Return: -* A GenTree* which points to the new tree. If the tree is not for simd intrinsic, -* return nullptr. -*/ + * Transform an assignment of a SIMD struct field to SIMD intrinsic + * SIMDIntrinsicSet*, and return a new tree. If it is not such an assignment, + * then return the old tree. + * Argument: + * tree - GenTree*. If this pointer points to simd struct which is used for simd + * intrinsic, we will morph it as simd intrinsic set. + * Return: + * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, + * return nullptr. + */ GenTree* Compiler::fgMorphFieldAssignToSIMDIntrinsicSet(GenTree* tree) { @@ -11843,20 +11846,21 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_CAST: return fgMorphCast(tree); - case GT_MUL: + case GT_MUL: // -a * C => a * -C, where C is constant // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (op1->OperIs(GT_NEG) && !op1->IsCnsIntOrI() && op2->IsCnsIntOrI()) + if (fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && + !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && + op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) { // tree: MUL // op1: a // op2: NEG // op2Child: C - op1 = op1->AsOp()->gtOp1; // a - op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C - tree->AsOp()->gtOp1 = op1; - tree->AsOp()->gtOp2 = op2; + tree->AsOp()->gtOp1 = op1->gtGetOp1(); + DEBUG_DESTROY_NODE(op1); + op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C return tree; } @@ -12008,17 +12012,18 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // -a / C => a / -C, where C is constant // DIV(NEG(a), C) => DIV(a, NEG(C)) - if (op1->OperIs(GT_NEG) && !op1->IsCnsIntOrI() && op2->IsCnsIntOrI()) + if (fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && + !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && + op2->AsIntCon()->IconValue() != -1) { // tree: DIV // op1: a // op2: NEG // op2Child: C - op1 = op1->AsOp()->gtOp1; // a - op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C - tree->AsOp()->gtOp1 = op1; - tree->AsOp()->gtOp2 = op2; + tree->AsOp()->gtOp1 = op1->gtGetOp1(); + DEBUG_DESTROY_NODE(op1); + op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C return tree; } @@ -12230,25 +12235,25 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } } #else // !TARGET_ARM64 - // If b is not a power of 2 constant then lowering replaces a % b - // with a - (a / b) * b and applies magic division optimization to - // a / b. The code may already contain an a / b expression (e.g. - // x = a / 10; y = a % 10;) and then we end up with redundant code. - // If we convert % to / here we give CSE the opportunity to eliminate - // the redundant division. If there's no redundant division then - // nothing is lost, lowering would have done this transform anyway. + // If b is not a power of 2 constant then lowering replaces a % b + // with a - (a / b) * b and applies magic division optimization to + // a / b. The code may already contain an a / b expression (e.g. + // x = a / 10; y = a % 10;) and then we end up with redundant code. + // If we convert % to / here we give CSE the opportunity to eliminate + // the redundant division. If there's no redundant division then + // nothing is lost, lowering would have done this transform anyway. if (!optValnumCSE_phase && ((tree->OperGet() == GT_MOD) && op2->IsIntegralConst())) { - ssize_t divisorValue = op2->AsIntCon()->IconValue(); - size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) + ssize_t divisorValue = op2->AsIntCon()->IconValue(); + size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) : static_cast(abs(divisorValue)); if (!isPow2(absDivisorValue)) { tree = fgMorphModToSubMulDiv(tree->AsOp()); - op1 = tree->AsOp()->gtOp1; - op2 = tree->AsOp()->gtOp2; + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; } } #endif // !TARGET_ARM64 @@ -13399,7 +13404,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_MUL: - #ifndef TARGET_64BIT if (typ == TYP_LONG) { @@ -13889,7 +13893,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } // Distribute negation over simple multiplication/division expressions - if (op1->OperIs(GT_MUL) || op1->OperIs(GT_DIV)) { + if (op1->OperIs(GT_MUL) || op1->OperIs(GT_DIV)) + { if (!op1->AsOp()->gtOp1->IsCnsIntOrI() && op1->AsOp()->gtOp2->IsCnsIntOrI()) { // -(a * C) => a * -C @@ -13905,7 +13910,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) tree->AsOp()->gtOp1 = op1 = newOp1; tree->AsOp()->gtOp2 = op2 = newOp2; - } + } } /* Any constant cases should have been folded earlier */ @@ -14756,8 +14761,8 @@ GenTree* Compiler::fgMorphRetInd(GenTreeUnOp* ret) #if defined(TARGET_64BIT) bool canFold = (indSize == lclVarSize); #else // !TARGET_64BIT - // TODO: improve 32 bit targets handling for LONG returns if necessary, nowadays we do not support `BITCAST - // long<->double` there. + // TODO: improve 32 bit targets handling for LONG returns if necessary, nowadays we do not support `BITCAST + // long<->double` there. bool canFold = (indSize == lclVarSize) && (lclVarSize <= REGSIZE_BYTES); #endif // TODO: support `genReturnBB != nullptr`, it requires #11413 to avoid `Incompatible types for @@ -15467,7 +15472,7 @@ GenTree* Compiler::fgMorphToEmulatedFP(GenTree* tree) case GT_DIV: helper = CPX_R4_DIV; break; - // case GT_MOD: helper = CPX_R4_REM; break; + // case GT_MOD: helper = CPX_R4_REM; break; case GT_EQ: helper = CPX_R4_EQ; @@ -15641,12 +15646,12 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) } } -/*------------------------------------------------------------------------- - * fgMorphTree() can potentially replace a tree with another, and the - * caller has to store the return value correctly. - * Turn this on to always make copy of "tree" here to shake out - * hidden/unupdated references. - */ + /*------------------------------------------------------------------------- + * fgMorphTree() can potentially replace a tree with another, and the + * caller has to store the return value correctly. + * Turn this on to always make copy of "tree" here to shake out + * hidden/unupdated references. + */ #ifdef DEBUG @@ -16003,7 +16008,7 @@ void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree) void Compiler::fgMorphTreeDone(GenTree* tree, GenTree* oldTree /* == NULL */ - DEBUGARG(int morphNum)) + DEBUGARG(int morphNum)) { #ifdef DEBUG if (verbose && treesBeforeAfterMorph) @@ -18450,7 +18455,7 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTree* tree) if (tree->AsOp()->gtOp1->gtOper == GT_LCL_VAR) { GenTree* morphedTree = fgMorphImplicitByRefArgs(tree, true); - changed = (morphedTree != nullptr); + changed = (morphedTree != nullptr); assert(!changed || (morphedTree == tree)); } } @@ -18459,13 +18464,13 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTree* tree) for (GenTree** pTree : tree->UseEdges()) { GenTree** pTreeCopy = pTree; - GenTree* childTree = *pTree; + GenTree* childTree = *pTree; if (childTree->gtOper == GT_LCL_VAR) { GenTree* newChildTree = fgMorphImplicitByRefArgs(childTree, false); if (newChildTree != nullptr) { - changed = true; + changed = true; *pTreeCopy = newChildTree; } } From 9727f8eeec4c8516155baadfe09cf5a41a8428f3 Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Mon, 14 Dec 2020 14:09:38 -0800 Subject: [PATCH 05/16] Resolved merge conflict and cleanup morph.cpp --- src/coreclr/jit/morph.cpp | 227 ++++++++++++++++++++++++-------------- 1 file changed, 142 insertions(+), 85 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 3b7b08ae65b2e..7a507e1dd6c77 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -175,7 +175,7 @@ GenTree* Compiler::fgMorphCast(GenTree* tree) // x86: src = float, dst = uint32/int64/uint64 or overflow conversion. && (tree->gtOverflow() || varTypeIsLong(dstType) || (dstType == TYP_UINT)) #endif - ) + ) { oper = gtNewCastNode(TYP_DOUBLE, oper, false, TYP_DOUBLE); } @@ -1309,7 +1309,7 @@ void fgArgInfo::ArgsComplete() || curArgTabEntry->isTmp // I protect this by "FEATURE_FIXED_OUT_ARGS" to preserve the property // that we only have late non-register args when that feature is on. #endif // FEATURE_FIXED_OUT_ARGS - ) + ) { curArgTabEntry->needTmp = true; needsTemps = true; @@ -1575,7 +1575,7 @@ void fgArgInfo::ArgsComplete() if (compiler->fgWalkTreePre(&argx, Compiler::fgChkLocAllocCB) == Compiler::WALK_ABORT) { curArgTabEntry->needTmp = true; - needsTemps = true; + needsTemps = true; continue; } } @@ -2723,7 +2723,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) } *insertionPoint = gtNewCallArgs(arg); #else // !defined(TARGET_X86) - // All other architectures pass the cookie in a register. + // All other architectures pass the cookie in a register. call->gtCallArgs = gtPrependNewCallArg(arg, call->gtCallArgs); #endif // defined(TARGET_X86) @@ -2791,8 +2791,9 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // This is a register argument - put it in the table. call->fgArgInfo->AddRegArg(argIndex, argx, call->gtCallThisArg, regNum, numRegs, byteSize, byteAlignment, - isStruct, callIsVararg UNIX_AMD64_ABI_ONLY_ARG(REG_STK) UNIX_AMD64_ABI_ONLY_ARG(0) - UNIX_AMD64_ABI_ONLY_ARG(0) UNIX_AMD64_ABI_ONLY_ARG(nullptr)); + isStruct, + callIsVararg UNIX_AMD64_ABI_ONLY_ARG(REG_STK) UNIX_AMD64_ABI_ONLY_ARG(0) + UNIX_AMD64_ABI_ONLY_ARG(0) UNIX_AMD64_ABI_ONLY_ARG(nullptr)); intArgRegNum++; #ifdef WINDOWS_AMD64_ABI @@ -2827,8 +2828,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) { noway_assert(call->gtCallArgs->GetNode()->TypeGet() == TYP_I_IMPL || call->gtCallArgs->GetNode()->TypeGet() == TYP_BYREF || - call->gtCallArgs->GetNode()->gtOper == - GT_NOP); // the arg was already morphed to a register (fgMorph called twice) + call->gtCallArgs->GetNode()->gtOper == GT_NOP); // the arg was already morphed to a register + // (fgMorph called twice) maxRegArgs = 1; } else @@ -3122,8 +3123,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // HFA structs are passed by value in multiple registers. // The "size" in registers may differ the size in pointer-sized units. CORINFO_CLASS_HANDLE structHnd = gtGetStructHandle(argx); - size = GetHfaCount(structHnd); - byteSize = info.compCompHnd->getClassSize(structHnd); + size = GetHfaCount(structHnd); + byteSize = info.compCompHnd->getClassSize(structHnd); } else { @@ -3132,7 +3133,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // if sufficient registers are available. // Structs that are larger than 2 pointers (except for HFAs) are passed by // reference (to a copy) - size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; + size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; byteSize = structSize; if (size > 2) { @@ -3144,20 +3145,20 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) } else { - size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot' + size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot' byteSize = genTypeSize(argx); } #elif defined(TARGET_ARM) || defined(TARGET_X86) if (isStructArg) { - size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; + size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; byteSize = structSize; } else { // The typical case. // Long/double type argument(s) will be modified as needed in Lowering. - size = genTypeStSz(argx->gtType); + size = genTypeStSz(argx->gtType); byteSize = genTypeSize(argx); } #else @@ -3240,7 +3241,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) #elif defined(TARGET_X86) || (isStructArg && isTrivialPointerSizedStruct(objClass)) #endif - ) + ) { #ifdef TARGET_ARM if (passUsingFloatRegs) @@ -3285,7 +3286,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // registers // unsigned roundupSize = (unsigned)roundUp(structSize, TARGET_POINTER_SIZE); - size = roundupSize / TARGET_POINTER_SIZE; + size = roundupSize / TARGET_POINTER_SIZE; // We also must update fltArgRegNum so that we no longer try to // allocate any new floating point registers for args @@ -3332,7 +3333,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) if (isStructArg) { unsigned int structFloatRegs = 0; - unsigned int structIntRegs = 0; + unsigned int structIntRegs = 0; for (unsigned int i = 0; i < structDesc.eightByteCount; i++) { if (structDesc.IsIntegralSlot(i)) @@ -3473,11 +3474,12 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) #endif // This is a register argument - put it in the table - newArgEntry = call->fgArgInfo->AddRegArg(argIndex, argx, args, nextRegNum, size, byteSize, argAlignBytes, - isStructArg, callIsVararg UNIX_AMD64_ABI_ONLY_ARG(nextOtherRegNum) - UNIX_AMD64_ABI_ONLY_ARG(structIntRegs) - UNIX_AMD64_ABI_ONLY_ARG(structFloatRegs) - UNIX_AMD64_ABI_ONLY_ARG(&structDesc)); + newArgEntry = + call->fgArgInfo->AddRegArg(argIndex, argx, args, nextRegNum, size, byteSize, argAlignBytes, isStructArg, + callIsVararg UNIX_AMD64_ABI_ONLY_ARG(nextOtherRegNum) + UNIX_AMD64_ABI_ONLY_ARG(structIntRegs) + UNIX_AMD64_ABI_ONLY_ARG(structFloatRegs) + UNIX_AMD64_ABI_ONLY_ARG(&structDesc)); newArgEntry->SetIsBackFilled(isBackFilled); newArgEntry->isNonStandard = isNonStandard; @@ -3519,7 +3521,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // we skip the corresponding floating point register argument intArgRegNum = min(intArgRegNum + size, MAX_REG_ARG); #endif // WINDOWS_AMD64_ABI - // No supported architecture supports partial structs using float registers. + // No supported architecture supports partial structs using float registers. assert(fltArgRegNum <= MAX_FLOAT_REG_ARG); } else @@ -3812,12 +3814,12 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) assert(size == 1); copyBlkClass = objClass; #else // UNIX_AMD64_ABI - // On Unix, structs are always passed by value. - // We only need a copy if we have one of the following: - // - The sizes don't match for a non-lclVar argument. - // - We have a known struct type (e.g. SIMD) that requires multiple registers. - // TODO-Amd64-Unix-Throughput: We don't need to keep the structDesc in the argEntry if it's not - // actually passed in registers. + // On Unix, structs are always passed by value. + // We only need a copy if we have one of the following: + // - The sizes don't match for a non-lclVar argument. + // - We have a known struct type (e.g. SIMD) that requires multiple registers. + // TODO-Amd64-Unix-Throughput: We don't need to keep the structDesc in the argEntry if it's not + // actually passed in registers. if (argEntry->isPassedInRegisters()) { if (argObj->OperIs(GT_OBJ)) @@ -4029,16 +4031,16 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // Build the mkrefany as a comma node: // (tmp.ptr=argx),(tmp.type=handle) - GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); + GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); GenTreeLclFld* destTypeSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__type); destPtrSlot->SetFieldSeq(GetFieldSeqStore()->CreateSingleton(GetRefanyDataField())); destPtrSlot->gtFlags |= GTF_VAR_DEF; destTypeSlot->SetFieldSeq(GetFieldSeqStore()->CreateSingleton(GetRefanyTypeField())); destTypeSlot->gtFlags |= GTF_VAR_DEF; - GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); + GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); GenTree* asgTypeSlot = gtNewAssignNode(destTypeSlot, argx->AsOp()->gtOp2); - GenTree* asg = gtNewOperNode(GT_COMMA, TYP_VOID, asgPtrSlot, asgTypeSlot); + GenTree* asg = gtNewOperNode(GT_COMMA, TYP_VOID, asgPtrSlot, asgTypeSlot); // Change the expression to "(tmp=val)" args->SetNode(asg); @@ -4475,7 +4477,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if !defined(HOST_UNIX) && defined(TARGET_ARM64) && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - ) + ) { elemType = hfaType; elemSize = genTypeSize(elemType); @@ -4587,7 +4589,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if !defined(HOST_UNIX) && defined(TARGET_ARM64) && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - ) + ) { // We have a HFA struct. noway_assert(elemType == varDsc->GetHfaType()); @@ -4633,11 +4635,12 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if defined(TARGET_ARM64) || defined(UNIX_AMD64_ABI) // Is this LclVar a promoted struct with exactly 2 fields? // TODO-ARM64-CQ: Support struct promoted HFA types here - if (varDsc->lvPromoted && (varDsc->lvFieldCnt == 2) && (!varDsc->lvIsHfa() + if (varDsc->lvPromoted && (varDsc->lvFieldCnt == 2) && + (!varDsc->lvIsHfa() #if !defined(HOST_UNIX) && defined(TARGET_ARM64) - && !fgEntryPtr->IsVararg() + && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - )) + )) { // See if we have two promoted fields that start at offset 0 and 8? unsigned loVarNum = lvaGetFieldLocal(varDsc, 0); @@ -5071,7 +5074,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, // Structs are always on the stack, and thus never need temps // so we have to put the copy and temp all into one expression. argEntry->tmpNum = tmp; - GenTree* arg = fgMakeTmpArgNode(argEntry); + GenTree* arg = fgMakeTmpArgNode(argEntry); // Change the expression to "(tmp=val),tmp" arg = gtNewOperNode(GT_COMMA, arg->TypeGet(), copyBlk, arg); @@ -6317,7 +6320,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) // Create "comma2" node and link it to "tree". // GenTree* comma2; - comma2 = gtNewOperNode(GT_COMMA, + comma2 = gtNewOperNode(GT_COMMA, addr->TypeGet(), // The type of "comma2" node is the same as the type of "addr" node. comma, addr); tree->AsOp()->gtOp1 = comma2; @@ -9091,8 +9094,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) // copy-back). unsigned retValTmpNum = BAD_VAR_NUM; CORINFO_CLASS_HANDLE structHnd = nullptr; - if (call->HasRetBufArg() && - call->gtCallLateArgs == nullptr) // Don't do this if we're re-morphing (which will make late args non-null). + if (call->HasRetBufArg() && call->gtCallLateArgs == nullptr) // Don't do this if we're re-morphing (which will make + // late args non-null). { // We're enforcing the invariant that return buffers pointers (at least for // struct return types containing GC pointers) are never pointers into the heap. @@ -11606,16 +11609,16 @@ GenTree* Compiler::getSIMDStructFromField(GenTree* tree, } /***************************************************************************** -* If a read operation tries to access simd struct field, then transform the -* operation to the SIMD intrinsic SIMDIntrinsicGetItem, and return the new tree. -* Otherwise, return the old tree. -* Argument: -* tree - GenTree*. If this pointer points to simd struct which is used for simd -* intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. -* Return: -* A GenTree* which points to the new tree. If the tree is not for simd intrinsic, -* return nullptr. -*/ + * If a read operation tries to access simd struct field, then transform the + * operation to the SIMD intrinsic SIMDIntrinsicGetItem, and return the new tree. + * Otherwise, return the old tree. + * Argument: + * tree - GenTree*. If this pointer points to simd struct which is used for simd + * intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. + * Return: + * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, + * return nullptr. + */ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) { @@ -11636,16 +11639,16 @@ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) } /***************************************************************************** -* Transform an assignment of a SIMD struct field to SIMD intrinsic -* SIMDIntrinsicSet*, and return a new tree. If it is not such an assignment, -* then return the old tree. -* Argument: -* tree - GenTree*. If this pointer points to simd struct which is used for simd -* intrinsic, we will morph it as simd intrinsic set. -* Return: -* A GenTree* which points to the new tree. If the tree is not for simd intrinsic, -* return nullptr. -*/ + * Transform an assignment of a SIMD struct field to SIMD intrinsic + * SIMDIntrinsicSet*, and return a new tree. If it is not such an assignment, + * then return the old tree. + * Argument: + * tree - GenTree*. If this pointer points to simd struct which is used for simd + * intrinsic, we will morph it as simd intrinsic set. + * Return: + * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, + * return nullptr. + */ GenTree* Compiler::fgMorphFieldAssignToSIMDIntrinsicSet(GenTree* tree) { @@ -11910,6 +11913,22 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return fgMorphCast(tree); case GT_MUL: + // -a * C => a * -C, where C is constant + // MUL(NEG(a), C) => MUL(a, NEG(C)) + if (fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && + !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && + op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) + { + // tree: MUL + // op1: a + // op2: NEG + // op2Child: C + + tree->AsOp()->gtOp1 = op1->gtGetOp1(); + DEBUG_DESTROY_NODE(op1); + op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C + return tree; + } #ifndef TARGET_64BIT if (typ == TYP_LONG) @@ -12057,6 +12076,23 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return fgMorphSmpOp(tree, mac); } + // -a / C => a / -C, where C is constant + // DIV(NEG(a), C) => DIV(a, NEG(C)) + if (fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && + !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && + op2->AsIntCon()->IconValue() != -1) + { + // tree: DIV + // op1: a + // op2: NEG + // op2Child: C + + tree->AsOp()->gtOp1 = op1->gtGetOp1(); + DEBUG_DESTROY_NODE(op1); + op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C + return tree; + } + #ifndef TARGET_64BIT if (typ == TYP_LONG) { @@ -12264,25 +12300,25 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } } #else // !TARGET_ARM64 - // If b is not a power of 2 constant then lowering replaces a % b - // with a - (a / b) * b and applies magic division optimization to - // a / b. The code may already contain an a / b expression (e.g. - // x = a / 10; y = a % 10;) and then we end up with redundant code. - // If we convert % to / here we give CSE the opportunity to eliminate - // the redundant division. If there's no redundant division then - // nothing is lost, lowering would have done this transform anyway. + // If b is not a power of 2 constant then lowering replaces a % b + // with a - (a / b) * b and applies magic division optimization to + // a / b. The code may already contain an a / b expression (e.g. + // x = a / 10; y = a % 10;) and then we end up with redundant code. + // If we convert % to / here we give CSE the opportunity to eliminate + // the redundant division. If there's no redundant division then + // nothing is lost, lowering would have done this transform anyway. if (!optValnumCSE_phase && ((tree->OperGet() == GT_MOD) && op2->IsIntegralConst())) { - ssize_t divisorValue = op2->AsIntCon()->IconValue(); - size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) + ssize_t divisorValue = op2->AsIntCon()->IconValue(); + size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) : static_cast(abs(divisorValue)); if (!isPow2(absDivisorValue)) { tree = fgMorphModToSubMulDiv(tree->AsOp()); - op1 = tree->AsOp()->gtOp1; - op2 = tree->AsOp()->gtOp2; + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; } } #endif // !TARGET_ARM64 @@ -13921,6 +13957,27 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return child; } + // Distribute negation over simple multiplication/division expressions + if (op1->OperIs(GT_MUL) || op1->OperIs(GT_DIV)) + { + if (!op1->AsOp()->gtOp1->IsCnsIntOrI() && op1->AsOp()->gtOp2->IsCnsIntOrI()) + { + // -(a * C) => a * -C + // -(a / C) => a / -C + oper = op1->gtOper; + tree->SetOper(op1->gtOper); + GenTree* newOp1 = op1->AsOp()->gtOp1; // a + GenTree* newOp2 = op1->AsOp()->gtOp2; // C + newOp2->AsIntCon()->SetIconValue(-newOp2->AsIntCon()->IconValue()); + + // Only need to destroy op1 since op2 will be null already + DEBUG_DESTROY_NODE(op1); + + tree->AsOp()->gtOp1 = op1 = newOp1; + tree->AsOp()->gtOp2 = op2 = newOp2; + } + } + /* Any constant cases should have been folded earlier */ noway_assert(!op1->OperIsConst() || !opts.OptEnabled(CLFLG_CONSTANTFOLD) || optValnumCSE_phase); break; @@ -14769,8 +14826,8 @@ GenTree* Compiler::fgMorphRetInd(GenTreeUnOp* ret) #if defined(TARGET_64BIT) bool canFold = (indSize == lclVarSize); #else // !TARGET_64BIT - // TODO: improve 32 bit targets handling for LONG returns if necessary, nowadays we do not support `BITCAST - // long<->double` there. + // TODO: improve 32 bit targets handling for LONG returns if necessary, nowadays we do not support `BITCAST + // long<->double` there. bool canFold = (indSize == lclVarSize) && (lclVarSize <= REGSIZE_BYTES); #endif // TODO: support `genReturnBB != nullptr`, it requires #11413 to avoid `Incompatible types for @@ -15480,7 +15537,7 @@ GenTree* Compiler::fgMorphToEmulatedFP(GenTree* tree) case GT_DIV: helper = CPX_R4_DIV; break; - // case GT_MOD: helper = CPX_R4_REM; break; + // case GT_MOD: helper = CPX_R4_REM; break; case GT_EQ: helper = CPX_R4_EQ; @@ -15654,12 +15711,12 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) } } -/*------------------------------------------------------------------------- - * fgMorphTree() can potentially replace a tree with another, and the - * caller has to store the return value correctly. - * Turn this on to always make copy of "tree" here to shake out - * hidden/unupdated references. - */ + /*------------------------------------------------------------------------- + * fgMorphTree() can potentially replace a tree with another, and the + * caller has to store the return value correctly. + * Turn this on to always make copy of "tree" here to shake out + * hidden/unupdated references. + */ #ifdef DEBUG @@ -16016,7 +16073,7 @@ void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree) void Compiler::fgMorphTreeDone(GenTree* tree, GenTree* oldTree /* == NULL */ - DEBUGARG(int morphNum)) + DEBUGARG(int morphNum)) { #ifdef DEBUG if (verbose && treesBeforeAfterMorph) @@ -18487,7 +18544,7 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTree* tree) if (tree->AsOp()->gtOp1->gtOper == GT_LCL_VAR) { GenTree* morphedTree = fgMorphImplicitByRefArgs(tree, true); - changed = (morphedTree != nullptr); + changed = (morphedTree != nullptr); assert(!changed || (morphedTree == tree)); } } @@ -18496,13 +18553,13 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTree* tree) for (GenTree** pTree : tree->UseEdges()) { GenTree** pTreeCopy = pTree; - GenTree* childTree = *pTree; + GenTree* childTree = *pTree; if (childTree->gtOper == GT_LCL_VAR) { GenTree* newChildTree = fgMorphImplicitByRefArgs(childTree, false); if (newChildTree != nullptr) { - changed = true; + changed = true; *pTreeCopy = newChildTree; } } From b4bf7e9aadd6b8b62857e3f3c5b2fb8bddd10227 Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Mon, 28 Dec 2020 19:03:51 -0800 Subject: [PATCH 06/16] Formatting morph.cpp --- src/coreclr/jit/morph.cpp | 111 +++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 56 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 01d8aa14dbb06..39840e12ad298 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -175,7 +175,7 @@ GenTree* Compiler::fgMorphCast(GenTree* tree) // x86: src = float, dst = uint32/int64/uint64 or overflow conversion. && (tree->gtOverflow() || varTypeIsLong(dstType) || (dstType == TYP_UINT)) #endif - ) + ) { oper = gtNewCastNode(TYP_DOUBLE, oper, false, TYP_DOUBLE); } @@ -1313,7 +1313,7 @@ void fgArgInfo::ArgsComplete() || curArgTabEntry->isTmp // I protect this by "FEATURE_FIXED_OUT_ARGS" to preserve the property // that we only have late non-register args when that feature is on. #endif // FEATURE_FIXED_OUT_ARGS - ) + ) { curArgTabEntry->needTmp = true; needsTemps = true; @@ -1579,7 +1579,7 @@ void fgArgInfo::ArgsComplete() if (compiler->fgWalkTreePre(&argx, Compiler::fgChkLocAllocCB) == Compiler::WALK_ABORT) { curArgTabEntry->needTmp = true; - needsTemps = true; + needsTemps = true; continue; } } @@ -2727,7 +2727,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) } *insertionPoint = gtNewCallArgs(arg); #else // !defined(TARGET_X86) - // All other architectures pass the cookie in a register. + // All other architectures pass the cookie in a register. call->gtCallArgs = gtPrependNewCallArg(arg, call->gtCallArgs); #endif // defined(TARGET_X86) @@ -3130,8 +3130,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // HFA structs are passed by value in multiple registers. // The "size" in registers may differ the size in pointer-sized units. CORINFO_CLASS_HANDLE structHnd = gtGetStructHandle(argx); - size = GetHfaCount(structHnd); - byteSize = info.compCompHnd->getClassSize(structHnd); + size = GetHfaCount(structHnd); + byteSize = info.compCompHnd->getClassSize(structHnd); } else { @@ -3140,7 +3140,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // if sufficient registers are available. // Structs that are larger than 2 pointers (except for HFAs) are passed by // reference (to a copy) - size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; + size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; byteSize = structSize; if (size > 2) { @@ -3152,20 +3152,20 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) } else { - size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot' + size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot' byteSize = genTypeSize(argx); } #elif defined(TARGET_ARM) || defined(TARGET_X86) if (isStructArg) { - size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; + size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE; byteSize = structSize; } else { // The typical case. // Long/double type argument(s) will be modified as needed in Lowering. - size = genTypeStSz(argx->gtType); + size = genTypeStSz(argx->gtType); byteSize = genTypeSize(argx); } #else @@ -3240,7 +3240,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) #elif defined(TARGET_X86) || (isStructArg && isTrivialPointerSizedStruct(objClass)) #endif - ) + ) { #ifdef TARGET_ARM if (passUsingFloatRegs) @@ -3285,7 +3285,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // registers // unsigned roundupSize = (unsigned)roundUp(structSize, TARGET_POINTER_SIZE); - size = roundupSize / TARGET_POINTER_SIZE; + size = roundupSize / TARGET_POINTER_SIZE; // We also must update fltArgRegNum so that we no longer try to // allocate any new floating point registers for args @@ -3332,7 +3332,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) if (isStructArg) { unsigned int structFloatRegs = 0; - unsigned int structIntRegs = 0; + unsigned int structIntRegs = 0; for (unsigned int i = 0; i < structDesc.eightByteCount; i++) { if (structDesc.IsIntegralSlot(i)) @@ -3518,8 +3518,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // Whenever we pass an integer register argument // we skip the corresponding floating point register argument intArgRegNum = min(intArgRegNum + size, MAX_REG_ARG); -#endif // WINDOWS_AMD64_ABI - // No supported architecture supports partial structs using float registers. +#endif // WINDOWS_AMD64_ABI + // No supported architecture supports partial structs using float registers. assert(fltArgRegNum <= MAX_FLOAT_REG_ARG); } else @@ -3812,12 +3812,12 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) assert(size == 1); copyBlkClass = objClass; #else // UNIX_AMD64_ABI - // On Unix, structs are always passed by value. - // We only need a copy if we have one of the following: - // - The sizes don't match for a non-lclVar argument. - // - We have a known struct type (e.g. SIMD) that requires multiple registers. - // TODO-Amd64-Unix-Throughput: We don't need to keep the structDesc in the argEntry if it's not - // actually passed in registers. + // On Unix, structs are always passed by value. + // We only need a copy if we have one of the following: + // - The sizes don't match for a non-lclVar argument. + // - We have a known struct type (e.g. SIMD) that requires multiple registers. + // TODO-Amd64-Unix-Throughput: We don't need to keep the structDesc in the argEntry if it's not + // actually passed in registers. if (argEntry->isPassedInRegisters()) { if (argObj->OperIs(GT_OBJ)) @@ -4029,16 +4029,16 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // Build the mkrefany as a comma node: // (tmp.ptr=argx),(tmp.type=handle) - GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); + GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); GenTreeLclFld* destTypeSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__type); destPtrSlot->SetFieldSeq(GetFieldSeqStore()->CreateSingleton(GetRefanyDataField())); destPtrSlot->gtFlags |= GTF_VAR_DEF; destTypeSlot->SetFieldSeq(GetFieldSeqStore()->CreateSingleton(GetRefanyTypeField())); destTypeSlot->gtFlags |= GTF_VAR_DEF; - GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); + GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); GenTree* asgTypeSlot = gtNewAssignNode(destTypeSlot, argx->AsOp()->gtOp2); - GenTree* asg = gtNewOperNode(GT_COMMA, TYP_VOID, asgPtrSlot, asgTypeSlot); + GenTree* asg = gtNewOperNode(GT_COMMA, TYP_VOID, asgPtrSlot, asgTypeSlot); // Change the expression to "(tmp=val)" args->SetNode(asg); @@ -4475,7 +4475,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if !defined(HOST_UNIX) && defined(TARGET_ARM64) && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - ) + ) { elemType = hfaType; elemSize = genTypeSize(elemType); @@ -4587,7 +4587,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if !defined(HOST_UNIX) && defined(TARGET_ARM64) && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - ) + ) { // We have a HFA struct. noway_assert(elemType == varDsc->GetHfaType()); @@ -4633,12 +4633,11 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #if defined(TARGET_ARM64) || defined(UNIX_AMD64_ABI) // Is this LclVar a promoted struct with exactly 2 fields? // TODO-ARM64-CQ: Support struct promoted HFA types here - if (varDsc->lvPromoted && (varDsc->lvFieldCnt == 2) && - (!varDsc->lvIsHfa() + if (varDsc->lvPromoted && (varDsc->lvFieldCnt == 2) && (!varDsc->lvIsHfa() #if !defined(HOST_UNIX) && defined(TARGET_ARM64) - && !fgEntryPtr->IsVararg() + && !fgEntryPtr->IsVararg() #endif // !defined(HOST_UNIX) && defined(TARGET_ARM64) - )) + )) { // See if we have two promoted fields that start at offset 0 and 8? unsigned loVarNum = lvaGetFieldLocal(varDsc, 0); @@ -5072,7 +5071,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, // Structs are always on the stack, and thus never need temps // so we have to put the copy and temp all into one expression. argEntry->tmpNum = tmp; - GenTree* arg = fgMakeTmpArgNode(argEntry); + GenTree* arg = fgMakeTmpArgNode(argEntry); // Change the expression to "(tmp=val),tmp" arg = gtNewOperNode(GT_COMMA, arg->TypeGet(), copyBlk, arg); @@ -6318,7 +6317,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) // Create "comma2" node and link it to "tree". // GenTree* comma2; - comma2 = gtNewOperNode(GT_COMMA, + comma2 = gtNewOperNode(GT_COMMA, addr->TypeGet(), // The type of "comma2" node is the same as the type of "addr" node. comma, addr); tree->AsOp()->gtOp1 = comma2; @@ -12297,25 +12296,25 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } } #else // !TARGET_ARM64 - // If b is not a power of 2 constant then lowering replaces a % b - // with a - (a / b) * b and applies magic division optimization to - // a / b. The code may already contain an a / b expression (e.g. - // x = a / 10; y = a % 10;) and then we end up with redundant code. - // If we convert % to / here we give CSE the opportunity to eliminate - // the redundant division. If there's no redundant division then - // nothing is lost, lowering would have done this transform anyway. + // If b is not a power of 2 constant then lowering replaces a % b + // with a - (a / b) * b and applies magic division optimization to + // a / b. The code may already contain an a / b expression (e.g. + // x = a / 10; y = a % 10;) and then we end up with redundant code. + // If we convert % to / here we give CSE the opportunity to eliminate + // the redundant division. If there's no redundant division then + // nothing is lost, lowering would have done this transform anyway. if (!optValnumCSE_phase && ((tree->OperGet() == GT_MOD) && op2->IsIntegralConst())) { - ssize_t divisorValue = op2->AsIntCon()->IconValue(); - size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) + ssize_t divisorValue = op2->AsIntCon()->IconValue(); + size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) : static_cast(abs(divisorValue)); if (!isPow2(absDivisorValue)) { tree = fgMorphModToSubMulDiv(tree->AsOp()); - op1 = tree->AsOp()->gtOp1; - op2 = tree->AsOp()->gtOp2; + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; } } #endif // !TARGET_ARM64 @@ -14823,8 +14822,8 @@ GenTree* Compiler::fgMorphRetInd(GenTreeUnOp* ret) #if defined(TARGET_64BIT) bool canFold = (indSize == lclVarSize); #else // !TARGET_64BIT - // TODO: improve 32 bit targets handling for LONG returns if necessary, nowadays we do not support `BITCAST - // long<->double` there. + // TODO: improve 32 bit targets handling for LONG returns if necessary, nowadays we do not support `BITCAST + // long<->double` there. bool canFold = (indSize == lclVarSize) && (lclVarSize <= REGSIZE_BYTES); #endif // TODO: support `genReturnBB != nullptr`, it requires #11413 to avoid `Incompatible types for @@ -15534,7 +15533,7 @@ GenTree* Compiler::fgMorphToEmulatedFP(GenTree* tree) case GT_DIV: helper = CPX_R4_DIV; break; - // case GT_MOD: helper = CPX_R4_REM; break; + // case GT_MOD: helper = CPX_R4_REM; break; case GT_EQ: helper = CPX_R4_EQ; @@ -15708,12 +15707,12 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) } } - /*------------------------------------------------------------------------- - * fgMorphTree() can potentially replace a tree with another, and the - * caller has to store the return value correctly. - * Turn this on to always make copy of "tree" here to shake out - * hidden/unupdated references. - */ +/*------------------------------------------------------------------------- + * fgMorphTree() can potentially replace a tree with another, and the + * caller has to store the return value correctly. + * Turn this on to always make copy of "tree" here to shake out + * hidden/unupdated references. + */ #ifdef DEBUG @@ -16070,7 +16069,7 @@ void Compiler::fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree) void Compiler::fgMorphTreeDone(GenTree* tree, GenTree* oldTree /* == NULL */ - DEBUGARG(int morphNum)) + DEBUGARG(int morphNum)) { #ifdef DEBUG if (verbose && treesBeforeAfterMorph) @@ -18541,7 +18540,7 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTree* tree) if (tree->AsOp()->gtOp1->gtOper == GT_LCL_VAR) { GenTree* morphedTree = fgMorphImplicitByRefArgs(tree, true); - changed = (morphedTree != nullptr); + changed = (morphedTree != nullptr); assert(!changed || (morphedTree == tree)); } } @@ -18550,13 +18549,13 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTree* tree) for (GenTree** pTree : tree->UseEdges()) { GenTree** pTreeCopy = pTree; - GenTree* childTree = *pTree; + GenTree* childTree = *pTree; if (childTree->gtOper == GT_LCL_VAR) { GenTree* newChildTree = fgMorphImplicitByRefArgs(childTree, false); if (newChildTree != nullptr) { - changed = true; + changed = true; *pTreeCopy = newChildTree; } } From bd9a46490c44327438b92bf2796ffc55bc250b7e Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Tue, 5 Jan 2021 20:08:32 -0800 Subject: [PATCH 07/16] Returning tree after performing smpop again to fix flags --- src/coreclr/jit/morph.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 39840e12ad298..1a6e9df939a59 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11923,7 +11923,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C - return tree; + return fgMorphSmpOp(tree, mac); } #ifndef TARGET_64BIT @@ -12086,7 +12086,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C - return tree; + return fgMorphSmpOp(tree, mac); } #ifndef TARGET_64BIT From 913699954e4acd6182573254dea4c0c57989c1ce Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Tue, 5 Jan 2021 20:28:38 -0800 Subject: [PATCH 08/16] Formatting --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1a6e9df939a59..7967de64ca4f4 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -3518,7 +3518,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // Whenever we pass an integer register argument // we skip the corresponding floating point register argument intArgRegNum = min(intArgRegNum + size, MAX_REG_ARG); -#endif // WINDOWS_AMD64_ABI +#endif // WINDOWS_AMD64_ABI // No supported architecture supports partial structs using float registers. assert(fltArgRegNum <= MAX_FLOAT_REG_ARG); } From 2e72f940ce1b421b2e1e130723f35d327635e82d Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Thu, 7 Jan 2021 14:16:13 -0800 Subject: [PATCH 09/16] Added check for optimizations, formatting. --- src/coreclr/jit/morph.cpp | 62 ++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 7967de64ca4f4..03194b3e2297d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11605,16 +11605,16 @@ GenTree* Compiler::getSIMDStructFromField(GenTree* tree, } /***************************************************************************** - * If a read operation tries to access simd struct field, then transform the - * operation to the SIMD intrinsic SIMDIntrinsicGetItem, and return the new tree. - * Otherwise, return the old tree. - * Argument: - * tree - GenTree*. If this pointer points to simd struct which is used for simd - * intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. - * Return: - * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, - * return nullptr. - */ +* If a read operation tries to access simd struct field, then transform the +* operation to the SIMD intrinsic SIMDIntrinsicGetItem, and return the new tree. +* Otherwise, return the old tree. +* Argument: +* tree - GenTree*. If this pointer points to simd struct which is used for simd +* intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. +* Return: +* A GenTree* which points to the new tree. If the tree is not for simd intrinsic, + return nullptr. +*/ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) { @@ -11635,16 +11635,16 @@ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) } /***************************************************************************** - * Transform an assignment of a SIMD struct field to SIMD intrinsic - * SIMDIntrinsicSet*, and return a new tree. If it is not such an assignment, - * then return the old tree. - * Argument: - * tree - GenTree*. If this pointer points to simd struct which is used for simd - * intrinsic, we will morph it as simd intrinsic set. - * Return: - * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, - * return nullptr. - */ +* Transform an assignment of a SIMD struct field to SIMD intrinsic +* SIMDIntrinsicSet*, and return a new tree. If it is not such an assignment, +* then return the old tree. +* Argument: +* tree - GenTree*. If this pointer points to simd struct which is used for simd +* intrinsic, we will morph it as simd intrinsic set. +* Return: +* A GenTree* which points to the new tree. If the tree is not for simd intrinsic, +* return nullptr. +*/ GenTree* Compiler::fgMorphFieldAssignToSIMDIntrinsicSet(GenTree* tree) { @@ -11911,15 +11911,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_MUL: // -a * C => a * -C, where C is constant // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && - !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && - op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) + if (opts.OptimizationEnabled() && fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && + op2->IsCnsIntOrI() && !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && + op2->AsIntCon()->IconValue() != 0 && op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) { - // tree: MUL - // op1: a - // op2: NEG - // op2Child: C - tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C @@ -12074,15 +12069,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // -a / C => a / -C, where C is constant // DIV(NEG(a), C) => DIV(a, NEG(C)) - if (fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && - !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && - op2->AsIntCon()->IconValue() != -1) + if (opts.OptimizationEnabled() && fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && + op2->IsCnsIntOrI() && !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && + op2->AsIntCon()->IconValue() != 0 && op2->AsIntCon()->IconValue() != -1) { - // tree: DIV - // op1: a - // op2: NEG - // op2Child: C - tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C From 6467520ea20b0aa80505d170a93ee27ee149bd0a Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Thu, 7 Jan 2021 15:04:41 -0800 Subject: [PATCH 10/16] Using gtIsActiveCSE_Candidate instead of fgGlobalMorph --- src/coreclr/jit/morph.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 03194b3e2297d..03ef7f9a4724d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11911,13 +11911,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_MUL: // -a * C => a * -C, where C is constant // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (opts.OptimizationEnabled() && fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && + if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) { tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); - op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C + tree->AsOp()->gtOp2 = gtNewIconNode(-op2->AsIntCon()->IconValue(), op2->TypeGet()); + DEBUG_DESTROY_NODE(op2); return fgMorphSmpOp(tree, mac); } @@ -12069,13 +12070,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // -a / C => a / -C, where C is constant // DIV(NEG(a), C) => DIV(a, NEG(C)) - if (opts.OptimizationEnabled() && fgGlobalMorph && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && + if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && op2->AsIntCon()->IconValue() != -1) { tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); - op2->AsIntCon()->SetIconValue(-op2->AsIntCon()->IconValue()); // -C + tree->AsOp()->gtOp2 = gtNewIconNode(-op2->AsIntCon()->IconValue(), op2->TypeGet()); + DEBUG_DESTROY_NODE(op2); return fgMorphSmpOp(tree, mac); } From 4a7f9b5f59c519ce5e0e4d952af8e6d2219562d7 Mon Sep 17 00:00:00 2001 From: Alex Covington <68252706+alexcovington@users.noreply.github.com> Date: Thu, 7 Jan 2021 15:12:14 -0800 Subject: [PATCH 11/16] Update src/coreclr/jit/morph.cpp Co-authored-by: Sergey Andreenko --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 03ef7f9a4724d..2de6757a23522 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11613,7 +11613,7 @@ GenTree* Compiler::getSIMDStructFromField(GenTree* tree, * intrinsic, we will morph it as simd intrinsic SIMDIntrinsicGetItem. * Return: * A GenTree* which points to the new tree. If the tree is not for simd intrinsic, - return nullptr. +* return nullptr. */ GenTree* Compiler::fgMorphFieldToSIMDIntrinsicGet(GenTree* tree) From cb71a90c0ca2f1be3c76b0f769fe15aaea7a1713 Mon Sep 17 00:00:00 2001 From: Alex Covington Date: Thu, 7 Jan 2021 15:32:24 -0800 Subject: [PATCH 12/16] Formatting --- src/coreclr/jit/morph.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 2de6757a23522..0777183cce405 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11911,9 +11911,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_MUL: // -a * C => a * -C, where C is constant // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && - op2->IsCnsIntOrI() && !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && - op2->AsIntCon()->IconValue() != 0 && op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) + if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && + !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && !op2->IsIconHandle() && + op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && + op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) { tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); @@ -12070,9 +12071,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // -a / C => a / -C, where C is constant // DIV(NEG(a), C) => DIV(a, NEG(C)) - if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && - op2->IsCnsIntOrI() && !op2->IsIconHandle() && op2->AsIntCon()->IconValue() != 1 && - op2->AsIntCon()->IconValue() != 0 && op2->AsIntCon()->IconValue() != -1) + if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && + !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && !op2->IsIconHandle() && + op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && + op2->AsIntCon()->IconValue() != -1) { tree->AsOp()->gtOp1 = op1->gtGetOp1(); DEBUG_DESTROY_NODE(op1); From fea92de11ce54ef1ce09d54307443a0d0bf3b734 Mon Sep 17 00:00:00 2001 From: Sergey Date: Fri, 8 Jan 2021 09:33:07 -0800 Subject: [PATCH 13/16] delete formatting changes. --- src/coreclr/jit/morph.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 0777183cce405..2ea353d575cef 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2833,8 +2833,8 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) { noway_assert(call->gtCallArgs->GetNode()->TypeGet() == TYP_I_IMPL || call->gtCallArgs->GetNode()->TypeGet() == TYP_BYREF || - call->gtCallArgs->GetNode()->gtOper == GT_NOP); // the arg was already morphed to a register - // (fgMorph called twice) + call->gtCallArgs->GetNode()->gtOper == + GT_NOP); // the arg was already morphed to a register (fgMorph called twice) maxRegArgs = 1; } else @@ -9090,8 +9090,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) // copy-back). unsigned retValTmpNum = BAD_VAR_NUM; CORINFO_CLASS_HANDLE structHnd = nullptr; - if (call->HasRetBufArg() && call->gtCallLateArgs == nullptr) // Don't do this if we're re-morphing (which will make - // late args non-null). + if (call->HasRetBufArg() && + call->gtCallLateArgs == nullptr) // Don't do this if we're re-morphing (which will make late args non-null). { // We're enforcing the invariant that return buffers pointers (at least for // struct return types containing GC pointers) are never pointers into the heap. From 8d2dc8ec9d22032bf7485f4e54dcd09a7bddddab Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 11 Jan 2021 02:25:38 -0800 Subject: [PATCH 14/16] Add a test. --- .../NegMulOrDivToConst.cs | 337 ++++++++++++++++++ .../NegMulOrDivToConst.csproj | 12 + 2 files changed, 349 insertions(+) create mode 100644 src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs create mode 100644 src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.csproj diff --git a/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs b/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs new file mode 100644 index 0000000000000..dbb87c9953d5a --- /dev/null +++ b/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs @@ -0,0 +1,337 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Test the following optimizations: +// -(v * const) => v * -const; +// -v * const => v * -const; +// -(v / const) => v / -const; +// -v / const => v / -const; +// +// Note that C# spec tells that `int.MinValue / -1` result is implementation specific, but +// ecma-335 requires it to throw `System.ArithmeticException` in such case, so we should not +// change 1 and -1 sign. + +using System; +using System.Runtime.CompilerServices; + +namespace TestIntLimits +{ + class Program + { + [MethodImpl(MethodImplOptions.NoInlining)] + static int CheckMulNeg() + { + bool fail = false; + if (Mul1_1(3) != -7 * 3) + { + fail = true; + } + + if (Mul1_2(100) != 0) + { + fail = true; + } + + try + { + Mul1_3(2); + } + catch + { + fail = true; + } + + + try + { + Mul1_4(1); + fail = true; + } + catch + { } + + try + { + Mul1_4(0); + + } + catch + { + fail = true; + } + + if (Mul1_5(1) != -int.MaxValue) + { + fail = true; + } + if (Mul1_5(0) != 0) + { + fail = true; + } + if (Mul1_5(-1) != int.MaxValue) + { + fail = true; + } + + try + { + Mul1_6(int.MinValue); + fail = true; + } + catch + { } + + + if (fail) + { + Console.WriteLine("CheckMulNeg failed"); + return 101; + } + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul1_1(int a) => -a * 7; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul1_2(int a) => -a * 0; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul1_3(int a) => -a * int.MinValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul1_4(int a) + { + checked + { + return -a * int.MinValue; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul1_5(int a) => -(a * int.MaxValue); + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul1_6(int a) + { + checked + { + return -a * 0; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int CheckNegMul() + { + bool fail = false; + if (Mul2_1(3) != -7 * 3) + { + fail = true; + } + + try + { + Mul2_2(100); + } + catch + { + fail = true; + } + + + try + { + Mul2_3(1); + fail = true; + } + catch + { } + + try + { + Mul2_3(0); + + } + catch + { + fail = true; + } + + if (fail) + { + Console.WriteLine("CheckNegMul failed"); + return 101; + } + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul2_1(int a) => -(a * 7); + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul2_2(int a) => -(a * int.MinValue); + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Mul2_3(int a) + { + checked + { + return -(a * int.MinValue); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int CheckDivNeg() + { + bool fail = false; + + if (Div1_1(110) != -10) + { + fail = true; + } + + if (Div1_2(100000000000) != -100000000) + { + fail = true; + } + + try + { + Div1_3(1); + Div1_3(int.MinValue); + Div1_4(1); + Div1_4(long.MinValue); + Div1_5(int.MinValue); + Div1_6(long.MinValue); + } + catch + { + fail = true; + } + + if (fail) + { + Console.WriteLine("CheckDivNeg failed"); + return 101; + } + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Div1_1(int a) => -a / 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div1_2(long a) => -a / 1000; + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div1_3(int a) => -a / int.MinValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div1_4(long a) => -a / long.MinValue; + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div1_5(int a) => -a / 1; + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div1_6(long a) => -a / 1; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int CheckNegDiv() + { + bool fail = false; + + if (Div2_1(110) != -10) + { + fail = true; + } + + if (Div2_2(100000000000) != -100000000) + { + fail = true; + } + + try + { + Div2_3(1); + Div2_3(int.MinValue); + Div2_4(1); + Div2_4(long.MinValue); + Div2_5(int.MinValue); + Div2_6(long.MinValue); + } + catch + { + fail = true; + } + + try + { + Div2_7(int.MinValue); + fail = true; + } + catch + { } + + try + { + Div2_8(long.MinValue); + fail = true; + } + catch + { } + + if (fail) + { + Console.WriteLine("CheckNegDiv failed"); + return 101; + } + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Div2_1(int a) => -(a / 11); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div2_2(long a) => -(a / 1000); + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Div2_3(int a) => -(a / int.MinValue); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div2_4(long a) => -(a / long.MinValue); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div2_5(int a) => -(a / 1); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div2_6(long a) => -(a / 1); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div2_7(int a) => -(a / -1); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long Div2_8(long a) => -(a / -1); + + static int Main() + { + if (CheckMulNeg() != 100) + { + return 101; + } + if (CheckNegMul() != 100) + { + return 101; + } + if (CheckDivNeg() != 100) + { + return 101; + } + if (CheckNegDiv() != 100) + { + return 101; + } + + return 100; + } + } +} \ No newline at end of file diff --git a/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.csproj b/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.csproj new file mode 100644 index 0000000000000..c6f2eea5464a4 --- /dev/null +++ b/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + From 46b8fff4973b4f567933d5e0196f6af7d5f434e4 Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 11 Jan 2021 12:08:06 -0800 Subject: [PATCH 15/16] Change the conditions a bit. --- src/coreclr/jit/morph.cpp | 86 +++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 2ea353d575cef..3d96acbe79851 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11909,18 +11909,20 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return fgMorphCast(tree); case GT_MUL: - // -a * C => a * -C, where C is constant - // MUL(NEG(a), C) => MUL(a, NEG(C)) - if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && - !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && !op2->IsIconHandle() && - op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && - op2->AsIntCon()->IconValue() != -1 && !tree->gtOverflow()) - { - tree->AsOp()->gtOp1 = op1->gtGetOp1(); - DEBUG_DESTROY_NODE(op1); - tree->AsOp()->gtOp2 = gtNewIconNode(-op2->AsIntCon()->IconValue(), op2->TypeGet()); - DEBUG_DESTROY_NODE(op2); - return fgMorphSmpOp(tree, mac); + if (opts.OptimizationEnabled() && !optValnumCSE_phase && !tree->gtOverflow()) + { + // MUL(NEG(a), C) => MUL(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && + !op2->IsIconHandle()) + { + GenTree* newOp1 = op1->gtGetOp1(); + GenTree* newConst = gtNewIconNode(-op2->AsIntCon()->IconValue(), op2->TypeGet()); + DEBUG_DESTROY_NODE(op1); + DEBUG_DESTROY_NODE(op2); + tree->AsOp()->gtOp1 = newOp1; + tree->AsOp()->gtOp2 = newConst; + return fgMorphSmpOp(tree, mac); + } } #ifndef TARGET_64BIT @@ -12069,18 +12071,22 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return fgMorphSmpOp(tree, mac); } - // -a / C => a / -C, where C is constant - // DIV(NEG(a), C) => DIV(a, NEG(C)) - if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && op1->OperIs(GT_NEG) && - !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && !op2->IsIconHandle() && - op2->AsIntCon()->IconValue() != 1 && op2->AsIntCon()->IconValue() != 0 && - op2->AsIntCon()->IconValue() != -1) + if (opts.OptimizationEnabled() && !optValnumCSE_phase) { - tree->AsOp()->gtOp1 = op1->gtGetOp1(); - DEBUG_DESTROY_NODE(op1); - tree->AsOp()->gtOp2 = gtNewIconNode(-op2->AsIntCon()->IconValue(), op2->TypeGet()); - DEBUG_DESTROY_NODE(op2); - return fgMorphSmpOp(tree, mac); + // DIV(NEG(a), C) => DIV(a, NEG(C)) + if (op1->OperIs(GT_NEG) && !op1->gtGetOp1()->IsCnsIntOrI() && op2->IsCnsIntOrI() && + !op2->IsIconHandle()) + { + ssize_t op2Value = op2->AsIntCon()->IconValue(); + if (op2Value != 1 && op2Value != -1) // Div must throw exception for int(long).MinValue / -1. + { + tree->AsOp()->gtOp1 = op1->gtGetOp1(); + DEBUG_DESTROY_NODE(op1); + tree->AsOp()->gtOp2 = gtNewIconNode(-op2Value, op2->TypeGet()); + DEBUG_DESTROY_NODE(op2); + return fgMorphSmpOp(tree, mac); + } + } } #ifndef TARGET_64BIT @@ -13948,23 +13954,31 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } // Distribute negation over simple multiplication/division expressions - if (op1->OperIs(GT_MUL) || op1->OperIs(GT_DIV)) + if (opts.OptimizationEnabled() && !optValnumCSE_phase && tree->OperIs(GT_NEG) && + op1->OperIs(GT_MUL, GT_DIV)) { - if (!op1->AsOp()->gtOp1->IsCnsIntOrI() && op1->AsOp()->gtOp2->IsCnsIntOrI()) + GenTreeOp* mulOrDiv = op1->AsOp(); + GenTree* op1op1 = mulOrDiv->gtGetOp1(); + GenTree* op1op2 = mulOrDiv->gtGetOp2(); + + if (!op1op1->IsCnsIntOrI() && op1op2->IsCnsIntOrI() && !op1op2->IsIconHandle()) { - // -(a * C) => a * -C - // -(a / C) => a / -C - oper = op1->gtOper; - tree->SetOper(op1->gtOper); - GenTree* newOp1 = op1->AsOp()->gtOp1; // a - GenTree* newOp2 = op1->AsOp()->gtOp2; // C - newOp2->AsIntCon()->SetIconValue(-newOp2->AsIntCon()->IconValue()); + // NEG(MUL(a, C)) => MUL(a, -C) + // NEG(DIV(a, C)) => DIV(a, -C), except when C = {-1, 1} + ssize_t constVal = op1op2->AsIntCon()->IconValue(); + if ((mulOrDiv->OperIs(GT_DIV) && (constVal != -1) && (constVal != 1)) || + (mulOrDiv->OperIs(GT_MUL) && !mulOrDiv->gtOverflow())) + { + GenTree* newOp1 = op1op1; // a + GenTree* newOp2 = gtNewIconNode(-constVal, op1op2->TypeGet()); // -C + mulOrDiv->gtOp1 = newOp1; + mulOrDiv->gtOp2 = newOp2; - // Only need to destroy op1 since op2 will be null already - DEBUG_DESTROY_NODE(op1); + DEBUG_DESTROY_NODE(tree); + DEBUG_DESTROY_NODE(op1op2); - tree->AsOp()->gtOp1 = op1 = newOp1; - tree->AsOp()->gtOp2 = op2 = newOp2; + return mulOrDiv; + } } } From 55d54e63158c27ab9100b20f3f2b42abe93c7fc5 Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 11 Jan 2021 15:47:02 -0800 Subject: [PATCH 16/16] Better names for the tests. --- .../NegMulOrDivToConst.cs | 110 +++++++++--------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs b/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs index dbb87c9953d5a..ddb97363678d4 100644 --- a/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs +++ b/src/tests/JIT/opt/InstructionCombining/NegMulOrDivToConst.cs @@ -22,19 +22,19 @@ class Program static int CheckMulNeg() { bool fail = false; - if (Mul1_1(3) != -7 * 3) + if (MulNeg7(3) != -7 * 3) { fail = true; } - if (Mul1_2(100) != 0) + if (MulNeg0(100) != 0) { fail = true; } try { - Mul1_3(2); + MulNegIntMin(2); } catch { @@ -44,7 +44,7 @@ static int CheckMulNeg() try { - Mul1_4(1); + CheckedMulNenIntMin(1); fail = true; } catch @@ -52,7 +52,7 @@ static int CheckMulNeg() try { - Mul1_4(0); + CheckedMulNenIntMin(0); } catch @@ -60,22 +60,22 @@ static int CheckMulNeg() fail = true; } - if (Mul1_5(1) != -int.MaxValue) + if (NegMulIntMaxValue(1) != -int.MaxValue) { fail = true; } - if (Mul1_5(0) != 0) + if (NegMulIntMaxValue(0) != 0) { fail = true; } - if (Mul1_5(-1) != int.MaxValue) + if (NegMulIntMaxValue(-1) != int.MaxValue) { fail = true; } try { - Mul1_6(int.MinValue); + CheckedMulNeg0(int.MinValue); fail = true; } catch @@ -91,16 +91,16 @@ static int CheckMulNeg() } [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul1_1(int a) => -a * 7; + static int MulNeg7(int a) => -a * 7; [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul1_2(int a) => -a * 0; + static int MulNeg0(int a) => -a * 0; [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul1_3(int a) => -a * int.MinValue; + static int MulNegIntMin(int a) => -a * int.MinValue; [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul1_4(int a) + static int CheckedMulNenIntMin(int a) { checked { @@ -109,10 +109,10 @@ static int Mul1_4(int a) } [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul1_5(int a) => -(a * int.MaxValue); + static int NegMulIntMaxValue(int a) => -(a * int.MaxValue); [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul1_6(int a) + static int CheckedMulNeg0(int a) { checked { @@ -124,14 +124,14 @@ static int Mul1_6(int a) static int CheckNegMul() { bool fail = false; - if (Mul2_1(3) != -7 * 3) + if (NegMul7(3) != -7 * 3) { fail = true; } try { - Mul2_2(100); + NegMulIntMinValue(100); } catch { @@ -141,7 +141,7 @@ static int CheckNegMul() try { - Mul2_3(1); + CheckedNegMulIntMinValue(1); fail = true; } catch @@ -149,7 +149,7 @@ static int CheckNegMul() try { - Mul2_3(0); + CheckedNegMulIntMinValue(0); } catch @@ -166,13 +166,13 @@ static int CheckNegMul() } [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul2_1(int a) => -(a * 7); + static int NegMul7(int a) => -(a * 7); [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul2_2(int a) => -(a * int.MinValue); + static int NegMulIntMinValue(int a) => -(a * int.MinValue); [MethodImpl(MethodImplOptions.NoInlining)] - static int Mul2_3(int a) + static int CheckedNegMulIntMinValue(int a) { checked { @@ -185,24 +185,24 @@ static int CheckDivNeg() { bool fail = false; - if (Div1_1(110) != -10) + if (DivNeg11(110) != -10) { fail = true; } - if (Div1_2(100000000000) != -100000000) + if (LongDivNeg1000(100000000000) != -100000000) { fail = true; } try { - Div1_3(1); - Div1_3(int.MinValue); - Div1_4(1); - Div1_4(long.MinValue); - Div1_5(int.MinValue); - Div1_6(long.MinValue); + DivNegIntMinValue(1); + DivNegIntMinValue(int.MinValue); + LongDivNegLongMinValue(1); + LongDivNegLongMinValue(long.MinValue); + DivNeg1(int.MinValue); + LongDivNeg1(long.MinValue); } catch { @@ -218,46 +218,46 @@ static int CheckDivNeg() } [MethodImpl(MethodImplOptions.NoInlining)] - static int Div1_1(int a) => -a / 11; + static int DivNeg11(int a) => -a / 11; [MethodImpl(MethodImplOptions.NoInlining)] - static long Div1_2(long a) => -a / 1000; + static long LongDivNeg1000(long a) => -a / 1000; [MethodImpl(MethodImplOptions.NoInlining)] - static long Div1_3(int a) => -a / int.MinValue; + static int DivNegIntMinValue(int a) => -a / int.MinValue; [MethodImpl(MethodImplOptions.NoInlining)] - static long Div1_4(long a) => -a / long.MinValue; + static long LongDivNegLongMinValue(long a) => -a / long.MinValue; [MethodImpl(MethodImplOptions.NoInlining)] - static long Div1_5(int a) => -a / 1; + static int DivNeg1(int a) => -a / 1; [MethodImpl(MethodImplOptions.NoInlining)] - static long Div1_6(long a) => -a / 1; + static long LongDivNeg1(long a) => -a / 1; [MethodImpl(MethodImplOptions.NoInlining)] static int CheckNegDiv() { bool fail = false; - if (Div2_1(110) != -10) + if (NegDiv11(110) != -10) { fail = true; } - if (Div2_2(100000000000) != -100000000) + if (LongNegDiv1000(100000000000) != -100000000) { fail = true; } try { - Div2_3(1); - Div2_3(int.MinValue); - Div2_4(1); - Div2_4(long.MinValue); - Div2_5(int.MinValue); - Div2_6(long.MinValue); + NegDivIntMinValue(1); + NegDivIntMinValue(int.MinValue); + LongNegDivLongMinValue(1); + LongNegDivLongMinValue(long.MinValue); + NegDiv1(int.MinValue); + LongNegDiv1(long.MinValue); } catch { @@ -266,7 +266,7 @@ static int CheckNegDiv() try { - Div2_7(int.MinValue); + NegDivMinus1(int.MinValue); fail = true; } catch @@ -274,7 +274,7 @@ static int CheckNegDiv() try { - Div2_8(long.MinValue); + LongNegDivMinus1(long.MinValue); fail = true; } catch @@ -289,28 +289,28 @@ static int CheckNegDiv() } [MethodImpl(MethodImplOptions.NoInlining)] - static int Div2_1(int a) => -(a / 11); + static int NegDiv11(int a) => -(a / 11); [MethodImpl(MethodImplOptions.NoInlining)] - static long Div2_2(long a) => -(a / 1000); + static long LongNegDiv1000(long a) => -(a / 1000); [MethodImpl(MethodImplOptions.NoInlining)] - static int Div2_3(int a) => -(a / int.MinValue); + static int NegDivIntMinValue(int a) => -(a / int.MinValue); [MethodImpl(MethodImplOptions.NoInlining)] - static long Div2_4(long a) => -(a / long.MinValue); + static long LongNegDivLongMinValue(long a) => -(a / long.MinValue); [MethodImpl(MethodImplOptions.NoInlining)] - static long Div2_5(int a) => -(a / 1); + static int NegDiv1(int a) => -(a / 1); [MethodImpl(MethodImplOptions.NoInlining)] - static long Div2_6(long a) => -(a / 1); + static long LongNegDiv1(long a) => -(a / 1); [MethodImpl(MethodImplOptions.NoInlining)] - static long Div2_7(int a) => -(a / -1); + static int NegDivMinus1(int a) => -(a / -1); [MethodImpl(MethodImplOptions.NoInlining)] - static long Div2_8(long a) => -(a / -1); + static long LongNegDivMinus1(long a) => -(a / -1); static int Main() { @@ -334,4 +334,4 @@ static int Main() return 100; } } -} \ No newline at end of file +}