Re: macppc clang: fix va_arg in Objective-C
> Date: Mon, 16 Nov 2020 15:59:13 -0500 > From: George Koehler > > On Wed, 4 Nov 2020 00:21:15 -0500 > George Koehler wrote: > > > Hello tech list, > > > > clang for 32-bit powerpc has a bug that breaks va_arg(3) when the > > argument type is an object or block in Objective-C. This breaks > > GNUstep on macppc. This clang diff fixes GNUstep. Objective-C uses > > pointers to represent objects and blocks, so this diff tells clang's > > va_arg to handle things with pointer representations as pointers. > > > > Anthony Richardby found and reported the bug, > > https://bugs.llvm.org/show_bug.cgi?id=47921 > > I want this diff (also at https://reviews.llvm.org/D90329). The first > change (isInt) fixes GNUstep, in a simpler way than my diff from 2 weeks > ago. The second change (isIndirect) fixes va_arg(3) with some C++ types. > The diff only affects 32-bit powerpc. > > ok to commit? Looks reasonable to me; ok kettenis@ > Index: clang/lib/CodeGen/TargetInfo.cpp > === > RCS file: /cvs/src/gnu/llvm/clang/lib/CodeGen/TargetInfo.cpp,v > retrieving revision 1.1.1.2 > diff -u -p -r1.1.1.2 TargetInfo.cpp > --- clang/lib/CodeGen/TargetInfo.cpp 9 Aug 2020 15:51:11 - 1.1.1.2 > +++ clang/lib/CodeGen/TargetInfo.cpp 10 Nov 2020 11:41:53 - > @@ -4248,13 +4248,12 @@ Address PPC32_SVR4_ABIInfo::EmitVAArg(Co >// }; > >bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; > - bool isInt = > - Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType(); > + bool isInt = !Ty->isFloatingType(); >bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; > >// All aggregates are passed indirectly? That doesn't seem consistent >// with the argument-lowering code. > - bool isIndirect = Ty->isAggregateType(); > + bool isIndirect = isAggregateTypeForABI(Ty); > >CGBuilderTy &Builder = CGF.Builder; > > >
Re: macppc clang: fix va_arg in Objective-C
On Wed, 4 Nov 2020 00:21:15 -0500 George Koehler wrote: > Hello tech list, > > clang for 32-bit powerpc has a bug that breaks va_arg(3) when the > argument type is an object or block in Objective-C. This breaks > GNUstep on macppc. This clang diff fixes GNUstep. Objective-C uses > pointers to represent objects and blocks, so this diff tells clang's > va_arg to handle things with pointer representations as pointers. > > Anthony Richardby found and reported the bug, > https://bugs.llvm.org/show_bug.cgi?id=47921 I want this diff (also at https://reviews.llvm.org/D90329). The first change (isInt) fixes GNUstep, in a simpler way than my diff from 2 weeks ago. The second change (isIndirect) fixes va_arg(3) with some C++ types. The diff only affects 32-bit powerpc. ok to commit? Index: clang/lib/CodeGen/TargetInfo.cpp === RCS file: /cvs/src/gnu/llvm/clang/lib/CodeGen/TargetInfo.cpp,v retrieving revision 1.1.1.2 diff -u -p -r1.1.1.2 TargetInfo.cpp --- clang/lib/CodeGen/TargetInfo.cpp9 Aug 2020 15:51:11 - 1.1.1.2 +++ clang/lib/CodeGen/TargetInfo.cpp10 Nov 2020 11:41:53 - @@ -4248,13 +4248,12 @@ Address PPC32_SVR4_ABIInfo::EmitVAArg(Co // }; bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; - bool isInt = - Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType(); + bool isInt = !Ty->isFloatingType(); bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; // All aggregates are passed indirectly? That doesn't seem consistent // with the argument-lowering code. - bool isIndirect = Ty->isAggregateType(); + bool isIndirect = isAggregateTypeForABI(Ty); CGBuilderTy &Builder = CGF.Builder;
Re: macppc clang: fix va_arg in Objective-C
On Wed, 4 Nov 2020 00:21:15 -0500 George Koehler wrote: > I posted this diff at https://reviews.llvm.org/D90329 ... > > ok to commit? --George No, I withdraw my diff. In about a week, I might have a better diff that also fixes va_arg with some C++ types. This only affects 32-bit powerpc; nothing will change on powerpc64 or elsewhere. --George > Index: clang/lib/CodeGen/TargetInfo.cpp > === > RCS file: /cvs/src/gnu/llvm/clang/lib/CodeGen/TargetInfo.cpp,v > retrieving revision 1.1.1.2 > diff -u -p -r1.1.1.2 TargetInfo.cpp > --- clang/lib/CodeGen/TargetInfo.cpp 9 Aug 2020 15:51:11 - 1.1.1.2 > +++ clang/lib/CodeGen/TargetInfo.cpp 28 Oct 2020 23:43:54 - > @@ -4248,8 +4248,8 @@ Address PPC32_SVR4_ABIInfo::EmitVAArg(Co >// }; > >bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; > - bool isInt = > - Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType(); > + bool isInt = Ty->isIntegerType() || Ty->hasPointerRepresentation() || > + Ty->isAggregateType(); >bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; > >// All aggregates are passed indirectly? That doesn't seem consistent -- George Koehler
macppc clang: fix va_arg in Objective-C
Hello tech list, clang for 32-bit powerpc has a bug that breaks va_arg(3) when the argument type is an object or block in Objective-C. This breaks GNUstep on macppc. This clang diff fixes GNUstep. Objective-C uses pointers to represent objects and blocks, so this diff tells clang's va_arg to handle things with pointer representations as pointers. Anthony Richardby found and reported the bug, https://bugs.llvm.org/show_bug.cgi?id=47921 I posted this diff at https://reviews.llvm.org/D90329 but upstream llvm might do something else. I used a clang with this diff to rebuild src and xenocara on macppc. ok to commit? --George Index: clang/lib/CodeGen/TargetInfo.cpp === RCS file: /cvs/src/gnu/llvm/clang/lib/CodeGen/TargetInfo.cpp,v retrieving revision 1.1.1.2 diff -u -p -r1.1.1.2 TargetInfo.cpp --- clang/lib/CodeGen/TargetInfo.cpp9 Aug 2020 15:51:11 - 1.1.1.2 +++ clang/lib/CodeGen/TargetInfo.cpp28 Oct 2020 23:43:54 - @@ -4248,8 +4248,8 @@ Address PPC32_SVR4_ABIInfo::EmitVAArg(Co // }; bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; - bool isInt = - Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType(); + bool isInt = Ty->isIntegerType() || Ty->hasPointerRepresentation() || + Ty->isAggregateType(); bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; // All aggregates are passed indirectly? That doesn't seem consistent
