[PATCH] D81365: [Alignment][NFC] Migrate HandleByVal to Align
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 8 04:19:45 PDT 2020
gchatelet marked 2 inline comments as done.
gchatelet added inline comments.
================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3840
/// Target-specific cleanup for formal ByVal parameters.
- virtual void HandleByVal(CCState *, unsigned &, unsigned) const {}
+ virtual void HandleByVal(CCState *, unsigned &, Align) const {}
----------------
courbet wrote:
> 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`.
> ```
>
>
Thx I've updated the commit message and review summary accordingly.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81365/new/
https://reviews.llvm.org/D81365
More information about the llvm-commits
mailing list