[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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 inboun
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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 i
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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
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
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
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
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
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 inbounds
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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 i
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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 &M) 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
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
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
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 &M) 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 &CGF, 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
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
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 &CGT, bool SoftFloatABI) + PowerPC32ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI) : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {} Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, @@ -4190,7 +4191,7 @@ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo { public: PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) - : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {} + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { // This is recovered from gcc output. @@ -4200,9 +4201,22 @@ bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, llvm::Value *Address) const override; }; + +class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo { +public: + PPCAIX32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} + + int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { +return 1; // r1 is the dedicated stack pointer + } + + bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, + 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->getElementTy
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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 &CGT, bool SoftFloatABI) + PowerPC32ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI) : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {} Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, @@ -4190,7 +4191,7 @@ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo { public: PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) - : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {} + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { // This is recovered from gcc output. @@ -4200,9 +4201,22 @@ bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, llvm::Value *Address) const override; }; + +class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo { +public: + PPCAIX32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} + + int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { +return 1; // r1 is the dedicated stack pointer + } + + bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, + 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->
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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
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 &CGT, bool SoftFloatABI) + PowerPC32ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI) : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {} Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, @@ -4190,7 +4191,7 @@ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo { public: PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) - : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {} + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { // This is recovered from gcc output. @@ -4200,9 +4201,22 @@ bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, llvm::Value *Address) const override; }; + +class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo { +public: + PPCAIX32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} + + int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { +return 1; // r1 is the dedicated stack pointer + } + + bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, + 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 (co
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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
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
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 &CGT, bool SoftFloatABI) + PowerPC32ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI) : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {} Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, @@ -4190,7 +4191,7 @@ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo { public: PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) - : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {} + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { // This is recovered from gcc output. @@ -4200,9 +4201,22 @@ bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, llvm::Value *Address) const override; }; + +class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo { +public: + PPCAIX32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} + + int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override { +return 1; // r1 is the dedicated stack pointer + } + + bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, + 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
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
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 &CGF, 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
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
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
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 &CGT, bool SoftFloatABI) + PowerPC32ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI) : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {} Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, @@ -4189,7 +4190,7 @@ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo { public: PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI) - : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {} + : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {} int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) 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 &CGF, Address VAList, +Address PowerPC32ABIInfo::EmitVAArg(CodeGenFunction &CGF, 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 an
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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
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
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 &CGF, 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 = bitca
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
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
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
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
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 &CGF, 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 PPC32_SVR4_ABIInfo::Emit