Skip to content

Commit

Permalink
[csa] Fix RawPtrT comparison
Browse files Browse the repository at this point in the history 
Update comparisons in BuildFastLoop to unsigned variants.
Prohibit unsigned types to use signed comparisons.

Fixed: chromium:1458837
Change-Id: I7d699fcd9dcc4a197911a537828863cf52f4e6cd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4659513
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#88642}
  • Loading branch information
@victorgomes
victorgomes authored and V8 LUCI CQ committed Jul 4, 2023
1 parent 10aebe5 commit 77b14d9
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 23 deletions.
33 changes: 20 additions & 13 deletions src/codegen/code-stub-assembler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12483,6 +12483,12 @@ TNode<TIndex> CodeStubAssembler::BuildFastLoop(
TNode<TIndex> start_index, TNode<TIndex> end_index,
const FastLoopBody<TIndex>& body, int increment,
LoopUnrollingMode unrolling_mode, IndexAdvanceMode advance_mode) {
// Update the index comparisons below in case we'd ever want to use Smi
// indexes.
static_assert(
!std::is_same<TIndex, Smi>::value,
"Smi indices are currently not supported because it's not clear whether "
"the use case allows unsigned comparisons or not");
var_index = start_index;
VariableList vars_copy(vars.begin(), vars.end(), zone());
vars_copy.push_back(&var_index);
Expand All @@ -12507,7 +12513,7 @@ TNode<TIndex> CodeStubAssembler::BuildFastLoop(
// forward to it from the pre-header). The extra branch is slower in the
// case that the loop actually iterates.
if (unrolling_mode == LoopUnrollingMode::kNo) {
TNode<BoolT> first_check = IntPtrOrSmiEqual(var_index.value(), end_index);
TNode<BoolT> first_check = UintPtrOrSmiEqual(var_index.value(), end_index);
int32_t first_check_val;
if (TryToInt32Constant(first_check, &first_check_val)) {
if (first_check_val) return var_index.value();
Expand All @@ -12519,25 +12525,25 @@ TNode<TIndex> CodeStubAssembler::BuildFastLoop(
BIND(&loop);
{
loop_body();
CSA_DCHECK(
this, increment > 0
? IntPtrOrSmiLessThanOrEqual(var_index.value(), end_index)
: IntPtrOrSmiLessThanOrEqual(end_index, var_index.value()));
Branch(IntPtrOrSmiNotEqual(var_index.value(), end_index), &loop, &done);
CSA_DCHECK(this, increment > 0 ? UintPtrOrSmiLessThanOrEqual(
var_index.value(), end_index)
: UintPtrOrSmiLessThanOrEqual(
end_index, var_index.value()));
Branch(UintPtrOrSmiNotEqual(var_index.value(), end_index), &loop, &done);
}
BIND(&done);
} else {
// Check if there are at least two elements between start_index and
// end_index.
DCHECK_EQ(unrolling_mode, LoopUnrollingMode::kYes);
CSA_DCHECK(this, increment > 0
? IntPtrOrSmiLessThanOrEqual(start_index, end_index)
: IntPtrOrSmiLessThanOrEqual(end_index, start_index));
? UintPtrOrSmiLessThanOrEqual(start_index, end_index)
: UintPtrOrSmiLessThanOrEqual(end_index, start_index));
TNode<TIndex> last_index =
IntPtrOrSmiSub(end_index, IntPtrOrSmiConstant<TIndex>(increment));
TNode<BoolT> first_check =
increment > 0 ? IntPtrOrSmiLessThan(start_index, last_index)
: IntPtrOrSmiGreaterThan(start_index, last_index);
increment > 0 ? UintPtrOrSmiLessThan(start_index, last_index)
: UintPtrOrSmiGreaterThan(start_index, last_index);
int32_t first_check_val;
if (TryToInt32Constant(first_check, &first_check_val)) {
if (first_check_val) {
Expand All @@ -12555,13 +12561,14 @@ TNode<TIndex> CodeStubAssembler::BuildFastLoop(
loop_body();
loop_body();
TNode<BoolT> loop_check =
increment > 0 ? IntPtrOrSmiLessThan(var_index.value(), last_index)
: IntPtrOrSmiGreaterThan(var_index.value(), last_index);
increment > 0
? UintPtrOrSmiLessThan(var_index.value(), last_index)
: UintPtrOrSmiGreaterThan(var_index.value(), last_index);
Branch(loop_check, &loop, &after_loop);
}
BIND(&after_loop);
{
GotoIfNot(IntPtrOrSmiEqual(var_index.value(), last_index), &done);
GotoIfNot(UintPtrOrSmiEqual(var_index.value(), last_index), &done);
// Iteration count is odd.
loop_body();
Goto(&done);
Expand Down
39 changes: 29 additions & 10 deletions src/codegen/code-stub-assembler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,25 +496,42 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
PARAMETER_BINOP(IntPtrOrSmiSub, IntPtrSub, SmiSub)
#undef PARAMETER_BINOP

#define PARAMETER_BINOP(OpName, IntPtrOpName, SmiOpName) \
#define PARAMETER_BINOP(OpName, IntPtrOpName, SmiOpName) \
TNode<BoolT> OpName(TNode<Smi> a, TNode<Smi> b) { return SmiOpName(a, b); } \
TNode<BoolT> OpName(TNode<IntPtrT> a, TNode<IntPtrT> b) { \
return IntPtrOpName(a, b); \
} \
/* IntPtrXXX comparisons shouldn't be used with unsigned types, use \
* UintPtrXXX operations explicitly instead. */ \
TNode<BoolT> OpName(TNode<UintPtrT> a, TNode<UintPtrT> b) { UNREACHABLE(); } \
TNode<BoolT> OpName(TNode<RawPtrT> a, TNode<RawPtrT> b) { UNREACHABLE(); }
// TODO(v8:9708): Define BInt operations once all uses are ported.
PARAMETER_BINOP(IntPtrOrSmiEqual, WordEqual, SmiEqual)
PARAMETER_BINOP(IntPtrOrSmiNotEqual, WordNotEqual, SmiNotEqual)
PARAMETER_BINOP(IntPtrOrSmiLessThan, IntPtrLessThan, SmiLessThan)
PARAMETER_BINOP(IntPtrOrSmiLessThanOrEqual, IntPtrLessThanOrEqual,
SmiLessThanOrEqual)
PARAMETER_BINOP(IntPtrOrSmiGreaterThan, IntPtrGreaterThan, SmiGreaterThan)
#undef PARAMETER_BINOP

#define PARAMETER_BINOP(OpName, UintPtrOpName, SmiOpName) \
TNode<BoolT> OpName(TNode<Smi> a, TNode<Smi> b) { return SmiOpName(a, b); } \
TNode<BoolT> OpName(TNode<IntPtrT> a, TNode<IntPtrT> b) { \
return IntPtrOpName(a, b); \
return UintPtrOpName(Unsigned(a), Unsigned(b)); \
} \
TNode<BoolT> OpName(TNode<UintPtrT> a, TNode<UintPtrT> b) { \
return IntPtrOpName(Signed(a), Signed(b)); \
return UintPtrOpName(a, b); \
} \
TNode<BoolT> OpName(TNode<RawPtrT> a, TNode<RawPtrT> b) { \
return IntPtrOpName(a, b); \
return UintPtrOpName(a, b); \
}
// TODO(v8:9708): Define BInt operations once all uses are ported.
PARAMETER_BINOP(IntPtrOrSmiEqual, WordEqual, SmiEqual)
PARAMETER_BINOP(IntPtrOrSmiNotEqual, WordNotEqual, SmiNotEqual)
PARAMETER_BINOP(IntPtrOrSmiLessThan, IntPtrLessThan, SmiLessThan)
PARAMETER_BINOP(IntPtrOrSmiLessThanOrEqual, IntPtrLessThanOrEqual,
SmiLessThanOrEqual)
PARAMETER_BINOP(IntPtrOrSmiGreaterThan, IntPtrGreaterThan, SmiGreaterThan)
PARAMETER_BINOP(UintPtrOrSmiEqual, WordEqual, SmiEqual)
PARAMETER_BINOP(UintPtrOrSmiNotEqual, WordNotEqual, SmiNotEqual)
PARAMETER_BINOP(UintPtrOrSmiLessThan, UintPtrLessThan, SmiBelow)
PARAMETER_BINOP(UintPtrOrSmiLessThanOrEqual, UintPtrLessThanOrEqual,
SmiBelowOrEqual)
PARAMETER_BINOP(UintPtrOrSmiGreaterThan, UintPtrGreaterThan, SmiAbove)
PARAMETER_BINOP(UintPtrOrSmiGreaterThanOrEqual, UintPtrGreaterThanOrEqual,
SmiAboveOrEqual)
#undef PARAMETER_BINOP
Expand Down Expand Up @@ -732,6 +749,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
SMI_COMPARISON_OP(SmiAboveOrEqual, UintPtrGreaterThanOrEqual,
Uint32GreaterThanOrEqual)
SMI_COMPARISON_OP(SmiBelow, UintPtrLessThan, Uint32LessThan)
SMI_COMPARISON_OP(SmiBelowOrEqual, UintPtrLessThanOrEqual,
Uint32LessThanOrEqual)
SMI_COMPARISON_OP(SmiLessThan, IntPtrLessThan, Int32LessThan)
SMI_COMPARISON_OP(SmiLessThanOrEqual, IntPtrLessThanOrEqual,
Int32LessThanOrEqual)
Expand Down

0 comments on commit 77b14d9

Please sign in to comment.