https://github.com/efriedma-quic approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/107581
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kmclaughlin-arm updated
https://github.com/llvm/llvm-project/pull/107581
>From 1e6f25c517d8d1adeeaf59f826141efdcad8f05a Mon Sep 17 00:00:00 2001
From: Kerry McLaughlin
Date: Fri, 6 Sep 2024 10:13:33 +
Subject: [PATCH 1/6] [Clang] Emit error for duplicate mangled names wit
efriedma-quic wrote:
Can you add a testcase that doesn't involve lambdas? Maybe something like the
following:
```
inline void f(int) asm("foo");
inline void f(int) {}
inline void f() asm("foo");
inline void f(){}
```
https://github.com/llvm/llvm-project/pull/107581
___
kmclaughlin-arm wrote:
> Oh, that makes sense... so the issue is generally with functions we emit
> lazily?
> I'd say it's reasonable to emit an error if we have two definitions for the
> same symbol, even if we don't end up emitting them because they're deferred.
Yes, I believe so. I've moved
https://github.com/kmclaughlin-arm updated
https://github.com/llvm/llvm-project/pull/107581
>From 1e6f25c517d8d1adeeaf59f826141efdcad8f05a Mon Sep 17 00:00:00 2001
From: Kerry McLaughlin
Date: Fri, 6 Sep 2024 10:13:33 +
Subject: [PATCH 1/5] [Clang] Emit error for duplicate mangled names wit
efriedma-quic wrote:
Oh, that makes sense... so the issue is generally with functions we emit lazily?
I'd say it's reasonable to emit an error if we have two definitions for the
same symbol, even if we don't end up emitting them because they're deferred.
Lazy emission is an optimization, and
kmclaughlin-arm wrote:
> For the lambda example, there are only three relevant calls to
> GetOrCreateLLVMFunction; one for each function with IsInDefinition false, but
> then only one with IsInDefinition true.
>
> It's not clear to me why the two cases are different, and I don't really want
>
efriedma-quic wrote:
In the example in
https://github.com/llvm/llvm-project/pull/107581#issuecomment-2343651051 ,
there are four relevant calls to GetOrCreateLLVMFunction: one for each function
with IsInDefinition false, and one for each function with IsInDefinition true.
The last of those c
kmclaughlin-arm wrote:
> For non-lambda methods, the way this works it that we call
> GetOrCreateLLVMFunction for both methods... for the first method, the
> `!Entry->isDeclaration()` would be false, but for the second one, it would be
> true because we've emitted the definition of the first m
@@ -0,0 +1,31 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm -o - %s -verify -DTEST1
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2
-emit-llvm -o - %s -verify -DTEST2
kmclaughlin-arm wro
https://github.com/kmclaughlin-arm updated
https://github.com/llvm/llvm-project/pull/107581
>From 1e6f25c517d8d1adeeaf59f826141efdcad8f05a Mon Sep 17 00:00:00 2001
From: Kerry McLaughlin
Date: Fri, 6 Sep 2024 10:13:33 +
Subject: [PATCH 1/4] [Clang] Emit error for duplicate mangled names wit
efriedma-quic wrote:
> I see that it is set and the reason the existing error is not emitted is
> because of the !Entry->isDeclaration() condition.
For non-lambda methods, the way this works it that we call
GetOrCreateLLVMFunction for both methods... for the first method, the
`!Entry->isDecla
@@ -0,0 +1,31 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm -o - %s -verify -DTEST1
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2
-emit-llvm -o - %s -verify -DTEST2
sdesmalen-arm wrote
https://github.com/kmclaughlin-arm updated
https://github.com/llvm/llvm-project/pull/107581
>From 1e6f25c517d8d1adeeaf59f826141efdcad8f05a Mon Sep 17 00:00:00 2001
From: Kerry McLaughlin
Date: Fri, 6 Sep 2024 10:13:33 +
Subject: [PATCH 1/3] [Clang] Emit error for duplicate mangled names wit
@@ -4652,18 +4652,35 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
// If there are two attempts to define the same mangled name, issue an
// error.
-if (IsForDefinition && !Entry->isDeclaration()) {
- GlobalDecl OtherGD;
- // Check that GD is
@@ -0,0 +1,30 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2
-emit-llvm -o - %s -verify -DTEST1
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2
-emit-llvm -o - %s -verify
@@ -0,0 +1,30 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2
-emit-llvm -o - %s -verify -DTEST1
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2
-emit-llvm -o - %s -verify
@@ -4652,18 +4652,35 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
// If there are two attempts to define the same mangled name, issue an
// error.
-if (IsForDefinition && !Entry->isDeclaration()) {
- GlobalDecl OtherGD;
- // Check that GD is
kmclaughlin-arm wrote:
>
> I'm having a bit of trouble understanding the way the new code is structured.
> What makes the definition of lambda call operators special here? Do we not
> call GetOrCreateLLVMFunction with IsForDefinition set?
When I added this error I incorrectly thought GetOrCre
https://github.com/kmclaughlin-arm updated
https://github.com/llvm/llvm-project/pull/107581
>From 1e6f25c517d8d1adeeaf59f826141efdcad8f05a Mon Sep 17 00:00:00 2001
From: Kerry McLaughlin
Date: Fri, 6 Sep 2024 10:13:33 +
Subject: [PATCH 1/2] [Clang] Emit error for duplicate mangled names wit
efriedma-quic wrote:
Sure, it makes sense to print a diagnostic for lambdas.
-
I'm having a bit of trouble understanding the way the new code is structured.
What makes the definition of lambda call operators special here? Do we not
call GetOrCreateLLVMFunction with IsForDefinition set?
sdesmalen-arm wrote:
> > The SME type attributes are not part of the name mangling
>
> If `int(&)() __arm_streaming` is a different type from `int(&)()` for
> template instantiation, it should have different mangling. If it doesn't,
> that's a bug. If there is no spec for the correct mangling,
efriedma-quic wrote:
(You shouldn't need to special-case lambdas; any use of the type needs the
appropriate mangling, except maybe the function declaration itself.)
https://github.com/llvm/llvm-project/pull/107581
___
cfe-commits mailing list
cfe-comm
efriedma-quic wrote:
> The SME type attributes are not part of the name mangling
If `int(&)() __arm_streaming` is a different type from `int(&)()` for template
instantiation, it should have different mangling. If it doesn't, that's a bug.
If there is no spec for the correct mangling, someone
kmclaughlin-arm wrote:
> Isn't this a bug in the mangler? I mean, it's better to print an error rather
> than silently miscompile, but this doesn't really solve the issue.
Hi @efriedma-quic,
The SME type attributes are not part of the name mangling. We figured an error
message would be useful
efriedma-quic wrote:
Isn't this a bug in the mangler? I mean, it's better to print an error rather
than silently miscompile, but this doesn't really solve the issue.
https://github.com/llvm/llvm-project/pull/107581
___
cfe-commits mailing list
cfe-com
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: Kerry McLaughlin (kmclaughlin-arm)
Changes
When functions are passed as arguments to a lambda, it's possible for
the mangled names of these functions to be the same despite the prototypes
being different. For example:
```
int non
https://github.com/kmclaughlin-arm created
https://github.com/llvm/llvm-project/pull/107581
When functions are passed as arguments to a lambda, it's possible for
the mangled names of these functions to be the same despite the prototypes
being different. For example:
```
int non_streaming_fn(i
28 matches
Mail list logo
