Skip to content

Commit 9d06057

Browse files
Hamlin LiRealFYang
Hamlin Li
and
RealFYang
committed
8358892: RISC-V: jvm crash when running dacapo sunflow after JDK-8352504
8359045: RISC-V: construct test to verify invocation of C2_MacroAssembler::enc_cmove_cmp_fp => BoolTest::ge/gt Co-authored-by: Fei Yang <fyang@openjdk.org> Reviewed-by: fyang, fjiang
1 parent fedd0a0 commit 9d06057

File tree

4 files changed

+1073
-34
lines changed

4 files changed

+1073
-34
lines changed

‎src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,15 +2170,13 @@ void C2_MacroAssembler::enc_cmove_cmp_fp(int cmpFlag, FloatRegister op1, FloatRe
21702170
cmov_cmp_fp_le(op1, op2, dst, src, is_single);
21712171
break;
21722172
case BoolTest::ge:
2173-
assert(false, "Should go to BoolTest::le case");
2174-
ShouldNotReachHere();
2173+
cmov_cmp_fp_ge(op1, op2, dst, src, is_single);
21752174
break;
21762175
case BoolTest::lt:
21772176
cmov_cmp_fp_lt(op1, op2, dst, src, is_single);
21782177
break;
21792178
case BoolTest::gt:
2180-
assert(false, "Should go to BoolTest::lt case");
2181-
ShouldNotReachHere();
2179+
cmov_cmp_fp_gt(op1, op2, dst, src, is_single);
21822180
break;
21832181
default:
21842182
assert(false, "unsupported compare condition");

‎src/hotspot/cpu/riscv/macroAssembler_riscv.cpp‎

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,12 +1268,19 @@ void MacroAssembler::cmov_gtu(Register cmp1, Register cmp2, Register dst, Regist
12681268
}
12691269

12701270
// ----------- cmove, compare float -----------
1271+
//
1272+
// For CmpF/D + CMoveI/L, ordered ones are quite straight and simple,
1273+
// so, just list behaviour of unordered ones as follow.
1274+
//
1275+
// Set dst (CMoveI (Binary cop (CmpF/D op1 op2)) (Binary dst src))
1276+
// (If one or both inputs to the compare are NaN, then)
1277+
// 1. (op1 lt op2) => true => CMove: dst = src
1278+
// 2. (op1 le op2) => true => CMove: dst = src
1279+
// 3. (op1 gt op2) => false => CMove: dst = dst
1280+
// 4. (op1 ge op2) => false => CMove: dst = dst
1281+
// 5. (op1 eq op2) => false => CMove: dst = dst
1282+
// 6. (op1 ne op2) => true => CMove: dst = src
12711283

1272-
// Move src to dst only if cmp1 == cmp2,
1273-
// otherwise leave dst unchanged, including the case where one of them is NaN.
1274-
// Clarification:
1275-
// java code : cmp1 != cmp2 ? dst : src
1276-
// transformed to : CMove dst, (cmp1 eq cmp2), dst, src
12771284
void MacroAssembler::cmov_cmp_fp_eq(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single) {
12781285
if (UseZicond) {
12791286
if (is_single) {
@@ -1289,7 +1296,7 @@ void MacroAssembler::cmov_cmp_fp_eq(FloatRegister cmp1, FloatRegister cmp2, Regi
12891296
Label no_set;
12901297
if (is_single) {
12911298
// jump if cmp1 != cmp2, including the case of NaN
1292-
// not jump (i.e. move src to dst) if cmp1 == cmp2
1299+
// fallthrough (i.e. move src to dst) if cmp1 == cmp2
12931300
float_bne(cmp1, cmp2, no_set);
12941301
} else {
12951302
double_bne(cmp1, cmp2, no_set);
@@ -1298,11 +1305,6 @@ void MacroAssembler::cmov_cmp_fp_eq(FloatRegister cmp1, FloatRegister cmp2, Regi
12981305
bind(no_set);
12991306
}
13001307

1301-
// Keep dst unchanged only if cmp1 == cmp2,
1302-
// otherwise move src to dst, including the case where one of them is NaN.
1303-
// Clarification:
1304-
// java code : cmp1 == cmp2 ? dst : src
1305-
// transformed to : CMove dst, (cmp1 ne cmp2), dst, src
13061308
void MacroAssembler::cmov_cmp_fp_ne(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single) {
13071309
if (UseZicond) {
13081310
if (is_single) {
@@ -1318,7 +1320,7 @@ void MacroAssembler::cmov_cmp_fp_ne(FloatRegister cmp1, FloatRegister cmp2, Regi
13181320
Label no_set;
13191321
if (is_single) {
13201322
// jump if cmp1 == cmp2
1321-
// not jump (i.e. move src to dst) if cmp1 != cmp2, including the case of NaN
1323+
// fallthrough (i.e. move src to dst) if cmp1 != cmp2, including the case of NaN
13221324
float_beq(cmp1, cmp2, no_set);
13231325
} else {
13241326
double_beq(cmp1, cmp2, no_set);
@@ -1327,14 +1329,6 @@ void MacroAssembler::cmov_cmp_fp_ne(FloatRegister cmp1, FloatRegister cmp2, Regi
13271329
bind(no_set);
13281330
}
13291331

1330-
// When cmp1 <= cmp2 or any of them is NaN then dst = src, otherwise, dst = dst
1331-
// Clarification
1332-
// scenario 1:
1333-
// java code : cmp2 < cmp1 ? dst : src
1334-
// transformed to : CMove dst, (cmp1 le cmp2), dst, src
1335-
// scenario 2:
1336-
// java code : cmp1 > cmp2 ? dst : src
1337-
// transformed to : CMove dst, (cmp1 le cmp2), dst, src
13381332
void MacroAssembler::cmov_cmp_fp_le(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single) {
13391333
if (UseZicond) {
13401334
if (is_single) {
@@ -1350,7 +1344,7 @@ void MacroAssembler::cmov_cmp_fp_le(FloatRegister cmp1, FloatRegister cmp2, Regi
13501344
Label no_set;
13511345
if (is_single) {
13521346
// jump if cmp1 > cmp2
1353-
// not jump (i.e. move src to dst) if cmp1 <= cmp2 or either is NaN
1347+
// fallthrough (i.e. move src to dst) if cmp1 <= cmp2 or either is NaN
13541348
float_bgt(cmp1, cmp2, no_set);
13551349
} else {
13561350
double_bgt(cmp1, cmp2, no_set);
@@ -1359,14 +1353,30 @@ void MacroAssembler::cmov_cmp_fp_le(FloatRegister cmp1, FloatRegister cmp2, Regi
13591353
bind(no_set);
13601354
}
13611355

1362-
// When cmp1 < cmp2 or any of them is NaN then dst = src, otherwise, dst = dst
1363-
// Clarification
1364-
// scenario 1:
1365-
// java code : cmp2 <= cmp1 ? dst : src
1366-
// transformed to : CMove dst, (cmp1 lt cmp2), dst, src
1367-
// scenario 2:
1368-
// java code : cmp1 >= cmp2 ? dst : src
1369-
// transformed to : CMove dst, (cmp1 lt cmp2), dst, src
1356+
void MacroAssembler::cmov_cmp_fp_ge(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single) {
1357+
if (UseZicond) {
1358+
if (is_single) {
1359+
fle_s(t0, cmp2, cmp1);
1360+
} else {
1361+
fle_d(t0, cmp2, cmp1);
1362+
}
1363+
czero_nez(dst, dst, t0);
1364+
czero_eqz(t0 , src, t0);
1365+
orr(dst, dst, t0);
1366+
return;
1367+
}
1368+
Label no_set;
1369+
if (is_single) {
1370+
// jump if cmp1 < cmp2 or either is NaN
1371+
// fallthrough (i.e. move src to dst) if cmp1 >= cmp2
1372+
float_blt(cmp1, cmp2, no_set, false, true);
1373+
} else {
1374+
double_blt(cmp1, cmp2, no_set, false, true);
1375+
}
1376+
mv(dst, src);
1377+
bind(no_set);
1378+
}
1379+
13701380
void MacroAssembler::cmov_cmp_fp_lt(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single) {
13711381
if (UseZicond) {
13721382
if (is_single) {
@@ -1382,7 +1392,7 @@ void MacroAssembler::cmov_cmp_fp_lt(FloatRegister cmp1, FloatRegister cmp2, Regi
13821392
Label no_set;
13831393
if (is_single) {
13841394
// jump if cmp1 >= cmp2
1385-
// not jump (i.e. move src to dst) if cmp1 < cmp2 or either is NaN
1395+
// fallthrough (i.e. move src to dst) if cmp1 < cmp2 or either is NaN
13861396
float_bge(cmp1, cmp2, no_set);
13871397
} else {
13881398
double_bge(cmp1, cmp2, no_set);
@@ -1391,6 +1401,30 @@ void MacroAssembler::cmov_cmp_fp_lt(FloatRegister cmp1, FloatRegister cmp2, Regi
13911401
bind(no_set);
13921402
}
13931403

1404+
void MacroAssembler::cmov_cmp_fp_gt(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single) {
1405+
if (UseZicond) {
1406+
if (is_single) {
1407+
flt_s(t0, cmp2, cmp1);
1408+
} else {
1409+
flt_d(t0, cmp2, cmp1);
1410+
}
1411+
czero_nez(dst, dst, t0);
1412+
czero_eqz(t0 , src, t0);
1413+
orr(dst, dst, t0);
1414+
return;
1415+
}
1416+
Label no_set;
1417+
if (is_single) {
1418+
// jump if cmp1 <= cmp2 or either is NaN
1419+
// fallthrough (i.e. move src to dst) if cmp1 > cmp2
1420+
float_ble(cmp1, cmp2, no_set, false, true);
1421+
} else {
1422+
double_ble(cmp1, cmp2, no_set, false, true);
1423+
}
1424+
mv(dst, src);
1425+
bind(no_set);
1426+
}
1427+
13941428
// Float compare branch instructions
13951429

13961430
#define INSN(NAME, FLOATCMP, BRANCH) \

‎src/hotspot/cpu/riscv/macroAssembler_riscv.hpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,9 @@ class MacroAssembler: public Assembler {
660660
void cmov_cmp_fp_eq(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single);
661661
void cmov_cmp_fp_ne(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single);
662662
void cmov_cmp_fp_le(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single);
663+
void cmov_cmp_fp_ge(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single);
663664
void cmov_cmp_fp_lt(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single);
665+
void cmov_cmp_fp_gt(FloatRegister cmp1, FloatRegister cmp2, Register dst, Register src, bool is_single);
664666

665667
public:
666668
// We try to follow risc-v asm menomics.

0 commit comments

Comments
 (0)