This is an archive of the discontinued LLVM Phabricator instance.

[BasicAA] Remove checks for GEP decomposition limit reached
ClosedPublic

Authored by nikic on Nov 7 2020, 11:40 AM.

Details

Summary

The GEP aliasing code currently checks for the GEP decomposition limit being reached (i.e., we did not reach the "final" underlying object). As far as I can see, these checks are not necessary. It is perfectly fine to work with a GEP whose base can still be further decomposed.

Looking back through the commit history, these checks were originally introduced in https://github.com/llvm/llvm-project/commit/1a444489e9d90915cfdda0720489893896ef1503. However, I believe that the problem this was intended to address was later properly fixed with https://github.com/llvm/llvm-project/commit/1726fc698ccb85fe4bb23c200a50f28b57fc53cb, and the checks are no longer necessary since then (and were not the right fix in the first place).

Is there something I'm missing here?

Diff Detail

Event Timeline

nikic created this revision.Nov 7 2020, 11:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2020, 11:40 AM
nikic requested review of this revision.Nov 7 2020, 11:40 AM

I would assume the "leftover" parts could just as well be part of the base, right? We'd still run the same logic, wouldn't we?

nikic added a comment.Nov 9 2020, 9:08 AM

I would assume the "leftover" parts could just as well be part of the base, right? We'd still run the same logic, wouldn't we?

Right. If the limit is reached, then we would for example have another GEP as base, but that shouldn't influence the treatment it gets in this function.

OK, this sounds sane to me. Can we create a test though?

nikic updated this revision to Diff 303951.Nov 9 2020, 12:01 PM

Add test for behavior at decomposition limit.

nikic added inline comments.Nov 9 2020, 12:07 PM
llvm/test/Analysis/BasicAA/gep-decomposition-limit.ll
11

The effective change here is that results at the decomposition limit are now as good as results below the limit. Results above it have less precision.

15

Slightly non-obvious: The reason why we get a better result on the last one is that we strip off an offset 6 from both sides (from inc6 because that's all there is, from inc7 because of the limit). As the offset is the same, the recursive query can preserves access sizes, so we get a precise end result. For the other cases we'll strip off an asymmetric offset (5 and 6, or 7 and 6) and thus the recursive query is performed without sizes, so we don't get precise results there.

This revision is now accepted and ready to land.Nov 9 2020, 12:58 PM
This revision was automatically updated to reflect the committed changes.