rsmith created this revision.
Herald added a subscriber: sanjoy.

This patch adds a flag `-fpass-indirect-ignore-move` that can be used to undo 
the ABI change in r310401, reverting Clang to its prior C++ ABI for pass/return 
by value of class types affected by that change.

This flag is enabled by default for PS4.

Perhaps we should consider adding a "clang ABI version" flag for this kind of 
thing, rather than flags for the individual settings? There are a collection of 
other ABI changes that a PS4 triple currently turns off in similar ad-hoc ways, 
and it would seem reasonable to have a more organized and disciplined way of 
handling them.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/uncopyable-args.cpp
  test/Driver/flags.c
  test/Driver/ps4-cpu-defaults.cpp

Index: test/Driver/ps4-cpu-defaults.cpp
===================================================================
--- test/Driver/ps4-cpu-defaults.cpp
+++ test/Driver/ps4-cpu-defaults.cpp
@@ -3,4 +3,5 @@
 
 // RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
 // CHECK: "-target-cpu" "btver2"
+// CHECK: "-fpass-indirect-ignore-move"
 // CHECK-NOT: exceptions
Index: test/Driver/flags.c
===================================================================
--- test/Driver/flags.c
+++ test/Driver/flags.c
@@ -24,3 +24,12 @@
 
 // RUN: %clang -target armv7-apple-darwin10 -### -S -mno-implicit-float -mimplicit-float %s 2>&1 | FileCheck -check-prefix=TEST8 %s
 // TEST8-NOT: "-no-implicit-float"
+
+// RUN: %clang -target x86_64-linux-gnu -### -c -fpass-indirect-ignore-move %s 2>&1 | FileCheck -check-prefix=TEST9 %s
+// RUN: %clang -target x86_64-scei-ps4 -### -c %s 2>&1 | FileCheck -check-prefix=TEST9 %s
+// TEST9: "-fpass-indirect-ignore-move"
+
+// RUN: %clang -target x86_64-linux-gnu -### -c -fno-pass-indirect-ignore-move %s 2>&1 | FileCheck -check-prefix=TEST10 %s
+// RUN: %clang -target x86_64-scei-ps4 -### -c -fno-pass-indirect-ignore-move %s 2>&1 | FileCheck -check-prefix=TEST10 %s
+// RUN: %clang -target x86_64-linux-gnu -### -c %s 2>&1 | FileCheck -check-prefix=TEST10 %s
+// TEST10-NOT: "-fpass-indirect-ignore-move"
Index: test/CodeGenCXX/uncopyable-args.cpp
===================================================================
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=NEWABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -fpass-indirect-ignore-move -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=18 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-18
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=19 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-19
 
@@ -56,8 +57,10 @@
 // CHECK-LABEL: define void @_ZN9move_ctor3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// NEWABI: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// OLDABI: call void @_ZN9move_ctor3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// OLDABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
 }
@@ -76,8 +79,10 @@
 // CHECK-LABEL: define void @_ZN11all_deleted3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// NEWABI: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// OLDABI: call void @_ZN11all_deleted3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// OLDABI-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
 }
@@ -95,8 +100,10 @@
 // CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// NEWABI: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// OLDABI: call void @_ZN18implicitly_deleted3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// OLDABI-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(i8*)
 
 // In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC 2015, it is.
 // WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64
@@ -116,8 +123,10 @@
 // CHECK-LABEL: define void @_ZN11one_deleted3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// NEWABI: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// OLDABI: call void @_ZN11one_deleted3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// OLDABI-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
 }
@@ -196,8 +205,10 @@
 }
 // CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// NEWABI: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// OLDABI: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* byval
+// NEWABI-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// OLDABI-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* byval
 
 // WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
 }
@@ -209,7 +220,8 @@
   void *p;
 };
 void *foo(A a) { return a.p; }
-// CHECK-LABEL: define i8* @_ZN15definition_only3fooENS_1AE(%"struct.definition_only::A"*
+// NEWABI-LABEL: define i8* @_ZN15definition_only3fooENS_1AE(%"struct.definition_only::A"*
+// OLDABI-LABEL: define i8* @_ZN15definition_only3fooENS_1AE(i8*
 // WIN64-LABEL: define i8* @"\01?foo@definition_only@@YAPEAXUA@1@@Z"(%"struct.definition_only::A"*
 }
 
@@ -224,7 +236,8 @@
   B b;
 };
 void *foo(A a) { return a.b.p; }
-// CHECK-LABEL: define i8* @_ZN17deleted_by_member3fooENS_1AE(%"struct.deleted_by_member::A"*
+// NEWABI-LABEL: define i8* @_ZN17deleted_by_member3fooENS_1AE(%"struct.deleted_by_member::A"*
+// OLDABI-LABEL: define i8* @_ZN17deleted_by_member3fooENS_1AE(i8*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_member@@YAPEAXUA@1@@Z"(%"struct.deleted_by_member::A"*
 }
 
@@ -238,7 +251,8 @@
   A();
 };
 void *foo(A a) { return a.p; }
-// CHECK-LABEL: define i8* @_ZN15deleted_by_base3fooENS_1AE(%"struct.deleted_by_base::A"*
+// NEWABI-LABEL: define i8* @_ZN15deleted_by_base3fooENS_1AE(%"struct.deleted_by_base::A"*
+// OLDABI-LABEL: define i8* @_ZN15deleted_by_base3fooENS_1AE(i8*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_base@@YAPEAXUA@1@@Z"(%"struct.deleted_by_base::A"*
 }
 
@@ -253,7 +267,8 @@
   B b;
 };
 void *foo(A a) { return a.b.p; }
-// CHECK-LABEL: define i8* @_ZN22deleted_by_member_copy3fooENS_1AE(%"struct.deleted_by_member_copy::A"*
+// NEWABI-LABEL: define i8* @_ZN22deleted_by_member_copy3fooENS_1AE(%"struct.deleted_by_member_copy::A"*
+// OLDABI-LABEL: define i8* @_ZN22deleted_by_member_copy3fooENS_1AE(i8*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_member_copy@@YAPEAXUA@1@@Z"(%"struct.deleted_by_member_copy::A"*
 }
 
@@ -267,7 +282,8 @@
   A();
 };
 void *foo(A a) { return a.p; }
-// CHECK-LABEL: define i8* @_ZN20deleted_by_base_copy3fooENS_1AE(%"struct.deleted_by_base_copy::A"*
+// NEWABI-LABEL: define i8* @_ZN20deleted_by_base_copy3fooENS_1AE(%"struct.deleted_by_base_copy::A"*
+// OLDABI-LABEL: define i8* @_ZN20deleted_by_base_copy3fooENS_1AE(i8*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_base_copy@@YAPEAXUA@1@@Z"(%"struct.deleted_by_base_copy::A"*
 }
 
@@ -277,7 +293,8 @@
   A(const A &o) = delete;
   void *p;
 };
-// CHECK-LABEL: define i8* @_ZN15explicit_delete3fooENS_1AE(%"struct.explicit_delete::A"*
+// NEWABI-LABEL: define i8* @_ZN15explicit_delete3fooENS_1AE(%"struct.explicit_delete::A"*
+// OLDABI-LABEL: define i8* @_ZN15explicit_delete3fooENS_1AE(i8*
 // WIN64-LABEL: define i8* @"\01?foo@explicit_delete@@YAPEAXUA@1@@Z"(%"struct.explicit_delete::A"*
 void *foo(A a) { return a.p; }
 }
@@ -289,7 +306,8 @@
   // Deleted copy ctor due to rvalue ref member.
   int &&ref;
 };
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1AE(%"struct.implicitly_deleted_copy_ctor::A"*
+// NEWABI-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1AE(%"struct.implicitly_deleted_copy_ctor::A"*
+// OLDABI-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1AE(i32*
 // WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAAEAHUA@1@@Z"(%"struct.implicitly_deleted_copy_ctor::A"*
 int &foo(A a) { return a.ref; }
 
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -622,6 +622,7 @@
   Opts.PIECopyRelocations =
       Args.hasArg(OPT_mpie_copy_relocations);
   Opts.OmitLeafFramePointer = Args.hasArg(OPT_momit_leaf_frame_pointer);
+  Opts.PassIndirectIgnoreMove = Args.hasArg(OPT_fpass_indirect_ignore_move);
   Opts.SaveTempLabels = Args.hasArg(OPT_msave_temp_labels);
   Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
   Opts.SoftFloat = Args.hasArg(OPT_msoft_float);
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2916,6 +2916,11 @@
 
   addPGOAndCoverageFlags(C, D, Output, Args, CmdArgs);
 
+  if (Args.hasFlag(options::OPT_fpass_indirect_ignore_move,
+                   options::OPT_fno_pass_indirect_ignore_move,
+                   getToolChain().getTriple().isPS4()))
+    CmdArgs.push_back("-fpass-indirect-ignore-move");
+
   // Add runtime flag for PS4 when PGO or Coverage are enabled.
   if (getToolChain().getTriple().isPS4CPU())
     PS4cpu::addProfileRTArgs(getToolChain(), Args, CmdArgs);
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -56,15 +56,23 @@
   ItaniumCXXABI(CodeGen::CodeGenModule &CGM,
                 bool UseARMMethodPtrABI = false,
                 bool UseARMGuardVarABI = false) :
-    CGCXXABI(CGM), UseARMMethodPtrABI(UseARMMethodPtrABI),
+    CGCXXABI(CGM),
+    UseARMMethodPtrABI(UseARMMethodPtrABI),
     UseARMGuardVarABI(UseARMGuardVarABI),
     Use32BitVTableOffsetABI(false) { }
 
   bool classifyReturnType(CGFunctionInfo &FI) const override;
 
+  bool passClassIndirect(const CXXRecordDecl *RD) const {
+    if (CGM.getCodeGenOpts().PassIndirectIgnoreMove)
+      return RD->hasNonTrivialDestructor() ||
+             RD->hasNonTrivialCopyConstructor();
+    return !canCopyArgument(RD);
+  }
+
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
     // If C++ prohibits us from making a copy, pass by address.
-    if (!canCopyArgument(RD))
+    if (passClassIndirect(RD))
       return RAA_Indirect;
     return RAA_Default;
   }
@@ -1012,7 +1020,7 @@
     return false;
 
   // If C++ prohibits us from making a copy, return by address.
-  if (!canCopyArgument(RD)) {
+  if (passClassIndirect(RD)) {
     auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
     FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
     return true;
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -123,6 +123,10 @@
 VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
 VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is specified.
 
+/// Do not consider the move constructor when determining whether to pass a
+/// class type indirectly.
+CODEGENOPT(PassIndirectIgnoreMove, 1, 0)
+
 /// \brief Choose profile instrumenation kind or no instrumentation.
 ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNone)
 /// \brief Choose profile kind for PGO use compilation.
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -630,6 +630,11 @@
   Flags<[DriverOption, CC1Option]>,
   HelpText<"Disable GNU style inline asm">;
 
+def fpass_indirect_ignore_move : Flag<["-"], "fpass-indirect-ignore-move">,
+    Group<f_Group>, Flags<[CC1Option]>;
+def fno_pass_indirect_ignore_move : Flag<["-"], "fno-pass-indirect-ignore-move">,
+    Group<f_Group>;
+
 def fprofile_sample_use : Flag<["-"], "fprofile-sample-use">, Group<f_Group>,
     Flags<[CoreOption]>;
 def fno_profile_sample_use : Flag<["-"], "fno-profile-sample-use">, Group<f_Group>,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D36501: a... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to