Eli,
Why is it generating lower-quality code? What kind of resolution do
you think we could come to on this issue?
For PNaCl, we lower first-class struct passing in a target-specific
way (in llc) so that we can approximately (but not exactly) match the
platform ABIs. We'd prefer full ABI compatibility, but we're willing
to live with partial compatibility for now.
Attached is a new patch. Also note, I don't (yet?) have commit access.
Thanks,
- pdox
On Tue, Sep 20, 2011 at 2:03 PM, Eli Friedman <[email protected]> wrote:
> On Tue, Sep 20, 2011 at 10:09 AM, David Meyer <[email protected]> wrote:
>> Ping! Anybody available to review?
>
> I'm not really sure the ABIArgInfo::Abstract stuff is really
> appropriate, in that it is likely to generate low-quality code as-is.
> I was sort of waiting for a resolution to that discussion on cfe-dev.
>
> I don't see any issues with the patch otherwise, though.
>
> + if (!DestPtr) {
> + DestPtr = CreateMemTemp(RetTy, "agg.tmp");
> + }
>
> Nit: LLVM style is no braces here.
>
> -Eli
>
Index: test/CodeGen/pnacl-arguments.c
===================================================================
--- test/CodeGen/pnacl-arguments.c (revision 0)
+++ test/CodeGen/pnacl-arguments.c (revision 0)
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -w -fblocks -triple le32-unknown-nacl -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+// CHECK: %struct.foo = type { i8 }
+struct foo { char x; };
+
+// CHECK: define signext i8 @f100(%struct.foo %a.value)
+char f100(struct foo a) {
+ return a.x;
+}
+
+char f200() {
+ struct foo a = { 'a' };
+// CHECK: %call = call signext i8 @f100(%struct.foo %0)
+// CHECK: ret i8 %call
+ return f100(a);
+}
Index: lib/CodeGen/ABIInfo.h
===================================================================
--- lib/CodeGen/ABIInfo.h (revision 140189)
+++ lib/CodeGen/ABIInfo.h (working copy)
@@ -63,7 +63,12 @@
/// are all scalar types or are themselves expandable types.
Expand,
- KindFirst=Direct, KindLast=Expand
+ /// Abstract - Pass each argument (including aggregates) as a
+ /// single argument, using the LLVM aggregate type. For returns,
+ /// aggregate arguments are returned directly, without using sret.
+ Abstract,
+
+ KindFirst=Direct, KindLast=Abstract
};
private:
@@ -96,6 +101,9 @@
static ABIArgInfo getExpand() {
return ABIArgInfo(Expand);
}
+ static ABIArgInfo getAbstract() {
+ return ABIArgInfo(Abstract);
+ }
Kind getKind() const { return TheKind; }
bool isDirect() const { return TheKind == Direct; }
@@ -103,6 +111,7 @@
bool isIgnore() const { return TheKind == Ignore; }
bool isIndirect() const { return TheKind == Indirect; }
bool isExpand() const { return TheKind == Expand; }
+ bool isAbstract() const { return TheKind == Abstract; }
bool canHaveCoerceToType() const {
return TheKind == Direct || TheKind == Extend;
Index: lib/CodeGen/TargetInfo.cpp
===================================================================
--- lib/CodeGen/TargetInfo.cpp (revision 140189)
+++ lib/CodeGen/TargetInfo.cpp (working copy)
@@ -81,6 +81,9 @@
case Expand:
OS << "Expand";
break;
+ case Abstract:
+ OS << "Abstract";
+ break;
}
OS << ")\n";
}
@@ -3155,7 +3158,68 @@
return false;
}
+//===----------------------------------------------------------------------===//
+// PNaCl ABI Implementation.
+//===----------------------------------------------------------------------===//
+/// PNaClABIInfo - The PNaCl implementation for ABI specific details.
+class PNaClABIInfo : public ABIInfo {
+public:
+ PNaClABIInfo(CodeGen::CodeGenTypes &CGT) : ABIInfo(CGT) {}
+
+ ABIArgInfo classifyReturnType(QualType RetTy) const;
+ ABIArgInfo classifyArgumentType(QualType RetTy) const;
+
+ virtual void computeInfo(CGFunctionInfo &FI) const {
+ FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+ for (CGFunctionInfo::arg_iterator it = FI.arg_begin(), ie = FI.arg_end();
+ it != ie; ++it)
+ it->info = classifyArgumentType(it->type);
+ }
+
+ virtual llvm::Value *EmitVAArg(llvm::Value *VAListAddr, QualType Ty,
+ CodeGenFunction &CGF) const;
+};
+
+class PNaClTargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+ PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
+ : TargetCodeGenInfo(new PNaClABIInfo(CGT)) {}
+};
+
+llvm::Value *PNaClABIInfo::EmitVAArg(llvm::Value *VAListAddr, QualType Ty,
+ CodeGenFunction &CGF) const {
+ return 0;
+}
+
+ABIArgInfo PNaClABIInfo::classifyArgumentType(QualType Ty) const {
+ if (isAggregateTypeForABI(Ty))
+ return ABIArgInfo::getAbstract();
+
+ // Treat an enum type as its underlying type.
+ if (const EnumType *EnumTy = Ty->getAs<EnumType>())
+ Ty = EnumTy->getDecl()->getIntegerType();
+
+ return (Ty->isPromotableIntegerType() ?
+ ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+}
+
+ABIArgInfo PNaClABIInfo::classifyReturnType(QualType RetTy) const {
+ if (RetTy->isVoidType())
+ return ABIArgInfo::getIgnore();
+
+ if (isAggregateTypeForABI(RetTy))
+ return ABIArgInfo::getIndirect(0);
+
+ // Treat an enum type as its underlying type.
+ if (const EnumType *EnumTy = RetTy->getAs<EnumType>())
+ RetTy = EnumTy->getDecl()->getIntegerType();
+
+ return (RetTy->isPromotableIntegerType() ?
+ ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+}
+
+
const TargetCodeGenInfo &CodeGenModule::getTargetCodeGenInfo() {
if (TheTargetCodeGenInfo)
return *TheTargetCodeGenInfo;
@@ -3199,6 +3263,9 @@
case llvm::Triple::systemz:
return *(TheTargetCodeGenInfo = new SystemZTargetCodeGenInfo(Types));
+ case llvm::Triple::le32:
+ return *(TheTargetCodeGenInfo = new PNaClTargetCodeGenInfo(Types));
+
case llvm::Triple::mblaze:
return *(TheTargetCodeGenInfo = new MBlazeTargetCodeGenInfo(Types));
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp (revision 140189)
+++ lib/CodeGen/CGCall.cpp (working copy)
@@ -656,6 +656,10 @@
case ABIArgInfo::Ignore:
resultType = llvm::Type::getVoidTy(getLLVMContext());
break;
+
+ case ABIArgInfo::Abstract:
+ resultType = ConvertType(FI.getReturnType());
+ break;
}
for (CGFunctionInfo::const_arg_iterator it = FI.arg_begin(),
@@ -691,6 +695,10 @@
case ABIArgInfo::Expand:
GetExpandedTypes(it->type, argTypes);
break;
+
+ case ABIArgInfo::Abstract:
+ argTypes.push_back(ConvertType(it->type));
+ break;
}
}
@@ -771,6 +779,7 @@
break;
case ABIArgInfo::Direct:
case ABIArgInfo::Ignore:
+ case ABIArgInfo::Abstract:
break;
case ABIArgInfo::Indirect:
@@ -843,6 +852,10 @@
// Skip increment, no matching LLVM parameter.
continue;
+ case ABIArgInfo::Abstract:
+ // No attributes
+ break;
+
case ABIArgInfo::Expand: {
SmallVector<llvm::Type*, 8> types;
// FIXME: This is rather inefficient. Do we ever actually need to do
@@ -1064,7 +1077,21 @@
// Skip increment, no matching LLVM parameter.
continue;
+
+ case ABIArgInfo::Abstract: {
+ assert(AI != Fn->arg_end() && "Argument mismatch!");
+ llvm::Value *V = AI;
+ if (hasAggregateLLVMType(Ty)) {
+ AI->setName(Arg->getName() + ".value");
+ llvm::Value *Alloca = CreateMemTemp(Ty);
+ Builder.CreateStore(V, Alloca);
+ EmitParmDecl(*Arg, Alloca, ArgNo);
+ } else {
+ EmitParmDecl(*Arg, V, ArgNo);
+ }
+ break;
}
+ }
++AI;
}
@@ -1189,8 +1216,10 @@
case ABIArgInfo::Extend:
case ABIArgInfo::Direct:
- if (RetAI.getCoerceToType() == ConvertType(RetTy) &&
- RetAI.getDirectOffset() == 0) {
+ case ABIArgInfo::Abstract:
+ if (RetAI.getKind() == ABIArgInfo::Abstract ||
+ (RetAI.getCoerceToType() == ConvertType(RetTy) &&
+ RetAI.getDirectOffset() == 0)) {
// The internal return value temp always will have pointer-to-return-type
// type, just do a load.
@@ -1632,6 +1661,20 @@
case ABIArgInfo::Ignore:
break;
+ case ABIArgInfo::Abstract: {
+ llvm::Value *V;
+ if (RV.isScalar())
+ V = RV.getScalarVal();
+ else if (RV.isAggregate())
+ V = Builder.CreateLoad(RV.getAggregateAddr());
+ else
+ assert(0 && "Unhandled RValue for Abstract ArgInfo");
+
+ Args.push_back(V);
+ checkArgMatches(V, IRArgNo, IRFuncTy);
+ break;
+ }
+
case ABIArgInfo::Extend:
case ABIArgInfo::Direct: {
if (!isa<llvm::StructType>(ArgInfo.getCoerceToType()) &&
@@ -1804,6 +1847,19 @@
// construct the appropriate return value for our caller.
return GetUndefRValue(RetTy);
+ case ABIArgInfo::Abstract: {
+ llvm::Value *V = CI;
+ if (CodeGenFunction::hasAggregateLLVMType(RetTy)) {
+ llvm::Value *DestPtr = ReturnValue.getValue();
+ if (!DestPtr)
+ DestPtr = CreateMemTemp(RetTy, "agg.tmp");
+ Builder.CreateStore(V, DestPtr);
+ return RValue::getAggregate(DestPtr);
+ } else {
+ return RValue::get(V);
+ }
+ }
+
case ABIArgInfo::Extend:
case ABIArgInfo::Direct: {
llvm::Type *RetIRTy = ConvertType(RetTy);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
