[clang] [llvm] [X86] Support EGPR for inline assembly. (PR #92338)
https://github.com/nickdesaulniers approved this pull request. someone with more knowledge about EGPR should approve this as well, but I'm happy with the code style for the additions. https://github.com/llvm/llvm-project/pull/92338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const
FieldDecl *Field,
else
LambdaLV = MakeAddrLValue(AddrOfExplicitObject,
D->getType().getNonReferenceType());
+
+// Make sure we have an lvalue to the lambda itself and not a derived
class.
+auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl();
+auto *LambdaTy = cast(Field->getParent());
+if (ThisTy != LambdaTy) {
+ const CXXCastPath = getContext().LambdaCastPaths.at(MD);
+ Address Base = GetAddressOfBaseClass(
+ LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(),
+ BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation());
nickdesaulniers wrote:
https://github.com/llvm/llvm-project/commit/da88e7fbd74cef33411bb5115f20e4b474d2d8b1
https://github.com/llvm/llvm-project/pull/89828
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const
FieldDecl *Field,
else
LambdaLV = MakeAddrLValue(AddrOfExplicitObject,
D->getType().getNonReferenceType());
+
+// Make sure we have an lvalue to the lambda itself and not a derived
class.
+auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl();
+auto *LambdaTy = cast(Field->getParent());
+if (ThisTy != LambdaTy) {
+ const CXXCastPath = getContext().LambdaCastPaths.at(MD);
+ Address Base = GetAddressOfBaseClass(
+ LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(),
+ BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation());
nickdesaulniers wrote:
error: too many arguments to function call, expected 0, have 1
LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(),
~~~ ^
https://lab.llvm.org/buildbot/#/builders/225/builds/36615/steps/6/logs/stdio
https://github.com/llvm/llvm-project/pull/89828
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PowerPC] Support -fpatchable-function-entry (PR #92997)
@@ -909,6 +909,24 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr
*MI) {
// Lower multi-instruction pseudo operations.
switch (MI->getOpcode()) {
default: break;
+ case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
+assert(!Subtarget->isAIXABI() &&
+ "AIX does not support patchable function entry!");
+// PATCHABLE_FUNCTION_ENTER on little endian is for XRAY support which is
+// handled in PPCLinuxAsmPrinter.
+if (MAI->isLittleEndian())
+ return;
+const Function = MI->getParent()->getParent()->getFunction();
+if (F.hasFnAttribute("patchable-function-entry")) {
+ unsigned Num = 0;
+ if (F.getFnAttribute("patchable-function-entry")
nickdesaulniers wrote:
I think you can also just call getFnAttribute then check isValid on the result.
https://github.com/llvm/llvm-project/pull/92997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PowerPC] Support -fpatchable-function-entry (PR #92997)
@@ -909,6 +909,24 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr
*MI) {
// Lower multi-instruction pseudo operations.
switch (MI->getOpcode()) {
default: break;
+ case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
+assert(!Subtarget->isAIXABI() &&
+ "AIX does not support patchable function entry!");
+// PATCHABLE_FUNCTION_ENTER on little endian is for XRAY support which is
+// handled in PPCLinuxAsmPrinter.
+if (MAI->isLittleEndian())
+ return;
+const Function = MI->getParent()->getParent()->getFunction();
nickdesaulniers wrote:
```suggestion
const Function = MF->getFunction();
```
https://github.com/llvm/llvm-project/pull/92997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PowerPC] Support -fpatchable-function-entry (PR #92997)
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not yet
supported on AIX}}
+__attribute__((patchable_function_entry(0))) void f();
nickdesaulniers wrote:
Can this test case simply be added to
clang/test/Sema/patchable-function-entry-attr.cpp?
https://github.com/llvm/llvm-project/pull/92997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Support EGPR for inline assembly. (PR #92338)
@@ -58016,15 +58035,27 @@ X86TargetLowering::getRegForInlineAsmConstraint(const
TargetRegisterInfo *TRI,
break;
case 'r': // GENERAL_REGS
case 'l': // INDEX_REGS
+ if (Subtarget.useInlineAsmGPR32()) {
+if (VT == MVT::i8 || VT == MVT::i1)
+ return std::make_pair(0U, ::GR8_NOREX2RegClass);
+if (VT == MVT::i16)
+ return std::make_pair(0U, ::GR16_NOREX2RegClass);
+if (VT == MVT::i32 || VT == MVT::f32 ||
+(!VT.isVector() && !Subtarget.is64Bit()))
+ return std::make_pair(0U, ::GR32_NOREX2RegClass);
+if (VT != MVT::f80 && !VT.isVector())
+ return std::make_pair(0U, ::GR64_NOREX2RegClass);
+break;
+ }
if (VT == MVT::i8 || VT == MVT::i1)
-return std::make_pair(0U, ::GR8_NOREX2RegClass);
+return std::make_pair(0U, ::GR8RegClass);
if (VT == MVT::i16)
-return std::make_pair(0U, ::GR16_NOREX2RegClass);
+return std::make_pair(0U, ::GR16RegClass);
if (VT == MVT::i32 || VT == MVT::f32 ||
(!VT.isVector() && !Subtarget.is64Bit()))
-return std::make_pair(0U, ::GR32_NOREX2RegClass);
+return std::make_pair(0U, ::GR32RegClass);
if (VT != MVT::f80 && !VT.isVector())
-return std::make_pair(0U, ::GR64_NOREX2RegClass);
+return std::make_pair(0U, ::GR64RegClass);
nickdesaulniers wrote:
Is it more concise to use ternary expressions here rather than duplicate so
much logic?
```c++
if (VT == MVT::i8 || VT == MVT::i1)
return std::make_pair(0U, Subtarget.useInlineAsmGPR32() ?
::GR8_NOREX2RegClass : ::GR8RegClass);
if (VT == MVT::i16)
return std::make_pair(0U, Subtarget.useInlineAsmGPR32() ?
::GR16_NOREX2RegClass : ::GR16RegClass);
if (VT == MVT::i32 || VT == MVT::f32 ||
(!VT.isVector() && !Subtarget.is64Bit()))
return std::make_pair(0U, Subtarget.useInlineAsmGPR32() ?
::GR32_NOREX2RegClass : ::GR32RegClass);
if (VT != MVT::f80 && !VT.isVector())
return std::make_pair(0U, Subtarget.useInlineAsmGPR32() ?
::GR64_NOREX2RegClass : ::GR64RegClass);
```
https://github.com/llvm/llvm-project/pull/92338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Support EGPR for inline assembly. (PR #92338)
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not llc -mtriple=x86_64 < %s 2>&1 | FileCheck %s --check-prefix=ERR
+; RUN: not llc -mtriple=x86_64 -mattr=+egpr < %s 2>&1 | FileCheck %s
--check-prefix=ERR
+; RUN: llc -mtriple=x86_64 -mattr=+egpr,+inline-asm-use-gpr32 < %s | FileCheck
%s --check-prefix=USEGPR32
+
+; ERR: error: inline assembly requires more registers than available
+
+define void @constraint_r_test() nounwind "frame-pointer"="all" {
+; USEGPR32-LABEL: constraint_r_test:
+; USEGPR32:addq %r16, %rax
+entry:
+ %reg = alloca i64, align 8
+ %0 = load i64, ptr %reg, align 8
+ call void asm sideeffect "add $0, %rax",
"r,~{rax},~{rbx},~{rcx},~{rdx},~{rdi},~{rsi},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{dirflag},~{fpsr},~{flags}"(i64
%0)
+ ret void
+}
+
+define void @constraint_jR_test() nounwind "frame-pointer"="all" {
+; EGPR-LABEL: constraint_jR_test:
+; EGPR:addq %r16, %rax
+;
+; EGPRUSEGPR32-LABEL: constraint_jR_test:
+; EGPRUSEGPR32:addq %r16, %rax
nickdesaulniers wrote:
Are these dead check lines? No `RUN` lines above use this check prefix.
https://github.com/llvm/llvm-project/pull/92338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Support EGPR for inline assembly. (PR #92338)
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not llc -mtriple=x86_64 %s 2>&1 | FileCheck %s --check-prefix=ERR
+; RUN: llc -mtriple=x86_64 -mattr=+egpr < %s | FileCheck %s
--check-prefix=EGPR
+; RUN: llc -mtriple=x86_64 -mattr=+egpr,+inline-asm-use-gpr32 < %s |
FileCheck %s --check-prefix=EGPRUSEGPR32
+
+; ERR: error: inline assembly requires more registers than available
+
+define void @constraint_jR_test() nounwind "frame-pointer"="all" {
+; EGPR-LABEL: constraint_jR_test:
+; EGPR:addq %r16, %rax
+;
+; EGPRUSEGPR32-LABEL: constraint_jR_test:
+; EGPRUSEGPR32:addq %r16, %rax
nickdesaulniers wrote:
These 2 are the same; consider using the same check prefix. You can omit
`--check-prefix` and just use `CHECK:` here.
https://github.com/llvm/llvm-project/pull/92338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen][NFC] Make ConstExprEmitter a ConstStmtVisitor (PR #89041)
https://github.com/nickdesaulniers approved this pull request. https://github.com/llvm/llvm-project/pull/89041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; } // cxx-warning{{unused function 'f'}}
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; } //
expected-warning{{unused function 'f0'}}
+static float __attribute__((overloadable)) f0(float x) { return x; } //
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("?f0@@YAHH@Z")));
nickdesaulniers wrote:
got it; please add a TODO that the overload of `f0` that accepts an `int`
should not warn (similar to the TODO you added in
clang/test/Sema/alias-unused.cpp for the namespace part. Same for `g1` and the
overload of `f3` that accepts no parameters.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
nickdesaulniers wrote:
Please file a bug and link to it from a TODO comment in the tests and/or
sources.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers approved this pull request. LGTM; though I have a strong preference to get bugs on file and linked to regarding: - issues with non-itanium mangling - issues with namespaced identifiers. https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } //
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}
nickdesaulniers wrote:
> may not fit
?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libc] [llvm] Fix typos (PR #88565)
https://github.com/nickdesaulniers closed https://github.com/llvm/llvm-project/pull/88565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libc] [llvm] Fix typos (PR #88565)
nickdesaulniers wrote: Thanks for the patch. Do you need one of us to merge this for you? https://github.com/llvm/llvm-project/pull/88565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libc] [llvm] Fix typos (PR #88565)
https://github.com/nickdesaulniers approved this pull request. https://github.com/llvm/llvm-project/pull/88565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; } // cxx-warning{{unused function 'f'}}
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; } //
expected-warning{{unused function 'f0'}}
+static float __attribute__((overloadable)) f0(float x) { return x; } //
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("?f0@@YAHH@Z")));
nickdesaulniers wrote:
As someone not familiar with microsoft's ABI, is there a tool like
`llvm-cxxfilt` that can help me check this mangling from the command line, on
my Linux host?
---
I'm guessing this test file is demonstrating the lack of msabi mangling
support? And will function as a change detector in the future?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,36 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. Due to name mangling, we look up the
+// demangled name ignoring parameters. This should handle the majority of use
+// cases while leaving namespace scope names unmarked.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = nullptr;
+ if (S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft)
+Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+ std::unique_ptr MC(S.Context.createMangleContext());
+ SmallString<256> Name;
+
+ const DeclarationNameInfo Target(
+ (Demangled ? Demangled : Str), AL.getLoc());
+ LookupResult LR(S, Target, Sema::LookupOrdinaryName);
+ if (S.LookupName(LR, S.TUScope)) {
+for (NamedDecl *ND : LR) {
+ if (MC->shouldMangleDeclName(ND)) {
+llvm::raw_svector_ostream Out(Name);
+Name.clear();
+MC->mangleName(GlobalDecl(ND), Out);
+ } else {
+Name = ND->getIdentifier()->getName();
nickdesaulniers wrote:
So then oops! My bug! And leads to the exact issue I was hypothesizing about
wrt hiding things!
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
nickdesaulniers wrote:
> foo is mangled.
for clang, not GCC. :disappointed:
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
nickdesaulniers wrote:
> Improving GCC consistency is not the goal of this PR. This PR intends to make
> clangSema and clangCodeGen consistent.
Is there a bug on file wrt to this inconsistency? I would like to link to it
from the test, sources, and public docs on the function attribute if we don't
fix this inconsistency BEFORE landing this PR.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } //
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}
nickdesaulniers wrote:
Surely there's another case where we need to go from mangled string back to
named decl. Perhaps there's somewhere else in clang that already does so and
can be reused here?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s nickdesaulniers wrote: > but git does not preserve file move history... Sure it does; isn't that the point of the `--follow` flag to `git log `? https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
// expected-no-diagnostics
+#endif
static int f(void) { return 42; }
int g(void) __attribute__((alias("f")));
static int foo [] = { 42, 0xDEAD };
extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}
nickdesaulniers wrote:
`LookupParsedName` smells like it might handle namespaced lookups?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
nickdesaulniers wrote:
Why should we warn here? Is it because `bar` is itself unused, or because its
`alias` isn't mangled?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } //
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}
+int g3() __attribute__((alias("_ZN2nsL2f3Ev")));
+}
+
+template
+static void *f4(T) { return nullptr; }
+static void *f4() { return nullptr; } // cxx-warning{{unused function 'f4'}}
+extern void g4() __attribute__((ifunc("_ZL2f4IiEPvT_")));
+void *use3 = f4(0);
nickdesaulniers wrote:
I'm curious what happens without the explicit usage. Template instantiation
would fail. Then we'd error that no such function was found? Perhaps consider
adding another test case where that happens? I'd like to ensure folks can't
try to hide potential gadgets that way.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,36 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. Due to name mangling, we look up the
+// demangled name ignoring parameters. This should handle the majority of use
+// cases while leaving namespace scope names unmarked.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = nullptr;
+ if (S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft)
+Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+ std::unique_ptr MC(S.Context.createMangleContext());
+ SmallString<256> Name;
+
+ const DeclarationNameInfo Target(
+ (Demangled ? Demangled : Str), AL.getLoc());
+ LookupResult LR(S, Target, Sema::LookupOrdinaryName);
+ if (S.LookupName(LR, S.TUScope)) {
+for (NamedDecl *ND : LR) {
+ if (MC->shouldMangleDeclName(ND)) {
+llvm::raw_svector_ostream Out(Name);
+Name.clear();
+MC->mangleName(GlobalDecl(ND), Out);
+ } else {
+Name = ND->getIdentifier()->getName();
nickdesaulniers wrote:
The code before didn't need to compare `Str` against
`ND->getIdentifier()->getName()`. Is there a way to rewrite this loop such
that we don't need to reassign `Name` or perform any comparisons when we should
not mangle the decl name? Or did I have a bug in my code?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
nickdesaulniers wrote:
This seems wrong to me, a false negative.
FWICT, for C++ mode, even in an `extern "C"` block, it seems that functions
with static linkage get mangled. Or... Oh, look, more behavioral differences
from GCC: https://godbolt.org/z/sa4e7aYqj
We mangle, they do not. That's going to lead to further compatibility issues
when trying to use this function attribute.
---
Ah! right you pointed this out in your note. Perhaps we should fix/change that
first in clang?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused
function 'resolver'}}
nickdesaulniers wrote:
Why warn here? (Probably same reason as for `foo` above.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers commented: Progressing in the right direction and I'm in favor of fixing this issue in clang, with similar levels of caution as my fellow reviewers. I think we can support Microsoft ABI mangling trivially here. That we mangle globals with static linkage in `extern "C"` blocks looks to me like a bug in clang that should get fixed first BEFORE this lands. Otherwise that seems like a massive compatibility issue. Perhaps another approach would be to at least document this difference. But then again, this behavioral difference is surprising, and I worry that it might be used to try to hide something. The namespace lookup false positive is something that perhaps doesn't need to get fixed here; if we don't plan to remove that final false positive in the PR, then please file a bug @MaskRay and link to it from a TODO comment in the test case and/or sources. https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } //
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}
nickdesaulniers wrote:
> While our approach has false negatives for namespace scope names
Isn't this with `ns::f3()` here a false positive, not a false negative?
Consider adding `TODO: ` to make it even more blatant.
Is this a bug in Sema::LookupOrdinaryName ?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -0,0 +1,47 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s nickdesaulniers wrote: consider using `git mv` when you rename files or move test cases. Makes it very obvious in code review when existing test cases are deleted vs moved and or updated. https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
nickdesaulniers wrote:
ADT has a `FreeDeleter`; smells useful here.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
nickdesaulniers wrote:
> While we can add S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft
> check for downstream users, it doesn't appear to add any value?
If we know the target ABI uses Microsoft mangling, then we should be using
`microsoftDemangle` rather than `itaniumDemangle`. There's also
`llvm::demangle` which can try to figure this out.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
// expected-no-diagnostics
+#endif
static int f(void) { return 42; }
int g(void) __attribute__((alias("f")));
static int foo [] = { 42, 0xDEAD };
extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}
+int g2(void) __attribute__((alias("_ZN2nsL2f2Ev")));
nickdesaulniers wrote:
Dont forget `f1` and one of the overloads of `resolver1`.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
// expected-no-diagnostics
+#endif
static int f(void) { return 42; }
int g(void) __attribute__((alias("f")));
static int foo [] = { 42, 0xDEAD };
extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
nickdesaulniers wrote:
>but checking whether a declaration will have the specified mangled name is
>challenging.
Hence the TODO.
> We would need a MangleContext
Maybe `AST::createMangleContext` can help?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
nickdesaulniers wrote:
> I could imagine an invalid ifunc/alias being used to 'hide' a function that
> is otherwise not called (but I'm also not smart enough to figure out why that
> would be en exploit).
If a TU is compiled without optimizations enabled, dead code elimination may
not remove an usused function, which could be used to purposefully retain a
gadget.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
nickdesaulniers wrote:
Looks like there's `getLangOpts().CXXABI`. `TargetCXXABI::Microsoft` is the
one that doesn't use itanium FWICT (looking at `ASTContext::createCXXABI`) And
`TargetInfo::getCXXABI` (looking at `ASTContext::createMangleContext`).
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
// expected-no-diagnostics
+#endif
static int f(void) { return 42; }
int g(void) __attribute__((alias("f")));
static int foo [] = { 42, 0xDEAD };
extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}
+int g2(void) __attribute__((alias("_ZN2nsL2f2Ev")));
nickdesaulniers wrote:
If these functions are only checked in `__cplusplus` mode, omit the `void` from
the parameter list?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
// expected-no-diagnostics
+#endif
static int f(void) { return 42; }
int g(void) __attribute__((alias("f")));
static int foo [] = { 42, 0xDEAD };
extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}
nickdesaulniers wrote:
Why warn for `f2(void)`? Would `_ZNL2nsL2f2Ev` be the mangling? (an additional
`L` between `N` and `2`)?
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
// expected-no-diagnostics
+#endif
static int f(void) { return 42; }
int g(void) __attribute__((alias("f")));
static int foo [] = { 42, 0xDEAD };
extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
nickdesaulniers wrote:
This behavior differs from GCC; https://godbolt.org/z/fz8a153qY. Even which
function is retained for codegen purposes is different. That's sure to lead to
compatibility issues. So beyond this diagnostic related change, it looks like
we may have a pre-existing codegen bug here, too.
---
Given an alias to `_ZL2f1v`, that demangles to `f1()`. So we _should_ diagnose
that `f1(int)` is unused (`_ZL2f1i`).
Same for `_ZL9resolver1v` below.
You allude to fixing false positives in lieu of adding false negatives, but I
would prefer to not have to compromise like that.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers commented: As the author of https://reviews.llvm.org/D54188 and the TODO added from there, this patch is the right thing to do. I fixed a false positive diagnostic for C and knew that the fix was insufficient for C++. I do appreciate the abundance of caution here, but this looks generally like what we want to do. > We fix false positives while causing false negatives when overloads are > present. Can we avoid the false negatives? https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
nickdesaulniers wrote:
How does one specify a "non-itanium build?"
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/87130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
}
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+ char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+ if (Demangled)
+Str = Demangled;
+ const DeclarationNameInfo target((Str), AL.getLoc());
+ LookupResult LR(S, target, Sema::LookupOrdinaryName);
+ if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+ ND->markUsed(S.Context);
nickdesaulniers wrote:
I believe you can put `alias` attributes on non-functions:
https://godbolt.org/z/s1PbojYr4.
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ExprConst] Fix second arg of __builtin_{clzg,ctzg} not always being evaluated (PR #86742)
https://github.com/nickdesaulniers approved this pull request. https://github.com/llvm/llvm-project/pull/86742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
nickdesaulniers wrote: > I think I am generally deeply confused about what should be provided by the > compiler and what should be provided by the C Standard Library on any given > platform. +1 > Doing otherwise and trying to be "helpful" in Clang only creates confusion > and forces C libraries to jump through hoops to avoid colliding with stuff > already defines in Clang builtin headers. > I believe we should basically not provide any Clang builtin headers except > the ones that clearly provide compiler-related stuff (e.g. or > tmmintrin.h & friends). Weak +1 (+1, but similar levels of unsuredness). --- On the llvm-libc side, we're running into pain with some of the compiler provided headers, too, particularly because they tend to themselves include additional headers (the xmm intrinsic headers including stdlib for example). I'm very tempted to resort to `-nostdinc` and just have our own definitions for everything: ```c typedef __SIZE_TYPE__ size_t; ... ``` and just be done with compiler resource headers. --- I'll suspect that `#define __need_foo` `#include ` pattern makes precompiling stddef.h impossible. --- cc @enh for visibility. https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ExprConst] Fix second arg of __builtin_{clzg,ctzg} not always being evaluated (PR #86742)
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+constexpr int increment(int& x) {
+ x++;
+ return x;
+}
+
+constexpr int test_clzg_0() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_clzg(0U, increment(x));
+ return x;
+}
+
+static_assert(test_clzg_0() == 1);
+
+constexpr int test_clzg_1() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_clzg(1U, increment(x));
+ return x;
+}
+
+static_assert(test_clzg_1() == 1);
+
+constexpr int test_ctzg_0() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_ctzg(0U, increment(x));
+ return x;
+}
+
+static_assert(test_ctzg_0() == 1);
+
+constexpr int test_ctzg_1() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_ctzg(1U, increment(x));
nickdesaulniers wrote:
It's probably also possible to add -Wno-warn-unused-result to the RUN line.
https://github.com/llvm/llvm-project/pull/86742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ExprConst] Fix second arg of __builtin_{clzg,ctzg} not always being evaluated (PR #86742)
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+constexpr int increment(int& x) {
+ x++;
+ return x;
+}
+
+constexpr int test_clzg_0() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_clzg(0U, increment(x));
+ return x;
+}
+
+static_assert(test_clzg_0() == 1);
+
+constexpr int test_clzg_1() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_clzg(1U, increment(x));
+ return x;
+}
+
+static_assert(test_clzg_1() == 1);
+
+constexpr int test_ctzg_0() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_ctzg(0U, increment(x));
+ return x;
+}
+
+static_assert(test_ctzg_0() == 1);
+
+constexpr int test_ctzg_1() {
+ int x = 0;
+ [[maybe_unused]] int unused = __builtin_ctzg(1U, increment(x));
nickdesaulniers wrote:
I think you can omit `[[maybe_unused]] int unused =` from each test?
https://github.com/llvm/llvm-project/pull/86742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers closed https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers approved this pull request. thanks! are you able to merge or do you need me to? https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
+ if (BuiltinOp == Builtin::BI__builtin_clzg && E->getNumArgs() > 1) {
+APSInt Fallback;
+if (!EvaluateInteger(E->getArg(1), Fallback, Info))
+ return false;
+return Success(Fallback, E);
+ }
+
+ if (ZeroIsUndefined)
nickdesaulniers wrote:
Sink the definition of `ZeroIsUndefined` closer to the use to reduce its scope.
We might not need to evaluate that expression.
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12410,12 +12420,22 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
case Builtin::BI__builtin_ctz:
case Builtin::BI__builtin_ctzl:
case Builtin::BI__builtin_ctzll:
- case Builtin::BI__builtin_ctzs: {
+ case Builtin::BI__builtin_ctzs:
+ case Builtin::BI__builtin_ctzg: {
APSInt Val;
if (!EvaluateInteger(E->getArg(0), Val, Info))
return false;
-if (!Val)
+
+if (!Val) {
+ if (BuiltinOp == Builtin::BI__builtin_ctzg && E->getNumArgs() > 1) {
+APSInt Fallback;
+if (!EvaluateInteger(E->getArg(1), Fallback, Info))
nickdesaulniers wrote:
Reuse `Val`?
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
+ if (BuiltinOp == Builtin::BI__builtin_clzg && E->getNumArgs() > 1) {
+APSInt Fallback;
+if (!EvaluateInteger(E->getArg(1), Fallback, Info))
nickdesaulniers wrote:
Could just reuse `Val` here; we don't need extra storage for `Fallback`.
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
nickdesaulniers wrote:
though I guess that's basically what you have!
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
nickdesaulniers wrote:
In that case:
```c++
case Builtin::BI__builtin_clzg:
case Builtin::BI__builtin_ctzg: {
APSInt Val;
if (!EvaluateInteger(E->getArg(0), Val, Info))
return false;
if (E->getNumArgs() == 1) {
if (!Val)
return Error(E);
if (BuiltinOp == Builtin::BI__builtin_clzg)
return Success(Val.countl_zero(), E);
return Success(Val.countr_zero(), E);
}
if (!EvaluateInteger(E->getArg(1), Val, Info))
return false;
return Success(Val, E);
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
nickdesaulniers wrote:
ah, no sorry, I see now what you mean.
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
nickdesaulniers wrote:
```c++
if (BuiltinOp == Builtin::BI__builtin_clzg)
return Success(Val.countl_zero(), E);
return Success(Val.countr_zero(), E);
```
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
nickdesaulniers wrote:
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fclzg
> Similar to __builtin_clz, except the argument is type-generic unsigned
> integer (standard, extended or bit-precise) and there is optional second
> argument with int type. No integral argument promotions are performed on the
> first argument. If two arguments are specified, and first argument is 0, the
> result is the second argument. If only one argument is specified and it is 0,
> the result is undefined.
The code as written is doing:
1. evaluate arg 0.
2. if evaluation is 0 and the builtin is clzg and num args > 1
3. evaluate arg 1
Consider that it may be cheaper to evaluate the number of args than the full
subexpression (at least when the sub expression itself is more than just a
literal iteger).
The code can be rewritten as:
```suggestion
case Builtin::BI__builtin_clzg:
case Builtin::BI__builtin_ctzg: {
unsigned ArgToEval = E->getNumArgs() > 1 ? 1 : 0;
APSInt Val;
if (!EvaluateInteger(E->getArg(0), Val, Info))
return false;
if (E->getNumArgs() == 1 && !Val)
return Error(E);
return Success(...
```
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12354,6 +12354,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, case Builtin::BI__builtin_clzl: case Builtin::BI__builtin_clzll: case Builtin::BI__builtin_clzs: + case Builtin::BI__builtin_clzg: nickdesaulniers wrote: d'oh! https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12410,12 +12420,22 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
case Builtin::BI__builtin_ctz:
case Builtin::BI__builtin_ctzl:
case Builtin::BI__builtin_ctzll:
- case Builtin::BI__builtin_ctzs: {
+ case Builtin::BI__builtin_ctzs:
+ case Builtin::BI__builtin_ctzg: {
APSInt Val;
if (!EvaluateInteger(E->getArg(0), Val, Info))
return false;
-if (!Val)
+
+if (!Val) {
+ if (BuiltinOp == Builtin::BI__builtin_ctzg && E->getNumArgs() > 1) {
+APSInt Fallback;
+if (!EvaluateInteger(E->getArg(1), Fallback, Info))
+ return false;
+return Success(Fallback, E);
+ }
nickdesaulniers wrote:
same feedback applies here.
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12354,6 +12354,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, case Builtin::BI__builtin_clzl: case Builtin::BI__builtin_clzll: case Builtin::BI__builtin_clzs: + case Builtin::BI__builtin_clzg: nickdesaulniers wrote: ctzg too? https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
BuiltinOp != Builtin::BI__lzcnt &&
BuiltinOp != Builtin::BI__lzcnt64;
-if (ZeroIsUndefined && !Val)
- return Error(E);
+if (!Val) {
nickdesaulniers wrote:
Hmm...for these 2 type generic builtins, it might be nice to check the number
of arguments first, then call `EvaluateInteger` on the correct arg, rather than
calling `EvaluateInteger`, failing, then calling `EvaluateInteger` again on the
second arg.
https://github.com/llvm/llvm-project/pull/86577
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement constexpr support for __builtin_{clzg, ctzg} (PR #86577)
https://github.com/nickdesaulniers commented: thanks for the patch! https://github.com/llvm/llvm-project/pull/86577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement __builtin_{clzg,ctzg} (PR #83431)
@@ -660,15 +660,23 @@ def Clz : Builtin, BitShort_Int_Long_LongLongTemplate {
let Prototype = "int(unsigned T)";
}
-// FIXME: Add int clzimax(uintmax_t)
+def Clzg : Builtin {
+ let Spellings = ["__builtin_clzg"];
+ let Attributes = [NoThrow, Const, CustomTypeChecking];
nickdesaulniers wrote:
Filed https://github.com/llvm/llvm-project/issues/86549
https://github.com/llvm/llvm-project/pull/83431
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement __builtin_{clzg,ctzg} (PR #83431)
@@ -660,15 +660,23 @@ def Clz : Builtin, BitShort_Int_Long_LongLongTemplate {
let Prototype = "int(unsigned T)";
}
-// FIXME: Add int clzimax(uintmax_t)
+def Clzg : Builtin {
+ let Spellings = ["__builtin_clzg"];
+ let Attributes = [NoThrow, Const, CustomTypeChecking];
nickdesaulniers wrote:
Should `Constexpr` be in this list of attributes?
https://godbolt.org/z/Efd3EW5xE
https://github.com/llvm/llvm-project/pull/83431
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement __builtin_{clzg,ctzg} (PR #83431)
https://github.com/nickdesaulniers closed https://github.com/llvm/llvm-project/pull/83431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement __builtin_{clzg,ctzg} (PR #83431)
nickdesaulniers wrote: @overmighty do you need one of us to commit this for you? Thanks for working on it! https://github.com/llvm/llvm-project/pull/83431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -1398,6 +1400,42 @@ class TargetInfo : public TransferrableTargetInfo,
}
llvm_unreachable("Unexpected SignReturnAddressKeyKind");
}
+
+ public:
+BranchProtectionInfo() = default;
nickdesaulniers wrote:
Do we still need the default constructor, or no?
https://github.com/llvm/llvm-project/pull/82819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -43,4 +49,4 @@
// BTE-NOT: !"sign-return-address-with-bkey"
// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 1}
-void foo() {}
+void foo() {}
nickdesaulniers wrote:
retain newline at EOF
https://github.com/llvm/llvm-project/pull/82819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
@@ -116,37 +116,22 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo
{
if (!FD)
return;
-const auto *TA = FD->getAttr();
-if (TA == nullptr)
- return;
-
-ParsedTargetAttr Attr =
-CGM.getTarget().parseTargetAttr(TA->getFeaturesStr());
-if (Attr.BranchProtection.empty())
- return;
-
-TargetInfo::BranchProtectionInfo BPI;
-StringRef Error;
-(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
- Attr.CPU, BPI, Error);
-assert(Error.empty());
-
-auto *Fn = cast(GV);
-Fn->addFnAttr("sign-return-address", BPI.getSignReturnAddrStr());
-
-if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) {
- Fn->addFnAttr("sign-return-address-key",
-BPI.SignKey == LangOptions::SignReturnAddressKeyKind::AKey
-? "a_key"
-: "b_key");
+TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts());
+
+if (const auto *TA = FD->getAttr()) {
+ ParsedTargetAttr Attr =
+ CGM.getTarget().parseTargetAttr(TA->getFeaturesStr());
+ if (!Attr.BranchProtection.empty()) {
+StringRef Error;
+(void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
+ Attr.CPU, BPI, Error);
+#ifndef NDEBUG
+assert(Error.empty());
+#endif
nickdesaulniers wrote:
No need for the preprocessor guard here, as `assert` only does anything if
NDEBUG is not defined.
[My earlier
suggestion](https://github.com/llvm/llvm-project/pull/82819#discussion_r1508064741)
was to put it around this if statement, but I think that's a bad suggestion
now since we want validateBranchProtection to run regardless of NDEBUG.
https://github.com/llvm/llvm-project/pull/82819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/nickdesaulniers commented: This still has "foo"="true" style function attributes, which are problematic. Is the plan to change that? https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM][AArch64] Alway emit protection attributes for functions. (PR #82819)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/82819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][C23] N3006 Underspecified object declarations (PR #79845)
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c2x -verify %s
+
+/* WG14 N3006: Full
+ * Underspecified object declarations
+ */
+
+struct S1 { int x, y; };// expected-note {{previous definition is
here}}
+union U1 { int a; double b; }; // expected-note {{previous definition is
here}}
+enum E1 { FOO, BAR }; // expected-note {{previous definition is
here}}
+
+auto normal_struct = (struct S1){ 1, 2 };
nickdesaulniers wrote:
consider adding a test using compound literals
```c
auto normal_struct2 = (struct S1) { .x = 1, .y = 2 };
```
(I'm surprised we don't currently support this (`normal_struct2` or
`normal_struct`). That alone seems like an improvement.
https://github.com/llvm/llvm-project/pull/79845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] -fsanitize=undefined: don't expand to signed-integer-overflow if -fwrapv (PR #85501)
nickdesaulniers wrote: What's up with the commit message on this PR? Why is it so different from the PR description? https://github.com/llvm/llvm-project/pull/85501 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AArch64] Enable fp128 for aarch64 linux target (PR #85070)
nickdesaulniers wrote: Right, I'm more interested in being able to use on aarch64 the `_Float128` type as standardized by C23, than the clang specific `__float128` extension. Does this PR help get us there? https://github.com/llvm/llvm-project/pull/85070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement __builtin_{clzg,ctzg} (PR #83431)
nickdesaulniers wrote: @efriedma-quic parting thoughts here? https://github.com/llvm/llvm-project/pull/83431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
nickdesaulniers wrote:
But then we (and GCC and MSVC) support the `[0]` extension syntax. So it's not
like codegen is impossible.
https://godbolt.org/z/PGa9KWGxq
```c
union foo {
int a;
int b[]; // error: flexible array member 'b' in a union is not allowed
};
union bar {
int a;
int b[0]; // works just fine!
};
```
So to me, it seems like the standard can be amended to add the "or unions too"
and already 3 implementations could generate valid code (GCC and clang would
both need their semantic analyses relaxed here, but their codegen seems to work
for the `[0]` extension already).
https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
nickdesaulniers wrote: > C23 6.7.3.2p20: "As a special case, the last member of a structure with more > than one named member may have an incomplete array type; this is called a > flexible array member. ..." Thanks! Yeah, I wonder if that could have been "of a structure _or union_ " (as in was that an intentional or unintentional omission)? https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
nickdesaulniers wrote: > [Since the C standard requires the test case to be > diagnosed](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548#c3) I wonder what part of the spec discusses this? Perhaps it's worth having the citation, just in case it helps revise the spec here one day. https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
nickdesaulniers wrote: > Left my comment on the main list Sorry, I missed this. Where? https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
nickdesaulniers wrote: > Should I update this PR or create a new one? Consider updating this one. Worst case, if my fellow reviewers disagree with my suggestion, there's always `git reflog` for getting back to the initial version. > GCC does not support this in either! Do you have a bug on file with either us or them about this? Please cc me on those if so. https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
nickdesaulniers wrote: > I didn't do this because it seemed like this would change a lot of existing > test cases Can you give some examples of tests that would fail? If we have tests checking that these fail, then perhaps those tests should add `-Werror=pedantic` so that they can continue to fail. Weirder, clang and msvc already support this, but only in C++ mode! msvc supports this in C mode! GCC does not support this in either! C++ mode: https://godbolt.org/z/esjnE7bnY C mode: https://godbolt.org/z/hGE9dhjE1 https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 %s -verify=c -fsyntax-only -fflex-array-extensions
+
+// The test checks that flexible array members do not emit warnings when
+// -fflex-array-extensions when used in a union or alone in a structure.
+
+struct already_hidden {
+ int a;
nickdesaulniers wrote:
```sh
$ cat clang/test/.clang-format
DisableFormat: true
```
:(
https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 %s -verify=c -fsyntax-only -fflex-array-extensions
+
+// The test checks that flexible array members do not emit warnings when
+// -fflex-array-extensions when used in a union or alone in a structure.
+
+struct already_hidden {
+ int a;
nickdesaulniers wrote:
Please run `git clang-format HEAD~` on your commit. This looks like kernel
coding style and not llvm coding style (which is used even for unit tests).
https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
https://github.com/nickdesaulniers edited https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
https://github.com/nickdesaulniers commented: Rather than have a `-f` flag to opt into this extension, I think instead you should just make it always available, then have tests that it can be used, but will trigger diagnostics under `-Wpedantic` since it's technically a language extension (IIUC). https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
nickdesaulniers wrote: > Does anyone understand why Linux uses __builtin_return_address there? Grep turns up 257 callers of `__builtin_return_address` in the linux kernel sources. Hard to say definitely for all cases. It's possible that some are incorrect, or should have their callers explicitly marked `always_inline` or `noinline`. > Have we consider the alternative of just disabling inlining when a function > uses __builtin_return_address? Is that what GCC does ([Looks like no](https://godbolt.org/z/8zKfGKrzq))? Perhaps useful to match behavior of this compiler builtin, if so. Or document ways in which we intentionally diverge. > Is there any case in Linux where it's a correctness issue to use a caller's > address? 257 call sites. hard to say https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
nickdesaulniers wrote: > I'm seeing evidence that this might be a chatty diagnostic in practice: > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/kernel/fork.c?L311 > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/mm/util.c?L644 > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/arch/arm/mm/nommu.c?L224 > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/kernel/scs.c?L48 > (and quite a few others). > > CC @nathanchance @nickdesaulniers @rjmccall for opinions > > If we continue to move forward with the patch, you should add a release note > to `clang/docs/ReleaseNotes.rst` so users know about the new diagnostic. There's definitely cases where `__builtin_frame_address` is used in callers attributes `__attribute__((always_inline))` within the Linux kernel sources. Perhaps we should diagnose when the caller is neither noinline or always_inline, since then inline substitution will depend on optimizations? https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
