[PATCH] D69578: [AIX] Add support for lowering int, float and double formal arguments.

[Zarko Todorovski via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6841
+      if (ValVT == MVT::i1)
+        ArgValue = DAG.getNode(ISD::TRUNCATE, dl, MVT::i1, ArgValue);
+      // PPC64 passes i8, i16, and i32 values in i64 registers. Promote
----------------
sfertile wrote:
> ZarkoCA wrote:
> > sfertile wrote:
> > > Shouldn't we be doing the same thing `extendArgForPPC64` is doing, but with the extra generality that the zext/zext type should be i32 when targeting 32-bit codegen?
> > All other PPC targets look like they just truncate i1s. Adding a condition to check for any extension flags for all types, including i1s, led to some nodes missing.  I will investigate further, however. 
> > All other PPC targets look like they just truncate i1s. 
> The 32-bit targets only truncate, the 64-bit targets add the `Assert[SZ]Ext` nodes appropriate for the ArgFlags. 
> 
> 
> 
> > Adding a condition to check for any extension flags for all types, including i1s, led to some nodes missing.
> 
> Consider the following test case for 32-bit though:
> 
> ```
> @b = common local_unnamed_addr global i8 0, align 1
> 
> define void @callee(i1 zeroext %_b) local_unnamed_addr {
> entry:
>   %frombool = zext i1 %_b to i8
>   store i8 %frombool, i8* @b, align 1
>   ret void
> }
> ```
> 
> If we add the assert nodes, then we would expect a DAG that kind of looks like:
> ```
> t1: i8 ZeroExt(i1 Truncate(i32 AssertZext(i32 CopyFromReg %_b)))
> store<(store 1 into @b)> t1, GlobalAddress:i32<i8* @b>
> ```
> 
> Without the assert nodes:
> ```
> t1: i8 ZeroExt(i1 Truncate(i32 CopyFromReg %_b))
> store<(store 1 into @b)> t1, GlobalAddress:i32<i8* @b>
> ```
> 
> 
> 
> In the first DAG we have preserved the information from the IR: namely that the value was original created by zero extending an i1 into an i32. Becuase of the extra info we can optimize away the truncate and zero extend. In the second DAG we have thrown the extra info away, and have to insert an extra clear instruction. If the Argument doesn't have the zeroext or signext attribute, then we end up with the second DAG anyway, and we would end up with a clear instruction as expected.
> 
> 
I added a function that will use the flags passed in and work in a more general way than extendArgForPPC64, this way we do the same thing for 32 and 64Bit. 


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

https://reviews.llvm.org/D69578