This is an archive of the discontinued LLVM Phabricator instance.

[TargetTransformInfo] assert on nullptr
ClosedPublic

Authored by nickdesaulniers on May 21 2019, 7:11 PM.

Details

Summary

This was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No.
38".

Add an assertion, since it's unlikely that this parameter is nullptr.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2019, 7:11 PM
fhahn added a subscriber: fhahn.May 22 2019, 4:12 AM

I am pretty sure from the uses of getGEPCost that Ptr should never be null. It is also used without check later on. I think it would be better to drop the if (Ptr != nullptr) check.

I am pretty sure from the uses of getGEPCost that Ptr should never be null. It is also used without check later on. I think it would be better to drop the if (Ptr != nullptr) check.

If we're going to do that we need to add an assertion instead

  • prefer assertion to release build checks
nickdesaulniers retitled this revision from [TargetTransformInfo] early return on nullptr to [TargetTransformInfo] assert on nullptr.May 31 2019, 9:45 PM
nickdesaulniers edited the summary of this revision. (Show Details)
nickdesaulniers edited reviewers, added: RKSimon, fhahn; removed: Carrot, chandlerc.
  • git-clang-format HEAD~
RKSimon accepted this revision.Jun 1 2019, 10:54 AM

LGTM with a couple of optional minor cleanups

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
684 ↗(On Diff #202532)

assert for a non-null PointeeType as well ?

690 ↗(On Diff #202532)

auto *BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());

This revision is now accepted and ready to land.Jun 1 2019, 10:54 AM
fhahn added a comment.Jun 1 2019, 11:07 AM

Thanks for updating, LGTM, with Simon's suggestions.

nickdesaulniers marked 2 inline comments as done.Jun 2 2019, 7:23 PM
This comment was removed by nickdesaulniers.
RKSimon accepted this revision.Jun 3 2019, 2:27 AM

cheers @nickdesaulniers LGTM again

This revision was automatically updated to reflect the committed changes.