This is an archive of the discontinued LLVM Phabricator instance.

[Alignment][NFC] Migrate HandleByVal to Align
ClosedPublic

Authored by gchatelet on Jun 8 2020, 1:53 AM.

Details

Summary

Note to downstream target maintainers: this might silently change the semantics of your code if you override TargetLowering::HandleByVal without marking it override.

This patch is part of a series to introduce an Alignment type.
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

Diff Detail

Unit TestsFailed

Event Timeline

gchatelet created this revision.Jun 8 2020, 1:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2020, 1:53 AM
courbet accepted this revision.Jun 8 2020, 2:35 AM
courbet added inline comments.
llvm/include/llvm/CodeGen/TargetLowering.h
3840

This change means that any downstream user that do not mark their functions override now silently stops applying their target-specific cleanup.

Unfortunately overriding a deprecated function does not seem to issue a warning: https://godbolt.org/z/DvQYm8. So I don't have a good way to approach the problem, but maybe the commit message could include something like:

Note to downstream target maintainers: this might silently change the semantics of your code if you override `TargetLowering::HandleByVal` without marking it `override`.
This revision is now accepted and ready to land.Jun 8 2020, 2:35 AM
gchatelet updated this revision to Diff 269160.Jun 8 2020, 3:48 AM

Updated commit message

gchatelet edited the summary of this revision. (Show Details)Jun 8 2020, 3:49 AM
gchatelet marked 2 inline comments as done.
gchatelet added inline comments.
llvm/include/llvm/CodeGen/TargetLowering.h
3840

Thx I've updated the commit message and review summary accordingly.

This revision was automatically updated to reflect the committed changes.
gchatelet marked an inline comment as done.