Page MenuHomePhabricator

[LAA] Begin moving the logic of generating checks out of addRuntimeCheck
ClosedPublic

Authored by anemet on Jul 14 2015, 4:18 PM.

Details

Summary

The goal is to start moving us closer to the model where
RuntimePointerChecking will compute and store the checks. Then a client
can filter the check according to its requirements and then use the
filtered list of checks with addRuntimeCheck.

Before the patch, this is all done in addRuntimeCheck. So the patch
starts to split up addRuntimeCheck while providing the old API under
what's more or less a wrapper now.

The new underlying addRuntimeCheck takes a collection of checks now,
expands the code for the bounds then generates the code for the checks.

I am not completely happy with making expandBounds static because now it
needs so many explicit arguments but I don't want to make the type
PointerBounds part of LAI. This should get fixed when addRuntimeCheck
is moved to LoopVersioning where it really belongs, IMO.

Audited the assembly diff of the testsuite (including externals). There
is a tiny bit of assembly churn that is due to the different order the
code for the bounds is expanded now
(MultiSource/Benchmarks/Prolangs-C/bison/conflicts.s and with LoopDist
on 456.hmmer/fast_algorithms.s).

Diff Detail

Repository
rL LLVM

Event Timeline

anemet updated this revision to Diff 29728.Jul 14 2015, 4:18 PM
anemet retitled this revision from to [LAA] Begin moving the logic of generating checks out of addRuntimeCheck.
anemet updated this object.
anemet added a reviewer: hfinkel.
anemet added a subscriber: llvm-commits.
anemet updated this object.Jul 15 2015, 3:44 PM

Ping.

Hal, This is a pretty simple patch technically not NFC because it changes the order of how the bounds for checks are expanded. I have several follow-on patches queued behind this that continue on the road to split out generation and filtering of checks from addRuntimeChecks.

Thanks,
Adam

klimek added a subscriber: klimek.Jul 24 2015, 3:15 AM

Email test: trying whether I can trigger a bounce here...

hfinkel accepted this revision.Jul 25 2015, 12:59 PM
hfinkel edited edge metadata.

LGTM.

Not specific to this change, but I think that I'd rather the function be named addRuntimeChecks, instead of addRuntimeCheck, because in general it adds multiple checks.

This revision is now accepted and ready to land.Jul 25 2015, 12:59 PM

LGTM.

Many thanks, Hal.

Not specific to this change, but I think that I'd rather the function be named addRuntimeChecks, instead of addRuntimeCheck, because in general it adds multiple checks.

Absolutely, will change that in a follow-on commit.

This revision was automatically updated to reflect the committed changes.

Not specific to this change, but I think that I'd rather the function be named addRuntimeChecks, instead of addRuntimeCheck, because in general it adds multiple checks.

Absolutely, will change that in a follow-on commit.

r244540