[Clang] Add option to allow marking pass-by-value args as noalias.

[fhahn]

Summary

After the recent discussion on cfe-dev 'Can indirect class parameters be
noalias?' [1], it seems like using using noalias is problematic for
current C++, but should be allowed for C-only code.

This patch introduces a new option to let the user indicate that it is
safe to mark indirect class parameters as noalias. Note that this also
applies to external callers, e.g. it might not be safe to use this flag
for C functions that are called by C++ functions.

In targets that allocate indirect arguments in the called function, this
enables more agressive optimizations with respect to memory operations
and brings a ~1% - 2% codesize reduction for some programs.

I am not sure about the best name for the option and description. I
would appreciate any feedback.

[1] : http://lists.llvm.org/pipermail/cfe-dev/2020-July/066353.html

[rsmith]

clang/include/clang/Basic/LangOptions.def

371–374 ↗

(On Diff #283720)

Per the comment at the top of the file, the description should be a single noun phrase; we use this in diagnostics such as "AST file was built with %0 enabled but it is currently disabled". Something like "assumption that by-value parameters do not alias any other values" maybe?

clang/include/clang/Driver/Options.td

4325–4328

This should be in Group<f_Group>.

[fhahn]

Thanks for taking a look!

I adjusted the LangOpts description and added the flag to f_Group, as suggested.

[fhahn]

ping

[fhahn]

ping :)

[fhahn]

ping :)

[rjmccall]

Is it acceptable to leave this as a -cc1 option while we're pursuing this with the language committee? Do we have any intent to pursue this with the language committee?

clang/include/clang/Basic/LangOptions.def

372 ↗

(On Diff #283837)

This should be a code-gen option, I think. I can't really imagine how this would affect language processing.

clang/include/clang/Driver/Options.td

4325–4328

The "Note" clause seems to muddy more than it clarifies. Maybe "has no effect on non-trivially-copyable classes in C++"? Or add proper documentation somewhere instead of trying to jam this into the help text.

clang/lib/CodeGen/CGCall.cpp

2207

This definitely can't be added unconditionally to all types; you need to rule out non-trivial C++ class types, as well as types with ObjC weak references.

[fhahn]

Change to codegen option, adjust description for option, limit to trivially copyable types in C++, add corresponding test cases.

[fhahn]

clang/include/clang/Driver/Options.td

4325–4328

I've added the clarification, thanks!

clang/lib/CodeGen/CGCall.cpp

2207

Updated to rule out non trivially-copyable types.

I am not sure how to best handle types with weak references. I did not manage to find any helpers that traverse a whole type to check if it contains weak references.

I added clang/test/CodeGenObjC/pass-by-value-noalias.m and IIUC noalias should not be added for that case.

[fhahn]

Also restrict to types that are isNonTrivialToPrimitiveCopy, so we do not add noalias for problematic types, like ObjC structs with weak references.

[rjmccall]

clang/lib/CodeGen/CGCall.cpp

2207

The restriction should not be by triviality but by whether indirectness is forced semantically. That may currently only be available on RecordDecl (as RecordDecl::getArgPassingRestrictions), but you could definitely add an accessor to Type which checked for a RecordType and otherwise returned that there are no passing constraints. Tagging Akira, who worked on this.

[fhahn]

Update code to limit adding noalias to types with getArgPassingRestrictions() == RecordDecl::APK_CanPassInRegs. This rules out problematic types, like ones with non-trivial constructors or ObjC structs with weak pointers. Thanks @rjmccall

I also added the escape cases shared on cfe-dev by @rsmith

[fhahn]

clang/lib/CodeGen/CGCall.cpp

2207

Thanks John, that does indeed exactly what we need I think. I updated the code to use getAsRecordDecl here. I can also add a helper to Type if it is useful beyond here.

[fhahn]

Add check lines for escape examples. Note that for return-by-value we already add noalias to the pointer unconditionally.

[rjmccall]

Just a minor tweak and then LGTM.

clang/include/clang/Driver/Options.td

4325–4328

Oh, you need a space after the period.

[fhahn]

clang/include/clang/Driver/Options.td

4325–4328

Done, thanks!

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(Addrsig, 1, 0)
 
 /// Whether to emit unused static constants.
 CODEGENOPT(KeepStaticConsts, 1, 0)
 
 /// Whether to not follow the AAPCS that enforce at least one read before storing to a volatile bitfield
 CODEGENOPT(ForceAAPCSBitfieldLoad, 1, 0)
 
+/// Assume that by-value parameters do not alias any other values.
+CODEGENOPT(PassByValueIsNoAlias, 1, 0)
+
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT

clang/include/clang/Driver/Options.td

def fwchar_type_EQ : Joined<["-"], "fwchar-type=">,
   HelpText<"Select underlying type for wchar_t">, Values<"char,short,int">;
 def fsigned_wchar : Flag<["-"], "fsigned-wchar">,
   HelpText<"Use a signed type for wchar_t">;
 def fno_signed_wchar : Flag<["-"], "fno-signed-wchar">,
   HelpText<"Use an unsigned type for wchar_t">;
 def fcompatibility_qualified_id_block_param_type_checking : Flag<["-"], "fcompatibility-qualified-id-block-type-checking">,
   HelpText<"Allow using blocks with parameters of more specific type than "
            "the type system guarantees when a parameter is qualified id">;
+def fpass_by_value_is_noalias: Flag<["-"], "fpass-by-value-is-noalias">,
+  HelpText<"Allows assuming by-value parameters do not alias any other value. "
+           "Has no effect on non-trivially-copyable classes in C++.">, Group<f_Group>;
 
 // FIXME: Remove these entirely once functionality/tests have been excised.
 def fobjc_gc_only : Flag<["-"], "fobjc-gc-only">, Group<f_Group>,
   HelpText<"Use GC exclusively for Objective-C related memory management">;
 def fobjc_gc : Flag<["-"], "fobjc-gc">, Group<f_Group>,
   HelpText<"Enable Objective-C garbage collection">;
 
 //===----------------------------------------------------------------------===//
 // Header Search Options

clang/lib/CodeGen/CGCall.cpp

for (CGFunctionInfo::const_arg_iterator I = FI.arg_begin(),
 
     case ABIArgInfo::Indirect: {
       if (AI.getInReg())
         Attrs.addAttribute(llvm::Attribute::InReg);
 
       if (AI.getIndirectByVal())
         Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));
 
+      auto *Decl = ParamType->getAsRecordDecl();
+      if (CodeGenOpts.PassByValueIsNoAlias && Decl &&
+          Decl->getArgPassingRestrictions() == RecordDecl::APK_CanPassInRegs)
+        // When calling the function, the pointer passed in will be the only
+        // reference to the underlying object. Mark it accordingly.
+        Attrs.addAttribute(llvm::Attribute::NoAlias);
+
       // TODO: We could add the byref attribute if not byval, but it would
       // require updating many testcases.
 
       CharUnits Align = AI.getIndirectAlign();
 
       // In a byval argument, it is important that the required
       // alignment of the type is honored, as LLVM might be creating a
       // *new* stack object, and needs to know what alignment to give

clang/lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
 
   Opts.PassPlugins = Args.getAllArgValues(OPT_fpass_plugin_EQ);
 
   Opts.SymbolPartition =
       std::string(Args.getLastArgValue(OPT_fsymbol_partition_EQ));
 
   Opts.ForceAAPCSBitfieldLoad = Args.hasArg(OPT_ForceAAPCSBitfieldLoad);
+
+  Opts.PassByValueIsNoAlias = Args.hasArg(OPT_fpass_by_value_is_noalias);
   return Success;
 }
 
 static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
                                       ArgList &Args) {
   Opts.OutputFile = std::string(Args.getLastArgValue(OPT_dependency_file));
   Opts.Targets = Args.getAllArgValues(OPT_MT);
   Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);

clang/test/CodeGen/pass-by-value-noalias.c

+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A struct large enough so it is not passed in registers on ARM64.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}

clang/test/CodeGenCXX/pass-by-value-noalias.cpp

+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A trivial struct large enough so it is not passed in registers on ARM64.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// Make sure noalias is added to indirect arguments with trivially copyable types
+// if -fpass-by-value-is-noalias is provided.
+
+// WITH_NOALIAS: define void @_Z4take3Foo(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @_Z4take3Foo(%struct.Foo* %arg)
+void take(Foo arg) {}
+
+int G;
+
+// NonTrivial is not trivially-copyable, because it has a non-trivial copy
+// constructor.
+struct NonTrivial {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+
+  NonTrivial(const NonTrivial &Other) {
+    a = G + 10 + Other.a;
+  }
+};
+
+// Make sure noalias is not added to indirect arguments that are not trivially
+// copyable even if -fpass-by-value-is-noalias is provided.
+
+// WITH_NOALIAS: define void @_Z4take10NonTrivial(%struct.NonTrivial* %arg)
+// NO_NOALIAS:   define void @_Z4take10NonTrivial(%struct.NonTrivial* %arg)
+void take(NonTrivial arg) {}
+
+// Escape examples. Pointers to the objects passed to take() may escape, depending on whether a temporary copy is created or not (e.g. due to NRVO).
+struct A {
+  A(A **where) : data{"hello world 1"} {
+    *where = this; //Escaped pointer 1 (proposed UB?)
+  }
+
+  A() : data{"hello world 2"} {}
+
+  char data[32];
+};
+A *p;
+
+// WITH_NOALIAS: define void @_Z4take1A(%struct.A* noalias %arg)
+// NO_NOALIAS: define void @_Z4take1A(%struct.A* %arg)
+void take(A arg) {}
+
+// WITH_NOALIAS: define void @_Z7CreateAPP1A(%struct.A* noalias sret align 1 %agg.result, %struct.A** %where)
+// NO_NOALIAS: define void @_Z7CreateAPP1A(%struct.A* noalias sret align 1 %agg.result, %struct.A** %where)
+A CreateA(A **where) {
+  A justlikethis;
+  *where = &justlikethis; //Escaped pointer 2 (should also be UB, then)
+  return justlikethis;
+}
+
+// elsewhere, perhaps compiled by a smarter compiler that doesn't make a copy here
+void test() {
+  take({&p});        // 1
+  take(CreateA(&p)); // 2
+}

clang/test/CodeGenObjC/pass-by-value-noalias.m

+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns -fobjc-runtime-has-weak -fobjc-arc -fobjc-dispatch-method=mixed %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns -fobjc-runtime-has-weak -fobjc-arc -fobjc-dispatch-method=mixed %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+@interface Bar
+@property char value;
+@end
+
+// A struct large enough so it is not passed in registers on ARM64, but with a
+// weak reference, so noalias should not be added even with
+// -fpass-by-value-is-noalias.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  Bar *__weak f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}