RFR[M]: 8151779: Some intrinsic flags could be replaced with one general flag

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu May 7 20:37:50 UTC 2020


On 5/5/20 2:37 AM, Liu, Xin wrote:
> Hello, David and Nils
> 
> Thank you to review the patch.  I went to brush up my English grammar and then update my patch to rev04.
> https://cr.openjdk.java.net/~xliu/8151779/04/webrev/
> Here is the incremental diff:  https://cr.openjdk.java.net/~xliu/8151779/r3_to_r4.diff  It reflect changes based on David's feedbacks. I really appreciate that you review so carefully and found so many invaluable suggestions. TBH, I don't understand Amazon's copyright header neither. I choose the simple way to dodge that problem.
> 
> Nils points out a very tricky question. Yes, I also notice that each TriBool takes 4 bytes on x86_64. It's a natural machine word and supposed to be the most efficient form.  As a result, the vector control_words take about 1.3Kb for all intrinsics. I thought it's not a big deal, but Nils brought up that each DirectiveSet will increase from 128b to 1440b.  Theoretically, the user may provide a CompileCommandFile which consists of hundreds of directives. Will hotspot have hundreds of DirectiveSet in that case?

May be use hashtable instead of whole array in DirectiveSet.

> 
> Actually, I do have a compacted container of TriBool. It's like a vector<bool> specialization.
> https://cr.openjdk.java.net/~xliu/8151779/TriBool.cpp
> 
> The reason I didn't include it because I still feel that a few KiloBytes memories are not a big deal.  Nowadays, hotspot allows Java programmers allocate over 100G heap.  Is it wise to increase software complexity to save KBs?
> 
> If you think it matters, I can integrate it.  May I update TriBoolArray in a standalone JBS? I have made a lot of changes. I hope I can verify them using KitchenSink?

Yes, you can file separate issue for improvements.

> 
> For the second problem,  I think it's because I used 'memset' to initialize an array of objects in rev01.   Previously, I had code like this:
> memset(&_intrinsic_control_words[0], 0, sizeof(_intrinsic_control_words));
> 
> This kind of usage will be warned as -Werror=class-memaccess in g++-8.  I have fixed it since rev02. I use DirectiveSet::fill_in(). Please check out.

Okay.

Thanks,
Vladimir

> 
> Thanks,
> --lx
> 


More information about the hotspot-compiler-dev mailing list