Fix Issue 9097 - Value range propagation to disable some array bound tests #1493

Merged
merged 1 commit into from Jul 9, 2013

3 participants

@yebblies
D Programming Language member

Do not emit bounds check code if the index is statically known to be within bounds

http://d.puremagic.com/issues/show_bug.cgi?id=9097

@Poita Poita and 1 other commented on an outdated diff Jan 17, 2013
src/expression.c
@@ -10177,6 +10178,21 @@ Expression *IndexExp::semantic(Scope *sc)
case Terror:
goto Lerr;
}
+
+ if (t1->ty == Tsarray || t1->ty == Tarray)
+ {
+ Expression *el = new ArrayLengthExp(loc, e1);
+ el = el->semantic(sc);
+ el = el->optimize(WANTvalue);
+ if (el->op == TOKint64 && 0)
@Poita
Poita added a note Jan 17, 2013

&& 0

@yebblies
D Programming Language member

Whoops. This was me checking that it actually changed the asm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@WalterBright
D Programming Language member

This appears to be an amalgamation of several independent things? They should have separate pull requests.

@yebblies
D Programming Language member

This appears to be an amalgamation of several independent things? They should have separate pull requests.

Does this refer to the commits that were erroneously included?

If not, the changes are limited to the following:

  • Allow getting .length from more expressions.
  • Return NULL from VoidInitExp::toExpression to indicate "can't generate an expression" instead of erroring (New! hopefully fixes the phobos errors)
  • Attempt to const-fold the length, and if it succeeds and the index is inside bounds, set a flag to prevent bounds-checks being emitted.

I don't think these changes are complex enough to split up the pull request.

@don-clugston-sociomantic

This is segfaulting in the unittests

@yebblies
D Programming Language member

Yeah, it's a new one. I'll have a look later.

@WalterBright WalterBright commented on the diff Jul 8, 2013
src/constfold.c
@@ -1333,6 +1333,8 @@ Expression *ArrayLength(Type *type, Expression *e1)
e = new IntegerExp(loc, dim, type);
}
+ else if (e1->type->toBasetype()->ty == Tsarray)
+ e = ((TypeSArray *)e1->type->toBasetype())->dim;
@WalterBright
D Programming Language member

How about just making a pull request out of this and the change to optimize.c, add a test case for it, and let's get that pulled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@yebblies yebblies Fix Issue 9097 - Value range propagation to disable some array bound …
…tests

Do not emit bounds check code if the index is statically known to be within bounds
186acf4
@yebblies
D Programming Language member

Even better, I fixed it.

VoidInitializer::toExpression should return NULL, just like all the other Initializers do on error. The segfault was due to TypeTypedef::defaultInitLiteral assuming toExpression always succeeds, which is invalid.

@WalterBright WalterBright merged commit fe9bb4a into dlang:master Jul 9, 2013

1 check was pending

Details default Pass: 8, Pending: 2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment