[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david edited https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david created
https://github.com/llvm/llvm-project/pull/136792
Note: the patch is probably amending the wrong piece of code, I've tried to add
it to `buildThisParam` but hit an assertion because of a missing translation
unit context.
Clang, unlike GCC, does not transform the following example into a 128-bit load
and store:
```c++
class vector4f
{
private:
float _elements[4];
public:
explicit __attribute__((noinline)) vector4f(float const *src)
{
_elements[0] = src[0];
_elements[1] = src[1];
_elements[2] = src[2];
_elements[3] = src[3];
}
};
```
That's because `src` might overlap with `_elements`.
However, according to the standard in 11.10.4.2 under [class.cdtor]:
> "During the construction of an object, if the value of the object or any
> of its subobjects is accessed through a glvalue that is not obtained,
> directly or indirectly, from the constructor’s this pointer, the value
> of the object or subobject thus obtained is unspecified."
which sounds like `restrict`.
Relevant GCC chain-mail:
https://gcc.gnu.org/pipermail/gcc-patches/2018-May/498812.html.
>From ba3d52c74add481bfe35a448963424dc448c0fad Mon Sep 17 00:00:00 2001
From: Guy David
Date: Wed, 23 Apr 2025 02:24:41 +0300
Subject: [PATCH] [Clang] Add `noalias` to `this` pointer in C++ constructors
---
clang/lib/CodeGen/CodeGenFunction.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 4d29ceace646f..2da48cea7849d 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1567,8 +1567,10 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD,
llvm::Function *Fn,
PGO.assignRegionCounters(GD, CurFn);
if (isa(FD))
EmitDestructorBody(Args);
- else if (isa(FD))
+ else if (isa(FD)) {
+Fn->addParamAttr(0, llvm::Attribute::NoAlias);
EmitConstructorBody(Args);
+ }
else if (getLangOpts().CUDA &&
!getLangOpts().CUDAIsDevice &&
FD->hasAttr())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david edited https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david edited https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david edited https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
guy-david wrote: Yeah, I realize it's misplaced, I am not familiar with that part of the project, see the first paragraph in the PR description. I don't really agree with your second point about breaking people's existing assumptions on UB :) I am willing to run correctness suites to further validate this change, if you can recommend on any. I think an opt-out flag is the wrong way to go about this, because it would add more maintenance if this does go against the standard, in which case we should revert this PR. https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david edited https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
guy-david wrote: Comparison between latest Clang and GCC's output for a snippet out of a benchmark that could use this optimization: https://godbolt.org/z/35EEvcsPr. I've ran llvm-test-suite ten times for the before and after, it executed correctly and expectedly saw no performance gains: ``` Tests: 3326 Metric: exec_time exec_time l/r lhsrhs diff count 3326.003326.001554.00 mean 885.285851 885.132328 inf std20163.199507 20160.504672 NaN min0.00 0.00 -1.00 25%0.00 0.00 -0.004020 50%0.00 0.00 0.00 75%1.741165 1.840265 0.003943 max802971.672414 802823.530425 inf ``` Also built a bootstrap build and confirmed that `ninja stage2-check-all` passes successfully, except for a few libcxx benchmarks that seem to fail regardless. https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
https://github.com/guy-david ready_for_review https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
guy-david wrote: @efriedma-quic ping :) https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
guy-david wrote: > > @zygoloid Can you explain in your example why `a.n == 2` must be true, when > > your interpretation (which I understood in the same manner) of the > > standard's wording does indicate that the object's state is unspecified? > > My reading is that the standard says that the value of `a.n` after `*p = 1;` > is unspecified, but after `n = 2;` the value of `a.n` is 2. I read it as "the value obtained" in the context of the execution of the entire constructor, because no value is really obtained after `*p = 1;`. https://github.com/llvm/llvm-project/pull/136792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add `noalias` to `this` pointer in C++ constructors (PR #136792)
guy-david wrote:
> I just got a case:
>
> ```
> class A {
> public:
>class B {
>public:
> B(A *);
>
> // some non static data fields
>};
>
>B b(this);
> };
> ```
>
> Does this a valid prove that this optimization is not valid ?
This example is fine because `this` is passed directly to `B`'s constructor, so
alias analysis can still succesfully track when it might alias other pointers
and stop certain optimizations, despite the `noalias`.
https://github.com/llvm/llvm-project/pull/136792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
