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

[Vladimir Kozlov]

Thank you, Dean

Vladimir

On 8/28/18 4:44 PM, dean.long at oracle.com wrote:
> On 8/28/18 3:48 PM, Vladimir Kozlov wrote:
>> Thank you, Dean
>>
>> On 8/27/18 10:21 AM, dean.long at oracle.com wrote:
>>> Looks good.  I did not review avx changes.
>>>
>>> A couple questions:
>>>
>>> Did you consider adding "const char* file = NULL, int line = 0" for x86 only instead of all platforms?
>>
>> Unfortunately pd_patch_instruction() method which is called from shared Label::patch_instructions() is not virtual and 
>> is declared on all platforms.
>> Also I think other platforms can implement this functionality using new API.
>>
> 
> OK.
> 
>>>
>>> Are _file[] and _line[] arrays every used?
>>
>> They are used in macroAssembler_x86.hpp in guarantee() in pd_patch_instruction() which is called when forward branches 
>> are processed in bind(Label):
>>
>> http://cr.openjdk.java.net/~kvn/8209594/webrev.05/src/hotspot/cpu/x86/macroAssembler_x86.hpp.udiff.html
>>
> 
> Actually I was asking about these two:
> 
> +#ifdef ASSERT
> +  // Sourcre file and line location of jump instruction
> +  int _lines[PatchCacheSize];
> +  const char* _files[PatchCacheSize];
> +#endif
> 
> but I see now the use in Label::patch_instructions().
> 
> dl
> 
>> Thanks,
>> Vladimir
>>
>>>
>>> dl
>>>
>>> On 8/25/18 1:05 PM, Vladimir Kozlov 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.
>>>>
>>>
>