[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