This is an archive of the discontinued LLVM Phabricator instance.

[OpaquePtr] Use ArgListEntry indirect types more in ISel lowering
ClosedPublic

Authored by aeubanks on May 1 2021, 9:36 PM.

Details

Summary

To avoid calling PointerType::getElementType().

Without also examining the callee in Call->getParam*Type(), we may end
up seeing that we have a byval attribute on the callee, but only looking
at the caller argument attributes which could be missing the byval
attribute.

Diff Detail

Event Timeline

aeubanks created this revision.May 1 2021, 9:36 PM
aeubanks requested review of this revision.May 1 2021, 9:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2021, 9:37 PM
dblaikie added inline comments.May 2 2021, 9:54 AM
llvm/include/llvm/IR/InstrTypes.h
1746–1747

When does the 'Ty' come back as null? Would that only be it older bitcode, that should be auto-upgraded when loading?

aeubanks updated this revision to Diff 342264.May 2 2021, 1:08 PM

assume indirectly passed types are always available

aeubanks added inline comments.May 2 2021, 1:09 PM
llvm/include/llvm/IR/InstrTypes.h
1746–1747

ah you're right
there was one test that was failing when assuming the type was never null, turns out we need to check more closely for these attributes on the arguments, some tests were missing that: https://reviews.llvm.org/D101725

dblaikie accepted this revision.May 2 2021, 1:25 PM

Looks good, contingent on D101725, thanks!

This revision is now accepted and ready to land.May 2 2021, 1:25 PM
This revision was landed with ongoing or failed builds.May 10 2021, 1:05 PM
This revision was automatically updated to reflect the committed changes.
aeubanks reopened this revision.Jun 3 2021, 3:53 PM
This revision is now accepted and ready to land.Jun 3 2021, 3:53 PM
aeubanks retitled this revision from [NFC] Use ArgListEntry indirect types more in ISel lowering to [OpaquePtr] Use ArgListEntry indirect types more in ISel lowering.Jun 11 2021, 12:22 PM
aeubanks edited the summary of this revision. (Show Details)
aeubanks updated this revision to Diff 351530.Jun 11 2021, 12:23 PM

rebase
check callee for byval/preallocated/inalloca type

I don't see any messages in the reverts, but this change caused crashes both times it landed, in case that wasn't already the reason that it was reverted each time.

aeubanks closed this revision.Sep 2 2021, 9:51 AM

This was landed at some point