Implement codegen for MSVC unions with reference members

[domdom]

Summary

Currently, clang accepts a union with a reference member when given the -fms-extensions flag. This change fixes the codegen for this case.

[aaron.ballman]

This looks reasonable to me, but @rnk and @majnemer may have opinions as well.

clang/test/CodeGenCXX/ms-union-member-ref.cpp

4–7

Can you clang-format the patch?

[domdom]

clang-formatted the test.

[domdom]

Ping!

Cheers,
Dom

[aaron.ballman]

LGTM, thank you! I think this is actually needed to properly support MFC on Windows, IIRC.

[domdom]

Thanks @aaron.ballman!

I will need someone to commit this for me :)

Cheers,
Dom

[aaron.ballman]

In D62571#1642096, @domdom wrote:

Thanks @aaron.ballman!

I will need someone to commit this for me :)

I'm happy to commit for you, but I get merge conflicts when trying to apply your patch. Can you rebase on trunk?

[domdom]

Rebased onto master, clang format the patch.

Merge conflict resolve by having the bitcast of the field reference happening after recording access index.

[aaron.ballman]

In D62571#1646227, @domdom wrote:

Rebased onto master, clang format the patch.

Merge conflict resolve by having the bitcast of the field reference happening after recording access index.

Thanks! I've committed in r370052.

clang/lib/CodeGen/CGExpr.cpp

if (CGM.getCodeGenOpts().StrictVTablePointers &&
       auto *stripped = Builder.CreateStripInvariantGroup(addr.getPointer());
       addr = Address(stripped, addr.getAlignment());
     }
   }
 
   unsigned RecordCVR = base.getVRQualifiers();
   if (rec->isUnion()) {
     // For unions, there is no pointer adjustment.
-    assert(!FieldType->isReferenceType() && "union has reference member");
     if (CGM.getCodeGenOpts().StrictVTablePointers &&
         hasAnyVptr(FieldType, getContext()))
       // Because unions can easily skip invariant.barriers, we need to add
       // a barrier every time CXXRecord field with vptr is referenced.
       addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
                      addr.getAlignment());
 
     if (IsInPreservedAIRegion) {
       // Remember the original union field index
       llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType(
           getContext().getRecordType(rec), rec->getLocation());
       addr = Address(
           Builder.CreatePreserveUnionAccessIndex(
               addr.getPointer(), getDebugInfoFIndex(rec, field->getFieldIndex()), DbgInfo),
           addr.getAlignment());
     }
-  } else {
 
+    if (FieldType->isReferenceType())
+      addr = Builder.CreateElementBitCast(
+          addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
+  } else {
     if (!IsInPreservedAIRegion)
       // For structs, we GEP to the field that the record layout suggests.
       addr = emitAddrOfFieldStorage(*this, addr, field);
     else
       // Remember the original struct field index
       addr = emitPreserveStructAccess(*this, addr, field);
+  }
 
-    // If this is a reference field, load the reference right now.
-    if (FieldType->isReferenceType()) {
-      LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
-                                      FieldTBAAInfo);
-      if (RecordCVR & Qualifiers::Volatile)
-        RefLVal.getQuals().addVolatile();
-      addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
+    LValue RefLVal =
+        MakeAddrLValue(addr, FieldType, FieldBaseInfo, FieldTBAAInfo);
 
-      // Qualifiers on the struct don't apply to the referencee.
-      RecordCVR = 0;
-      FieldType = FieldType->getPointeeType();
-    }
-  }
 
   // Make sure that the address is pointing to the right type.  This is critical
   // for both unions and structs.  A union needs a bitcast, a struct element
   // will need a bitcast if the LLVM type laid out doesn't match the desired
   // type.
   addr = Builder.CreateElementBitCast(
       addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
 

clang/test/CodeGenCXX/ms-union-member-ref.cpp

+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *&ref;
+  int **ptr;
+};
+
+int *f1(A *a) {
+  return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:       [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:       [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:       [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:       ret i32* [[IP]]
+
+void f2(A *a) {
+  *a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:       [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:       [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:       [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:       store i32 1, i32* [[IP]]
+
+bool f3(A *a, int *b) {
+  return a->ref != b;
+}
+// CHECK-LABEL: define {{.*}}i1 @_Z2f3P1APi(%union.A* %a, i32* %b)
+// CHECK:       [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:       [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:       [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:       [[IP2:%[^[:space:]]+]]  = load i32*, i32** %b.addr
+// CHECK:       icmp ne i32* [[IP]], [[IP2]]