Skip to content

[clang] Add value_type attr, use it to add noalias when pass-by-value. #95004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fhahn wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[clang] Add value_type attr, use it to add noalias when pass-by-value. #95004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c3b6943
[clang] Add value_type attr, use it to add noalias when pass-by-value.
fhahn Jun 10, 2024
82e6bdb
!fixup add attr docs.
fhahn Mar 14, 2025
e0ff9c7
!fixup update wording
fhahn Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,16 @@ def TrivialABI : InheritableAttr {
let SimpleHandler = 1;
}

def ValueType : InheritableAttr {
// This attribute does not have a C [[]] spelling because it requires the
// CPlusPlus language option.
let Spellings = [Clang<"value_type", 0>];
let Subjects = SubjectList<[CXXRecord]>;
let Documentation = [TrivialABIDocs];
Copy link
Collaborator

@erichkeane erichkeane Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding docs, but setting the documentation link here to something else?

let LangOpts = [CPlusPlus];
let SimpleHandler = 1;
}

def MaxFieldAlignment : InheritableAttr {
// This attribute has no spellings as it is only ever created implicitly.
let Spellings = [];
Expand Down
12 changes: 12 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9242,3 +9242,15 @@ Declares that a function potentially allocates heap memory, and prevents any pot
of ``nonallocating`` by the compiler.
}];
}

def ValueTypeDocs : Documentation {
let Category = DocCatDecl;
let Content = [{
The ``value_type`` attribute can be used to mark user-defined types as 'value
types'. When objects of value types are passed value to functions, the objects
Copy link
Collaborator

@erichkeane erichkeane Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
types'. When objects of value types are passed value to functions, the objects
types'. When objects of value types are passed by value to functions, the objects

are always considered to be formally copied into a new object. This means the
argument itself must be the only value referencing the passed object. This
Copy link
Collaborator

@erichkeane erichkeane Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am having a hard time figuring out why pass-by-value doesn't already imply this? Pass-by-value means it was copied into the new object, right? So I guess I have a hard time figuring out what we really mean here.

Copy link
Member

@jyknight jyknight Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NRVO copy-elision can enable a function which returns by value to stash the address of the returned value. Or, C++ constructors can do the same.

Then, copy-elision on the call can result in the callee receiving a object whose address is escaped, despite it receiving it "passed by value". E.g. in the following example, &a == global in the body of foo.

struct NonTrivial { NonTrivial(); };
NonTrivial *global;
NonTrivial frob() {
    NonTrivial result;
    global = &result;
    return result;
}

void foo(NonTrivial a);
int main() {
  foo(frob());
}

But, I don't think this new attribute is viable; the intent expressed by the PR message is to use it in libc++ to mark stdlib types. But, this cannot be a type property, because any code could do the above with any type.

Copy link
Collaborator

@erichkeane erichkeane Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see what you mean now. Yeah, that doesn't look like something that could be a property of the type so much as the uses itself. So I again find myself wondering what property of the type we can annotate that makes it usable in the opt requested.

Copy link
Contributor

@rjmccall rjmccall Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the argument satisfies the formal conditions for being passed in registers, the implementation is allowed to introduce a copy despite NRVO, which might be enough to make any escaped/self pointers formally invalid even if we don't actually copy. But yeah, NRVO might mean we're out of luck for an arbitrary type to which that doesn't apply.

We could probably talk to the C++ committee about this if we can show that we could do useful optimizations except for some contrived and dangerous code pattern like escaping the address of a copyable class value prior to returning it via NRVO.

Copy link
Contributor Author

@fhahn fhahn Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyknight thanks for sharing the example! I made a few changes to make it runable: https://clang.godbolt.org/z/WrE8Ga6d9

What's interesting is that we get different results with Clang with and without optimizations (without optimizations, there's a copy due to byval, while with optimizations to function is inlined and the byval is gone). GCC seems to create copies both with and without optimizations and gives consistent results.

On AArch64, the argument isn't marked as byval, so we get the same results with and w/o optimizations. But it behaves differently to X86 w/o optimizations.

So overall it seems like there already are a number of inconsistencies when trying to capture the address of a pass-by-value argument across multiple dimensions; IIUC the program should return the same results for both GCC, Clang, with opts, w/o opts and different architectures.

The attribute itself is intended to forbid such uses of the type, but I agree it is not clear if it would be possible to use other for types in libc++.

Copy link
Contributor

@rjmccall rjmccall Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, right, stack arguments.

Anyway, that code specifically has... at least unspecified behavior. NonTrivial satisfies the conditions of [class.temporary]p3, and so the compiler is permitted (but not required) to perform additional copies when passing and returning it, which makes it impossible to specify under the standard whether the object addresses compare the same. They do if all copies are avoided, they don't if the return copy is avoided but the parameter copy is not, and it's UB if there's a return copy because the pointer no longer points to allocated storage.

I don't know whether that's a strong enough argument to actually treat the existence of pointers to the parameter as UB, though. And it's specific to formally trivial types, which presumably doesn't really help us with our optimization efforts because we want to optimize std::string, std::vector, and the like.

allows the optimizer to assume that no other pointers may alias the object when
it is passed indirectly to a function..
Copy link
Collaborator

@erichkeane erichkeane Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it is passed indirectly to a function..
it is passed indirectly to a function.

}];
}
7 changes: 4 additions & 3 deletions clang/lib/CodeGen/CGCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2797,9 +2797,10 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));

auto *Decl = ParamType->getAsRecordDecl();
if (CodeGenOpts.PassByValueIsNoAlias && Decl &&
Decl->getArgPassingRestrictions() ==
RecordArgPassingKind::CanPassInRegs)
if (Decl && ((CodeGenOpts.PassByValueIsNoAlias &&
Decl->getArgPassingRestrictions() ==
RecordArgPassingKind::CanPassInRegs) ||
Decl->hasAttr<ValueTypeAttr>()))
// 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);
Expand Down
40 changes: 40 additions & 0 deletions clang/test/CodeGenCXX/value-type-attr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s

struct Small {
int *p;
Small();
~Small();
Small(const Small &) noexcept;
Small &operator=(const Small &);
};


struct __attribute__((value_type)) SmallVT {
int *p;
SmallVT();
~SmallVT();
SmallVT(const SmallVT &) noexcept;
SmallVT &operator=(const SmallVT &);
};

struct Large {
int *p;
int a[128];
Large();
~Large();
Large(const Large &) noexcept;
Large &operator=(const Large &);
};

struct __attribute__((value_type)) LargeVT {
int *p;
int a[128];
LargeVT();
~LargeVT();
LargeVT(const LargeVT &) noexcept;
LargeVT &operator=(const LargeVT &);
};

void foo(Small a, SmallVT b, Large c, LargeVT d) {}
// CHECK: define void @_Z3foo5Small7SmallVT5Large7LargeVT(ptr noundef %a, ptr noalias noundef %b, ptr noundef %c, ptr noalias noundef %d)
1 change: 1 addition & 0 deletions clang/test/Misc/pragma-attribute-supported-attributes-list.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@
// CHECK-NEXT: UnsafeBufferUsage (SubjectMatchRule_function, SubjectMatchRule_field)
// CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
// CHECK-NEXT: VTablePointerAuthentication (SubjectMatchRule_record)
// CHECK-NEXT: ValueType (SubjectMatchRule_record)
// CHECK-NEXT: VecReturn (SubjectMatchRule_record)
// CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
// CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
Expand Down