[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-05-05 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA abandoned this revision.
ZarkoCA added a comment.

This work is included in https://reviews.llvm.org/D79035. Abandoning this 
revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-05-04 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 261837.
ZarkoCA edited the summary of this revision.
ZarkoCA added a comment.

Rebased patch to include latest changes in trunk.  Removed that it depended on 
https://reviews.llvm.org/D73290 in the summary, as that patch has been landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/ppc-dwarf.c
  clang/test/CodeGen/ppc32-struct-return.c
  clang/test/CodeGen/ppc64-dwarf.c

Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ /dev/null
@@ -1,128 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-static unsigned char dwarf_reg_size_table[1024];
-
-int test() {
-  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table);
-
-  return __builtin_dwarf_sp_column();
-}
-
-// CHECK-LABEL: define signext i32 @test()
-// CHECK:  store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 0), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 1), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 2), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 3), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 4), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 5), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 6), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 7), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 8), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 9), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 10), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 11), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 12), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 13), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 14), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 15), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 16), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 17), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 18), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 19), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 20), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 21), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 22), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 23), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 24), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 25), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 26), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 27), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 28), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 29), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-16 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 258110.
ZarkoCA added a comment.

Added a TODO to remove the error for `msvr4-struct-return` on AIX when we 
verify it works as expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/ppc-dwarf.c
  clang/test/CodeGen/ppc32-struct-return.c
  clang/test/CodeGen/ppc64-dwarf.c

Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ /dev/null
@@ -1,128 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-static unsigned char dwarf_reg_size_table[1024];
-
-int test() {
-  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table);
-
-  return __builtin_dwarf_sp_column();
-}
-
-// CHECK-LABEL: define signext i32 @test()
-// CHECK:  store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 0), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 1), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 2), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 3), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 4), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 5), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 6), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 7), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 8), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 9), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 10), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 11), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 12), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 13), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 14), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 15), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 16), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 17), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 18), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 19), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 20), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 21), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 22), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 23), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 24), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 25), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 26), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 27), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 28), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 29), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 30), align 1
-// CHECK-NEXT: store 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-15 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked an inline comment as done.
ZarkoCA added inline comments.



Comment at: clang/test/CodeGen/ppc32-struct-return.c:53
+
+// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported 
on AIX
+ 

jasonliu wrote:
> jasonliu wrote:
> > sfertile wrote:
> > > jasonliu wrote:
> > > > If certain front end option is not supported on certain target, I think 
> > > > it makes more sense to have a standard diagnostic in the driver 
> > > > component, instead of "crash" in the backend. 
> > > > i.e. What if we specify this option on a Windows machine? Maybe we 
> > > > should pursue the same behavior. 
> > > Its not that we don't intend to support the option on AIX, but that 
> > > support for the option takes further verification on AIX.  Since there is 
> > > a difference how  AIX justifies subregister sized values in its argument 
> > > passing, we can't just assume that this option will pack the return 
> > > values the same way on AIX and Linux. 
> > > 
> > > The focus of this patch was originally to enable and verify the proper IR 
> > > generation of va-arg/va-lis/va-start, however the scope of the patch has 
> > > kept growing. If there are codegen differences in packing the return 
> > > register with the svr4-return option enabled it will grow this patch once 
> > > again. The fatal error lets us limit the scope of this patch, while not 
> > > silently emiting bad codegen if there is a difference in how gcc 
> > > initializes the return  registers. If during verification we decide we 
> > > don't want to support the option on AIX, then adopting  a standard 
> > > diagnostic in the driver component becomes the appropriate behavior. 
> > It wasn't clear for me from the code that this is not a permanent thing.  
> > In that case, does it make sense to leave a TODO here and say we need to 
> > re-evaluate this in the future to decide if we want to support it or not on 
> > AIX?
> > On another note, I'm not sure if this is really needed on AIX target 
> > though. But I guess we could discuss it down the road. 
> Just to avoid ambiguity, I meant I'm not sure if we really need this *option* 
> on AIX.
>The focus of this patch was originally to enable and verify the proper IR 
>generation of va-arg/va-lis/va-start, however the scope of the patch has kept 
>growing. If there are codegen differences in packing the return register with 
>the svr4-return option enabled it will grow this patch once again. The fatal 
>error lets us limit the scope of this patch, while not silently emiting bad 
>codegen if there is a difference in how gcc initializes the return registers. 
>If during verification we decide we don't want to support the option on AIX, 
>then adopting a standard diagnostic in the driver component becomes the 
>appropriate behavior.

@sfertile basically articulated my reasoning quite well here.  I just want to 
add, if verified to work on AIX, then we can simply remove the error in that 
one place and have the option working. 

GCC on AIX has those options and they work in the same way as described in 
https://reviews.llvm.org/D73290.  I think implementing a compatible GCC option 
in a fairly straightforward way doesn't hurt us.

I agree with your point made earlier, I will add a TODO to make it clearer that 
we need to completely verify the behaviour. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-14 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/test/CodeGen/ppc32-struct-return.c:53
+
+// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported 
on AIX
+ 

sfertile wrote:
> jasonliu wrote:
> > If certain front end option is not supported on certain target, I think it 
> > makes more sense to have a standard diagnostic in the driver component, 
> > instead of "crash" in the backend. 
> > i.e. What if we specify this option on a Windows machine? Maybe we should 
> > pursue the same behavior. 
> Its not that we don't intend to support the option on AIX, but that support 
> for the option takes further verification on AIX.  Since there is a 
> difference how  AIX justifies subregister sized values in its argument 
> passing, we can't just assume that this option will pack the return values 
> the same way on AIX and Linux. 
> 
> The focus of this patch was originally to enable and verify the proper IR 
> generation of va-arg/va-lis/va-start, however the scope of the patch has kept 
> growing. If there are codegen differences in packing the return register with 
> the svr4-return option enabled it will grow this patch once again. The fatal 
> error lets us limit the scope of this patch, while not silently emiting bad 
> codegen if there is a difference in how gcc initializes the return  
> registers. If during verification we decide we don't want to support the 
> option on AIX, then adopting  a standard diagnostic in the driver component 
> becomes the appropriate behavior. 
It wasn't clear for me from the code that this is not a permanent thing.  In 
that case, does it make sense to leave a TODO here and say we need to 
re-evaluate this in the future to decide if we want to support it or not on AIX?
On another note, I'm not sure if this is really needed on AIX target though. 
But I guess we could discuss it down the road. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-14 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/test/CodeGen/ppc32-struct-return.c:53
+
+// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported 
on AIX
+ 

jasonliu wrote:
> sfertile wrote:
> > jasonliu wrote:
> > > If certain front end option is not supported on certain target, I think 
> > > it makes more sense to have a standard diagnostic in the driver 
> > > component, instead of "crash" in the backend. 
> > > i.e. What if we specify this option on a Windows machine? Maybe we should 
> > > pursue the same behavior. 
> > Its not that we don't intend to support the option on AIX, but that support 
> > for the option takes further verification on AIX.  Since there is a 
> > difference how  AIX justifies subregister sized values in its argument 
> > passing, we can't just assume that this option will pack the return values 
> > the same way on AIX and Linux. 
> > 
> > The focus of this patch was originally to enable and verify the proper IR 
> > generation of va-arg/va-lis/va-start, however the scope of the patch has 
> > kept growing. If there are codegen differences in packing the return 
> > register with the svr4-return option enabled it will grow this patch once 
> > again. The fatal error lets us limit the scope of this patch, while not 
> > silently emiting bad codegen if there is a difference in how gcc 
> > initializes the return  registers. If during verification we decide we 
> > don't want to support the option on AIX, then adopting  a standard 
> > diagnostic in the driver component becomes the appropriate behavior. 
> It wasn't clear for me from the code that this is not a permanent thing.  In 
> that case, does it make sense to leave a TODO here and say we need to 
> re-evaluate this in the future to decide if we want to support it or not on 
> AIX?
> On another note, I'm not sure if this is really needed on AIX target though. 
> But I guess we could discuss it down the road. 
Just to avoid ambiguity, I meant I'm not sure if we really need this *option* 
on AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-14 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: clang/test/CodeGen/ppc32-struct-return.c:53
+
+// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported 
on AIX
+ 

jasonliu wrote:
> If certain front end option is not supported on certain target, I think it 
> makes more sense to have a standard diagnostic in the driver component, 
> instead of "crash" in the backend. 
> i.e. What if we specify this option on a Windows machine? Maybe we should 
> pursue the same behavior. 
Its not that we don't intend to support the option on AIX, but that support for 
the option takes further verification on AIX.  Since there is a difference how  
AIX justifies subregister sized values in its argument passing, we can't just 
assume that this option will pack the return values the same way on AIX and 
Linux. 

The focus of this patch was originally to enable and verify the proper IR 
generation of va-arg/va-lis/va-start, however the scope of the patch has kept 
growing. If there are codegen differences in packing the return register with 
the svr4-return option enabled it will grow this patch once again. The fatal 
error lets us limit the scope of this patch, while not silently emiting bad 
codegen if there is a difference in how gcc initializes the return  registers. 
If during verification we decide we don't want to support the option on AIX, 
then adopting  a standard diagnostic in the driver component becomes the 
appropriate behavior. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-13 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/test/CodeGen/ppc32-struct-return.c:53
+
+// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported 
on AIX
+ 

If certain front end option is not supported on certain target, I think it 
makes more sense to have a standard diagnostic in the driver component, instead 
of "crash" in the backend. 
i.e. What if we specify this option on a Windows machine? Maybe we should 
pursue the same behavior. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-13 Thread Sean Fertile via Phabricator via cfe-commits
sfertile accepted this revision.
sfertile added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-09 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 256398.
ZarkoCA edited the summary of this revision.
ZarkoCA added a comment.

Addressed comments
-added error for -msvr4-struct-return on AIX and modified appropriate test case
-changed code structure as per comment
-renamed test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/ppc-dwarf.c
  clang/test/CodeGen/ppc32-struct-return.c
  clang/test/CodeGen/ppc64-dwarf.c

Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ /dev/null
@@ -1,128 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-static unsigned char dwarf_reg_size_table[1024];
-
-int test() {
-  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table);
-
-  return __builtin_dwarf_sp_column();
-}
-
-// CHECK-LABEL: define signext i32 @test()
-// CHECK:  store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 0), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 1), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 2), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 3), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 4), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 5), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 6), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 7), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 8), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 9), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 10), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 11), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 12), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 13), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 14), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 15), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 16), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 17), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 18), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 19), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 20), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 21), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 22), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 23), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 24), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 25), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 26), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 27), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 28), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 29), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-09 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added a comment.

A couple minor comments, but patch is almost ready otherwise.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:4249
   uint64_t Size;
 
   // -msvr4-struct-return puts small aggregates in GPR3 and GPR4.

Pedantic nit: Can we emit a fatal error if `-msvr4-struct-return` is specified 
on AIX. Then we can add a lit test which checks for the error, which will 
change once we verify the options behavior on AIX and enable it. 

Note: FWIW both Zarko and I have verified the option is accepted and work as 
expected by passing in r3/r4, I'm just not sure if any padding is handled the 
same way on AIX as described below.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4483
+  // AIX does not have SPE registers so we don't set 111 - 113.
+  if (getABIInfo().getTarget().getTriple().isOSAIX()) {
+AssignToArrayRange(Builder, Address, Four8, 109, 110);

Minor nit:

Can we restructure like this:

```
 // 109: vrsave
// 110: vscr
AssignToArrayRange(Builder, Address, Four8, 109, 110);

  // AIX does not have SPE registers so we don't set 111 - 113.
  if (getABIInfo().getTarget().getTriple().isOSAIX()) 
  return false;

// 111: spe_acc
  // 112: spefscr
  // 113: sfp
  AssignToArrayRange(Builder, Address, Four8, 111, 113);

return false;
```



Comment at: clang/test/CodeGen/ppc64-aix32-dwarf.c:1
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s --check-prefix=PPC64
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm  %s -o - | 
FileCheck %s --check-prefix=AIX32

Minor nit: How about naming the test file ppc-dwarf.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-09 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 256351.
ZarkoCA marked 2 inline comments as done.
ZarkoCA added a comment.

Rebased on https://reviews.llvm.org/D73290 and this patch now depends on it.
Removed PPCAIX32TargetCodegenClass from previous diff.
Corrected behaviour `PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable()` for 
32BIT AIX, and added the expected output to the test case which has also been 
renamed to reflect that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/ppc64-aix32-dwarf.c
  clang/test/CodeGen/ppc64-dwarf.c

Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ /dev/null
@@ -1,128 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-static unsigned char dwarf_reg_size_table[1024];
-
-int test() {
-  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table);
-
-  return __builtin_dwarf_sp_column();
-}
-
-// CHECK-LABEL: define signext i32 @test()
-// CHECK:  store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 0), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 1), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 2), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 3), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 4), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 5), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 6), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 7), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 8), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 9), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 10), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 11), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 12), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 13), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 14), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 15), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 16), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 17), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 18), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 19), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 20), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 21), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 22), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 23), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 24), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 25), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 26), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 27), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 28), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-09 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 8 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:

Xiangling_L wrote:
> sfertile wrote:
> > Xiangling_L wrote:
> > > I have a question here. AIX32 falls into PPC32 target, so why we don't 
> > > inherit from `PPC32TargetCodeGenInfo` instead?
> > Do we need a separate AIX specific class? We are implementing 2 functions, 
> > 1 of which is the same implementation as its `PPC32TargetCodeGenInfo` 
> > counterpart. If we have access to the triple, we can  return true when the 
> > OS is AIX in `PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable`. With the 
> > implementations being nearly identical (and after enabling 
> > DwarfEHRegSizeTable they will be identical) I think we are better to not 
> > add a new class if we can avoid it.
> Not adding a new class makes sense to me if we are sure that 
> `DwarfEHRegSizeTable` will be identical/viable for AIX.
I had an offline conversation with @sfertile, verified that the existing 
initDwarfEHRegSizeTable() function can work for AIX with minor changes.  

At first, I thought it may be useuful to create a stub class for for AIX at 
least. But I was able to access the AIX triple info through getABIInfo() and we 
can keep the one PPC32 class and diverge for AIX within the function when 
required. 



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4210
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer

Xiangling_L wrote:
> Is `getDwarfEHStackPointer` necessary to be correct for vararg of AIX to 
> work[I guess possibly not]? If not, should it fall into Dwarf related patch 
> rather than in this one? BTW, if your `PPCAIX32TargetCodeGenInfo` inherits 
> from `PPC32TargetCodeGenInfo` instead as I mentioned above, then it would be 
> naturally correct.
It's not related to vaarg but the implementation is pretty simple so we might 
as well do it since we are here.  I also added a testcase with this enabled. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-09 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:

sfertile wrote:
> Xiangling_L wrote:
> > I have a question here. AIX32 falls into PPC32 target, so why we don't 
> > inherit from `PPC32TargetCodeGenInfo` instead?
> Do we need a separate AIX specific class? We are implementing 2 functions, 1 
> of which is the same implementation as its `PPC32TargetCodeGenInfo` 
> counterpart. If we have access to the triple, we can  return true when the OS 
> is AIX in `PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable`. With the 
> implementations being nearly identical (and after enabling 
> DwarfEHRegSizeTable they will be identical) I think we are better to not add 
> a new class if we can avoid it.
Not adding a new class makes sense to me if we are sure that 
`DwarfEHRegSizeTable` will be identical/viable for AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-09 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:

Xiangling_L wrote:
> I have a question here. AIX32 falls into PPC32 target, so why we don't 
> inherit from `PPC32TargetCodeGenInfo` instead?
Do we need a separate AIX specific class? We are implementing 2 functions, 1 of 
which is the same implementation as its `PPC32TargetCodeGenInfo` counterpart. 
If we have access to the triple, we can  return true when the OS is AIX in 
`PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable`. With the implementations 
being nearly identical (and after enabling DwarfEHRegSizeTable they will be 
identical) I think we are better to not add a new class if we can avoid it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-06 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:

I have a question here. AIX32 falls into PPC32 target, so why we don't inherit 
from `PPC32TargetCodeGenInfo` instead?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4210
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer

Is `getDwarfEHStackPointer` necessary to be correct for vararg of AIX to work[I 
guess possibly not]? If not, should it fall into Dwarf related patch rather 
than in this one? BTW, if your `PPCAIX32TargetCodeGenInfo` inherits from 
`PPC32TargetCodeGenInfo` instead as I mentioned above, then it would be 
naturally correct.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4447
+CodeGen::CodeGenFunction , llvm::Value *Address) const {
+  return true;
+}

As simple as this function is, does it make sense to move the body of `return 
true` into the class definition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-06 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/test/CodeGen/aix-vararg.c:15
+
+  // 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+  // 32BIT-NEXT:  entry:

`#0`, `#1`[the last three lines] are redundant, could you clean them up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-06 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 255323.
ZarkoCA added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/aix32-dwarf-error.c

Index: clang/test/CodeGen/aix32-dwarf-error.c
===
--- /dev/null
+++ clang/test/CodeGen/aix32-dwarf-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -emit-llvm %s
+
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table); //expected-error {{cannot compile this __builtin_init_dwarf_reg_size_table yet}}
+  return __builtin_dwarf_sp_column();
+}
Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,39 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+
+void aix_varg(int a, ...) {
+  __builtin_va_list arg;
+  __builtin_va_start(arg, a);
+  __builtin_va_list arg2;
+  __builtin_va_arg(arg, int);
+  __builtin_va_copy(arg2, arg);
+  __builtin_va_end(arg);
+  __builtin_va_end(arg2);
+}
+
+  // 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+  // 32BIT-NEXT:  entry:
+  // 32BIT-NEXT:%a.addr = alloca i32, align 4
+  // 32BIT-NEXT:%arg = alloca i8*, align 4
+  // 32BIT-NEXT:%arg2 = alloca i8*, align 4
+  // 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+  // 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+  // 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+  // 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+  // 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+  // 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+  // 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+  // 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+  // 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+  // 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+  // 32BIT-NEXT:ret void
+  // 32BIT-NEXT:  }
+  // 32BIT: declare void @llvm.va_start(i8*) #1
+  // 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+  // 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4173,14 +4173,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
@@ -4190,7 +4191,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -4200,9 +4201,22 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  PPCAIX32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer
+  }
+
+  bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
+   llvm::Value *Address) const override;
+};
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = Ty->getAs())
 Ty = CTy->getElementType();
@@ -4228,9 +4242,12 @@
 
 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-01 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 254214.
ZarkoCA marked an inline comment as done.
ZarkoCA added a comment.

Set isSoftFloat to return false for AIX.


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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/aix32-dwarf-error.c

Index: clang/test/CodeGen/aix32-dwarf-error.c
===
--- /dev/null
+++ clang/test/CodeGen/aix32-dwarf-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -emit-llvm %s
+
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table); //expected-error {{cannot compile this __builtin_init_dwarf_reg_size_table yet}}
+  return __builtin_dwarf_sp_column();
+}
Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,39 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+
+void aix_varg(int a, ...) {
+  __builtin_va_list arg;
+  __builtin_va_start(arg, a);
+  __builtin_va_list arg2;
+  __builtin_va_arg(arg, int);
+  __builtin_va_copy(arg2, arg);
+  __builtin_va_end(arg);
+  __builtin_va_end(arg2);
+}
+
+  // 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+  // 32BIT-NEXT:  entry:
+  // 32BIT-NEXT:%a.addr = alloca i32, align 4
+  // 32BIT-NEXT:%arg = alloca i8*, align 4
+  // 32BIT-NEXT:%arg2 = alloca i8*, align 4
+  // 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+  // 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+  // 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+  // 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+  // 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+  // 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+  // 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+  // 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+  // 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+  // 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+  // 32BIT-NEXT:ret void
+  // 32BIT-NEXT:  }
+  // 32BIT: declare void @llvm.va_start(i8*) #1
+  // 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+  // 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4173,14 +4173,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
@@ -4190,7 +4191,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -4200,9 +4201,22 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  PPCAIX32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer
+  }
+
+  bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
+   llvm::Value *Address) const override;
+};
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = Ty->getAs())
 Ty = 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-01 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 4 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10019
+  return SetCGInfo(
+  new PPCAIX32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == 
"soft"));
 return SetCGInfo(

jasonliu wrote:
> ZarkoCA wrote:
> > jasonliu wrote:
> > > Does AIX have soft Float? If not, do we want to always pass in 'false'? 
> > Thanks, missed changing this.  I set it to hard.
> I don't think `CodeGenOpts.FloatABI == "hard"` is what we want though. 
> Currently it means if CodeGenOpts.FloatABI is really "hard", then it will 
> pass in `true` for `SoftFloatABI` to indicate we are soft float ABI.
You're right. I wanted to keep the symmetry but that's not the correct thing to 
do. 


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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-04-01 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10019
+  return SetCGInfo(
+  new PPCAIX32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == 
"soft"));
 return SetCGInfo(

ZarkoCA wrote:
> jasonliu wrote:
> > Does AIX have soft Float? If not, do we want to always pass in 'false'? 
> Thanks, missed changing this.  I set it to hard.
I don't think `CodeGenOpts.FloatABI == "hard"` is what we want though. 
Currently it means if CodeGenOpts.FloatABI is really "hard", then it will pass 
in `true` for `SoftFloatABI` to indicate we are soft float ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-30 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 253776.
ZarkoCA added a comment.

Fixed test cases to use builtins again, set no soft float abi for AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/aix32-dwarf-error.c

Index: clang/test/CodeGen/aix32-dwarf-error.c
===
--- /dev/null
+++ clang/test/CodeGen/aix32-dwarf-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -emit-llvm %s
+
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table); //expected-error {{cannot compile this __builtin_init_dwarf_reg_size_table yet}}
+  return __builtin_dwarf_sp_column();
+}
Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,39 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+
+void aix_varg(int a, ...) {
+  __builtin_va_list arg;
+  __builtin_va_start(arg, a);
+  __builtin_va_list arg2;
+  __builtin_va_arg(arg, int);
+  __builtin_va_copy(arg2, arg);
+  __builtin_va_end(arg);
+  __builtin_va_end(arg2);
+}
+
+  // 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+  // 32BIT-NEXT:  entry:
+  // 32BIT-NEXT:%a.addr = alloca i32, align 4
+  // 32BIT-NEXT:%arg = alloca i8*, align 4
+  // 32BIT-NEXT:%arg2 = alloca i8*, align 4
+  // 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+  // 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+  // 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+  // 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+  // 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+  // 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+  // 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+  // 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+  // 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+  // 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+  // 32BIT-NEXT:ret void
+  // 32BIT-NEXT:  }
+  // 32BIT: declare void @llvm.va_start(i8*) #1
+  // 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+  // 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4173,14 +4173,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
@@ -4190,7 +4191,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -4200,9 +4201,22 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  PPCAIX32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer
+  }
+
+  bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
+   llvm::Value *Address) const override;
+};
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-30 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 4 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10019
+  return SetCGInfo(
+  new PPCAIX32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == 
"soft"));
 return SetCGInfo(

jasonliu wrote:
> Does AIX have soft Float? If not, do we want to always pass in 'false'? 
Thanks, missed changing this.  I set it to hard.



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

jasonliu wrote:
> Any reason we don't use __builtin_va... any more?
My mistake, I somehow included the old version of the test in the diff. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-30 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10019
+  return SetCGInfo(
+  new PPCAIX32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == 
"soft"));
 return SetCGInfo(

Does AIX have soft Float? If not, do we want to always pass in 'false'? 



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

Any reason we don't use __builtin_va... any more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-27 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 2 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements

jasonliu wrote:
> sfertile wrote:
> > jasonliu wrote:
> > > So for AIX we are taking PowerPC32ABIInfo right now. And we know 
> > > EmitVAArg of this target does the right thing on AIX after the change. 
> > > But for other functions, for example, getParamTypeAlignment, 
> > > initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> > > If we are not sure, is there anything we want to do (etc, put a comment 
> > > on where it gets used or at the function definition)? Or are we fine to 
> > > just leave it as it is and have a TODO in our head?
> > Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes 
> > for all the registers. Even though the OS is different the underlying 
> > hardware is the same. I'm not sure it's something that makes sense to 
> > support for AIX though, in which case I think its valid to return `true` to 
> > indicate its not supported. 
> > 
> > `getParamTypeAlignment` is used only in the context of the alignment for 
> > vaarg, we should make sure its correct for AIX since supporting vaarg is 
> > the scope of this patch.
> In that case, is it better if we have lit test to actually exercise the path 
> in getParamTypeAlignment to show that we actually confirmed the behavior is 
> correct for AIX? And if it is some path we do not care for now(ComplexType? 
> VectorType?), then we would want TODOs on them to show we need further 
> investigation later. 
>But for other functions, for example, getParamTypeAlignment, 
>initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?

The AIX stdarg headers show that alignment for the va_list is dependent on 
mode, so 4 on 32 and 8 on 64-bit. getParamAlignment looks like it aligns 4 for 
non-complex non-struct args. 



>If we are not sure, is there anything we want to do (etc, put a comment on 
>where it gets used or at the function definition)? Or are we fine to just 
>leave it as it is and have a TODO in our head?

We already took this path before this patch and I'm correcting some of the 
behaviour so it's correct for varargs on AIX. I think that we will need to 
address this depending on when/if we add an AIXABIInfo class.  I added a TODO 
on line 4247 for just that so I think that implies we will investigate 
everything required for the correct implementation of the whatever remains 
after the vaarg handling.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-27 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 253127.
ZarkoCA added a comment.

Created PPCAIX32TargetCodeGenInfo class so that initDwarfEHRegSizeTable now 
returns true on AIX and added a test.

Fixed formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/aix32-dwarf-error.c

Index: clang/test/CodeGen/aix32-dwarf-error.c
===
--- /dev/null
+++ clang/test/CodeGen/aix32-dwarf-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -emit-llvm %s
+
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table); //expected-error {{cannot compile this __builtin_init_dwarf_reg_size_table yet}}
+  return __builtin_dwarf_sp_column();
+}
Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,40 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+#include 
+
+void aix_varg(int a, ...) {
+  va_list arg;
+  va_start(arg, a);
+  va_list arg2;
+  va_arg(arg, int);
+  va_copy(arg2, arg);
+  va_end(arg);
+  va_end(arg2);
+}
+
+// 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+// 32BIT-NEXT:  entry:
+// 32BIT-NEXT:%a.addr = alloca i32, align 4
+// 32BIT-NEXT:%arg = alloca i8*, align 4
+// 32BIT-NEXT:%arg2 = alloca i8*, align 4
+// 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+// 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+// 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+// 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+// 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+// 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+// 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+// 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+// 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+// 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+// 32BIT-NEXT:ret void
+// 32BIT-NEXT:  }
+// 32BIT: declare void @llvm.va_start(i8*) #1
+// 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+// 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4173,14 +4173,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
@@ -4190,7 +4191,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -4200,9 +4201,22 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  PPCAIX32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer
+  }
+
+  bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
+   llvm::Value *Address) const override;
+};
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = Ty->getAs())
 Ty = CTy->getElementType();
@@ 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-26 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements

sfertile wrote:
> jasonliu wrote:
> > So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg 
> > of this target does the right thing on AIX after the change. 
> > But for other functions, for example, getParamTypeAlignment, 
> > initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> > If we are not sure, is there anything we want to do (etc, put a comment on 
> > where it gets used or at the function definition)? Or are we fine to just 
> > leave it as it is and have a TODO in our head?
> Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes for 
> all the registers. Even though the OS is different the underlying hardware is 
> the same. I'm not sure it's something that makes sense to support for AIX 
> though, in which case I think its valid to return `true` to indicate its not 
> supported. 
> 
> `getParamTypeAlignment` is used only in the context of the alignment for 
> vaarg, we should make sure its correct for AIX since supporting vaarg is the 
> scope of this patch.
In that case, is it better if we have lit test to actually exercise the path in 
getParamTypeAlignment to show that we actually confirmed the behavior is 
correct for AIX? And if it is some path we do not care for now(ComplexType? 
VectorType?), then we would want TODOs on them to show we need further 
investigation later. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-24 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements

jasonliu wrote:
> So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg of 
> this target does the right thing on AIX after the change. 
> But for other functions, for example, getParamTypeAlignment, 
> initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> If we are not sure, is there anything we want to do (etc, put a comment on 
> where it gets used or at the function definition)? Or are we fine to just 
> leave it as it is and have a TODO in our head?
Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes for 
all the registers. Even though the OS is different the underlying hardware is 
the same. I'm not sure it's something that makes sense to support for AIX 
though, in which case I think its valid to return `true` to indicate its not 
supported. 

`getParamTypeAlignment` is used only in the context of the alignment for vaarg, 
we should make sure its correct for AIX since supporting vaarg is the scope of 
this patch.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232
+Address PowerPC32ABIInfo::EmitVAArg(CodeGenFunction , Address VAList,
   QualType Ty) const {
+  // TODO: Add AIX ABI Info. Currently, we are relying on PowerPC32ABIInfo to

Make sure to address the formatting here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-24 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements

So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg of 
this target does the right thing on AIX after the change. 
But for other functions, for example, getParamTypeAlignment, 
initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
If we are not sure, is there anything we want to do (etc, put a comment on 
where it gets used or at the function definition)? Or are we fine to just leave 
it as it is and have a TODO in our head?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-24 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 6 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4175
 namespace {
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {

sfertile wrote:
> ZarkoCA wrote:
> > sfertile wrote:
> > > This name and comment is misleading, the class is used for both SVR4 and 
> > > Darwin, and after this patch AIX. We need to fix the name comment to 
> > > reflect that.
> > Does this wording of the comment work? 
> Missed updating the class name. Suggested: `PPC32_SVR4_ABIInfo` --> 
> `PowerPC32ABIInfo`.
Sorry, missed updating the class name the first time.  



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

hubert.reinterpretcast wrote:
> ZarkoCA wrote:
> > hubert.reinterpretcast wrote:
> > > Can we use built-in types and functions in place of a header inclusion?
> > I'm worried about legal/copyright issues with using contents from AIX 
> > system headers to LLVM testcases. But I can do that for sure once I 
> > understand what I am allowed to do.   
> I just mean:
> `__builtin_va_list`
> `__builtin_va_start`
> `__builtin_va_copy`
> `__builtin_va_arg`
> `__builtin_va_end`
> 
Thanks, I wasn't clear.  Done as that now. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-24 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 252325.
ZarkoCA marked 2 inline comments as done.
ZarkoCA added a comment.

Renamed PPC32_SVR4ABIInfo class to PPC32ABIInfo.
Fixed test case to use builtins.
Changed comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c

Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,39 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+//
+void aix_varg(int a, ...) {
+  __builtin_va_list arg;
+  __builtin_va_start(arg, a);
+  __builtin_va_list arg2;
+  __builtin_va_arg(arg, int);
+  __builtin_va_copy(arg2, arg);
+  __builtin_va_end(arg);
+  __builtin_va_end(arg2);
+}
+
+// 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+// 32BIT-NEXT:  entry:
+// 32BIT-NEXT:%a.addr = alloca i32, align 4
+// 32BIT-NEXT:%arg = alloca i8*, align 4
+// 32BIT-NEXT:%arg2 = alloca i8*, align 4
+// 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+// 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+// 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+// 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+// 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+// 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+// 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+// 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+// 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+// 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+// 32BIT-NEXT:ret void
+// 32BIT-NEXT:  }
+// 32BIT: declare void @llvm.va_start(i8*) #1
+// 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+// 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4172,14 +4172,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin, and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
@@ -4189,7 +4190,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -4201,7 +4202,7 @@
 };
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = Ty->getAs())
 Ty = CTy->getElementType();
@@ -4227,9 +4228,12 @@
 
 // TODO: this implementation is now likely redundant with
 // DefaultABIInfo::EmitVAArg.
-Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction , Address VAList,
+Address PowerPC32ABIInfo::EmitVAArg(CodeGenFunction , Address VAList,
   QualType Ty) const {
-  if (getTarget().getTriple().isOSDarwin()) {
+  // TODO: Add AIX ABI Info. Currently, we are relying on PowerPC32ABIInfo to
+  // emit correct VAArg.
+  if (getTarget().getTriple().isOSDarwin() ||
+  getTarget().getTriple().isOSAIX()) {
 auto TI = getContext().getTypeInfoInChars(Ty);
 TI.second = getParamTypeAlignment(Ty);
 
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -369,7 +369,8 @@
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
-// This is the ELF definition, and is overridden by the Darwin sub-target
+// This is the ELF definition, and is overridden by the Darwin and AIX
+// 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-23 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4175
 namespace {
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {

ZarkoCA wrote:
> sfertile wrote:
> > This name and comment is misleading, the class is used for both SVR4 and 
> > Darwin, and after this patch AIX. We need to fix the name comment to 
> > reflect that.
> Does this wording of the comment work? 
Missed updating the class name. Suggested: `PPC32_SVR4_ABIInfo` --> 
`PowerPC32ABIInfo`.


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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-19 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4176
+/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC 
ELF
+/// (SVR4), Darwin and AIX.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {

I would suggest using an "Oxford comma" before the "and". Also, colon instead 
of comma before "used".



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

ZarkoCA wrote:
> hubert.reinterpretcast wrote:
> > Can we use built-in types and functions in place of a header inclusion?
> I'm worried about legal/copyright issues with using contents from AIX system 
> headers to LLVM testcases. But I can do that for sure once I understand what 
> I am allowed to do.   
I just mean:
`__builtin_va_list`
`__builtin_va_start`
`__builtin_va_copy`
`__builtin_va_arg`
`__builtin_va_end`



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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-19 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 8 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4175
 namespace {
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {

sfertile wrote:
> This name and comment is misleading, the class is used for both SVR4 and 
> Darwin, and after this patch AIX. We need to fix the name comment to reflect 
> that.
Does this wording of the comment work? 



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232
   QualType Ty) const {
-  if (getTarget().getTriple().isOSDarwin()) {
+  // TODO: Add AIX ABI Info.  Currently we are relying on PPC32_SVR4_ABIInfo to
+  // emit correct VAArg.

hubert.reinterpretcast wrote:
> No need for two spaces. Add comma after "Currently".
Thanks, I've found it hard to shake the habits my Grade 8 typing teacher taught 
me. 



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

hubert.reinterpretcast wrote:
> Can we use built-in types and functions in place of a header inclusion?
I'm worried about legal/copyright issues with using contents from AIX system 
headers to LLVM testcases. But I can do that for sure once I understand what I 
am allowed to do.   



Comment at: clang/test/CodeGen/aix-vararg.c:10
+  va_arg(arg, int);
+  va_end(arg);
+}

jasonliu wrote:
> As part of a "va_..." family, do we also want to test va_copy? 
Good suggestion, added. 


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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-19 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 251407.
ZarkoCA marked 3 inline comments as done.
ZarkoCA added a comment.

Changed comments per suggestions. 
Added `va_copy` in test case.


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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c


Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,40 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+
+void aix_varg(int a, ...) {
+  va_list arg;
+  va_start(arg, a);
+  va_list arg2;
+  va_arg(arg, int);
+  va_copy(arg2, arg);
+  va_end(arg);
+  va_end(arg2);
+}
+
+// 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+// 32BIT-NEXT:  entry:
+// 32BIT-NEXT:%a.addr = alloca i32, align 4
+// 32BIT-NEXT:%arg = alloca i8*, align 4
+// 32BIT-NEXT:%arg2 = alloca i8*, align 4
+// 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+// 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+// 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+// 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+// 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+// 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+// 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+// 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+// 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+// 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+// 32BIT-NEXT:ret void
+// 32BIT-NEXT:  }
+// 32BIT: declare void @llvm.va_start(i8*) #1
+// 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+// 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4172,7 +4172,8 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
+/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC 
ELF
+/// (SVR4), Darwin and AIX.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
@@ -4229,7 +4230,10 @@
 // DefaultABIInfo::EmitVAArg.
 Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction , Address VAList,
   QualType Ty) const {
-  if (getTarget().getTriple().isOSDarwin()) {
+  // TODO: Add AIX ABI Info. Currently, we are relying on PPC32_SVR4_ABIInfo to
+  // emit correct VAArg.
+  if (getTarget().getTriple().isOSDarwin() ||
+  getTarget().getTriple().isOSAIX()) {
 auto TI = getContext().getTypeInfoInChars(Ty);
 TI.second = getParamTypeAlignment(Ty);
 
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -369,7 +369,8 @@
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
-// This is the ELF definition, and is overridden by the Darwin sub-target
+// This is the ELF definition, and is overridden by the Darwin and AIX
+// sub-targets.
 return TargetInfo::PowerABIBuiltinVaList;
   }
 };


Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,40 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+#include 
+
+void aix_varg(int a, ...) {
+  va_list arg;
+  va_start(arg, a);
+  va_list arg2;
+  va_arg(arg, int);
+  va_copy(arg2, arg);
+  va_end(arg);
+  va_end(arg2);
+}
+
+// 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+// 32BIT-NEXT:  entry:
+// 32BIT-NEXT:%a.addr = alloca i32, align 4
+// 32BIT-NEXT:%arg = alloca i8*, align 4
+// 32BIT-NEXT:%arg2 = alloca i8*, align 4
+// 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+// 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+// 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+// 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+// 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+// 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+// 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+// 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+// 32BIT-NEXT:%3 = bitcast 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-18 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/test/CodeGen/aix-vararg.c:10
+  va_arg(arg, int);
+  va_end(arg);
+}

As part of a "va_..." family, do we also want to test va_copy? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-18 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232
   QualType Ty) const {
-  if (getTarget().getTriple().isOSDarwin()) {
+  // TODO: Add AIX ABI Info.  Currently we are relying on PPC32_SVR4_ABIInfo to
+  // emit correct VAArg.

No need for two spaces. Add comma after "Currently".



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

Can we use built-in types and functions in place of a header inclusion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-18 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:373
+// This is the ELF definition, and is overridden by the Darwin and AIX
+// sub-target.
 return TargetInfo::PowerABIBuiltinVaList;

Minor nit: `target` --> `targets`.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4175
 namespace {
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {

This name and comment is misleading, the class is used for both SVR4 and 
Darwin, and after this patch AIX. We need to fix the name comment to reflect 
that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-18 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA created this revision.
Herald added subscribers: cfe-commits, kbarton, nemanjai.
Herald added a project: clang.
ZarkoCA added reviewers: jasonliu, sfertile, cebowleratibm.
Herald added a subscriber: wuzish.

This patch contains only the FE changes previously found in 
https://reviews.llvm.org/D76130. 
Currently all PPC32-bit targets take the PPC32SVRABI path, this patch branches 
within that path to consume the va_list emitted for AIX and emit the correct 
VAArg.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c


Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,30 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+
+void aix_varg(int a, ...) {
+  va_list arg;
+  va_start(arg, a);
+  va_arg(arg, int);
+  va_end(arg);
+}
+
+// 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+// 32BIT:   entry:
+// 32BIT-NEXT:%a.addr = alloca i32, align 4
+// 32BIT-NEXT:%arg = alloca i8*, align 4
+// 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+// 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+// 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+// 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, 
i32 4
+// 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+// 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+// 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+// 32BIT-NEXT:%arg2 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg2)
+// 32BIT-NEXT:ret void
+// 32BIT-NEXT:  }
+// 32BIT:declare void @llvm.va_start(i8*)
+// 32BIT:declare void @llvm.va_end(i8*)
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4229,7 +4229,10 @@
 // DefaultABIInfo::EmitVAArg.
 Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction , Address VAList,
   QualType Ty) const {
-  if (getTarget().getTriple().isOSDarwin()) {
+  // TODO: Add AIX ABI Info.  Currently we are relying on PPC32_SVR4_ABIInfo to
+  // emit correct VAArg.
+  if (getTarget().getTriple().isOSDarwin() ||
+  getTarget().getTriple().isOSAIX()) {
 auto TI = getContext().getTypeInfoInChars(Ty);
 TI.second = getParamTypeAlignment(Ty);
 
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -369,7 +369,8 @@
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
-// This is the ELF definition, and is overridden by the Darwin sub-target
+// This is the ELF definition, and is overridden by the Darwin and AIX
+// sub-target.
 return TargetInfo::PowerABIBuiltinVaList;
   }
 };


Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,30 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+#include 
+
+void aix_varg(int a, ...) {
+  va_list arg;
+  va_start(arg, a);
+  va_arg(arg, int);
+  va_end(arg);
+}
+
+// 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+// 32BIT:   entry:
+// 32BIT-NEXT:%a.addr = alloca i32, align 4
+// 32BIT-NEXT:%arg = alloca i8*, align 4
+// 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+// 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+// 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+// 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+// 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+// 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+// 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+// 32BIT-NEXT:%arg2 = bitcast i8** %arg to i8*
+// 32BIT-NEXT:call void @llvm.va_end(i8* %arg2)
+// 32BIT-NEXT:ret void
+// 32BIT-NEXT:  }
+// 32BIT:declare void @llvm.va_start(i8*)
+// 32BIT:declare void @llvm.va_end(i8*)
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4229,7 +4229,10 @@
 // DefaultABIInfo::EmitVAArg.
 Address