[PATCH] [ms-cxxabi] Implement member pointer comparisons

[John McCall]

On Apr 19, 2013, at 11:51 AM, Reid Kleckner <rnk at google.com> wrote:
> Like Itanium, comparisons are basically bitwise comparisons of the two
> values, with an exception for null member function pointers.  If two
> function pointers are null, only the function pointer field matters for
> comparison purposes.  The rest of the bits can be arbitrary.  We take
> advantage of this in isZeroInitializable(), and it may matter once we
> start emitting conversions.
> 
> http://llvm-reviews.chandlerc.com/D695
> 
> Files:
>  lib/CodeGen/MicrosoftCXXABI.cpp
>  test/CodeGenCXX/microsoft-abi-member-pointers.cpp
> 
> Index: lib/CodeGen/MicrosoftCXXABI.cpp
> ===================================================================
> --- lib/CodeGen/MicrosoftCXXABI.cpp
> +++ lib/CodeGen/MicrosoftCXXABI.cpp
> @@ -149,6 +149,12 @@
>   virtual llvm::Constant *EmitMemberDataPointer(const MemberPointerType *MPT,
>                                                 CharUnits offset);
> 
> +  virtual llvm::Value *EmitMemberPointerComparison(CodeGenFunction &CGF,
> +                                                   llvm::Value *L,
> +                                                   llvm::Value *R,
> +                                                   const MemberPointerType *MPT,
> +                                                   bool Inequality);
> +
>   virtual llvm::Value *EmitMemberPointerIsNotNull(CodeGenFunction &CGF,
>                                                   llvm::Value *MemPtr,
>                                                   const MemberPointerType *MPT);
> @@ -415,6 +421,10 @@
>   return Inheritance == MSIM_Unspecified;
> }
> 
> +static bool hasOnlyOneField(MSInheritanceModel Inheritance) {
> +  return Inheritance <= MSIM_SinglePolymorphic;
> +}
> +
> // Only member pointers to functions need a this adjustment, since it can be
> // combined with the field offset for data pointers.
> static bool hasNonVirtualBaseAdjustmentField(const MemberPointerType *MPT,
> @@ -531,6 +541,66 @@
>   return llvm::ConstantStruct::getAnon(fields);
> }
> 
> +/// Member pointers are the same if they're either bitwise identical *or* both
> +/// null.  Null-ness for function members is determined by the first field,
> +/// while for data member pointers we must compare all fields.
> +llvm::Value *
> +MicrosoftCXXABI::EmitMemberPointerComparison(CodeGenFunction &CGF,
> +                                             llvm::Value *L,
> +                                             llvm::Value *R,
> +                                             const MemberPointerType *MPT,
> +                                             bool Inequality) {
> +  CGBuilderTy &Builder = CGF.Builder;
> +  llvm::ICmpInst::Predicate Eq;
> +  llvm::Instruction::BinaryOps And, Or;
> +  if (Inequality) {
> +    Eq = llvm::ICmpInst::ICMP_NE;
> +    And = llvm::Instruction::Or;
> +    Or = llvm::Instruction::And;
> +  } else {
> +    Eq = llvm::ICmpInst::ICMP_EQ;
> +    And = llvm::Instruction::And;
> +    Or = llvm::Instruction::Or;
> +  }

Please leave a comment explaining what you're doing here.

> +  // If this is a single field member pointer (single inheritance), this is a
> +  // single icmp.
> +  const CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl();
> +  MSInheritanceModel Inheritance = RD->getMSInheritanceModel();
> +  if (hasOnlyOneField(Inheritance))
> +    return Builder.CreateICmp(Eq, L, R);
> +
> +  // Compare the first field.
> +  llvm::Value *L0 = Builder.CreateExtractValue(L, 0, "lhs.0");
> +  llvm::Value *R0 = Builder.CreateExtractValue(R, 0, "rhs.0");
> +  llvm::Value *Cmp0 = Builder.CreateICmp(Eq, L0, R0);
> +
> +  // Compare everything other than the first field.
> +  llvm::Value *Res = 0;
> +  for (unsigned I = 1, E = cast<llvm::StructType>(L->getType())->getNumElements();
> +       I != E; ++I) {
> +    llvm::Value *LField = Builder.CreateExtractValue(L, I);
> +    llvm::Value *RField = Builder.CreateExtractValue(R, I);
> +    llvm::Value *Cmp = Builder.CreateICmp(Eq, LField, RField);
> +    if (Res)
> +      Res = Builder.CreateBinOp(And, Res, Cmp, "memptr.tobool");

The value labels you're using here seem to be for a different operation.

Otherwise, this all seems to look good.

John.