This is an archive of the discontinued LLVM Phabricator instance.

Convert an APInt to int64_t properly in TTI::getGEPCost().
ClosedPublic

Authored by jlebar on Oct 4 2017, 12:43 PM.

Details

Summary

If the pointer width is 32 bits and the calculated GEP offset is
negative, we call APInt::getLimitedValue(), which does a
*zero*-extension of the offset. That's wrong -- we should do an sext.

Fixes a bug introduced in rL314362 and found by Evgeny Astigeevich.

Diff Detail

Repository
rL LLVM

Event Timeline

jlebar created this revision.Oct 4 2017, 12:43 PM
efriedma added inline comments.Oct 4 2017, 1:00 PM
llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
726 ↗(On Diff #117712)

This doesn't actually clamp the way you want it to; suppose BaseOffset is less than int64_t::min.

I'd suggest just BaseOffset.sextOrTrunc(64).getSExtValue(); not precisely the same thing, but good enough until someone comes along with an architecture which actually needs 128-bit pointer offsets.

jlebar updated this revision to Diff 117720.Oct 4 2017, 1:08 PM
jlebar marked an inline comment as done.

Address efriedma's comment.

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
726 ↗(On Diff #117712)

Indeed, that's better, thank you.

efriedma accepted this revision.Oct 4 2017, 1:36 PM

LGTM

This revision is now accepted and ready to land.Oct 4 2017, 1:36 PM
jlebar added a comment.Oct 4 2017, 1:48 PM

Thank you for the review.

This revision was automatically updated to reflect the committed changes.