Review request (S): 6646109 array subscript expressions become top() with -d64

[John Rose]

Reviewed.  But I have one reservation.  (CC-ing the list because it  
touches on an interesting property of our system.)

On Apr 16, 2008, at 3:05 PM, Chuck Rasbold wrote:

> http://webrev.invokedynamic.info/rasbold/6646019/
>
> Several "Tester" programs fail (in different ways) with -d64 when a
> negative array length or negative subscript is used.  C2 proves that
> the subscript expression does not meet the array bounds and a
> ConvI2LNode construction returns top.

But top is the correct answer here.  It should be OK, since the  
control edges that enable the computation will be certainly be  
topping out also.  While the dead code folds up, the compiler should  
not crash, regardless of which order the nodes top out.  Does this  
fix mask any bugs of that sort?

> There are two parts to this fix:
>
> By itself, the change in graphKit.cpp fixes the problem in all
> observed cases.  If the ConvI2LNode construction returns top(), we
> simply back off on the type of ConvI2LNode to TypeLong::INT.

This works because the fold-up happens immediately, and the code can  
done a special-case backoff.  But the same problems could re-appear  
if something slightly delays the fold-up until a later GVN step, so  
this fixup code misses its cue.

Our system works best when it is insensitive to such ordering of  
optimization steps (Church Rosser confluence property).

So, is there a crash downstream (hopefully just in the parser) that  
needs to be fixed instead?

> The fix in parse2.cpp preemptively handles the cases when the array
> bound is negative. The CmpU bounds check trick doesn't work if the
> array bound is negative. However, if the parser observes the bound is
> negative, then it can proceed as if it is 0, and the CmpU mechanism
> causes the path to disappear into an uncommon_trap.

That's a reasonable tweak.  It is simple and saves the compiler some  
useless thrashing.

-- John