rnk added inline comments.
> erichkeane wrote in ItaniumMangle.cpp:1236-1237
> Right, good catch. I looked at Mangle.cpp which does something very similar,
> and assumes that FunctionType is a valid cast here, so I've switched this
> here too, please let me know if that is a wrong assumption.
IMO this would be simpler if we did:
if (IsRegCall)
mangleRegCallName(II); // implement this elsewhere
else
mangleSourceName(II);
I don't like that the standard mangleSourceName as you have it has to be
concerned with IsRegCall. Without your change, it is a simple function that
clearly expresses that the basic unit of Itanium name mangling is decimal
length prefixed strings.
> ItaniumMangle.cpp:1203
> switch (Name.getNameKind()) {
> case DeclarationName::Identifier: {
> const IdentifierInfo *II = Name.getAsIdentifierInfo();
What mangling should happen for operator overloads and all other kinds of
DeclarationName? Please add tests for these cases
> TargetInfo.cpp:1954
> + ABIArgInfo classifyRegCallStructTypeImpl(QualType Ty,
> + unsigned &neededInt,
> + unsigned &neededSSE) const;
format
> TargetInfo.cpp:3297
> +ABIArgInfo X86_64ABIInfo::classifyRegCallStructTypeImpl(
> + QualType Ty, unsigned &neededInt, unsigned &neededSSE) const {
> + auto RT = Ty->getAs<RecordType>();
Please name variables in LLVM style
> TargetInfo.cpp:3321
> + } else {
> + unsigned localNeededInt, localNeededSSE;
> + if (classifyArgumentType(FD->getType(),
> std::numeric_limits<unsigned>::max(),
variable naming
> TargetInfo.cpp:3352-3354
> + unsigned freeIntRegs = IsRegCall ? 11 : 6;
> + unsigned freeSSERegs = IsRegCall ? 16 : 8;
> + unsigned neededInt, neededSSE;
Since you're touching most of this, can you make this code standardize on
LLVM's variable naming convention:
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> erichkeane wrote in TargetInfo.cpp:3306
> Hmmm... I'm not sure what behavior would be expected in that case. Also, I
> just looked at a bunch of similar 'for' loops for various other reasons, and
> I don't really see where that is tested elsewhere (so I don't really see how
> to, besides I.isVirtual?). In that case, I would suspect that the class is
> not put into a register?
You don't need to check if each base is dynamic, just check
CXXRD->isDynamicClass() and make it return indirectly if so.
https://reviews.llvm.org/D25204
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
