Bug ID: JDK-8288467 remove memory_operand assert for spilled instructions

Type: Bug
Component: hotspot
Sub-Component: compiler
Affected Version: 11,17,19,20

Priority: P3
Status: Resolved
Resolution: Fixed

Submitted: 2022-06-15
Updated: 2022-07-27
Resolved: 2022-06-21

JDK 11	JDK 17	JDK 19	JDK 20
11.0.17-oracleFixed	17.0.5-oracleFixed	19 b28Fixed	20Fixed

$ CONF=linux-x86-server-fastdebug make run-test TEST=compiler/eliminateAutobox/TestFloatBoxing.java TEST_VM_OPTS="-XX:UseAVX=0 -XX:UseSSE=0" 

# To suppress the following error report, specify this argument
# after -XX: or in .hotspotrc:  SuppressErrorAt=/chaitin.cpp:1736
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/shade/trunks/jdk/src/hotspot/share/opto/chaitin.cpp:1736), pid=1609602, tid=1609632
#  assert(cisc->memory_operand() == nullptr) failed: no memory operand, only stack
#
# JRE version: OpenJDK Runtime Environment (20.0) (fastdebug build 20-internal-adhoc.shade.jdk)
# Java VM: OpenJDK Server VM (fastdebug 20-internal-adhoc.shade.jdk, mixed mode, sharing, tiered, g1 gc, linux-x86)
# Problematic frame:
# V  [libjvm.so+0x821956]  PhaseChaitin::fixup_spills()+0x716

The node in question is:
   67  addFPR24_mem_cisc  === 23 62 19 72  [[ ]] #32/0x00000020#8/0x00000008

Fix request [17u] No risk, only removing an assertion. Required follow up for 8282555. Clean backport.
27-06-2022
Fix request [11u] No risk, only removing an assertion. Required follow up for 8282555. Needed a trivial resolve.
27-06-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jdk11u-dev/pull/1188 Date: 2022-06-27 06:07:37 +0000
27-06-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jdk17u-dev/pull/508 Date: 2022-06-27 06:12:28 +0000
27-06-2022
Changeset: af051391 Author: Emanuel Peter <epeter@openjdk.org> Date: 2022-06-21 15:22:02 +0000 URL: https://git.openjdk.org/jdk19/commit/af05139133530871c88991aa0340205cfc44972a
21-06-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jdk19/pull/33 Date: 2022-06-17 09:32:40 +0000
17-06-2022
Additional information: We have a stack spill, from a mach node addFPR24_reg_mem to a cisc node addFPR24_mem_cisc // Cisc-alternate to addFPR_reg // Spill to obtain 24-bit precision instruct addFPR24_reg_mem(stackSlotF dst, regFPR src1, memory src2) %{ predicate(UseSSE==0 && Compile::current()->select_24_bit_instr()); match(Set dst (AddF src1 (LoadF src2))); ... src1 used to be a register. After spilling it is a memory node, reading from stack (frame pointer fp). BTW, both cisc and mach node have mach->oper_input_base() == 2. That is why we go into the else case. Before spilling: 23: control 62: memory for src2 68: float input src1 72: address input dst addFPR24_reg_mem === 23 62 68 72 [[ 66 ]] #8/0x00000008 after spilling: 19: MachProj for frame pointer 67 addFPR24_mem_cisc === 23 62 19 72 something that makes me nervous, is that src is not a memory write but simply a data phi node with type #float. 68 Phi === 23 69 54 [[ 67 ]] #float So far I have usually seen src be a DefinitionSpillCopy node. They are supposed to be allocated in PhaseChaitin::get_spillcopy_wide PhaseChaitin::split_DEF PhaseChaitin::Split PhaseChaitin::Register_Allocate Compile::Code_Gen Ahaa, this is a really special case: 54 roundFloat_mem_reg === _ 55 [[ 102 68 ]] 69 roundFloat_mem_reg === _ 70 [[ 68 ]] 23 Region === 23 84 83 [[ 23 22 59 62 67 68 72 67 ]] 68 Phi === 23 69 54 [[ 67 ]] #float Here, we have two separate spills, both roundFloat_mem_reg: roundFloat_mem_reg(stackSlotF dst, regFPR src) They take a register input, round it, and write it to stack. Since the control flow has just merged in a Region, we also need to merge the value (spilling) in a Phi. So this seems correct.
17-06-2022
On x86 32bit, we have the following instructions with multiple memory_operand, all because of USE_of_memory == 2, because both src1 and src2 are addresses from which the instruction is supposed to read. addFPR24_mem_cisc addFPR24_mem_cisc_0 addFPR24_mem_mem mulFPR24_mem_mem addFPR24_mem_cisc addFPR24_mem_cisc_0 addFPR24_mem_mem mulFPR24_mem_mem
17-06-2022
[~shade] I see the instruction addFPR24_mem_cisc is specific to src/hotspot/cpu/x86/x86_32.ad // Cisc-spill variant // Spill to obtain 24-bit precision instruct addFPR24_mem_cisc(stackSlotF dst, memory src1, memory src2) %{ predicate(UseSSE==0 && Compile::current()->select_24_bit_instr()); match(Set dst (AddF src1 (LoadF src2))); ... We load two values from memory, add them, and store that to stack. This instruction seems to have oper_input_base()==1, because the output is not memory but a stackSlot, and for stackSlots we do unfortunately not say that we need a memory edge in MatchNode::needs_ideal_memory_edge. So then we add the memory edge as an additional req. That seems fine. The problem here is that we do not expect a memory operand. [~jbhateja] You asked for that assert in the PR. Is it possible that this instruction does have a memory_operand, because of src1 and src2 are memory? If that is true, we probably have to remove the assert. cisc->memory_operand() returns 0xffffffff in build/slowdebug-x32/hotspot/variant-server/support/adlc/ad_x86.cpp const MachOper* addFPR24_mem_ciscNode::memory_operand() const { return (MachOper*)-1; } This is generated in ArchDesc::defineClasses becasue we have instr->memory_operand(_globalNames) == InstructForm::MANY_MEMORY_OPERANDS
16-06-2022
[~epeter], any suggestions?
15-06-2022
Since the assert was added in JDK 19, we should target this fix to 19.
15-06-2022
ILW = C2 assertion failure of newly introduced assertion, single test and x86 only (?), possibly use UseAVX>0 and/or UseSSE>0 or disable compilation of affected method = HLM = P3
15-06-2022
Bisection points to JDK-8282555.
15-06-2022