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

John Rose john.r.rose at oracle.com
Tue Dec 10 23:31:13 UTC 2019


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> 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/ <https://software.intel.com/sites/landingpage/IntrinsicsGuide/>
>> 
>> Anyway, I got your idea and it makes perfect sense to me to collect such ideas.
> 



More information about the hotspot-compiler-dev mailing list