This is an archive of the discontinued LLVM Phabricator instance.

[LAA] Use DL to get element size for bound computation.
ClosedPublic

Authored by fhahn on Oct 7 2020, 3:27 AM.

Details

Summary

Currently LAA uses getScalarSizeInBits to compute the size of an element
when computing the end bound of an access.

This does not work as expected for pointers to pointers, because
getScalarSizeInBits will return 0 for pointer types.

By using DataLayout to get the size of the element we can also correctly
handle pointer element types.

Note the changes to the existing test, which seems to also use the wrong
offset for the end.

Fixes PR47751.

Diff Detail

Event Timeline

fhahn created this revision.Oct 7 2020, 3:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2020, 3:27 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
fhahn requested review of this revision.Oct 7 2020, 3:27 AM

The code change looks right to me, but it's my first time looking at this code.

I verified this fixes the original program I was looking at.

On a related note, the amount of code we generate in runtime checks for the given testcase is completely ridiculous. But that's not a correctness issue. :)

anemet accepted this revision.Oct 7 2020, 10:14 AM

LGTM too.

This revision is now accepted and ready to land.Oct 7 2020, 10:14 AM
fhahn added a comment.Oct 7 2020, 10:18 AM

On a related note, the amount of code we generate in runtime checks for the given testcase is completely ridiculous. But that's not a correctness issue. :)

Agreed, but I think this mostly boils down to improving SCEV expansion and follow-up simplifications, as LAA mostly just hands off SCEV expression to the expander.

This revision was landed with ongoing or failed builds.Oct 7 2020, 10:57 AM
This revision was automatically updated to reflect the committed changes.