[PATCH] D74225: [AIX] Implement formal arguments passed in stack memory

Zarko Todorovski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 24 10:12:08 PST 2020


ZarkoCA marked 16 inline comments as done.
ZarkoCA added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7009
+    const unsigned ValueSize = ValVT.getStoreSize();
+    assert((ValueSize <= LocSize) &&
+           "Object size is larger than size of MemLoc");
----------------
ZarkoCA wrote:
> cebowleratibm wrote:
> > Do we need to add a truncate node when ValueSize is less than LocSize?
> > 
> > Suggest "ValSize".
> From what I've looked into, no other PPC targets add a truncate node in their respective LowerFormalArguments() functions.  I did find it being done on x86 for i1s, however, and poked around to see why the difference is there.  
> 
> As far as I understand, it looks like we don't need to do on PPC since PPCTargetLowering::LowerLOAD() will do that when we set the load node in LowerFormalArguments.  
> 
> The relevant code snippet from PPCTargetLowering::LowerLOAD() is below: 
> 
> ```
> SDValue NewLD =
>       DAG.getExtLoad(ISD::EXTLOAD, dl, getPointerTy(DAG.getDataLayout()), Chain,
>                      BasePtr, MVT::i8, MMO);
>   SDValue Result = DAG.getNode(ISD::TRUNCATE, dl, MVT::i1, NewLD);
> ```
> 
> I'm fairly sure that's the reason why we don't see truncation to loads from memory in the various PPC LowerFormalArguments() since it's done in LowerLoad for certain types.  
To add further, the special load handling is needed for i1s since we cant do i1 sized loads on Power.  We extend to an i8 and then truncate.

Also, in 64Bit mode for example, the caller will store parameters sized 4 bytes or less with 8 byte loads and sign or zero extended to 8bytes. For the callee, we adjust the offset where the load happens to load only the 4bytes containing the argument and then do a 4byte load from there.  There shouldn't be a need to truncate in that case.   


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74225





More information about the llvm-commits mailing list