Skip to content

Commit df032fb

Browse files
authored
JIT: Fix GenTree::IsPow2 functions (#127615)
`IsIntegralConstUnsignedPow2` was passing in sign extended value to `isPow2`. So it wasn't actually unsigned. For example: 1 << 31 would not be recognized as pow2. Add a `UnsignedIntegralValue` that gives the zero-extended literal. Use it in `IsIntegralConstUnsignedPow2` to fix it's impl. Also use it in `fgMorphUModToAndSub` and a place in lower.
1 parent 033157d commit df032fb

6 files changed

Lines changed: 28 additions & 23 deletions

File tree

src/coreclr/jit/emitarm64.cpp

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2595,20 +2595,9 @@ emitter::code_t emitter::emitInsCode(instruction ins, insFormat fmt)
25952595
{
25962596
assert(width <= 64);
25972597

2598-
UINT64 result = ~value;
2599-
2600-
if (width < 64)
2601-
{
2602-
// Check that 'value' fits in 'width' bits. Don't consider "sign" bits above width.
2603-
UINT64 maxVal = 1ULL << width;
2604-
UINT64 lowBitsMask = maxVal - 1;
2605-
UINT64 signBitsMask = ~lowBitsMask | (1ULL << (width - 1)); // The high bits must be set, and the top bit
2606-
// (sign bit) must be set.
2607-
assert((value < maxVal) || ((value & signBitsMask) == signBitsMask));
2608-
2609-
// mask off any extra bits that we got from the complement operation
2610-
result &= lowBitsMask;
2611-
}
2598+
// Mask for zero'ing bits above width.
2599+
UINT64 mask = UINT64_MAX >> (64 - width);
2600+
UINT64 result = ~value & mask;
26122601

26132602
return result;
26142603
}

src/coreclr/jit/gentree.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19614,6 +19614,15 @@ bool GenTreeIntConCommon::ImmedValCanBeFolded(Compiler* comp, genTreeOps op)
1961419614
return !ImmedValNeedsReloc(comp) || (op == GT_EQ) || (op == GT_NE);
1961519615
}
1961619616

19617+
UINT64 GenTreeIntConCommon::UnsignedIntegralValue() const
19618+
{
19619+
uint64_t mask = (UINT64_MAX >> (64 - (genTypeSize(this) * BITS_PER_BYTE)));
19620+
19621+
int64_t signExtended = IntegralValue();
19622+
uint64_t zeroExtended = signExtended & mask;
19623+
return zeroExtended;
19624+
}
19625+
1961719626
#if defined(TARGET_AMD64) || defined(TARGET_RISCV64)
1961819627
// Returns true if this absolute address fits within the base of an addr mode.
1961919628
// On Amd64 this effectively means, whether an absolute indirect address can

src/coreclr/jit/gentree.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,6 +3340,7 @@ struct GenTreeIntConCommon : public GenTree
33403340
inline ssize_t IconValue() const;
33413341
inline void SetIconValue(ssize_t val);
33423342
inline INT64 IntegralValue() const;
3343+
UINT64 UnsignedIntegralValue() const;
33433344
inline void SetIntegralValue(int64_t value);
33443345

33453346
template <typename T>
@@ -3539,7 +3540,7 @@ inline INT64 GenTreeIntConCommon::IntegralValue() const
35393540
#ifdef TARGET_64BIT
35403541
return LngValue();
35413542
#else
3542-
return OperIs(GT_CNS_LNG) ? LngValue() : (INT64)IconValue();
3543+
return OperIs(GT_CNS_LNG) ? LngValue() : static_cast<INT64>(IconValue());
35433544
#endif // TARGET_64BIT
35443545
}
35453546

@@ -10476,7 +10477,7 @@ inline bool GenTree::IsIntegralConstUnsignedPow2() const
1047610477
{
1047710478
if (IsIntegralConst())
1047810479
{
10479-
return isPow2((UINT64)AsIntConCommon()->IntegralValue());
10480+
return isPow2(AsIntConCommon()->UnsignedIntegralValue());
1048010481
}
1048110482

1048210483
return false;
@@ -10494,9 +10495,7 @@ inline bool GenTree::IsIntegralConstAbsPow2() const
1049410495
{
1049510496
if (IsIntegralConst())
1049610497
{
10497-
INT64 svalue = AsIntConCommon()->IntegralValue();
10498-
size_t value = (svalue == SSIZE_T_MIN) ? static_cast<size_t>(svalue) : static_cast<size_t>(abs(svalue));
10499-
return isPow2(value);
10498+
return isAbsPow2(AsIntConCommon()->IntegralValue());
1050010499
}
1050110500

1050210501
return false;

src/coreclr/jit/lower.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4326,8 +4326,8 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp)
43264326
#ifdef TARGET_RISCV64
43274327
if (bitOp->IsIntegralConstUnsignedPow2())
43284328
{
4329-
INT64 bit = bitOp->AsIntConCommon()->IntegralValue();
4330-
int log2 = BitOperations::Log2((UINT64)bit);
4329+
UINT64 bit = bitOp->AsIntConCommon()->UnsignedIntegralValue();
4330+
int log2 = BitOperations::Log2(bit);
43314331
bitOp->AsIntConCommon()->SetIntegralValue(log2);
43324332
return true;
43334333
}

src/coreclr/jit/morph.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12229,8 +12229,8 @@ GenTree* Compiler::fgMorphUModToAndSub(GenTreeOp* tree)
1222912229

1223012230
const var_types type = tree->TypeGet();
1223112231

12232-
const size_t cnsValue = (static_cast<size_t>(tree->gtOp2->AsIntConCommon()->IntegralValue())) - 1;
12233-
GenTree* const newTree = gtNewOperNode(GT_AND, type, tree->gtOp1, gtNewIconNodeWithVN(this, cnsValue, type));
12232+
const size_t mask = static_cast<size_t>(tree->gtOp2->AsIntConCommon()->UnsignedIntegralValue() - 1);
12233+
GenTree* const newTree = gtNewOperNode(GT_AND, type, tree->gtOp1, gtNewIconNodeWithVN(this, mask, type));
1223412234

1223512235
newTree->SetMorphed(this);
1223612236
DEBUG_DESTROY_NODE(tree->gtOp2);

src/coreclr/jit/utils.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ inline bool isPow2(T i)
6767
return (i > 0 && ((i - 1) & i) == 0);
6868
}
6969

70+
// return true if abs(arg) is a power of 2
71+
template <typename T>
72+
inline bool isAbsPow2(T i)
73+
{
74+
static_assert(std::numeric_limits<T>::is_signed);
75+
return (i == std::numeric_limits<T>::min()) || isPow2(std::abs(i));
76+
}
77+
7078
template <typename T>
7179
constexpr bool AreContiguous(T val1, T val2)
7280
{

0 commit comments

Comments
 (0)