Skip to content

Commit

Permalink
JIT: Remove FEATURE_SET_FLAGS, clean up arm32 flags (#104778)
Browse files Browse the repository at this point in the history
- Remove `FEATURE_SET_FLAGS`
- Remove `GenTree::gtRequestSetFlags`
- Remove all manipulation of `GTF_SET_FLAGS` from morph
- Switch arm32 to be similar to other backends: only set
  `GTF_SET_FLAGS`  on IR nodes that need to set flags that upcoming
  nodes may use. Do not set it for IR nodes whose internal codegen sets
  and uses flags.
  • Loading branch information
jakobbotsch authored and pull[bot] committed Oct 7, 2024
1 parent ed3a399 commit 59ee134
Show file tree
Hide file tree
Showing 12 changed files with 3 additions and 148 deletions.
7 changes: 1 addition & 6 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10391,15 +10391,10 @@ JITDBGAPI void __cdecl cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[SPILLED_OPER]");
}
#if FEATURE_SET_FLAGS
if (tree->gtFlags & GTF_SET_FLAGS)
{
if ((op != GT_IND) && (op != GT_STOREIND))
{
chars += printf("[ZSF_SET_FLAGS]");
}
chars += printf("[SET_FLAGS]");
}
#endif
if (tree->gtFlags & GTF_IND_NONFAULTING)
{
if (tree->OperIsIndirOrArrMetaData())
Expand Down
84 changes: 1 addition & 83 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9961,81 +9961,7 @@ void Compiler::gtUpdateNodeSideEffects(GenTree* tree)

bool GenTree::gtSetFlags() const
{
//
// When FEATURE_SET_FLAGS (TARGET_ARM) is active the method returns true
// when the gtFlags has the flag GTF_SET_FLAGS set
// otherwise the architecture will be have instructions that typically set
// the flags and this method will return true.
//
// Exceptions: GT_IND (load/store) is not allowed to set the flags
// and on XARCH the GT_MUL/GT_DIV and all overflow instructions
// do not set the condition flags
//
// Precondition we have a GTK_SMPOP
//
if (!varTypeIsIntegralOrI(TypeGet()) && (TypeGet() != TYP_VOID))
{
return false;
}

if (((gtFlags & GTF_SET_FLAGS) != 0) && (gtOper != GT_IND))
{
// GTF_SET_FLAGS is not valid on GT_IND and is overlaid with GTF_NONFAULTING_IND
return true;
}
else
{
return false;
}
}

bool GenTree::gtRequestSetFlags()
{
bool result = false;

#if FEATURE_SET_FLAGS
// This method is a Nop unless FEATURE_SET_FLAGS is defined

// In order to set GTF_SET_FLAGS
// we must have a GTK_SMPOP
// and we have a integer or machine size type (not floating point or TYP_LONG on 32-bit)
//
if (!OperIsSimple())
return false;

if (!varTypeIsIntegralOrI(TypeGet()))
return false;

switch (gtOper)
{
case GT_IND:
case GT_ARR_LENGTH:
case GT_MDARR_LENGTH:
case GT_MDARR_LOWER_BOUND:
// These will turn into simple load from memory instructions
// and we can't force the setting of the flags on load from memory
break;

case GT_MUL:
case GT_DIV:
// These instructions don't set the flags (on x86/x64)
//
break;

default:
// Otherwise we can set the flags for this gtOper
// and codegen must set the condition flags.
//
gtFlags |= GTF_SET_FLAGS;
result = true;
break;
}
#endif // FEATURE_SET_FLAGS

// Codegen for this tree must set the condition flags if
// this method returns true.
//
return result;
return (gtFlags & GTF_SET_FLAGS) != 0;
}

GenTreeUseEdgeIterator::GenTreeUseEdgeIterator()
Expand Down Expand Up @@ -10680,10 +10606,6 @@ bool GenTree::HandleKindDataIsInvariant(GenTreeFlags flags)
: '-'); // H is for Hoist this expr
printf("%c", (flags & GTF_REVERSE_OPS) ? 'R' : '-');
printf("%c", (flags & GTF_UNSIGNED) ? 'U' : (flags & GTF_BOOLEAN) ? 'B' : '-');
#if FEATURE_SET_FLAGS
printf("%c", (flags & GTF_SET_FLAGS) ? 'S' : '-');
++charsDisplayed;
#endif

// Both GTF_SPILL and GTF_SPILLED: '#'
// Only GTF_SPILLED: 'z'
Expand Down Expand Up @@ -13281,10 +13203,6 @@ void Compiler::gtDispLIRNode(GenTree* node, const char* prefixMsg /* = nullptr *

// 60 spaces for alignment
printf("%-60s", "");
#if FEATURE_SET_FLAGS
// additional flag enlarges the flag field by one character
printf(" ");
#endif

indentStack.Push(operandArc);
indentStack.print();
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -2271,7 +2271,6 @@ struct GenTree
bool gtOverflow() const;
bool gtOverflowEx() const;
bool gtSetFlags() const;
bool gtRequestSetFlags();

#ifdef DEBUG
static int gtDispFlags(GenTreeFlags flags, GenTreeDebugFlags debugFlags);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const
emitAttr attr = emitActualTypeSize(childNode->TypeGet());
emitAttr size = EA_SIZE(attr);
#ifdef TARGET_ARM
insFlags flags = parentNode->gtSetFlags() ? INS_FLAGS_SET : INS_FLAGS_DONT_CARE;
insFlags flags = (parentNode->gtOverflowEx() || parentNode->gtSetFlags()) ? INS_FLAGS_SET : INS_FLAGS_DONT_CARE;
#endif

switch (parentNode->OperGet())
Expand Down
27 changes: 0 additions & 27 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7610,14 +7610,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA

/* We also mark it as DONT_CSE, as we don't handle QMARKs with nonRELOP op1s */
op1->gtFlags |= (GTF_RELOP_JMP_USED | GTF_DONT_CSE);

// Request that the codegen for op1 sets the condition flags
// when it generates the code for op1.
//
// Codegen for op1 must set the condition flags if
// this method returns true.
//
op1->gtRequestSetFlags();
}
else
{
Expand Down Expand Up @@ -8311,23 +8303,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
op1 = tree->AsOp()->gtOp1;
op2 = tree->gtGetOp2IfPresent();

// Do we have an integer compare operation?
//
if (tree->OperIsCompare() && varTypeIsIntegralOrI(tree->TypeGet()))
{
// Are we comparing against zero?
//
if (op2->IsIntegralConst(0))
{
// Request that the codegen for op1 sets the condition flags
// when it generates the code for op1.
//
// Codegen for op1 must set the condition flags if
// this method returns true.
//
op1->gtRequestSetFlags();
}
}
/*-------------------------------------------------------------------------
* Perform the required oper-specific postorder morphing
*/
Expand Down Expand Up @@ -8619,8 +8594,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
CM_OVF_OP:
if (tree->gtOverflow())
{
tree->gtRequestSetFlags();

// Add the excptn-throwing basic block to jump to on overflow

fgAddCodeRef(compCurBB, SCK_OVERFLOW);
Expand Down
24 changes: 0 additions & 24 deletions src/coreclr/jit/optimizebools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1273,30 +1273,6 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees()
--m_comp->fgReturnCount;
}

#if FEATURE_SET_FLAGS
// For comparisons against zero we will have the GTF_SET_FLAGS set
// and this can cause an assert to fire in fgMoveOpsLeft(GenTree* tree)
// during the CSE phase.
//
// So make sure to clear any GTF_SET_FLAGS bit on these operations
// as they are no longer feeding directly into a comparisons against zero

// Make sure that the GTF_SET_FLAGS bit is cleared.
// Fix 388436 ARM JitStress WP7
m_c1->gtFlags &= ~GTF_SET_FLAGS;
m_c2->gtFlags &= ~GTF_SET_FLAGS;

// The new top level node that we just created does feed directly into
// a comparison against zero, so set the GTF_SET_FLAGS bit so that
// we generate an instruction that sets the flags, which allows us
// to omit the cmp with zero instruction.

// Request that the codegen for cmpOp1 sets the condition flags
// when it generates the code for cmpOp1.
//
cmpOp1->gtRequestSetFlags();
#endif

// Recost/rethread the tree if necessary
//
if (m_comp->fgNodeThreading != NodeThreading::None)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/targetamd64.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#define FEATURE_STRUCTPROMOTE 1 // JIT Optimization to promote fields of structs into registers
#define FEATURE_FASTTAILCALL 1 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 1 // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 0 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
#define MAX_PASS_SINGLEREG_BYTES 8 // Maximum size of a struct passed in a single register (double).
#ifdef UNIX_AMD64_ABI
#define FEATURE_IMPLICIT_BYREFS 1 // Support for struct parameters passed via pointers to shadow copies
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/targetarm.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#define FEATURE_MULTIREG_STRUCT_PROMOTE 0 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_FASTTAILCALL 1 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 1 // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 1 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
#define FEATURE_IMPLICIT_BYREFS 0 // Support for struct parameters passed via pointers to shadow copies
#define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register (including HFA support)
#define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register (including passing HFAs)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/targetarm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#define FEATURE_MULTIREG_STRUCT_PROMOTE 1 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_FASTTAILCALL 1 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 1 // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 0 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
#define FEATURE_IMPLICIT_BYREFS 1 // Support for struct parameters passed via pointers to shadow copies
#define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register
#define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/targetloongarch64.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#define FEATURE_MULTIREG_STRUCT_PROMOTE 1 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_FASTTAILCALL 1 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 1 // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 0 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
#define FEATURE_IMPLICIT_BYREFS 1 // Support for struct parameters passed via pointers to shadow copies
#define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register
#define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/targetriscv64.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#define FEATURE_MULTIREG_STRUCT_PROMOTE 1 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_FASTTAILCALL 1 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 1 // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 0 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
#define FEATURE_IMPLICIT_BYREFS 1 // Support for struct parameters passed via pointers to shadow copies
#define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register
#define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/targetx86.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#define FEATURE_MULTIREG_STRUCT_PROMOTE 0 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_FASTTAILCALL 0 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 0 // opportunistic Tail calls (without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 0 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when
// the flags need to be set
#define FEATURE_IMPLICIT_BYREFS 0 // Support for struct parameters passed via pointers to shadow copies
#define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register
Expand Down

0 comments on commit 59ee134

Please sign in to comment.