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

[David Holmes]

Hi,

Sorry for the delay in getting back to this.

On 5/05/2020 7:37 pm, 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.

In vmSymbols.hpp

+  // 1. Disable/Control Intrinsic accept a list of intrinsic IDs.

s/accept/accepts/

+  //    their final value are subject to hardware inspection 
(VM_Version::initialize).

s/value/values/

Otherwise all my nits have been addressed - thanks.

I don't need to see a further webrev.

Thanks,
David
-----

> 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?
> 
> 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?
> 
> 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.
> 
> Thanks,
> --lx
>