[12] RFR(M) 8209594: guarantee(this->is8bit(imm8)) failed: Short forward jump exceeds 8-bit offset

[Vladimir Kozlov]

Thank you, Chuck

On 8/27/18 11:00 AM, Chuck Rasbold wrote:
> Thanks, Vladimir. This looks good to me also.
> 
> We also recently detected the failing guarantee for jmpb(CLEANUP) in
> MacroAssembler::string_indexof().
> Your new diagnostics are better than the ones I was working on.
> 
> How did you instrument the code to generate the largest possible code
> sequence? I found a couple of incorrect short jumps in
> vectorized_mismatch() which may be considered false positives; the xmm
> registers always are in the lower bank.

Manual work :(

http://cr.openjdk.java.net/~kvn/8209594/webrev.04/

I instrumented few methods which may generate different code in assembler_x86.cpp.
And I found all avx512 macroAssembler instructions and faked upper bank usage to generate worse 
code. See changes which include instrumentation in macroAssembler_x86.cpp:

I also disable AVX512BW and AVX512VL cpu features in vm_version_x86.cpp to execute .ad instructions 
guarded by supports_avx512nobw() and other changes.

Unfortunately it failed several tests related to String intrinsics so I did not want to spent time 
on investigation and excluded instrumentation.

You are right about false positive in vectorized_mismatch() but it is not guarantied that it will be 
used only in this one place in a future.

Regards,
Vladimir

> 
> 
> 
> 
> On Sat, Aug 25, 2018 at 1:05 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com
>> wrote:
> 
>> http://cr.openjdk.java.net/~kvn/8209594/webrev.05/
>> https://bugs.openjdk.java.net/browse/JDK-8209594
>>
>> All platforms are affected. Please, test.
>>
>> I instrumented code to generated biggest code and find all possible
>> incorrect short jumps. This is the result.
>>
>> Changed jump instruction patching API to added jump's source location in
>> debug build but implemented it only on x86. I tired to search instructions
>> by code offsets. I used very simple macro:
>>
>> #define jmpb(L) jmpb_0(L, __FILE__, __LINE__)
>>
>> 'do {} while(0)' does not work here because I need to replace non-static
>> method. I will be glad if someone can give better suggestion how implement
>> this macro.
>>
>> Fixed incorrect avx512 code in macroAssembler_x86.cpp and x86.ad file.
>> There were missing instructions and incorrect instructions (copy-paste
>> typos).
>>
>> Fixed C2 scratch buffer sizing. It did not take into account everything
>> and as result from MAX_inst_size=1024 only 700 bytes were available. I hit
>> this issue when RTM locking was generated for FastLock node.
>>
>> Tested tier1-3 on all our platforms. And also running these tiers on
>> avx512 machine.
>>
>> --
>> Thanks,
>> Vladimir
>>