tingwang updated this revision to Diff 458721. tingwang added a comment. Update according to comments: (1) The argument type is not touched. (2) Add flag in APIs to force right-adjust the parameter for this issue. (3) One change on if check logic: now use CoerceTy->getIntegerBitWidth() directly compare with GPRBits, previously used llvm::alignTo(CoerceTy->getIntegerBitWidth(), 8). I think `llvm::alignTo()` is redundant here. (4) Created NFC patch to pre-commit the test case.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133338/new/ https://reviews.llvm.org/D133338 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGen/PowerPC/ppc64-align-struct.c
Index: clang/test/CodeGen/PowerPC/ppc64-align-struct.c =================================================================== --- clang/test/CodeGen/PowerPC/ppc64-align-struct.c +++ clang/test/CodeGen/PowerPC/ppc64-align-struct.c @@ -129,15 +129,15 @@ return y; } -// Error pattern will be fixed in https://reviews.llvm.org/D133338 // CHECK: define{{.*}} void @test8va(%struct.test8* noalias sret(%struct.test8) align 1 %[[AGG_RESULT:.*]], i32 noundef signext %x, ...) // CHECK: %[[CUR:[^ ]+]] = load i8*, i8** %ap // CHECK: %[[NEXT:[^ ]+]] = getelementptr inbounds i8, i8* %[[CUR]], i64 8 // CHECK: store i8* %[[NEXT]], i8** %ap -// CHECK: [[T0:%.*]] = bitcast i8* %[[CUR]] to %struct.test8* +// CHECK: [[T0:%.*]] = getelementptr inbounds i8, i8* %[[CUR]], i64 7 +// CHECK: [[T1:%.*]] = bitcast i8* [[T0]] to %struct.test8* // CHECK: [[DEST:%.*]] = bitcast %struct.test8* %[[AGG_RESULT]] to i8* -// CHECK: [[SRC:%.*]] = bitcast %struct.test8* [[T0]] to i8* -// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[DEST]], i8* align 8 [[SRC]], i64 1, i1 false) +// CHECK: [[SRC:%.*]] = bitcast %struct.test8* [[T1]] to i8* +// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[DEST]], i8* align 1 [[SRC]], i64 1, i1 false) struct test8 test8va (int x, ...) { struct test8 y; Index: clang/lib/CodeGen/TargetInfo.cpp =================================================================== --- clang/lib/CodeGen/TargetInfo.cpp +++ clang/lib/CodeGen/TargetInfo.cpp @@ -322,13 +322,17 @@ /// leaving one or more empty slots behind as padding. If this /// is false, the returned address might be less-aligned than /// DirectAlign. +/// \param ForceRightAdjust - Default is false. On big-endian platform and +/// if the argument is smaller than a slot, set this flag will force +/// right-adjust the argument in its slot irrespective of the type. static Address emitVoidPtrDirectVAArg(CodeGenFunction &CGF, Address VAListAddr, llvm::Type *DirectTy, CharUnits DirectSize, CharUnits DirectAlign, CharUnits SlotSize, - bool AllowHigherAlign) { + bool AllowHigherAlign, + bool ForceRightAdjust = false) { // Cast the element type to i8* if necessary. Some platforms define // va_list as a struct containing an i8* instead of just an i8*. if (VAListAddr.getElementType() != CGF.Int8PtrTy) @@ -354,7 +358,7 @@ // If the argument is smaller than a slot, and this is a big-endian // target, the argument will be right-adjusted in its slot. if (DirectSize < SlotSize && CGF.CGM.getDataLayout().isBigEndian() && - !DirectTy->isStructTy()) { + (!DirectTy->isStructTy() || ForceRightAdjust)) { Addr = CGF.Builder.CreateConstInBoundsByteGEP(Addr, SlotSize - DirectSize); } @@ -375,11 +379,15 @@ /// an argument type with an alignment greater than the slot size /// will be emitted on a higher-alignment address, potentially /// leaving one or more empty slots behind as padding. +/// \param ForceRightAdjust - Default is false. On big-endian platform and +/// if the argument is smaller than a slot, set this flag will force +/// right-adjust the argument in its slot irrespective of the type. static Address emitVoidPtrVAArg(CodeGenFunction &CGF, Address VAListAddr, QualType ValueTy, bool IsIndirect, TypeInfoChars ValueInfo, CharUnits SlotSizeAndAlign, - bool AllowHigherAlign) { + bool AllowHigherAlign, + bool ForceRightAdjust = false) { // The size and alignment of the value that was passed directly. CharUnits DirectSize, DirectAlign; if (IsIndirect) { @@ -395,9 +403,9 @@ if (IsIndirect) DirectTy = DirectTy->getPointerTo(0); - Address Addr = - emitVoidPtrDirectVAArg(CGF, VAListAddr, DirectTy, DirectSize, DirectAlign, - SlotSizeAndAlign, AllowHigherAlign); + Address Addr = emitVoidPtrDirectVAArg(CGF, VAListAddr, DirectTy, DirectSize, + DirectAlign, SlotSizeAndAlign, + AllowHigherAlign, ForceRightAdjust); if (IsIndirect) { Addr = Address(CGF.Builder.CreateLoad(Addr), ElementTy, ValueInfo.Align); @@ -5451,9 +5459,21 @@ return complexTempStructure(CGF, VAListAddr, Ty, SlotSize, EltSize, CTy); } + // On big-endian, need to check that an aggregate may end up coerced to + // integer type in single register. When DirectSize is less than SlotSize, set + // force right-adjust flag. + bool ForceRightAdjust = false; + if (CGF.CGM.getDataLayout().isBigEndian()) { + ABIArgInfo AI = classifyArgumentType(Ty); + if (AI.isDirect()) + if (llvm::Type *CoerceTy = AI.getCoerceToType()) + if (CoerceTy->isIntegerTy() && CoerceTy->getIntegerBitWidth() < GPRBits) + ForceRightAdjust = true; + } + // Otherwise, just use the general rule. - return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false, - TypeInfo, SlotSize, /*AllowHigher*/ true); + return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false, TypeInfo, + SlotSize, /*AllowHigher*/ true, ForceRightAdjust); } bool
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
