ZarkoCA added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4368
+
+ return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false, TypeInfo,
+ SlotSize, /*AllowHigher*/ true);
----------------
Is there a reason why Indirect is set to `false` instead of querying for it
using `classifyArgumentType(Ty).isIndirect()`?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4641
return false;
case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return
return true;
----------------
Has this option been verified to work correctly on AIX? In
https://reviews.llvm.org/D76360 we added a defensive error because we weren't
sure whether padding was handled correctly as described in the code.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4654
llvm::Value *Address) const {
- // This is calculated from the LLVM and GCC tables and verified
- // against gcc output. AFAIK all ABIs use the same encoding.
-
- CodeGen::CGBuilderTy &Builder = CGF.Builder;
-
- llvm::IntegerType *i8 = CGF.Int8Ty;
- llvm::Value *Four8 = llvm::ConstantInt::get(i8, 4);
- llvm::Value *Eight8 = llvm::ConstantInt::get(i8, 8);
- llvm::Value *Sixteen8 = llvm::ConstantInt::get(i8, 16);
-
- // 0-31: r0-31, the 4-byte general-purpose registers
- AssignToArrayRange(Builder, Address, Four8, 0, 31);
-
- // 32-63: fp0-31, the 8-byte floating-point registers
- AssignToArrayRange(Builder, Address, Eight8, 32, 63);
-
- // 64-76 are various 4-byte special-purpose registers:
- // 64: mq
- // 65: lr
- // 66: ctr
- // 67: ap
- // 68-75 cr0-7
- // 76: xer
- AssignToArrayRange(Builder, Address, Four8, 64, 76);
-
- // 77-108: v0-31, the 16-byte vector registers
- AssignToArrayRange(Builder, Address, Sixteen8, 77, 108);
-
- // 109: vrsave
- // 110: vscr
- // 111: spe_acc
- // 112: spefscr
- // 113: sfp
- AssignToArrayRange(Builder, Address, Four8, 109, 113);
-
- return false;
+ return PPC_initDwarfEHRegSizeTable(CGF, Address, /* Is64Bit*/ false,
+ /*IsAIX*/ false);
----------------
Should be changed to `/*Is64BIT*/`? Or you can add spaces to`/*IsAIX*/` so that
it's consistent.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:5195
+ return PPC_initDwarfEHRegSizeTable(CGF, Address, /* Is64Bit*/ true,
+ /*IsAIX*/ false);
}
----------------
Remove extra space from `/* Is64Bit*/`
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10454
+ if (Triple.isOSAIX())
+ return SetCGInfo(new AIXTargetCodeGenInfo(Types, /* Is64Bit */ false));
+
----------------
same as above about removing extra spaces.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10465
+ if (Triple.isOSAIX())
+ return SetCGInfo(new AIXTargetCodeGenInfo(Types, /* Is64Bit */ true));
+
----------------
spaces
================
Comment at: clang/test/CodeGen/aix-return.c:3
+// RUN: %clang_cc1 -triple powerpc-unknown-aix \
+// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefixes=AIX-COM,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix \
----------------
I think it would be simpler to just use `AIX` instead of `AIX-COM`.
================
Comment at: clang/test/CodeGen/aix-struct-arg.c:3
+// RUN: %clang_cc1 -triple powerpc-unknown-aix \
+// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix \
----------------
Same here, I think it be simpler to use either `CHECK` or `AIX`. My preference
is to use `CHECK` since other targets aren't being tested here but I would
leave it up to you.
================
Comment at: clang/test/CodeGen/aix-vaargs.c:3
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | FileCheck
%s --check-prefixes=AIX-COM,AIX-M32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s |
FileCheck %s --check-prefixes=AIX-COM,AIX-M64
----------------
jasonliu wrote:
> Xiangling_L wrote:
> > Consistent with other testcases to use `AIX32/AIX64`?
> I chose AIX-M32/AIX-M64 mainly because the length is the same as AIX-COM so
> we don't need to worry about aligning the space. I would prefer to keep it
> that.
I agree with Xiangling, it would be better to be consistent with the other
testcases. To get things to line up properly you can use `AIX32/AIX64` and
`CHECK` instead of `AIX-COM`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
