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

[dean.long at oracle.com]

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.
>>>
>>