RFR(XS): 8167300: Local scheduling failures during gcm should be fatal

[Vladimir Kozlov]

On 10/7/16 4:25 AM, Roland Westrelin wrote:
>
> Thanks for looking at this, Vladimir.
>
>> First, I can find who can set failure reason in LCM. Can you point me?
>
> It's not set in lcm.cpp. PhaseCFG::schedule_local() returns false and
> the caller (PhaseCFG::global_code_motion()) sets the failure reason.
>
>> Second, is not better for debugging to add assert in a place where we
>> record failure instead of waiting when we exit compilation?
>
> Are you suggesting this?

Yes, it is in addition to your changes.

>
> diff --git a/src/share/vm/opto/lcm.cpp b/src/share/vm/opto/lcm.cpp
> --- a/src/share/vm/opto/lcm.cpp
> +++ b/src/share/vm/opto/lcm.cpp
> @@ -1147,6 +1147,8 @@
>        // If this is the first failure, the sentinel string will "stick"
>        // to the Compile object, and the C2Compiler will see it and retry.
>        C->record_failure(C2Compiler::retry_no_subsuming_loads());
> +    } else {
> +      assert(false, "graph should be schedulable");
>      }
>      // assert( phi_cnt == end_idx(), "did not schedule all" );
>      return false;
>
>
>> Third, should we do the same for late schedule failure?:
>>    C->record_method_not_compilable("late schedule failed: incorrect graph");
>
> Ok. But why don't we do the same for all the other
> record_method_not_compilable() calls in gcm.cpp that don't cause the
> compilation to be reattempted:

Yes, that is what I meant. I gave only one example but it should be for 
all cases.

Thanks,
Vladimir

>
> C->record_method_not_compilable("late schedule failed: LCA == NULL");
> C->record_method_not_compilable("late schedule failed: incorrect graph");
> C->record_method_not_compilable("early schedule failed");
>
> ?
>
> Roland.
>