This is an archive of the discontinued LLVM Phabricator instance.

[TargetLowering] Remove optional arguments passing to makeLibCall
ClosedPublic

Authored by shiva0217 on Aug 6 2019, 2:17 AM.

Details

Summary

The patch introduces MakeLibCallOptions struct as suggested by @efriedma on D65497.
The struct contains bits to pass argument and return value flags to makeLibCall. The patch should not has any functionality changing.

Diff Detail

Repository
rL LLVM

Event Timeline

shiva0217 created this revision.Aug 6 2019, 2:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2019, 2:17 AM
efriedma added inline comments.Aug 8 2019, 3:58 PM
include/llvm/CodeGen/TargetLowering.h
3547 ↗(On Diff #213544)

LLVM standard style is that all type names are capitalized.

3549 ↗(On Diff #213544)

Does it really make sense to have these two big structs here, when makeLibCall itself only uses a few bits from them?

lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
2478 ↗(On Diff #213544)

The "irrelevant" is supposed to be attached to whether the result is signed; an integer that's too large to fit in a register probably isn't going to get extended on any target.

It isn't really a useful comment, though; I'd just get rid of it.

shiva0217 marked 3 inline comments as done.Aug 8 2019, 11:24 PM
shiva0217 added inline comments.
include/llvm/CodeGen/TargetLowering.h
3547 ↗(On Diff #213544)

Ok, Thanks for your kind reminder.

3549 ↗(On Diff #213544)

You're right. It seems we only need to define the bits will be used for makeLibCall. I'll update the structure, thanks.

lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
2478 ↗(On Diff #213544)

I'll remove the comment, thanks.

shiva0217 updated this revision to Diff 214308.Aug 8 2019, 11:30 PM
shiva0217 edited the summary of this revision. (Show Details)

Only define the bits will be used for makelibCall in MakeLibCallOptions struct.

lenary edited reviewers, added: luismarques; removed: lenary.Aug 12 2019, 7:53 AM
luismarques accepted this revision.Aug 15 2019, 12:54 PM

LGTM with the latest changes.

This revision is now accepted and ready to land.Aug 15 2019, 12:54 PM
This revision was automatically updated to reflect the committed changes.