From 01e99f26b53c1e9b340e650664e10d5bf464ce5c Mon Sep 17 00:00:00 2001 From: LuYahan Date: Mon, 7 Sep 2026 13:48:46 +0800 Subject: [PATCH] deps: V8: backport 8521e2e77535 Original commit message: [riscv]Fix unordered FP comparison and emit at the consumer The lowering for kFloatLessThanOrUnordered and kFloatLessThanOrEqualOrUnordered incorrectly mapped to (LT, true) and (LE, true), respectively. Since ComputeBranchInfo can negate a branch condition after emitting the comparison, complementary conditions must use the same comparison and invert its predicate. The incorrect mappings dropped the negation, breaking NaN semantics. For example, optimized Wasm loops like `do { x += 1.0; } while (!(x > 0.0));` produced wrong results. Fix the mappings: kFloatLessThanOrUnordered now uses negated GE, and kFloatLessThanOrEqualOrUnordered uses negated GT. To prevent this class of bug, also convert kRiscvCmpS and kRiscvCmpD into pseudo-instructions like the integer kRiscvCmp. The actual comparison is now emitted at the single flags consumer (branch, boolean materialization, or select) through the new EmitFPCompare helper, using the consumer's final FlagsCondition. This ensures the predicate and machine condition are resolved consistently. Generated code remains byte-identical. Bug: 557938134 Change-Id: I230e181753c5e9c6e7fbea651e551192a76e4005 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8360836 Reviewed-by: Ji Qiu Auto-Submit: Yahan Lu (LuYahan) Commit-Queue: Ji Qiu Cr-Commit-Position: refs/heads/main@{#109690} Refs: https://github.com/v8/v8/commit/8521e2e77535a5c255d15ff8deb459fc7260c54d Co-authored-by: LuYahan --- common.gypi | 2 +- .../backend/riscv/code-generator-riscv.cc | 98 ++++++++++--------- 2 files changed, 53 insertions(+), 47 deletions(-) diff --git a/common.gypi b/common.gypi index e6a17d3505a5..563259e21ea7 100644 --- a/common.gypi +++ b/common.gypi @@ -43,7 +43,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.32', + 'v8_embedder_string': '-node.33', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/compiler/backend/riscv/code-generator-riscv.cc b/deps/v8/src/compiler/backend/riscv/code-generator-riscv.cc index ed33d5d3c671..15ba9ca08efd 100644 --- a/deps/v8/src/compiler/backend/riscv/code-generator-riscv.cc +++ b/deps/v8/src/compiler/backend/riscv/code-generator-riscv.cc @@ -433,8 +433,8 @@ FPUCondition FlagsConditionToConditionCmpFPU(bool* predicate, *predicate = true; return GE; case kFloatLessThanOrUnordered: - *predicate = true; - return LT; + *predicate = false; + return GE; case kFloatGreaterThanOrUnordered: *predicate = false; return LE; @@ -442,8 +442,8 @@ FPUCondition FlagsConditionToConditionCmpFPU(bool* predicate, *predicate = false; return LT; case kFloatLessThanOrEqualOrUnordered: - *predicate = true; - return LE; + *predicate = false; + return GT; default: *predicate = true; break; @@ -451,6 +451,44 @@ FPUCondition FlagsConditionToConditionCmpFPU(bool* predicate, UNREACHABLE(); } +// Emits the actual floating-point comparison for a kRiscvCmpS/kRiscvCmpD +// instruction, materializing its boolean result (0 or 1) into {dst}. +// The comparison itself is a pseudo-instruction (like the integer kRiscvCmp): +// nothing is emitted at the instruction site. Instead, the comparison and the +// FPUCondition-to-machine mapping are resolved together here, at the single +// flags consumer (branch, boolean materialization, or select), using the +// consumer's final FlagsCondition {condition} (which ComputeBranchInfo may +// have negated for branches). This makes it structurally impossible for +// complementary conditions to observe a comparison that was emitted for a +// different condition. +// The returned predicate tells the consumer how to interpret {dst}: when +// false, the comparison result must be inverted to obtain the condition's +// truth value (this is how conditions involving "unordered" are expressed +// with RISC-V's NaN-false flt/fle/feq instructions). +void EmitFPCompare(MacroAssembler* masm, RiscvOperandConverter& i, + Instruction* instr, FlagsCondition condition, Register dst, + bool* predicate) { + FPUCondition cc = FlagsConditionToConditionCmpFPU(predicate, condition); + if (instr->arch_opcode() == kRiscvCmpS) { + FPURegister left = i.InputOrZeroSingleRegister(0); + FPURegister right = i.InputOrZeroSingleRegister(1); + if ((left == kSingleRegZero || right == kSingleRegZero) && + !masm->IsSingleZeroRegSet()) { + masm->LoadFPRImmediate(kSingleRegZero, 0.0f); + } + masm->CompareF32(dst, cc, left, right); + } else { + DCHECK_EQ(instr->arch_opcode(), kRiscvCmpD); + FPURegister left = i.InputOrZeroDoubleRegister(0); + FPURegister right = i.InputOrZeroDoubleRegister(1); + if ((left == kDoubleRegZero || right == kDoubleRegZero) && + !masm->IsDoubleZeroRegSet()) { + masm->LoadFPRImmediate(kDoubleRegZero, 0.0); + } + masm->CompareF64(dst, cc, left, right); + } +} + #if V8_ENABLE_WEBASSEMBLY class WasmOutOfLineTrap : public OutOfLineCode { public: @@ -1597,26 +1635,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction( // Pseudo-instruction used for cmpzero/branch. No opcode emitted here. break; - case kRiscvCmpS: { - FPURegister left = i.InputOrZeroSingleRegister(0); - FPURegister right = i.InputOrZeroSingleRegister(1); - bool predicate; - FPUCondition cc = - FlagsConditionToConditionCmpFPU(&predicate, instr->flags_condition()); - - if ((left == kSingleRegZero || right == kSingleRegZero) && - !__ IsSingleZeroRegSet()) { - __ LoadFPRImmediate(kSingleRegZero, 0.0f); - } - switch (FlagsModeField::decode(instr->opcode())) { - case kFlags_set: - __ CompareF32(i.OutputRegister(), cc, left, right); - break; - default: - __ CompareF32(kScratchReg, cc, left, right); - break; - } - } break; + case kRiscvCmpS: + // Pseudo-instruction used for FP cmp/branch. No opcode emitted here; + // the comparison is emitted by the flags consumer via EmitFPCompare. + break; case kRiscvAddS: // TODO(plind): add special case: combine mult & add. __ fadd_s(i.OutputDoubleRegister(), i.InputDoubleRegister(0), @@ -1645,25 +1667,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction( __ fsqrt_s(i.OutputDoubleRegister(), i.InputDoubleRegister(0)); break; } - case kRiscvCmpD: { - FPURegister left = i.InputOrZeroDoubleRegister(0); - FPURegister right = i.InputOrZeroDoubleRegister(1); - bool predicate; - FPUCondition cc = - FlagsConditionToConditionCmpFPU(&predicate, instr->flags_condition()); - if ((left == kDoubleRegZero || right == kDoubleRegZero) && - !__ IsDoubleZeroRegSet()) { - __ LoadFPRImmediate(kDoubleRegZero, 0.0); - } - switch (FlagsModeField::decode(instr->opcode())) { - case kFlags_set: - __ CompareF64(i.OutputRegister(), cc, left, right); - break; - default: - __ CompareF64(kScratchReg, cc, left, right); - break; - } - } break; + case kRiscvCmpD: + // Pseudo-instruction used for FP cmp/branch. No opcode emitted here; + // the comparison is emitted by the flags consumer via EmitFPCompare. + break; #if V8_TARGET_ARCH_RISCV32 case kRiscvAddPair: __ AddPair(i.OutputRegister(0), i.OutputRegister(1), i.InputRegister(0), @@ -4600,8 +4607,7 @@ void AssembleBranchToLabels(CodeGenerator* gen, MacroAssembler* masm, } else if (instr->arch_opcode() == kRiscvCmpS || instr->arch_opcode() == kRiscvCmpD) { bool predicate; - FlagsConditionToConditionCmpFPU(&predicate, condition); - // floating-point compare result is set in kScratchReg + EmitFPCompare(masm, i, instr, condition, kScratchReg, &predicate); if (predicate) { __ BranchTrueF(kScratchReg, tlabel); } else { @@ -5005,7 +5011,7 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, } else if (instr->arch_opcode() == kRiscvCmpD || instr->arch_opcode() == kRiscvCmpS) { bool predicate; - FlagsConditionToConditionCmpFPU(&predicate, condition); + EmitFPCompare(masm(), i, instr, condition, result, &predicate); // RISCV compare returns 0 or 1, do nothing when predicate; otherwise // toggle result (i.e., 0 -> 1, 1 -> 0) if (!predicate) {