RFR(S): 8194993: Loop Strip Mining has some leftover debugging code

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(S): 8194993: Loop Strip Mining has some leftover debugging code

Roland Westrelin-3

http://cr.openjdk.java.net/~roland/8194993/webrev.00/

I noticed I left some debugging code in the logic that finalized the
outer loop of strip mined loops. If the strip mined loop has stores that
were sunk out of the loop and stores on the back edge then a
ShouldNotReachHere() is hit. I never managed to write a test case that
triggers both conditions and they've never been caught by testing so
far. Anyway, we don't want a crash if that ever happens in the wild and
we probably don't want to ship untested code. What I propose instead is
that if that ever happens, we simply drop the outer loop and leave the
inner loop as it is (no strip mining). I left an assert so if that does
happen we have a chance to fix that corner case.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8194993: Loop Strip Mining has some leftover debugging code

Tobias Hartmann-2
Hi Roland,

this looks reasonable to me. Have you verified that the code that bails out works? For example, by always bailing out
and running some tests?

Best regards,
Tobias

On 12.01.2018 13:08, Roland Westrelin wrote:

>
> http://cr.openjdk.java.net/~roland/8194993/webrev.00/
>
> I noticed I left some debugging code in the logic that finalized the
> outer loop of strip mined loops. If the strip mined loop has stores that
> were sunk out of the loop and stores on the back edge then a
> ShouldNotReachHere() is hit. I never managed to write a test case that
> triggers both conditions and they've never been caught by testing so
> far. Anyway, we don't want a crash if that ever happens in the wild and
> we probably don't want to ship untested code. What I propose instead is
> that if that ever happens, we simply drop the outer loop and leave the
> inner loop as it is (no strip mining). I left an assert so if that does
> happen we have a chance to fix that corner case.
>
> Roland.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8194993: Loop Strip Mining has some leftover debugging code

Roland Westrelin-3

Hi Tobias,

Thanks for looking at this.

> this looks reasonable to me. Have you verified that the code that
> bails out works? For example, by always bailing out and running some
> tests?

Yes, I did.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8194993: Loop Strip Mining has some leftover debugging code

Tobias Hartmann-2

On 12.01.2018 13:47, Roland Westrelin wrote:
> Yes, I did.

Okay, great. I'll run testing and sponsor.

Best regards,
Tobias