subject:"\[PATCH\] D62571\: Implement codegen for MSVC unions with reference members"

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-08-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman closed this revision.
aaron.ballman added a comment.

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-08-26 Thread Dominic Ferreira via Phabricator via cfe-commits

domdom updated this revision to Diff 217289.
domdom added a comment.

Rebased onto master, clang format the patch.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/ms-union-member-ref.cpp


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *
+  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]]
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4057,7 +4057,6 @@
   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
@@ -4074,27 +4073,30 @@
   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, , );
-
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+  // 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, , );
+
+// 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


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *
+  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:

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-08-23 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

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?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-08-22 Thread Dominic Ferreira via Phabricator via cfe-commits

domdom added a comment.

Thanks @aaron.ballman!

I will need someone to commit this for me :)

Cheers,
Dom


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-08-20 Thread Dominic Ferreira via Phabricator via cfe-commits

domdom added a comment.

Ping!

Cheers,
Dom


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-06-18 Thread Dominic Ferreira via Phabricator via cfe-commits

domdom updated this revision to Diff 205495.
domdom added a comment.

clang-formatted the test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/ms-union-member-ref.cpp


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *
+  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]]
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3929,29 +3929,32 @@
   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 (FieldType->isReferenceType())
+  addr = Builder.CreateElementBitCast(
+  addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
   } else {
 // For structs, we GEP to the field that the record layout suggests.
 addr = emitAddrOfFieldStorage(*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, , );
-
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+  // 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, , );
+
+// 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


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *
+  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

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added reviewers: rnk, majnemer.
aaron.ballman added subscribers: majnemer, rnk.
aaron.ballman added a comment.

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




Comment at: clang/test/CodeGenCXX/ms-union-member-ref.cpp:3-6
+union A {
+int *
+int **ptr;
+};

Can you clang-format the patch?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-05-29 Thread Dominic Ferreira via Phabricator via cfe-commits

domdom created this revision.
domdom added a reviewer: asl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rC Clang

https://reviews.llvm.org/D62571

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/ms-union-member-ref.cpp


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+int *
+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]]
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3929,29 +3929,32 @@
   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 (FieldType->isReferenceType())
+  addr = Builder.CreateElementBitCast(
+  addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
   } else {
 // For structs, we GEP to the field that the record layout suggests.
 addr = emitAddrOfFieldStorage(*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, , );
-
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+  // 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, , );
+
+// 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


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+int *
+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*,

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[PATCH] D62571: Implement codegen for MSVC unions with reference members

9 matches

Site Navigation

Mail list logo

Footer information