[14] RFR (XS): 8234392: C2: Extend Matcher::match_rule_supported_vector() with element type information

[Vladimir Ivanov]

Yes, fully agree. Updated version:
   http://cr.openjdk.java.net/~vlivanov/8234392/webrev.01/

Got rid of ret_value and enhanced the comments, but also fixed 
long-standing bug you noticed: AbsVF should have the same additional 
checks as NegVF (since 512bit vandps is also introduced in AVX512DQ).

Best regards,
Vladimir Ivanov

On 11.12.2019 02:31, John Rose wrote:
> Actually I have one more comment about the new classification logic:
> 
> The “ret_value” idiom is terrible.
> 
> I see a function which is complex, with something like this at the top:
> 
>    if (… simple size check …) {
>      ret_value = false;   // size not supported
>    } …
> 
> I want to read something more decisive like this:
> 
>    if (… simple size check …) {
>      return false;   // size not supported
>    } …
> 
> The “ret_value” thingy adds only noise, and no clarity.
> 
> It is more than an annoyance, hence my comment here.
> The problem is that if I want to understand the quick check
> above, I have to scroll down *past all the other checks* to see
> if some joker does “ret_value = true” before the “return ret_value”,
> subverting my understanding of the code.  Basically, the “ret_value”
> nonsense makes the code impossible to break into understandable
> parts.
> 
> — Grumpy John
> 
> On Dec 10, 2019, at 12:34 PM, John Rose <john.r.rose at oracle.com 
> <mailto:john.r.rose at oracle.com>> wrote:
>>
>>> It would severely penalize Xeon Phis (which lack BW, DQ, and VL 
>>> extensions), but maybe there'll be a moment when Skylake 
>>> (F+CD+BW+DQ+VL) can be chosen as the baseline.
>>
>> Yeah, I saw that coming after I visited the trusty intrinsics guide
>> https://software.intel.com/sites/landingpage/IntrinsicsGuide/
>>>
>>> Anyway, I got your idea and it makes perfect sense to me to collect 
>>> such ideas.
>>
>