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