[PATCH] D108795: [PowerPC] Fix issue with lowering byval parameters.

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.

Please try out the simpler alternative of letting the legalizer take care of the store size. If that alternative is not possible, please add comments to the patch explaining why it is not.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:4403
 
-          if (ObjSize==1 || ObjSize==2 || ObjSize==4) {
-            EVT ObjType = (ObjSize == 1 ? MVT::i8 :
-                           (ObjSize == 2 ? MVT::i16 : MVT::i32));
+          switch (ObjSize) {
+          case 1:
----------------
lei wrote:
> This case stmt basically does:
> 1. generate truc store for i8|i16|i32
> 2. push truc store to MemOps
> 
> I think it would be more clean to create a lambda that does both steps with `MVT::i[8|16|32]` as an arg and call it within the case stmts.
This seems unnecessarily complicated. There is already code in the legalizer that does this, so we should be able to just emit the store that we need.

If for whatever reason we can't, I would much rather not grow this already huge function by all this code but fold this into the new function.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-byval-multi-store.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs --mtriple powerpc64le-unknown-linux-gnu \
----------------
Please pre-commit to show only the differences in code-gen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108795/new/

https://reviews.llvm.org/D108795