RFR: 8283737: riscv: MacroAssembler::stop() should emit fixed-length instruction sequence [v4]

[Aleksey Shipilev]

On Wed, 30 Mar 2022 08:53:26 GMT, Xiaolin Zheng <xlinzheng at openjdk.org> wrote:

>> Hi team,
>> 
>> Could I have a review of this simple patch? -
>> 
>> `PhaseOutput::fill_buffer` detects if the real size of a node matches (<=) the size of it in scratch_emit(). The call chain for MacroAssembler::stop() is:
>> 
>> 
>> MachEpilogNode::emit
>>     -> reserved_stack_check()
>>          -> should_not_reach_here()
>>               -> stop(const char *msg)
>> 
>> 
>> `li()` on RISCV could generate 1~6 instructions, and the msg argument could be an on-stack buffer; `stop()` also uses `__ pc()` that could also be different in `scratch_emit()` and `emit()`. They both have the potential issue here so the size generated in `MacroAssembler::stop()` needs to be a fixed value.
>> 
>> Could be reproduced in the fastdebug build by adding one line:
>> 
>> 
>> // Die now.
>> instruct ShouldNotReachHere() %{
>>   match(Halt);
>>   ins_cost(BRANCH_COST);
>>   format %{ "#@ShouldNotReachHere" %}
>>   ins_encode %{
>>     Assembler::CompressibleRegion cr(&_masm);
>>     if (is_reachable()) {
>>       __ halt();
>> +     __ unimplemented("this is an on-stack char literal");  // assertion fail at 'assert(false, "wrong size of mach node");'
>>     }
>>   %}
>>   ins_pipe(pipe_class_default);
>> %}
>> 
>> 
>> Tests passed in hotspot tier1 & jdk tier1 without new errors found.
>> 
>> Thanks,
>> Xiaolin
>
> Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert the c.beqz commit: put it into another patch as the suggestion
>   
>   This reverts commit cd4a37b655af41f20d00b71a542b00161ff88e22.

Yes, looks good. Submit another PR for that change, and I'll approve too.

-------------

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7982