RFR: 8240302: x64: Assembler::reachable redundantly call Relocation::type() more than once

[Claes Redestad]

Hi Ioi, Vladimir, Tobias,

thanks for reviewing!

Yes, const seems like it can't convince the compiler that it can elide
the redundant calls. Maybe a link-time optimization pass would have
allowed the toolchain to do the right thing here..?

/Claes

On 2020-03-03 03:25, Ioi Lam wrote:
> Looks good to me, too.
> 
> I was wondering if the C compiler would be a little smarter if 
> Relocation::type() was declared as a const function, like:
> 
> - virtual relocInfo::relocType type()       { return relocInfo::none; }
> + virtual relocInfo::relocType type() const { return relocInfo::none; }
> 
> But apparently GCC will not elide the multiple calls:
> 
> // test.cpp
> class X {
> public: virtual int get() const { return 1; }
> };
> 
> class Y : public X {
> public:  virtual int get() const { return 2; }
> };
> 
> X* xx = 0;
> int testfunc(char* argv[]) {
>    X* x = xx;
>    return x->get() + x->get() + x->get();
> }
> 
> $ gcc -O -S -o - test.cpp | grep call
>      call    *(%rax)
>      call    *(%rax)
>      call    *(%rax)
> 
> So it looks like we must manually save the return value of type() as 
> done in your patch.
> 
> Thanks
> - Ioi
> 
> On 3/2/20 4:11 PM, Vladimir Kozlov wrote:
>> Good cleanup.
>>
>> Thanks,
>> Vladimir
>>
>> On 3/2/20 7:07 AM, Claes Redestad wrote:
>>> Hi,
>>>
>>> in the x64-version Assembler::reachable method, various calls to
>>> adr.reloc() are not hoisted by the compiler, due calling into the
>>> virtual method Relocation::type(). This means each call to reachable
>>> typically do several redundant calls into Relocation::type()
>>>
>>> This patch refactors all these calls into a single one, which has a
>>> small, but measurable, effect on startup (and likely some small
>>> effect on JIT compiler throughput in general)
>>>
>>> Webrev: http://cr.openjdk.java.net/~redestad/8240302/open.00/
>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8240302
>>>
>>> Testing: tier1-3
>>>
>>> Thanks!
>>>
>>> /Claes
>