[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D138846#4657246 , @hans wrote:

>> I just saw @glandium's earlier comment:
>>
>>> Code built with older versions of LLVM (e.g. rust) and running with the 
>>> updated runtime crash at startup with this change.
>>
>> This is the exact same issue we encountered. Because there is a profile 
>> format change, it's expected to update both clang and rustc to use the same 
>> version of llvm in order for it to work.
>
> Thanks for figuring this out!
>
> Would it be possible to somehow make profile format mismatches a linker error 
> instead of a hard-to-debug runtime problem? For example could the 
> instrumentation depend on some symbol in the runtime whose name includes a 
> version number?
>
> I think the ASan (and maybe other sanitizer) instrumentations do this.

Te detect incompatibilities, we can either rename the metadata section or add a 
version detector field either to the format header or in individual datum.

Renaming seems infeasible since a lot of places need to be modified (difficult 
to make an atomic update everywhere).

This patch does introduce a new version: `Version11` and llvm-profdata will 
seem to report errors for older versions.
Does the Rust side fail to report a version mismatch error?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138846/new/

https://reviews.llvm.org/D138846

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:25
+
+static bool getPIE(const ArgList , const ToolChain ) {
+  if (Args.hasArg(options::OPT_static, options::OPT_shared,

I've simplified `Gnu.cpp` a bit for handling different link modes. 
ae623d16d50c9f12de7ae7ac1aa11c9d6857e081



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:51
+
+  if (IsPIE || IsStaticPIE)
+CmdArgs.push_back("-pie");

See ae623d16d50c9f12de7ae7ac1aa11c9d6857e081


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150930: [Driver] Accept and ignore -fno-lifetime-dse argument

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D150930#4656909 , @brunodf wrote:

> Hi,
>
> I found this review request and I just want to comment that I find it strange 
> that it was rejected.
>
> @MaskRay I understand that using a compile_commands.json configured for gcc 
> with clang-based tools is not a supported use case, but still the clang 
> driver was explicitly designed to emulate gcc: "The clang tool is the 
> compiler driver and front-end, which is designed to be a drop-in replacement 
> for the gcc command" and "The 'clang' driver is designed to work as closely 
> to GCC as possible to maximize portability." are quotes from 
> https://clang.llvm.org/get_started.html
> In that regard, `clang_ignored_gcc_optimization_f_Group` is logical and it 
> includes many options that you cite like `-falign-jumps=`, `-falign-loops=`, 
> `-fmerge-constants`, `-fschedule-insns`, etc.
> Sure, more projects support clang directly now, but I was not aware there is 
> a change in this policy, or that there is a "stop" on adding more options (in 
> that case, it would be consistent that the documentation is adapted to say 
> that clang is only drop-in compatible with some historic version of gcc).

Clang does emulate a lot of options, but this does not mean we will add every 
option GCC supports.
I think this sentence in my previous reply applies:

> For a useful option, we consider implementing it, but I think the bar is 
> higher than appeasing such unsupported use cases

We want that the project and users learn to remove unsupported options.

Nowadays, `clang_ignored_gcc_optimization_f_Group` should probably not be used 
anymore.

> In my view, the main objection to "accept and ignore" a GCC option is when 
> the option provides some guarantees that clang/LLVM cannot uphold. For 
> example, ignoring `-fno-strict-aliasing` would be dangerous if you actually 
> carry out type-based aliasing optimizations, because programs that compile 
> with it likely contain violations of the strict aliasing rules. It seems that 
> `-fno-lifetime-dse` similarly intends to allow violating a language rule. I'm 
> not aware if clang/LLVM contains optimization that exploit this language rule 
> (since the option appears in the context of the LLVM code base itself, and 
> because compiling this code base with clang itself is well tested in many 
> configurations, I suspect not), but if it does (now or in the future), 
> ignoring this option is dangerous.

I think it's possible that LLVM in the future make take advantage of the 
similar idea behind GCC's `-flifetime-dse`, but the semantics may or may not be 
exactly the same.
And we may choose to implement `-fno-lifetime-dse` or not.

MemorySanitizer's error checking is a bit like `-flifetime-dse` 
(https://github.com/llvm/llvm-project/issues/24952).
If we implement `-flifetime-dse`, we may need to think about its interaction 
with MemorySanitizer, which brings more complexity.
Right now, the justification for adding an ignored option is not significant 
enough.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150930/new/

https://reviews.llvm.org/D150930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/AST/Decl.cpp:380
+->getTemplatedDecl()
+->hasAttr();
 }

rjmccall wrote:
> Okay, this change seems wrong.  A visibility attribute on an explicit 
> specialization or instantiation should definitely override everything else.  
> A visibility attribute on a template pattern should only override other 
> visibility information that we would derive from that pattern, like the 
> visibility of the template parameters; it should not override visibility from 
> the template arguments.  You probably need this function to return an enum 
> with three cases: (1) factor in both template arguments and template 
> parameters, (2) factor in only template arguments, and (3) factor in nothing.
> 
> Also, the bug here doesn't seem to be specific to function templates, and you 
> should fix the code for all three paths (function templates, class templates, 
> and variable templates).  Basically we're universally mishandling visibility 
> attributes on template patterns for templates with non-type template 
> parameters with hidden visibility.
I have added test coverage to `test51` in `visibility.cpp` and studied GCC's 
behavior.

Sent https://github.com/llvm/llvm-project/pull/72092 to ignore visibility 
effects from template parameters. It will bring our behavior closer to GCC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154774/new/

https://reviews.llvm.org/D154774

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:607
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not 
valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");

Is this workaround still needed after b0e61de7075942ef5ac8af9ca1ec918317f62152 
(with a test `clang/test/Coverage/unresolved-ctor-expr.cpp`)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147073/new/

https://reviews.llvm.org/D147073

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.h:61
+  bool isPICDefault() const override { return true; }
+  bool isPIEDefault(const llvm::opt::ArgList &) const override { return true; }
+  bool isPICDefaultForced() const override { return false; }

Test that the -cc1 line contains `"-pic-level" "2" "-pic-is-pie"`



Comment at: clang/lib/Driver/ToolChains/Serenity.h:68
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList ) const override {
+return UnwindTableLevel::Asynchronous;
+  }

Test that the -cc1 line contains `"-funwind-tables=2"`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/serenity.cpp:20
+// SERENITY_X86_64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_X86_64: "-pie"
+// SERENITY_X86_64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"

Prefer `-SAME:` whenever applicable

ditto below



Comment at: clang/test/Driver/serenity.cpp:159
+// DEFAULT_LIBCXX: "-z" "pack-relative-relocs"
+// DEFAULT_LIBCXX: "crt0.o" "crti.o" "crtbeginS.o"
+// DEFAULT_LIBCXX: "--push-state"

`TC.GetFilePath("crti.o")` returns a path to `crti.o` if `crti.o` is found, 
otherwise a raw `crti.o` without a patch component. The raw `crti.o` indicates 
that clang driver fails to find `crti.o` in a search path.

I think you need to change `--sysroot=` to pointer to a directory tree in 
`Inputs/` where `crti.o` can be found.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:76
+  if (!IsStatic || IsStaticPIE)
+CmdArgs.push_back("--eh-frame-hdr");
+

ADKaster wrote:
> MaskRay wrote:
> > This is not tested
> Hm. this also seems like incorrect logic. In my next push I will remove this 
> condition around --eh-frame-hdr to match the other ToolChains.
https://maskray.me/blog/2020-11-08-stack-unwinding I have some notes on 
".eh_frame_hdr and PT_GNU_EH_FRAME". 

> Clang and GCC usually pass --eh-frame-hdr to ld, with the exception that gcc 
> -static does not pass --eh-frame-hdr. The difference is a historical choice 
> related to `__register_frame_info`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that all exception objects' destructors are non-throwing

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0a73918bfdd: [ItaniumCXXABI] Add 
-fassume-nothrow-exception-dtor to assume that all… (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D108905?vs=557976=558008#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/Driver/clang-exception-flags.cpp
  clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp

Index: clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only %s -fcxx-exceptions -fassume-nothrow-exception-dtor -verify
+
+namespace test1 {
+template  struct A { A(); ~A(); };
+struct B { ~B() noexcept(false); };
+struct B1 : B {};
+struct B2 { B b; };
+struct C { virtual void f(); } c;
+struct MoveOnly { MoveOnly(); MoveOnly(MoveOnly&&); };
+void run() {
+  throw A();
+  throw B();  // expected-error{{cannot throw object of type 'B' with a potentially-throwing destructor}}
+  throw new B;
+  throw B1(); // expected-error{{cannot throw object of type 'B1' with a potentially-throwing destructor}}
+  B2 b2;
+  throw b2;   // expected-error{{cannot throw object of type 'B2' with a potentially-throwing destructor}}
+  throw c;
+  MoveOnly m;
+  throw m;
+}
+}
Index: clang/test/Driver/clang-exception-flags.cpp
===
--- clang/test/Driver/clang-exception-flags.cpp
+++ clang/test/Driver/clang-exception-flags.cpp
@@ -27,3 +27,6 @@
 // RUN: %clang -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // RUN: %clang -### -target x86_64-sie-ps5 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // PS-OFF-NOT: "-cc1" {{.*}} "-f{{(cxx-)?}}exceptions"
+
+// RUN: %clang -### -fexceptions -fno-assume-nothrow-exception-dtor -fassume-nothrow-exception-dtor %s 2>&1 | FileCheck %s --check-prefix=NOTHROW-DTOR
+// NOTHROW-DTOR: "-cc1"{{.*}} "-fcxx-exceptions" "-fassume-nothrow-exception-dtor"
Index: clang/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -1,5 +1,6 @@
 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,THROWEND
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -fassume-nothrow-exception-dtor -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,NOTHROWEND
 
 namespace std {
 template  struct coroutine_traits;
@@ -49,7 +50,9 @@
   // CHECK: [[DeallocPad]]:
   // CHECK-NEXT: landingpad
   // CHECK-NEXT:   cleanup
-  // CHECK: br label %[[Dealloc:.+]]
+  // THROWEND:br label %[[Dealloc:.+]]
+  // NOTHROWEND:  icmp ne ptr %[[#]], null
+  // NOTHROWEND-NEXT: br i1 %[[#]], label %[[Dealloc:.+]], label
 
   Cleanup cleanup;
   may_throw();
@@ -68,13 +71,15 @@
   // CHECK: [[Catch]]:
   // CHECK:call ptr @__cxa_begin_catch(
   // CHECK:call void @_ZNSt16coroutine_traitsIJvEE12promise_type19unhandled_exceptionEv(
-  // CHECK:invoke void @__cxa_end_catch()
-  // CHECK-NEXT:to label %[[Cont:.+]] unwind
+  // THROWEND:invoke void @__cxa_end_catch()
+  // THROWEND-NEXT: to label %[[Cont:.+]] unwind
+  // NOTHROWEND:  call void @__cxa_end_catch()
+  // NOTHROWEND-NEXT:   br label %[[Cont2:.+]]
 
-  // CHECK: [[Cont]]:
-  // CHECK-NEXT: br label %[[Cont2:.+]]
-  // CHECK: [[Cont2]]:
-  // CHECK-NEXT: br label %[[Cleanup:.+]]
+  // THROWEND:  [[Cont]]:
+  // THROWEND-NEXT:   br label %[[Cont2:.+]]
+  // CHECK: [[Cont2]]:
+  // CHECK-NEXT:  br label %[[Cleanup:.+]]
 
   // CHECK: [[Cleanup]]:
   // CHECK: call void @_ZNSt16coroutine_traitsIJvEE12promise_typeD1Ev(
@@ -82,8 +87,8 @@
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem0]]
 
   // CHECK: [[Dealloc]]:
-  // CHECK:   %[[Mem:.+]] = call ptr @llvm.coro.free(
-  // CHECK:   call void @_ZdlPv(ptr noundef %[[Mem]])
+  

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:164
+if (crtend_path.empty()) {
+  const char *crtend = (IsShared || IsPIE) ? "crtendS.o" : "crtend.o";
+  crtend_path = TC.GetFilePath(crtend);

crt* files are not tested


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/serenity.cpp:2
+// UNSUPPORTED: system-windows
+
+/// Test a cross compiler.

https://github.com/MaskRay/Config/wiki/LLVM#clanglibdriver

The test fails in a 
`-DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind 
-DCLANG_DEFAULT_CXX_STDLIB=libc++ ` build



Comment at: clang/test/Driver/serenity.cpp:35
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^./]+}}.o

https://github.com/MaskRay/Config/wiki/LLVM#clanglibdriver

The `NOT -l`test fails in a 
`-DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind 
-DCLANG_DEFAULT_CXX_STDLIB=libc++ ` build


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:31
+  Arg *Last = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
+  options::OPT_nopie);
+  return Last ? Last->getOption().matches(options::OPT_pie) : true;

`-no-pie` is nowadays canonical and GCC does't support `-nopie`. You can drop 
`-nopie`.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:76
+  if (!IsStatic || IsStaticPIE)
+CmdArgs.push_back("--eh-frame-hdr");
+

This is not tested



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("pack-relative-relocs");
+

This is untested



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:211
+  options::OPT_fno_use_init_array, true))
+CC1Args.push_back("-fno-use-init-array");
+}

This is for systems that historically support .ctors/.dtors 
https://maskray.me/blog/2021-11-07-init-ctors-init-array

If Serenity doesn't, this should be removed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that all exception objects' destructors are non-throwing

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 557976.
MaskRay marked an inline comment as done.
MaskRay added a comment.

remove unused err_ arguments.
fix and test `throw new B`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/Driver/clang-exception-flags.cpp
  clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp

Index: clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only %s -fcxx-exceptions -fassume-nothrow-exception-dtor -verify
+
+namespace test1 {
+template  struct A { A(); ~A(); };
+struct B { ~B() noexcept(false); };
+struct B1 : B {};
+struct B2 { B b; };
+struct C { virtual void f(); } c;
+struct MoveOnly { MoveOnly(); MoveOnly(MoveOnly&&); };
+void run() {
+  throw A();
+  throw B();  // expected-error{{cannot throw object of type 'B' with a potentially-throwing destructor}}
+  throw new B;
+  throw B1(); // expected-error{{cannot throw object of type 'B1' with a potentially-throwing destructor}}
+  B2 b2;
+  throw b2;   // expected-error{{cannot throw object of type 'B2' with a potentially-throwing destructor}}
+  throw c;
+  MoveOnly m;
+  throw m;
+}
+}
Index: clang/test/Driver/clang-exception-flags.cpp
===
--- clang/test/Driver/clang-exception-flags.cpp
+++ clang/test/Driver/clang-exception-flags.cpp
@@ -27,3 +27,6 @@
 // RUN: %clang -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // RUN: %clang -### -target x86_64-sie-ps5 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // PS-OFF-NOT: "-cc1" {{.*}} "-f{{(cxx-)?}}exceptions"
+
+// RUN: %clang -### -fexceptions -fno-assume-nothrow-exception-dtor -fassume-nothrow-exception-dtor %s 2>&1 | FileCheck %s --check-prefix=NOTHROW-DTOR
+// NOTHROW-DTOR: "-cc1"{{.*}} "-fcxx-exceptions" "-fassume-nothrow-exception-dtor"
Index: clang/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -1,5 +1,6 @@
 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,THROWEND
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -fassume-nothrow-exception-dtor -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,NOTHROWEND
 
 namespace std {
 template  struct coroutine_traits;
@@ -49,7 +50,9 @@
   // CHECK: [[DeallocPad]]:
   // CHECK-NEXT: landingpad
   // CHECK-NEXT:   cleanup
-  // CHECK: br label %[[Dealloc:.+]]
+  // THROWEND:br label %[[Dealloc:.+]]
+  // NOTHROWEND:  icmp ne ptr %[[#]], null
+  // NOTHROWEND-NEXT: br i1 %[[#]], label %[[Dealloc:.+]], label
 
   Cleanup cleanup;
   may_throw();
@@ -68,13 +71,15 @@
   // CHECK: [[Catch]]:
   // CHECK:call ptr @__cxa_begin_catch(
   // CHECK:call void @_ZNSt16coroutine_traitsIJvEE12promise_type19unhandled_exceptionEv(
-  // CHECK:invoke void @__cxa_end_catch()
-  // CHECK-NEXT:to label %[[Cont:.+]] unwind
+  // THROWEND:invoke void @__cxa_end_catch()
+  // THROWEND-NEXT: to label %[[Cont:.+]] unwind
+  // NOTHROWEND:  call void @__cxa_end_catch()
+  // NOTHROWEND-NEXT:   br label %[[Cont2:.+]]
 
-  // CHECK: [[Cont]]:
-  // CHECK-NEXT: br label %[[Cont2:.+]]
-  // CHECK: [[Cont2]]:
-  // CHECK-NEXT: br label %[[Cleanup:.+]]
+  // THROWEND:  [[Cont]]:
+  // THROWEND-NEXT:   br label %[[Cont2:.+]]
+  // CHECK: [[Cont2]]:
+  // CHECK-NEXT:  br label %[[Cleanup:.+]]
 
   // CHECK: [[Cleanup]]:
   // CHECK: call void @_ZNSt16coroutine_traitsIJvEE12promise_typeD1Ev(
@@ -82,8 +87,8 @@
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem0]]
 
   // CHECK: [[Dealloc]]:
-  // CHECK:   %[[Mem:.+]] = call ptr @llvm.coro.free(
-  // CHECK:   call void @_ZdlPv(ptr noundef %[[Mem]])
+  // THROWEND:   %[[Mem:.+]] = call ptr @llvm.coro.free(
+  // THROWEND:   call void @_ZdlPv(ptr noundef %[[Mem]])
 

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that all exception objects' destructors are non-throwing

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 557975.
MaskRay marked an inline comment as done.
MaskRay retitled this revision from "[ItaniumCXXABI] Add 
-fassume-nothrow-exception-dtor to assume that an exception object' destructor 
is nothrow" to "[ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume 
that all exception objects' destructors are non-throwing".
MaskRay edited the summary of this revision.
MaskRay added a comment.

Add a diagnostic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/Driver/clang-exception-flags.cpp
  clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp

Index: clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/assume-nothrow-exception-dtor.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only %s -fcxx-exceptions -fassume-nothrow-exception-dtor -verify
+
+namespace test1 {
+struct A {};
+struct B { ~B() noexcept(false); };
+struct B1 : B {};
+struct B2 { B b; };
+struct C { virtual void f(); } c;
+void run() {
+  throw A();
+  throw B();  // expected-error{{thrown object has a potentially-throwing destructor}}
+  throw B1(); // expected-error{{thrown object has a potentially-throwing destructor}}
+  throw B2(); // expected-error{{thrown object has a potentially-throwing destructor}}
+  throw c;
+}
+}
Index: clang/test/Driver/clang-exception-flags.cpp
===
--- clang/test/Driver/clang-exception-flags.cpp
+++ clang/test/Driver/clang-exception-flags.cpp
@@ -27,3 +27,6 @@
 // RUN: %clang -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // RUN: %clang -### -target x86_64-sie-ps5 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // PS-OFF-NOT: "-cc1" {{.*}} "-f{{(cxx-)?}}exceptions"
+
+// RUN: %clang -### -fexceptions -fno-assume-nothrow-exception-dtor -fassume-nothrow-exception-dtor %s 2>&1 | FileCheck %s --check-prefix=NOTHROW-DTOR
+// NOTHROW-DTOR: "-cc1"{{.*}} "-fcxx-exceptions" "-fassume-nothrow-exception-dtor"
Index: clang/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -1,5 +1,6 @@
 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,THROWEND
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -fassume-nothrow-exception-dtor -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,NOTHROWEND
 
 namespace std {
 template  struct coroutine_traits;
@@ -49,7 +50,9 @@
   // CHECK: [[DeallocPad]]:
   // CHECK-NEXT: landingpad
   // CHECK-NEXT:   cleanup
-  // CHECK: br label %[[Dealloc:.+]]
+  // THROWEND:br label %[[Dealloc:.+]]
+  // NOTHROWEND:  icmp ne ptr %[[#]], null
+  // NOTHROWEND-NEXT: br i1 %[[#]], label %[[Dealloc:.+]], label
 
   Cleanup cleanup;
   may_throw();
@@ -68,13 +71,15 @@
   // CHECK: [[Catch]]:
   // CHECK:call ptr @__cxa_begin_catch(
   // CHECK:call void @_ZNSt16coroutine_traitsIJvEE12promise_type19unhandled_exceptionEv(
-  // CHECK:invoke void @__cxa_end_catch()
-  // CHECK-NEXT:to label %[[Cont:.+]] unwind
+  // THROWEND:invoke void @__cxa_end_catch()
+  // THROWEND-NEXT: to label %[[Cont:.+]] unwind
+  // NOTHROWEND:  call void @__cxa_end_catch()
+  // NOTHROWEND-NEXT:   br label %[[Cont2:.+]]
 
-  // CHECK: [[Cont]]:
-  // CHECK-NEXT: br label %[[Cont2:.+]]
-  // CHECK: [[Cont2]]:
-  // CHECK-NEXT: br label %[[Cleanup:.+]]
+  // THROWEND:  [[Cont]]:
+  // THROWEND-NEXT:   br label %[[Cont2:.+]]
+  // CHECK: [[Cont2]]:
+  // CHECK-NEXT:  br label %[[Cleanup:.+]]
 
   // CHECK: [[Cleanup]]:
   // CHECK: call void @_ZNSt16coroutine_traitsIJvEE12promise_typeD1Ev(
@@ -82,8 +87,8 @@
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem0]]
 
   // CHECK: [[Dealloc]]:
-  // CHECK:   %[[Mem:.+]] = call ptr @llvm.coro.free(
-  // CHECK:   call void @_ZdlPv(ptr noundef %[[Mem]])
+  // THROWEND:   

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D108905#4655694 , @ChuanqiXu wrote:

> Oh, I am not saying the legacy and old comment. I mean you need to touch 
> ReleaseNotes.rst and UserManual.rst since we add a new flag. Also we need 
> either add a TODO/FIXME saying we need to emit an error in Sema if we find 
> the the dtor of the exception we're throwing may throw or implement the 
> semantics actually.

Thanks for the reminder about `ReleaseNotes.rst` and `UsersManual.rst`!

I think many changes don't update `UsersManual.rst` but this option is probably 
quite useful and therefore deserves an entry. Added.
The primary change is to `clang/lib/CodeGen/ItaniumCXXABI.cpp`, which does not 
report warnings.
If we want to implement a warning, we should probably do it in 
clang/lib/Sema/SemaDeclCXX.cpp `Sema::BuildExceptionDeclaration`, which is not 
touched in this file, so a TODO seems not appropriate...

Is the warning to warn about `noexcept(false)` destructor in an 
exception-declaration ? When?
If at catch handlers, unfortunately we are cannot determine the exception 
object for a `catch (...) { ... }` (used by coroutines).
Technically, even if a destructor is `noexcept(false)`, the destructor may not 
throw when `__cxa_end_catch` destroys the object.

So we probably should warn about throw expressions, which can be done in 
`Sema::CheckCXXThrowOperand` and I will investigate it.
However, this warning appears orthogonal to `-fassume-nothrow-exception-dtor`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 557968.
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Update clang/docs/UsersManual.rst and clang/docs/ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/Driver/clang-exception-flags.cpp

Index: clang/test/Driver/clang-exception-flags.cpp
===
--- clang/test/Driver/clang-exception-flags.cpp
+++ clang/test/Driver/clang-exception-flags.cpp
@@ -27,3 +27,6 @@
 // RUN: %clang -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // RUN: %clang -### -target x86_64-sie-ps5 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // PS-OFF-NOT: "-cc1" {{.*}} "-f{{(cxx-)?}}exceptions"
+
+// RUN: %clang -### -fexceptions -fno-assume-nothrow-exception-dtor -fassume-nothrow-exception-dtor %s 2>&1 | FileCheck %s --check-prefix=NOTHROW-DTOR
+// NOTHROW-DTOR: "-cc1"{{.*}} "-fcxx-exceptions" "-fassume-nothrow-exception-dtor"
Index: clang/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -1,5 +1,6 @@
 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,THROWEND
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -fassume-nothrow-exception-dtor -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,NOTHROWEND
 
 namespace std {
 template  struct coroutine_traits;
@@ -49,7 +50,9 @@
   // CHECK: [[DeallocPad]]:
   // CHECK-NEXT: landingpad
   // CHECK-NEXT:   cleanup
-  // CHECK: br label %[[Dealloc:.+]]
+  // THROWEND:br label %[[Dealloc:.+]]
+  // NOTHROWEND:  icmp ne ptr %[[#]], null
+  // NOTHROWEND-NEXT: br i1 %[[#]], label %[[Dealloc:.+]], label
 
   Cleanup cleanup;
   may_throw();
@@ -68,13 +71,15 @@
   // CHECK: [[Catch]]:
   // CHECK:call ptr @__cxa_begin_catch(
   // CHECK:call void @_ZNSt16coroutine_traitsIJvEE12promise_type19unhandled_exceptionEv(
-  // CHECK:invoke void @__cxa_end_catch()
-  // CHECK-NEXT:to label %[[Cont:.+]] unwind
+  // THROWEND:invoke void @__cxa_end_catch()
+  // THROWEND-NEXT: to label %[[Cont:.+]] unwind
+  // NOTHROWEND:  call void @__cxa_end_catch()
+  // NOTHROWEND-NEXT:   br label %[[Cont2:.+]]
 
-  // CHECK: [[Cont]]:
-  // CHECK-NEXT: br label %[[Cont2:.+]]
-  // CHECK: [[Cont2]]:
-  // CHECK-NEXT: br label %[[Cleanup:.+]]
+  // THROWEND:  [[Cont]]:
+  // THROWEND-NEXT:   br label %[[Cont2:.+]]
+  // CHECK: [[Cont2]]:
+  // CHECK-NEXT:  br label %[[Cleanup:.+]]
 
   // CHECK: [[Cleanup]]:
   // CHECK: call void @_ZNSt16coroutine_traitsIJvEE12promise_typeD1Ev(
@@ -82,8 +87,8 @@
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem0]]
 
   // CHECK: [[Dealloc]]:
-  // CHECK:   %[[Mem:.+]] = call ptr @llvm.coro.free(
-  // CHECK:   call void @_ZdlPv(ptr noundef %[[Mem]])
+  // THROWEND:   %[[Mem:.+]] = call ptr @llvm.coro.free(
+  // THROWEND:   call void @_ZdlPv(ptr noundef %[[Mem]])
 
   co_return;
 }
Index: clang/test/CodeGenCXX/exceptions.cpp
===
--- clang/test/CodeGenCXX/exceptions.cpp
+++ clang/test/CodeGenCXX/exceptions.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++98 -o - -fcxx-exceptions -fexceptions | FileCheck -check-prefix=CHECK -check-prefix=CHECK98 %s
-// RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++11 -o - -fcxx-exceptions -fexceptions | FileCheck -check-prefix=CHECK -check-prefix=CHECK11 %s
+// RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++11 -o - -fcxx-exceptions -fexceptions | FileCheck --check-prefixes=CHECK,CHECK11,THROWEND11 %s
+// RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++11 -o - -fcxx-exceptions -fexceptions -fassume-nothrow-exception-dtor | FileCheck --check-prefixes=CHECK,CHECK11,NOTHROWEND11 %s
 
 // CHECK: %[[STRUCT_TEST13_A:.*]] = type 

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4435-4448
   struct CallEndCatch final : EHScopeStack::Cleanup {
 CallEndCatch(bool MightThrow) : MightThrow(MightThrow) {}
 bool MightThrow;
 
 void Emit(CodeGenFunction , Flags flags) override {
   if (!MightThrow) {
 CGF.EmitNounwindRuntimeCall(getEndCatchFn(CGF.CGM));

ChuanqiXu wrote:
> If `__cxa_end_catch ` is nounwind always now, do we need to refactor this 
> one? Like:
> ```
> struct CallEndCatch final : EHScopeStack::Cleanup {
> CallEndCatch() {}
> 
> void Emit(CodeGenFunction , Flags flags) override {
>   CGF.EmitNounwindRuntimeCall(getEndCatchFn(CGF.CGM));
> }
>   };
> ```
Since `__cxa_end_catch` is no longer `nounwind` in the latest revision, we 
cannot simplify the code...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I'll land this patch next week if there is no objection. This seems very useful 
to a few parties and the current behavior is opt-in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.
Herald added subscribers: wangpc, luke, sunshaoce, arichardson.
Herald added a project: All.



Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:40
+ const llvm::opt::ArgList ) {
+  if (Args.getLastArg(options::OPT_gcc_toolchain))
+return true;

This logic (if `--gcc-toolchain=` is available or `lib/crt0.o` is present, 
select `RISCVToolChain`; otherwise `BareMetal`) is strange.

Can someone shed light on what this `RISCVToolChain` is intended and which 
`crt0.o` is used?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91442/new/

https://reviews.llvm.org/D91442

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106371: [AIX] Generate large code model relocations when mcmodel=medium on AIX

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.
Herald added a project: All.



Comment at: clang/test/Driver/mcmodel.c:15
 // LARGE: "-mcmodel=large"
+// AIX-MCMEDIUM-OVERRIDE: "-mcmodel=large"
 

Is it intentional that -mcmodel=medium is changed to large for 32-bit systems? 
Is this AIX specific?

In GCC, I believe in general only 64-bit architectures support `-mcmodel=` with 
some exceptions like nds32.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106371/new/

https://reviews.llvm.org/D106371

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D108905#4654561 , @smeenai wrote:

> This looks great to me, thanks. @rjmccall should sign off on it though.

@rjmccall Are you happy with this opt-in option?

In D108905#4654626 , @ChuanqiXu wrote:

> LGTM since this is exactly what we do in the downstream. The effects of the 
> change in our workloads is 4% size reduction in a coroutine intensive 
> project. (But it requires the coroutine's final suspend can't except via 
> symetric transfer. I was meant to send a paper to WG21 to discuss this).   
> After all, this should be a win for most projects.
>
> We need to mention this in the docs and the ReleaseNotes since we add a new 
> flag.
>
> To make this self contained, we need to add a check in the frontend and emit 
> errors if the compiler find the throwing exception's destructor may throw. 
> This is not required in the current patch but it may be good to add a FIXME 
> or TODO somewhere.
>
> (BTW, the Phab itself is extremly slow now. So if we want more discuss on 
> this, let's move this to Github?)

The slowness was due to a malicious/aggressive crawler. I have fixed it and 
updated php-fpm config:) 
https://discourse.llvm.org/t/phabricator-is-very-slow/73132/14


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100509: Support GCC's -fstack-usage flag

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.
Herald added a subscriber: ormris.
Herald added a project: All.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1208
+
+  *StackUsageStream << MF.getFunction().getParent()->getName();
+  if (const DISubprogram *DSP = MF.getFunction().getSubprogram())

Created https://github.com/llvm/llvm-project/pull/69896 to fix the filename for 
an included file, to address https://github.com/llvm/llvm-project/issues/69889


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100509/new/

https://reviews.llvm.org/D100509

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 557798.
MaskRay retitled this revision from "[ItaniumCXXABI] Make __cxa_end_catch calls 
unconditionally nounwind" to "[ItaniumCXXABI] Add 
-fassume-nothrow-exception-dtor to assume that an exception object' destructor 
is nothrow".
MaskRay edited the summary of this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Switch to opt-in

Add -fassume-nothrow-exception-dtor


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/Driver/clang-exception-flags.cpp

Index: clang/test/Driver/clang-exception-flags.cpp
===
--- clang/test/Driver/clang-exception-flags.cpp
+++ clang/test/Driver/clang-exception-flags.cpp
@@ -27,3 +27,6 @@
 // RUN: %clang -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // RUN: %clang -### -target x86_64-sie-ps5 %s 2>&1 | FileCheck %s -check-prefix=PS-OFF
 // PS-OFF-NOT: "-cc1" {{.*}} "-f{{(cxx-)?}}exceptions"
+
+// RUN: %clang -### -fexceptions -fno-assume-nothrow-exception-dtor -fassume-nothrow-exception-dtor %s 2>&1 | FileCheck %s --check-prefix=NOTHROW-DTOR
+// NOTHROW-DTOR: "-cc1"{{.*}} "-fcxx-exceptions" "-fassume-nothrow-exception-dtor"
Index: clang/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -1,5 +1,6 @@
 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,THROWEND
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -fassume-nothrow-exception-dtor -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,NOTHROWEND
 
 namespace std {
 template  struct coroutine_traits;
@@ -49,7 +50,9 @@
   // CHECK: [[DeallocPad]]:
   // CHECK-NEXT: landingpad
   // CHECK-NEXT:   cleanup
-  // CHECK: br label %[[Dealloc:.+]]
+  // THROWEND:br label %[[Dealloc:.+]]
+  // NOTHROWEND:  icmp ne ptr %[[#]], null
+  // NOTHROWEND-NEXT: br i1 %[[#]], label %[[Dealloc:.+]], label
 
   Cleanup cleanup;
   may_throw();
@@ -64,17 +67,18 @@
   // CHECK-NEXT:   catch ptr null
   // CHECK:  call void @_ZN7CleanupD1Ev(
   // CHECK:  br label %[[Catch:.+]]
-
   // CHECK: [[Catch]]:
   // CHECK:call ptr @__cxa_begin_catch(
   // CHECK:call void @_ZNSt16coroutine_traitsIJvEE12promise_type19unhandled_exceptionEv(
-  // CHECK:invoke void @__cxa_end_catch()
-  // CHECK-NEXT:to label %[[Cont:.+]] unwind
+  // THROWEND:invoke void @__cxa_end_catch()
+  // THROWEND-NEXT: to label %[[Cont:.+]] unwind
+  // NOTHROWEND:  call void @__cxa_end_catch()
+  // NOTHROWEND-NEXT:   br label %[[Cont2:.+]]
 
-  // CHECK: [[Cont]]:
-  // CHECK-NEXT: br label %[[Cont2:.+]]
-  // CHECK: [[Cont2]]:
-  // CHECK-NEXT: br label %[[Cleanup:.+]]
+  // THROWEND: [[Cont]]:
+  // THROWEND-NEXT:   br label %[[Cont2:.+]]
+  // CHECK:[[Cont2]]:
+  // CHECK-NEXT:br label %[[Cleanup:.+]]
 
   // CHECK: [[Cleanup]]:
   // CHECK: call void @_ZNSt16coroutine_traitsIJvEE12promise_typeD1Ev(
@@ -82,8 +86,8 @@
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem0]]
 
   // CHECK: [[Dealloc]]:
-  // CHECK:   %[[Mem:.+]] = call ptr @llvm.coro.free(
-  // CHECK:   call void @_ZdlPv(ptr noundef %[[Mem]])
+  // THROWEND:   %[[Mem:.+]] = call ptr @llvm.coro.free(
+  // THROWEND:   call void @_ZdlPv(ptr noundef %[[Mem]])
 
   co_return;
 }
Index: clang/test/CodeGenCXX/exceptions.cpp
===
--- clang/test/CodeGenCXX/exceptions.cpp
+++ clang/test/CodeGenCXX/exceptions.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++98 -o - -fcxx-exceptions -fexceptions | FileCheck -check-prefix=CHECK -check-prefix=CHECK98 %s
-// RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++11 -o - -fcxx-exceptions -fexceptions | FileCheck -check-prefix=CHECK -check-prefix=CHECK11 %s
+// RUN: %clang_cc1 -no-enable-noundef-analysis %s -triple=x86_64-linux-gnu -emit-llvm -std=c++11 -o - -fcxx-exceptions -fexceptions | FileCheck 

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D108905#4654410 , @smeenai wrote:

> In D108905#4654403 , @ChuanqiXu 
> wrote:
>
>> In D108905#4654393 , @smeenai 
>> wrote:
>>
>>> In D108905#2975712 , @rsmith 
>>> wrote:
>>>
 No decision as yet, but so far it looks very likely that we'll settle on 
 the rule that exceptions cannot have potentially-throwing destructors, and 
 that we should reject `throw`s of such types. I don't think that should be 
 applied retroactively to C++98, though, because destructors were not 
 implicitly non-throwing back then.
>>>
>>> Were there any updates here? This seems like a useful change for us, and I 
>>> was wondering if it could be unblocked now.
>>
>> What @rsmith talked about is in the standard-wise. I mean, if you feel this 
>> is useful, it is possible to add an compiler option to enable this. I've 
>> filed an issue to track this: 
>> https://github.com/llvm/llvm-project/issues/57375. My plan was to make this 
>> happen this year.
>
> Yup, an option to enable this would be great for us.

I am happy to revive the patch with an option? Any suggestion to the name? CC1 
or Driver?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108905/new/

https://reviews.llvm.org/D108905

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 557659.
MaskRay added a comment.

test rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152279/new/

https://reviews.llvm.org/D152279

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c


Index: clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
===
--- clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
+++ clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
@@ -32,14 +32,14 @@
 
 void test(void) {}
 
-// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 0}
 // RV32-G4:  !{i32 8, !"SmallDataLimit", i32 4}
 // RV32-S0:  !{i32 8, !"SmallDataLimit", i32 0}
 // RV32-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
 // RV32-T16: !{i32 8, !"SmallDataLimit", i32 16}
 // RV32-PIC: !{i32 8, !"SmallDataLimit", i32 0}
 
-// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 0}
 // RV64-G4:  !{i32 8, !"SmallDataLimit", i32 4}
 // RV64-S0:  !{i32 8, !"SmallDataLimit", i32 0}
 // RV64-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2107,12 +2107,11 @@
   const Driver  = TC.getDriver();
   const llvm::Triple  = TC.getTriple();
   // Default small data limitation is eight.
-  const char *SmallDataLimit = "8";
+  const char *SmallDataLimit = "0";
   // Get small data limitation.
   if (Args.getLastArg(options::OPT_shared, options::OPT_fpic,
   options::OPT_fPIC)) {
 // Not support linker relaxation for PIC.
-SmallDataLimit = "0";
 if (Args.hasArg(options::OPT_G)) {
   D.Diag(diag::warn_drv_unsupported_sdata);
 }
@@ -2120,7 +2119,6 @@
  .equals_insensitive("large") &&
  (Triple.getArch() == llvm::Triple::riscv64)) {
 // Not support linker relaxation for RV64 with large code model.
-SmallDataLimit = "0";
 if (Args.hasArg(options::OPT_G)) {
   D.Diag(diag::warn_drv_unsupported_sdata);
 }


Index: clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
===
--- clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
+++ clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
@@ -32,14 +32,14 @@
 
 void test(void) {}
 
-// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 0}
 // RV32-G4:  !{i32 8, !"SmallDataLimit", i32 4}
 // RV32-S0:  !{i32 8, !"SmallDataLimit", i32 0}
 // RV32-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
 // RV32-T16: !{i32 8, !"SmallDataLimit", i32 16}
 // RV32-PIC: !{i32 8, !"SmallDataLimit", i32 0}
 
-// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 0}
 // RV64-G4:  !{i32 8, !"SmallDataLimit", i32 4}
 // RV64-S0:  !{i32 8, !"SmallDataLimit", i32 0}
 // RV64-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2107,12 +2107,11 @@
   const Driver  = TC.getDriver();
   const llvm::Triple  = TC.getTriple();
   // Default small data limitation is eight.
-  const char *SmallDataLimit = "8";
+  const char *SmallDataLimit = "0";
   // Get small data limitation.
   if (Args.getLastArg(options::OPT_shared, options::OPT_fpic,
   options::OPT_fPIC)) {
 // Not support linker relaxation for PIC.
-SmallDataLimit = "0";
 if (Args.hasArg(options::OPT_G)) {
   D.Diag(diag::warn_drv_unsupported_sdata);
 }
@@ -2120,7 +2119,6 @@
  .equals_insensitive("large") &&
  (Triple.getArch() == llvm::Triple::riscv64)) {
 // Not support linker relaxation for RV64 with large code model.
-SmallDataLimit = "0";
 if (Args.hasArg(options::OPT_G)) {
   D.Diag(diag::warn_drv_unsupported_sdata);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159541: [UEFI] X86_64 UEFI Clang Driver

[Fangrui Song via Phabricator via cfe-commits]

MaskRay requested changes to this revision.
MaskRay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:68
+  // "Terminal Service Aware" flag is not needed for UEFI applications.
+  CmdArgs.push_back("-tsaware:no");
+

These options are not tested.



Comment at: clang/test/Driver/uefi.c:4
+// RUN: | FileCheck -check-prefixes=CHECK %s
+// RUN: %clang -### %s --target=x86_64-uefi \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \

This RUN line is not needed. If you want to check that x86_64-uefi normalizes 
to unknown, you can add a unittest instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159541/new/

https://reviews.llvm.org/D159541

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:340
+  HasRocThrustLibrary = !HIPRocThrustPathArg.empty() &&
+D.getVFS().exists(HIPRocThrustPathArg + "/thrust");
+  HIPRocPrimPathArg =

AlexVlx wrote:
> MaskRay wrote:
> > Prefer `concat` so that if `HIPRocThrustPathArg` ends with `/`, we will not 
> > get `...//thrust`.
> If you don't mind, I'll do this in a subsequent patch since it's just 
> annoying and not an outright error. The issue being that 
> RocmInstallationDetector is not a ToolChain and isn't `friend`ly to one 
> either, so `concat` is inaccessible. Addressing that would add a bit of 
> unrelated noise (looking through the file we probably want concat in more 
> places).
Don't mind:)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155775/new/

https://reviews.llvm.org/D155775

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1281
+"rocThrust path, required by the HIP Standard Parallel Algorithm "
+"Acceleration library, used to implicitly include the rocThrust library.">;
+def hipstdpar_prim_path_EQ : Joined<["--"], "hipstdpar-prim-path=">,

The convention is to omit the trailing dot.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:340
+  HasRocThrustLibrary = !HIPRocThrustPathArg.empty() &&
+D.getVFS().exists(HIPRocThrustPathArg + "/thrust");
+  HIPRocPrimPathArg =

Prefer `concat` so that if `HIPRocThrustPathArg` ends with `/`, we will not get 
`...//thrust`.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6566
   options::OPT_fno_gpu_allow_device_init);
+if (Args.hasFlag(options::OPT_fgpu_allow_device_init,
+ options::OPT_fno_gpu_allow_device_init, false))

Note that I have refactored the code to use `addOptInFlag`. You need to remove 
the repeated OPT_fgpu_allow_device_init checking.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155775/new/

https://reviews.llvm.org/D155775

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Other than -Wframe-larger-than=656 and `const uptr indicator = 
0x0f00ull;` changes, others looks good to me.
I agree that this should be reviewed by @dvyukov :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform.h:971
 };
-const uptr indicator = 0x0e00ull;
+const uptr indicator = 0x0f00ull;
 const uptr ind_lsb = 1ull << LeastSignificantSetBitIndex(indicator);

alexfanqi wrote:
> jrtc27 wrote:
> > ?
> This is changed for sv48. Tsan compresses the address to 44 bits and uses the 
> top 3 bits (42-44) to uncompress it by comparing with the corresponding bits 
> in the mapping. So the 42-44th bits of each app mapping range must be all 
> different from each other.  But 3 bits are not enough to distinguish mappings 
> in this scheme of MappingRiscv64_48, so I increment it to 4 bits. An 
> alternative of this would be reducing app mapping size by half.
@dvyukov ^


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158641: [AArch64] Fix FMV ifunc resolver usage on old Android APIs. Rename internal compiler-rt FMV functions.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

The Clang Driver/CodeGen and compiler-rt changes look good to me. Of course 
please wait for an Android reviewer :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158641/new/

https://reviews.llvm.org/D158641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155769: [HIP][Clang][docs][RFC] Add documentation for C++ Parallel Algorithm Offload

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/docs/HIPSupport.rst:266
+
+- ``-hipstdpar`` / ``--hipstdpar`` enables algorithm offload, which depending 
on
+  phase, has the following effects:

Newer long options that don't use the common prefix like `-f` are preferred to 
only support `--foo`, not `-foo`.
Many `--hip*` options don't support `-hip*`. Why add `-hipstdpar`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155769/new/

https://reviews.llvm.org/D155769

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6569
+
+  if (Args.hasArg(options::OPT_hipstdpar_interpose_alloc))
+CmdArgs.push_back("-hipstdpar-interpose-alloc");

This can use AddLastArg.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155775/new/

https://reviews.llvm.org/D155775

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1261
   HelpText<"HIP runtime installation path, used for finding HIP version and 
adding HIP include path.">;
+def hipstdpar : Flag<["-", "--"], "hipstdpar">,
+  Visibility<[ClangOption, CC1Option]>,

Accept just `--` for new options. See `--hip-path=`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155775/new/

https://reviews.llvm.org/D155775

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: llvm/unittests/IR/DataLayoutTest.cpp:108
 
+TEST(DataLayoutTest, TargetTripleManglingComponent) {
+  Triple TT = Triple("x86_64-unknown-uefi");

UEFI may be a better name in case we test more properties in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152206/new/

https://reviews.llvm.org/D152206

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

DataLayout needs a unittest in llvm/unittests/IR/DataLayoutTest.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152206/new/

https://reviews.llvm.org/D152206

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> So this is truly a test of the ld.otherlinker feature pattern, not some 
> special case driver feature. I guess we should leave the test alone.
> Closing, we left the test alone, it still uses -fuse-ld=lld-link2. Perhaps in 
> the future we should reconsider this, but that's how things stand now, and we 
> aren't going to land this patch as is. + @MaskRay , who has thought about 
> -fuse-ld= semantics.

Yes, we can leave the test alone. The `ld.` convention is mainly for ELF.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69763/new/

https://reviews.llvm.org/D69763

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D147844#4646633 , @MaskRay wrote:

> FWIW: adding parentheses for expressions, such as `overflow = (4608 * 1024 * 
> 1024) ?  4608 * 1024 * 1024 : 0;`, and `results.reason = (actions & 
> _UA_SEARCH_PHASE) ? ... : ...`, looks unnatural and is too noisy to me.

From quuxplusone:

The first expression

  overflow = (4608 * 1024 * 1024) ?  4608 * 1024 * 1024 : 0;

is a tautology; it is not possible for (4608 * 1024 * 1024) to be zero. (If 
it's signed integer overflow, it's UB, not zero; and the compiler knows this.)
But if you make it defined by adding `u`, then IMO it's very little hardship to 
explicitly write the comparison to zero:

  overflow = (4608u * 1024 * 1024) != 0 ?  4608u * 1024 * 1024 : 0;  // OK, no 
UB, no warning

The second expression

  results.reason = (actions & _UA_SEARCH_PHASE) ? ... : ...

should IMHO also be written as

  results.reason = (actions & _UA_SEARCH_PHASE) != 0 ? ... : ...

but I think you would get good support if you proposed that the compiler should 
treat `&` as a "logical operator" in this specific case. This just means 
changing line 9312 to

  return OP->isComparisonOp() || OP->isLogicalOp() || (OP->getOpcode() == 
BO_And);


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147844/new/

https://reviews.llvm.org/D147844

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158476: [driver] Search for compatible Android runtime directories

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

nits




Comment at: clang/docs/ReleaseNotes.rst:295
+  target-specific runtime and standard libraries in directories named after the
+  target (e.g. if you're building with ``-target 
aarch64-none-linux-android21``,
+  Clang will look for ``lib/aarch64-none-linux-android21`` under its resource

`--target=`



Comment at: clang/test/Driver/android-unversioned-fallback-warning.cpp:14
+// Using an unversioned directory for an unversioned triple isn't a warning.
+// RUN: %clang -target aarch64-none-linux-android -ccc-install-dir %t/bin \
+// RUN: -resource-dir %t/resource -### -c %s 2>&1 | \

`--target=`

`-target ` has been deprecated since Clang 3.4


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158476/new/

https://reviews.llvm.org/D158476

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D152279#4638173 , @asb wrote:

> In D152279#4612099 , @craig.topper 
> wrote:
>
>> In D152279#4612087 , @MaskRay 
>> wrote:
>>
>>> I am still interested in moving this forward. What should be done here? If 
>>> the decision is to keep the current odd default 8 for 
>>> `toolchains::RISCVToolChain`, I guess I'll have to take the compromise as 
>>> making a step forward is better than nothing.
>>
>> On 1 RV64 CPU I tried in our RTL simulator, changing from 8 to 0 reduced 
>> dhrystone score by 2.7%. Using 16, or 32 gave the same score as 8. Reducing 
>> 8 to 4 improved the score by 0.5%.
>
> Thanks for testing - do you have a view one way or another on this default? 
> I'm somewhat torn personally.

Is this patch blocked by anything?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152279/new/

https://reviews.llvm.org/D152279

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

FWIW: adding parentheses for expressions, such as `overflow = (4608 * 1024 * 
1024) ?  4608 * 1024 * 1024 : 0;`, and `results.reason = (actions & 
_UA_SEARCH_PHASE) ? ... : ...`, looks unnatural and is too noisy to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147844/new/

https://reviews.llvm.org/D147844

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D130531#4644687 , @aeubanks wrote:

> I'm not understanding why this doesn't also apply to "PIE Level", doesn't it 
> also follow the same reasoning? pic -> PIC is the same as pie -> PIE
>
> e.g. if you merge a small PIC and large PIC file, the resulting file would 
> only be guaranteed to work with a "large PIC executable" (unsure what the 
> right term is) and not a "small PIC executable", so if we say it's a large 
> PIC file, that's wrong since it wouldn't link into a "large PIC executable", 
> so we have to conservatively say it's a small PIC file.
> and s/PIC/PIE for the same argument

"PIE Level" is a bit of a misdesign. We should treat the value as a boolean and 
ignore 1/2 difference.
"PIC Level" decides the small PIC vs large PIC difference, as well as the small 
PIE vs large PIE difference.

I think "PIE Level" should use "Min" as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130531/new/

https://reviews.llvm.org/D130531

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:208
   (`#61334 `_)
+- For function multi-versioning using the ``target`` or ``target_clones``
+  attributes, remove comdat for internal linkage functions.

ilinpv wrote:
> If I read the code right it applies for function multi-versioning using 
> `target_version` and `target_clones` attributes ( AArch64 ) as well, doesn't 
> it?
Yes, this patch fixed `target_version` as well.

Perhaps update_cc_test_checks.py should be updated to test comdat as well.

I'll adjust the release note to:

> For function multi-versioning using the ``target``, ``target_clones``, 
> ``target_version`` ...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158963/new/

https://reviews.llvm.org/D158963

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159373: [clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159373/new/

https://reviews.llvm.org/D159373

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85309: [Driver] Support GNU ld on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:55
+  StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
+  // FIXME: What about -fuse-ld=?
+  return UseLinker == "bfd" || UseLinker == "gld";

`-fuse-ld=` is deprecated by `--ld-path=`. So just drop this FIXME.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:84
+// FIXME: Could also use /usr/bin/gld here.
+return std::string("/usr/gnu/bin/ld");
+




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85309/new/

https://reviews.llvm.org/D85309

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159335: [Object] Change OffloadBinary::write to return SmallString<0>

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf93c271d4cc1: [Object] Change OffloadBinary::write to return 
SmallString0 (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159335/new/

https://reviews.llvm.org/D159335

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  llvm/include/llvm/Object/OffloadBinary.h
  llvm/lib/Object/OffloadBinary.cpp
  llvm/lib/ObjectYAML/OffloadEmitter.cpp
  llvm/unittests/Object/OffloadingTest.cpp

Index: llvm/unittests/Object/OffloadingTest.cpp
===
--- llvm/unittests/Object/OffloadingTest.cpp
+++ llvm/unittests/Object/OffloadingTest.cpp
@@ -43,8 +43,8 @@
   Data.StringData = StringData;
   Data.Image = std::move(ImageData);
 
-  auto BinaryBuffer = OffloadBinary::write(Data);
-
+  auto BinaryBuffer =
+  MemoryBuffer::getMemBufferCopy(OffloadBinary::write(Data));
   auto BinaryOrErr = OffloadBinary::create(*BinaryBuffer);
   if (!BinaryOrErr)
 FAIL();
Index: llvm/lib/ObjectYAML/OffloadEmitter.cpp
===
--- llvm/lib/ObjectYAML/OffloadEmitter.cpp
+++ llvm/lib/ObjectYAML/OffloadEmitter.cpp
@@ -38,14 +38,10 @@
   Member.Content->writeAsBinary(OS);
 Image.Image = MemoryBuffer::getMemBufferCopy(OS.str());
 
-std::unique_ptr Binary = object::OffloadBinary::write(Image);
-
 // Copy the data to a new buffer so we can modify the bytes directly.
-SmallVector NewBuffer;
-std::copy(Binary->getBufferStart(), Binary->getBufferEnd(),
-  std::back_inserter(NewBuffer));
+auto Buffer = object::OffloadBinary::write(Image);
 auto *TheHeader =
-reinterpret_cast([0]);
+reinterpret_cast([0]);
 if (Doc.Version)
   TheHeader->Version = *Doc.Version;
 if (Doc.Size)
@@ -55,7 +51,7 @@
 if (Doc.EntrySize)
   TheHeader->EntrySize = *Doc.EntrySize;
 
-Out.write(NewBuffer.begin(), NewBuffer.size());
+Out.write(Buffer.begin(), Buffer.size());
   }
 
   return true;
Index: llvm/lib/Object/OffloadBinary.cpp
===
--- llvm/lib/Object/OffloadBinary.cpp
+++ llvm/lib/Object/OffloadBinary.cpp
@@ -204,8 +204,7 @@
   new OffloadBinary(Buf, TheHeader, TheEntry));
 }
 
-std::unique_ptr
-OffloadBinary::write(const OffloadingImage ) {
+SmallString<0> OffloadBinary::write(const OffloadingImage ) {
   // Create a null-terminated string table with all the used strings.
   StringTableBuilder StrTab(StringTableBuilder::ELF);
   for (auto  : OffloadingData.StringData) {
@@ -243,7 +242,7 @@
   TheEntry.ImageOffset = BinaryDataSize;
   TheEntry.ImageSize = OffloadingData.Image->getBufferSize();
 
-  SmallVector Data;
+  SmallString<0> Data;
   Data.reserve(TheHeader.Size);
   raw_svector_ostream OS(Data);
   OS << StringRef(reinterpret_cast(), sizeof(Header));
@@ -264,7 +263,7 @@
   OS.write_zeros(TheHeader.Size - OS.tell());
   assert(TheHeader.Size == OS.tell() && "Size mismatch");
 
-  return MemoryBuffer::getMemBufferCopy(OS.str());
+  return Data;
 }
 
 Error object::extractOffloadBinaries(MemoryBufferRef Buffer,
Index: llvm/include/llvm/Object/OffloadBinary.h
===
--- llvm/include/llvm/Object/OffloadBinary.h
+++ llvm/include/llvm/Object/OffloadBinary.h
@@ -18,6 +18,7 @@
 #define LLVM_OBJECT_OFFLOADBINARY_H
 
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/Error.h"
@@ -78,7 +79,7 @@
   static Expected> create(MemoryBufferRef);
 
   /// Serialize the contents of \p File to a binary buffer to be read later.
-  static std::unique_ptr write(const OffloadingImage &);
+  static SmallString<0> write(const OffloadingImage &);
 
   static uint64_t getAlignment() { return 8; }
 
Index: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
===
--- clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
+++ clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
@@ -126,11 +126,11 @@
 ImageBinary.StringData[Key] = Value;
   }
 }
-std::unique_ptr Buffer = OffloadBinary::write(ImageBinary);
-if (Buffer->getBufferSize() % OffloadBinary::getAlignment() != 0)
+llvm::SmallString<0> Buffer = OffloadBinary::write(ImageBinary);
+if (Buffer.size() % OffloadBinary::getAlignment() != 0)
   return createStringError(inconvertibleErrorCode(),
"Offload binary has invalid size alignment");
-OS << Buffer->getBuffer();
+OS << Buffer;
   }
 
   if (Error E = writeFile(OutputFile,
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

[PATCH] D159352: [Driver] Don't default to DWARF 2 on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!




Comment at: clang/test/CodeGen/dwarf-version.c:7
 // RUN: %clang -target x86_64-linux-gnu -gdwarf -S -emit-llvm -o - %s | 
FileCheck %s --check-prefix=VER5
+// RUN: %clang -target i386-pc-solaris -g -S -emit-llvm -o - %s | FileCheck %s 
--check-prefix=VER5
+// RUN: %clang -target i386-pc-solaris -gdwarf -S -emit-llvm -o - %s | 
FileCheck %s --check-prefix=VER5

Switch to `--target=` while moving lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159352/new/

https://reviews.llvm.org/D159352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay abandoned this revision.
MaskRay added a comment.

Obsoleted by D159173 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158688/new/

https://reviews.llvm.org/D158688

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159173: [Driver] Report warnings for unclaimed TargetSpecific options for assembler input

[Fangrui Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe9d454d1c195: [Driver] Report warnings for unclaimed 
TargetSpecific options for assembler… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159173/new/

https://reviews.llvm.org/D159173

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Arch/X86.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aarch64-target-as-march.s
  clang/test/Driver/target-specific.s
  clang/test/Driver/x86-mfpmath.c

Index: clang/test/Driver/x86-mfpmath.c
===
--- clang/test/Driver/x86-mfpmath.c
+++ clang/test/Driver/x86-mfpmath.c
@@ -1,5 +1,5 @@
 // RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
 // CHECK: "-mfpmath" "sse"
 
-/// Don't warn for assembler input.
-// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck %s --check-prefix=WARN
+// WARN: warning: argument unused during compilation: '-mfpmath=sse'
Index: clang/test/Driver/target-specific.s
===
--- /dev/null
+++ clang/test/Driver/target-specific.s
@@ -0,0 +1,12 @@
+/// Check that we report a warning instead of an error for target-specific compilation only options.
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard -c %s 2>&1 | FileCheck %s
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard -c -fno-integrated-as %s 2>&1 | FileCheck %s
+
+/// Report a warning if we perform the link phase.
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard %s 2>&1 | FileCheck %s
+
+// CHECK: warning: argument unused during compilation: '-faddrsig'
+// CHECK: warning: argument unused during compilation: '-mbranch-protection=standard'
+
+/// assembler-with-cpp claims compile only options. Ideally we should emit a warning.
+// RUN: %clang -### -Werror --target=aarch64 -c -faddrsig -mbranch-protection=standard -x assembler-with-cpp %s
Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -35,7 +35,7 @@
 // MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
 
 /// march to compiler and assembler, we choose the one suited to the input file type
-// RUN: not %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
 // RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
 // RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
 // RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -528,7 +528,7 @@
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
-x86::getX86TargetFeatures(D, Triple, Args, Features, ForAS);
+x86::getX86TargetFeatures(D, Triple, Args, Features);
 break;
   case llvm::Triple::hexagon:
 hexagon::getHexagonTargetFeatures(D, Triple, Args, Features);
Index: clang/lib/Driver/ToolChains/Arch/X86.h
===
--- clang/lib/Driver/ToolChains/Arch/X86.h
+++ clang/lib/Driver/ToolChains/Arch/X86.h
@@ -26,7 +26,7 @@
 
 void getX86TargetFeatures(const Driver , const llvm::Triple ,
   const llvm::opt::ArgList ,
-  std::vector , bool ForAS);
+  std::vector );
 
 } // end namespace x86
 } // end namespace target
Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -118,13 +118,7 @@
 
 void x86::getX86TargetFeatures(const Driver , const llvm::Triple ,
const ArgList ,
-   std::vector , bool ForAS) {
-  if (ForAS) {
-// Some target-specific options are only handled in AddX86TargetArgs, which
-// is not called by ClangAs::ConstructJob. Claim them here.
-Args.claimAllArgs(options::OPT_mfpmath_EQ);
-  }
-
+   std::vector ) {
   // Claim and report unsupported -mabi=. Note: we don't support "sysv_abi" or
   

[PATCH] D159173: [Driver] Report warnings for unclaimed TargetSpecific options for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thank you. I am testing more thoroughly. We need this to catch up 17.0.0rc4...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159173/new/

https://reviews.llvm.org/D159173

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159335: [Object] Change OffloadBinary::write to return SmallString<0>

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 555258.
MaskRay added a comment.

update clang-offload-packager


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159335/new/

https://reviews.llvm.org/D159335

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  llvm/include/llvm/Object/OffloadBinary.h
  llvm/lib/Object/OffloadBinary.cpp
  llvm/lib/ObjectYAML/OffloadEmitter.cpp
  llvm/unittests/Object/OffloadingTest.cpp

Index: llvm/unittests/Object/OffloadingTest.cpp
===
--- llvm/unittests/Object/OffloadingTest.cpp
+++ llvm/unittests/Object/OffloadingTest.cpp
@@ -43,8 +43,8 @@
   Data.StringData = StringData;
   Data.Image = std::move(ImageData);
 
-  auto BinaryBuffer = OffloadBinary::write(Data);
-
+  auto BinaryBuffer =
+  MemoryBuffer::getMemBufferCopy(OffloadBinary::write(Data));
   auto BinaryOrErr = OffloadBinary::create(*BinaryBuffer);
   if (!BinaryOrErr)
 FAIL();
Index: llvm/lib/ObjectYAML/OffloadEmitter.cpp
===
--- llvm/lib/ObjectYAML/OffloadEmitter.cpp
+++ llvm/lib/ObjectYAML/OffloadEmitter.cpp
@@ -38,14 +38,10 @@
   Member.Content->writeAsBinary(OS);
 Image.Image = MemoryBuffer::getMemBufferCopy(OS.str());
 
-std::unique_ptr Binary = object::OffloadBinary::write(Image);
-
 // Copy the data to a new buffer so we can modify the bytes directly.
-SmallVector NewBuffer;
-std::copy(Binary->getBufferStart(), Binary->getBufferEnd(),
-  std::back_inserter(NewBuffer));
+auto Buffer = object::OffloadBinary::write(Image);
 auto *TheHeader =
-reinterpret_cast([0]);
+reinterpret_cast([0]);
 if (Doc.Version)
   TheHeader->Version = *Doc.Version;
 if (Doc.Size)
@@ -55,7 +51,7 @@
 if (Doc.EntrySize)
   TheHeader->EntrySize = *Doc.EntrySize;
 
-Out.write(NewBuffer.begin(), NewBuffer.size());
+Out.write(Buffer.begin(), Buffer.size());
   }
 
   return true;
Index: llvm/lib/Object/OffloadBinary.cpp
===
--- llvm/lib/Object/OffloadBinary.cpp
+++ llvm/lib/Object/OffloadBinary.cpp
@@ -204,8 +204,7 @@
   new OffloadBinary(Buf, TheHeader, TheEntry));
 }
 
-std::unique_ptr
-OffloadBinary::write(const OffloadingImage ) {
+SmallString<0> OffloadBinary::write(const OffloadingImage ) {
   // Create a null-terminated string table with all the used strings.
   StringTableBuilder StrTab(StringTableBuilder::ELF);
   for (auto  : OffloadingData.StringData) {
@@ -243,7 +242,7 @@
   TheEntry.ImageOffset = BinaryDataSize;
   TheEntry.ImageSize = OffloadingData.Image->getBufferSize();
 
-  SmallVector Data;
+  SmallString<0> Data;
   Data.reserve(TheHeader.Size);
   raw_svector_ostream OS(Data);
   OS << StringRef(reinterpret_cast(), sizeof(Header));
@@ -264,7 +263,7 @@
   OS.write_zeros(TheHeader.Size - OS.tell());
   assert(TheHeader.Size == OS.tell() && "Size mismatch");
 
-  return MemoryBuffer::getMemBufferCopy(OS.str());
+  return Data;
 }
 
 Error object::extractOffloadBinaries(MemoryBufferRef Buffer,
Index: llvm/include/llvm/Object/OffloadBinary.h
===
--- llvm/include/llvm/Object/OffloadBinary.h
+++ llvm/include/llvm/Object/OffloadBinary.h
@@ -18,6 +18,7 @@
 #define LLVM_OBJECT_OFFLOADBINARY_H
 
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/Error.h"
@@ -78,7 +79,7 @@
   static Expected> create(MemoryBufferRef);
 
   /// Serialize the contents of \p File to a binary buffer to be read later.
-  static std::unique_ptr write(const OffloadingImage &);
+  static SmallString<0> write(const OffloadingImage &);
 
   static uint64_t getAlignment() { return 8; }
 
Index: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
===
--- clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
+++ clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
@@ -126,11 +126,11 @@
 ImageBinary.StringData[Key] = Value;
   }
 }
-std::unique_ptr Buffer = OffloadBinary::write(ImageBinary);
-if (Buffer->getBufferSize() % OffloadBinary::getAlignment() != 0)
+llvm::SmallString<0> Buffer = OffloadBinary::write(ImageBinary);
+if (Buffer.size() % OffloadBinary::getAlignment() != 0)
   return createStringError(inconvertibleErrorCode(),
"Offload binary has invalid size alignment");
-OS << Buffer->getBuffer();
+OS << Buffer;
   }
 
   if (Error E = writeFile(OutputFile,
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- 

[PATCH] D159335: [Object] Change OffloadBinary::write to return SmallString<0>

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: jhuber6, JonChesterfield.
Herald added a subscriber: hiraditya.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

SmallString<0> is more flexible and avoids an unneeded copy in
ObjectYAML/OffloadEmitter.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159335

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  llvm/include/llvm/Object/OffloadBinary.h
  llvm/lib/Object/OffloadBinary.cpp
  llvm/lib/ObjectYAML/OffloadEmitter.cpp
  llvm/unittests/Object/OffloadingTest.cpp

Index: llvm/unittests/Object/OffloadingTest.cpp
===
--- llvm/unittests/Object/OffloadingTest.cpp
+++ llvm/unittests/Object/OffloadingTest.cpp
@@ -43,8 +43,8 @@
   Data.StringData = StringData;
   Data.Image = std::move(ImageData);
 
-  auto BinaryBuffer = OffloadBinary::write(Data);
-
+  auto BinaryBuffer =
+  MemoryBuffer::getMemBufferCopy(OffloadBinary::write(Data));
   auto BinaryOrErr = OffloadBinary::create(*BinaryBuffer);
   if (!BinaryOrErr)
 FAIL();
Index: llvm/lib/ObjectYAML/OffloadEmitter.cpp
===
--- llvm/lib/ObjectYAML/OffloadEmitter.cpp
+++ llvm/lib/ObjectYAML/OffloadEmitter.cpp
@@ -38,14 +38,10 @@
   Member.Content->writeAsBinary(OS);
 Image.Image = MemoryBuffer::getMemBufferCopy(OS.str());
 
-std::unique_ptr Binary = object::OffloadBinary::write(Image);
-
 // Copy the data to a new buffer so we can modify the bytes directly.
-SmallVector NewBuffer;
-std::copy(Binary->getBufferStart(), Binary->getBufferEnd(),
-  std::back_inserter(NewBuffer));
+auto Buffer = object::OffloadBinary::write(Image);
 auto *TheHeader =
-reinterpret_cast([0]);
+reinterpret_cast([0]);
 if (Doc.Version)
   TheHeader->Version = *Doc.Version;
 if (Doc.Size)
@@ -55,7 +51,7 @@
 if (Doc.EntrySize)
   TheHeader->EntrySize = *Doc.EntrySize;
 
-Out.write(NewBuffer.begin(), NewBuffer.size());
+Out.write(Buffer.begin(), Buffer.size());
   }
 
   return true;
Index: llvm/lib/Object/OffloadBinary.cpp
===
--- llvm/lib/Object/OffloadBinary.cpp
+++ llvm/lib/Object/OffloadBinary.cpp
@@ -204,8 +204,7 @@
   new OffloadBinary(Buf, TheHeader, TheEntry));
 }
 
-std::unique_ptr
-OffloadBinary::write(const OffloadingImage ) {
+SmallString<0> OffloadBinary::write(const OffloadingImage ) {
   // Create a null-terminated string table with all the used strings.
   StringTableBuilder StrTab(StringTableBuilder::ELF);
   for (auto  : OffloadingData.StringData) {
@@ -243,7 +242,7 @@
   TheEntry.ImageOffset = BinaryDataSize;
   TheEntry.ImageSize = OffloadingData.Image->getBufferSize();
 
-  SmallVector Data;
+  SmallString<0> Data;
   Data.reserve(TheHeader.Size);
   raw_svector_ostream OS(Data);
   OS << StringRef(reinterpret_cast(), sizeof(Header));
@@ -264,7 +263,7 @@
   OS.write_zeros(TheHeader.Size - OS.tell());
   assert(TheHeader.Size == OS.tell() && "Size mismatch");
 
-  return MemoryBuffer::getMemBufferCopy(OS.str());
+  return Data;
 }
 
 Error object::extractOffloadBinaries(MemoryBufferRef Buffer,
Index: llvm/include/llvm/Object/OffloadBinary.h
===
--- llvm/include/llvm/Object/OffloadBinary.h
+++ llvm/include/llvm/Object/OffloadBinary.h
@@ -18,6 +18,7 @@
 #define LLVM_OBJECT_OFFLOADBINARY_H
 
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/Error.h"
@@ -78,7 +79,7 @@
   static Expected> create(MemoryBufferRef);
 
   /// Serialize the contents of \p File to a binary buffer to be read later.
-  static std::unique_ptr write(const OffloadingImage &);
+  static SmallString<0> write(const OffloadingImage &);
 
   static uint64_t getAlignment() { return 8; }
 
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -184,7 +184,8 @@
   Image.StringData["arch"] = Arch;
   Image.Image = std::move(*ImageOrError);
 
-  std::unique_ptr Binary = OffloadBinary::write(Image);
+  std::unique_ptr Binary =
+  MemoryBuffer::getMemBufferCopy(OffloadBinary::write(Image));
   auto NewBinaryOrErr = OffloadBinary::create(*Binary);
   if (!NewBinaryOrErr)
 return NewBinaryOrErr.takeError();
@@ -909,7 +910,8 @@
 bundleOpenMP(ArrayRef Images) {
   SmallVector> Buffers;
   for (const OffloadingImage  : Images)
-Buffers.emplace_back(OffloadBinary::write(Image));
+Buffers.emplace_back(
+

[PATCH] D85309: [Driver] Support GNU ld on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/hip-link-bundle-archive.hip:59
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
-// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   --target=x86_64-pc-windows-msvc -fuse-ld= \
 // RUN:   -nogpuinc -nogpulib %s -fgpu-rdc -L%t -lhipBundled2 \

Any idea why `-fuse-ld=` is now needed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85309/new/

https://reviews.llvm.org/D85309

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130777: Enable embedded lto for XCOFF.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This needs rebase after fatLTO support.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130777/new/

https://reviews.llvm.org/D130777

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143305: [clang] Fix -Xarch_ for -mllvm and alike

[Fangrui Song via Phabricator via cfe-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: jplehr.

I assume that this is unneeded after :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143305/new/

https://reviews.llvm.org/D143305

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> This is a breaking change since some SCLs might use .* or (abc|def) which are 
> supported regexes but not valid globs. Since we have just cut clang 16.x this 
> is a good time to make this change.

This should be updated, too. 16.x => 17.x


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154014/new/

https://reviews.llvm.org/D154014

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

> [SpecialCaseList] Use Globs instead of Regex

Since Glob is enabled under a non-default header `#!special-case-list-v2`, this 
subject should be changed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154014/new/

https://reviews.llvm.org/D154014

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Basic/Sanitizers.cpp:130
+if (A->getOption().matches(clang::driver::options::OPT_mexecute_only) &&
+llvm::ARM::supportedExecuteOnly(Triple)) {
+  return true;

MaskRay wrote:
> I don't think we need an extra `llvm::ARM::supportedExecuteOnly` check. We 
> just return true when `-mexecute-only` is in effect.
Sorry, I just noticed that this adds a circular (if we consider headers) 
dependency from clangBasic to clangDriver. I'm removing it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158614/new/

https://reviews.llvm.org/D158614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Driver/Options.td:4586
+def print_supported_extensions : Flag<["-", "--"], 
"print-supported-extensions">,
+  Group, Flags<[CC1Option, CoreOption]>,
+  HelpText<"Print supported extensions for RISC-V">,

CC1Option/CoreOption has been removed. Please rebase. `Visibility<[ClangOption, 
CC1Option, CLOption]>,`

Remove `Group`. We don't need a workaround to claim the 
option in the case of `clang -xxx a.o`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146054/new/

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85309: [Driver] Support GNU ld on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:305
 
+bool tools::isLinkerGnuLd(const ToolChain , const ArgList ) {
+  // Only used if targetting Solaris.

I suppose that this should be in a Solaris specific file to indicate that it's 
not for other systems.

GNU ld is almost ubiquitous on Linux and is almost always available at 
/usr/bin/ld (with very few distributions using others linkers by default or 
providing an option).

Detecting linker to affect driver decisions is we Linux are very wary of. We 
are nervous even trying to do some stuff only related to lld.

We likely don't want this function to be in CommonArgs to lure other 
contributors to use.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85309/new/

https://reviews.llvm.org/D85309

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159054: [Driver] Removal of C_INCLUDE_DIRS feature

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D159054#4628278 , @probinson wrote:

> In D159054#4626772 , @brad wrote:
>
>> Just FYI I am not in a rush to commit this. I am posting this more so as a 
>> means of prodding for discussion of the feature.
>
> So far nobody has popped up to say they want it.
>
> @MaskRay I poked around a bit on sourcegraph.com and didn't see any statement 
> about what it actually searches, other than vague "all your repositories" and 
> "all your code." The "your" bit makes me wonder how broad it is really. The 
> website doesn't say.

They index quite a lot: 
https://about.sourcegraph.com/blog/indexing-the-oss-universe-update-more-code-hosts

I also tried using my company's internal code search and I cannot find 
environment variables using `C_INCLUDE_DIRS`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159054/new/

https://reviews.llvm.org/D159054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D152495#4628903 , @smeenai wrote:

> In D152495#4628877 , @hans wrote:
>
>> In D152495#4628870 , @goncharov 
>> wrote:
>>
>>> due to this change we have a enourmous number of new warnings, on the other 
>>> hand -Wunused-variable is a valuable warning. I am not sure what is the 
>>> policy and best practices for warnings but maybe there is a possiblity to 
>>> make a transition period for enabling this type of warning separetely and 
>>> to allow updating existing code?
>>
>> The usual policy is to put new warnings behind new flags so users can 
>> disable them selectively. It gets trickier when it's existing warnings 
>> getting enhanced like this. Would it be possible to put this new 
>> functionality behind a flag?
>
> Yeah, this has been a recurring issue when existing warnings get enhanced. 
> It's often not feasible to fix all new instances right away, but you don't 
> want to disable or `-Wno-error` the warning either cos then new issues can 
> start creeping in, leaving you with no good options.

(I haven't really read the technical side of this patch yet.)

Yes, from me observing how we internally treat such warnings.

> Maybe we should have a policy around even enhancements to existing warnings 
> always going in their own subgroup, so that we can disable them selectively 
> while not regressing anything? @aaron.ballman, what are your thoughts?

Sometimes an improved diagnostic just triggers in very few new places that can 
be easily handled.
Sometimes there are many more places so that it is difficult to fix in a 
satisfactory timeline (as a rolling update user doesn't want to postpone 
releases a long time).

I think sometimes it's just difficult for contributors to figure out whether a 
diagnostic is going to require substantial cleanup or just a little effort. In 
this case rolling update users (like we) providing feedback is useful whether a 
new sub `-Wxxx` is needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152495/new/

https://reviews.llvm.org/D152495

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

[Fangrui Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG651b2fbc1c7e: [CodeGen] Function multi-versioning: 
dont set comdat for internal linkage… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158963/new/

https://reviews.llvm.org/D158963

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-target-clones.c
  clang/test/CodeGen/attr-target-mv.c


Index: clang/test/CodeGen/attr-target-mv.c
===
--- clang/test/CodeGen/attr-target-mv.c
+++ clang/test/CodeGen/attr-target-mv.c
@@ -32,6 +32,13 @@
   return foo();
 }
 
+static int __attribute__((target("arch=meteorlake"))) foo_internal(void) 
{return 15;}
+static int __attribute__((target("default"))) foo_internal(void) { return 2; }
+
+int bar1(void) {
+  return foo_internal();
+}
+
 inline int __attribute__((target("sse4.2"))) foo_inline(void) { return 0; }
 inline int __attribute__((target("arch=sandybridge"))) foo_inline(void);
 inline int __attribute__((target("arch=ivybridge"))) foo_inline(void) {return 
1;}
@@ -128,6 +135,7 @@
 
 
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
+// LINUX: @foo_internal.ifunc = internal ifunc i32 (), ptr 
@foo_internal.resolver
 // LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
 // LINUX: @foo_decls.ifunc = weak_odr ifunc void (), ptr @foo_decls.resolver
 // LINUX: @foo_multi.ifunc = weak_odr ifunc void (i32, double), ptr 
@foo_multi.resolver
@@ -254,6 +262,11 @@
 // WINDOWS: call i32 @foo.sse4.2
 // WINDOWS: call i32 @foo
 
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @foo_internal.resolver() {
+
+// WINDOWS: define internal i32 @foo_internal.resolver() {
+
 // LINUX: define{{.*}} i32 @bar2()
 // LINUX: call i32 @foo_inline.ifunc()
 
Index: clang/test/CodeGen/attr-target-clones.c
===
--- clang/test/CodeGen/attr-target-clones.c
+++ clang/test/CodeGen/attr-target-clones.c
@@ -16,6 +16,7 @@
 // LINUX: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
 // LINUX: @__cpu_features2 = external dso_local global [3 x i32]
 
+// LINUX: @internal.ifunc = internal ifunc i32 (), ptr @internal.resolver
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
 // LINUX: @foo_dupes.ifunc = weak_odr ifunc void (), ptr @foo_dupes.resolver
 // LINUX: @unused.ifunc = weak_odr ifunc void (), ptr @unused.resolver
@@ -23,6 +24,12 @@
 // LINUX: @foo_inline2.ifunc = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
 // LINUX: @foo_used_no_defn.ifunc = weak_odr ifunc i32 (), ptr 
@foo_used_no_defn.resolver
 
+static int __attribute__((target_clones("sse4.2, default"))) internal(void) { 
return 0; }
+int use(void) { return internal(); }
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @internal.resolver() {
+// WINDOWS: define internal i32 @internal() {
+
 int __attribute__((target_clones("sse4.2, default"))) foo(void) { return 0; }
 // LINUX: define {{.*}}i32 @foo.sse4.2.0()
 // LINUX: define {{.*}}i32 @foo.default.1()
@@ -192,7 +199,5 @@
 
 // CHECK: attributes #[[SSE42]] =
 // CHECK-SAME: 
"target-features"="+crc32,+cx8,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
-// CHECK: attributes #[[DEF]] =
-// Don't bother checking features, we verified it is the same as a normal 
function.
 // CHECK: attributes #[[SB]] =
 // CHECK-SAME: 
"target-features"="+avx,+cmov,+crc32,+cx16,+cx8,+fxsr,+mmx,+pclmul,+popcnt,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4052,7 +4052,7 @@
 
 ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
 
-if (supportsCOMDAT())
+if (!ResolverFunc->hasLocalLinkage() && supportsCOMDAT())
   ResolverFunc->setComdat(
   getModule().getOrInsertComdat(ResolverFunc->getName()));
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -205,6 +205,9 @@
 - Clang's ``-Wunused-private-field`` no longer warns on fields whose type is
   declared with ``[[maybe_unused]]``.
   (`#61334 `_)
+- For function multi-versioning using the ``target`` or ``target_clones``
+  attributes, remove comdat for internal linkage functions.
+  (`#65114 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/CodeGen/attr-target-mv.c

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 554756.
MaskRay edited the summary of this revision.
MaskRay added a comment.

add release note


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158963/new/

https://reviews.llvm.org/D158963

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-target-clones.c
  clang/test/CodeGen/attr-target-mv.c


Index: clang/test/CodeGen/attr-target-mv.c
===
--- clang/test/CodeGen/attr-target-mv.c
+++ clang/test/CodeGen/attr-target-mv.c
@@ -32,6 +32,13 @@
   return foo();
 }
 
+static int __attribute__((target("arch=meteorlake"))) foo_internal(void) 
{return 15;}
+static int __attribute__((target("default"))) foo_internal(void) { return 2; }
+
+int bar1(void) {
+  return foo_internal();
+}
+
 inline int __attribute__((target("sse4.2"))) foo_inline(void) { return 0; }
 inline int __attribute__((target("arch=sandybridge"))) foo_inline(void);
 inline int __attribute__((target("arch=ivybridge"))) foo_inline(void) {return 
1;}
@@ -128,6 +135,7 @@
 
 
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
+// LINUX: @foo_internal.ifunc = internal ifunc i32 (), ptr 
@foo_internal.resolver
 // LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
 // LINUX: @foo_decls.ifunc = weak_odr ifunc void (), ptr @foo_decls.resolver
 // LINUX: @foo_multi.ifunc = weak_odr ifunc void (i32, double), ptr 
@foo_multi.resolver
@@ -254,6 +262,11 @@
 // WINDOWS: call i32 @foo.sse4.2
 // WINDOWS: call i32 @foo
 
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @foo_internal.resolver() {
+
+// WINDOWS: define internal i32 @foo_internal.resolver() {
+
 // LINUX: define{{.*}} i32 @bar2()
 // LINUX: call i32 @foo_inline.ifunc()
 
Index: clang/test/CodeGen/attr-target-clones.c
===
--- clang/test/CodeGen/attr-target-clones.c
+++ clang/test/CodeGen/attr-target-clones.c
@@ -16,6 +16,7 @@
 // LINUX: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
 // LINUX: @__cpu_features2 = external dso_local global [3 x i32]
 
+// LINUX: @internal.ifunc = internal ifunc i32 (), ptr @internal.resolver
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
 // LINUX: @foo_dupes.ifunc = weak_odr ifunc void (), ptr @foo_dupes.resolver
 // LINUX: @unused.ifunc = weak_odr ifunc void (), ptr @unused.resolver
@@ -23,6 +24,12 @@
 // LINUX: @foo_inline2.ifunc = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
 // LINUX: @foo_used_no_defn.ifunc = weak_odr ifunc i32 (), ptr 
@foo_used_no_defn.resolver
 
+static int __attribute__((target_clones("sse4.2, default"))) internal(void) { 
return 0; }
+int use(void) { return internal(); }
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @internal.resolver() {
+// WINDOWS: define internal i32 @internal() {
+
 int __attribute__((target_clones("sse4.2, default"))) foo(void) { return 0; }
 // LINUX: define {{.*}}i32 @foo.sse4.2.0()
 // LINUX: define {{.*}}i32 @foo.default.1()
@@ -192,7 +199,5 @@
 
 // CHECK: attributes #[[SSE42]] =
 // CHECK-SAME: 
"target-features"="+crc32,+cx8,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
-// CHECK: attributes #[[DEF]] =
-// Don't bother checking features, we verified it is the same as a normal 
function.
 // CHECK: attributes #[[SB]] =
 // CHECK-SAME: 
"target-features"="+avx,+cmov,+crc32,+cx16,+cx8,+fxsr,+mmx,+pclmul,+popcnt,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4052,7 +4052,7 @@
 
 ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
 
-if (supportsCOMDAT())
+if (!ResolverFunc->hasLocalLinkage() && supportsCOMDAT())
   ResolverFunc->setComdat(
   getModule().getOrInsertComdat(ResolverFunc->getName()));
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -205,6 +205,9 @@
 - Clang's ``-Wunused-private-field`` no longer warns on fields whose type is
   declared with ``[[maybe_unused]]``.
   (`#61334 `_)
+- For function multi-versioning using the ``target`` or ``target_clones``
+  attributes, remove comdat for internal linkage functions.
+  (`#65114 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/CodeGen/attr-target-mv.c
===
--- clang/test/CodeGen/attr-target-mv.c
+++ clang/test/CodeGen/attr-target-mv.c
@@ -32,6 +32,13 @@
   return 

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D158688#4625839 , @MaskRay wrote:

> In D158688#4624267 , @simon_tatham 
> wrote:
>
>> The change LGTM, and "agree with gcc" seems like a reasonable justification 
>> in this case.
>
> Thank you both!
>
>> But I'm curious more generally about what options should / shouldn't be 
>> covered by `-Wunused-command-line-argument`. Doesn't the same reasoning 
>> apply to //most// options that C compilation uses and assembly doesn't? If 
>> you have a command of the form `clang -someoption -c foo.c`, it's surely 
>> //always// convenient for a user to be able to change the `.c` into a `.s`, 
>> or to put a variable list of files on the end of the command line which 
>> might or might not include any `.c` files.
>
> `-Wunused-command-line-argument` does fire for most options when the only 
> input kind is assembly without preprocessing.
> It seems that the diagnostics are for `assembler` input, not 
> `assembler-with-cpp`...
>
>> Why is this option in particular different from others? Is there a 
>> documented policy anywhere?
>
> I am not aware of a documented policy anywhere, but I have some notes on 
> https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument#assembler-input
>  .
>
> Let me ask on Discourse: 
> https://discourse.llvm.org/t/wunused-command-line-argument-for-assembly-files/73111

I think we should do D159173  .. The warnings 
are still useful, but we can do it in Driver.cpp to avoid the `ForAS` hacks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158688/new/

https://reviews.llvm.org/D158688

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159173: [Driver] Report warnings for unlaimed TargetSpecific options for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: jansvoboda11, thesamesam, chill, peter.smith, 
simon_tatham, stuij.
Herald added subscribers: kadircet, kristof.beyls.
Herald added a reviewer: ctetreau.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, ilya-biryukov.
Herald added a project: clang.

This patch amends D151590  to not error for 
unlaimed TargetSpecific
options for `-x assembler` input files. This input type causes Driver to
construct tools::ClangAs (-fintegrated-as) or other assemblers (e.g.
tools::gnutools::Assembler) Their ConstructJobs methods, unlike
Clang::ConstructJobs, claim very few options. If an option is unclaimed,
it either leads to a -Wunused-command-line-argument warning or an error
(if `TargetSpecific` is set):

  % clang '-###' --target=aarch64 -mbranch-protection=bti -c a.s
  clang: error: unsupported option '-mbranch-protection=' for target 'aarch64'

It seems that downgrading the diagnostic to warning is most useful as
many users use CFLAGS even for `.s` files:

  clang --target=aarch64 -mbranch-protection=bti -S a.c
  clang --target=aarch64 -mbranch-protection=bti -c a.s

I decide not to suppress the warning so that
-Wunused-command-line-argument lovers still get a warning, and help
projects use proper ASFLAGS/CFLAGS/etc.

Note: `-mbranch-protection=bti a.S` currently has no warning as `-x 
assembler-with-cpp`
instructs clangDriver to select tools::Clang and claim most options.

Revert D159010  to demonstrate that we emit a 
warning for -mfpmath= for
`-x assembler` input.

Modify my AIX cleanup cd18efb61d759405956dbd30e4b5f2720d8e1783 
 to
add an err_drv_unsupported_opt_for_target.

Planned for main and release/17.x


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159173

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Arch/X86.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aarch64-target-as-march.s
  clang/test/Driver/target-specific.s
  clang/test/Driver/x86-mfpmath.c

Index: clang/test/Driver/x86-mfpmath.c
===
--- clang/test/Driver/x86-mfpmath.c
+++ clang/test/Driver/x86-mfpmath.c
@@ -1,5 +1,5 @@
 // RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
 // CHECK: "-mfpmath" "sse"
 
-/// Don't warn for assembler input.
-// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck %s --check-prefix=WARN
+// WARN: warning: argument unused during compilation: '-mfpmath=sse'
Index: clang/test/Driver/target-specific.s
===
--- /dev/null
+++ clang/test/Driver/target-specific.s
@@ -0,0 +1,12 @@
+/// Check that we report a warning instead of an error for target-specific compilation only options.
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard -c %s 2>&1 | FileCheck %s
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard -c -fno-integrated-as %s 2>&1 | FileCheck %s
+
+/// Report a warning if we perform the link phase.
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard %s 2>&1 | FileCheck %s
+
+// CHECK: warning: argument unused during compilation: '-faddrsig'
+// CHECK: warning: argument unused during compilation: '-mbranch-protection=standard'
+
+/// assembler-with-cpp claims compile only options. Ideally we should emit a warning.
+// RUN: %clang -### -Werror --target=aarch64 -c -faddrsig -mbranch-protection=standard -x assembler-with-cpp %s
Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -35,7 +35,7 @@
 // MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
 
 /// march to compiler and assembler, we choose the one suited to the input file type
-// RUN: not %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
 // RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
 // RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
 // RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ 

[PATCH] D159016: [clang] Fix assertion failure using -MJ with -fsyntax-only

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!




Comment at: clang/test/Driver/compilation_database_fsyntax_only.c:1
+// RUN: mkdir -p %t.workdir && cd %t.workdir
+// RUN: %clang -fsyntax-only %s -MJ - 2>&1 | FileCheck %s

The test can be added into `compilation_database.c`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159016/new/

https://reviews.llvm.org/D159016

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117929: [XRay] Add support for RISCV

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:54
+   // Handler address will be null if it is not set
+   beq a2, x0, FunctionEntry_restore
+

This local symbol doesn't seem useful. We can just use numbers (`1f` and `1:`) 
or `LOCAL_LABEL(...)`, so that the symbol table will not have the unneeded 
symbol entries.



Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:85
+FunctionEntry_end:
+   ASM_SIZE(__xray_FunctionEntry)
+   .cfi_endproc

stray `ASM_SIZE`?



Comment at: compiler-rt/lib/xray/xray_trampoline_riscv64.S:22
+   ASM_TYPE_FUNCTION(__xray_FunctionEntry)
+   ASM_SYMBOL(__xray_FunctionEntry):
+   .cfi_startproc

See the aarch64 implementation to use `ASM_SIZE(__xray_FunctionEntry)` to set 
`.size`



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:185
+if (F.hasFnAttribute("patchable-function-entry")) {
+  break;
+}

omit braces for single-line single statement body.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:164
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {

"does not support compressed instructions" makes this feature infeasible for 
many systems, I'll say probably almost all systems that may consider XRay. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117929/new/

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158376: [Driver] move DragonFly header search path management to the driver

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!




Comment at: clang/lib/Lex/InitHeaderSearch.cpp:331
-  case llvm::Triple::DragonFly:
-AddPath("/usr/include/c++/5.0", CXXSystem, false);
-break;

brad wrote:
> MaskRay wrote:
> > 5.0 becomes 8.0 after move?
> > 5.0 becomes 8.0 after move?
> 
> Read what the summary says. The current path is wrong.
Thank you! If this has been broken for so long, it means they don't care about 
libstdc++? I think we can just remove the include path...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158376/new/

https://reviews.llvm.org/D158376

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158376: [Driver] move DragonFly header search path management to the driver

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Lex/InitHeaderSearch.cpp:331
-  case llvm::Triple::DragonFly:
-AddPath("/usr/include/c++/5.0", CXXSystem, false);
-break;

5.0 becomes 8.0 after move?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158376/new/

https://reviews.llvm.org/D158376

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang/lib/Basic/Sanitizers.cpp:126
+  // execute-only output (no data access to code sections).
+  if (const llvm::opt::Arg *A =
+  Args.getLastArg(clang::driver::options::OPT_mexecute_only,

MaskRay wrote:
> The idiom is `hasFlag(clang::driver::options::OPT_mexecute_only, 
> clang::driver::options::OPT_mno_execute_only, false)`
`return Args.hasFlag(clang::driver::options::OPT_mexecute_only ...`



Comment at: clang/test/Driver/fsanitize.c:986
+// CHECK-UBSAN-FUNCTION-DAG: error: invalid argument '-fsanitize=function' not 
allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED-NOT: error: invalid argument '-fsanitize=function' 
not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED: 
"-fsanitize={{((alignment|array-bounds|bool|builtin|enum|float-cast-overflow|integer-divide-by-zero|nonnull-attribute|null|pointer-overflow|return|returns-nonnull-attribute|shift-base|shift-exponent|signed-integer-overflow|unreachable|vla-bound),?){17}"}}

Delete this `-NOT`. `%clang` returning 0 implies that there is no error.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158614/new/

https://reviews.llvm.org/D158614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157813: [Driver][VE] Support VPU flag as a feature for VE

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/VE.cpp:22
+ std::vector ) {
+  // Defaults.
+  bool EnableVPU = true;

kaz7 wrote:
> MaskRay wrote:
> > Delete the comment. The code speaks itself. 
> > 
> > ```
> > if (Args.hasArg(options::OPT_mvevpu, options::OPT_mno_vevpu, true)
> >   Features.push_back("+vpu");
> > ```
> Thanks.  I remove comments and simplify existing code.
Sorry for my typo. We should use the 3-argument `hasFlag` here. It's simpler 
than getLastArg+matches


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157813/new/

https://reviews.llvm.org/D157813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159137: [AIX] Fix Link Issue when `-fprofile-update=[atomic|prefer-atomic]` is in Effect

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440
+  StringRef Val = A->getValue();
+  if (Val == "atomic" || Val == "prefer-atomic")
+CmdArgs.push_back("-latomic");

qiongsiwu1 wrote:
> This check here is copied from 
> https://github.com/llvm/llvm-project/blob/77596e6b167bf0a5efa790597d6b75ac5e685b55/clang/lib/Driver/ToolChains/Clang.cpp#L867,
>  and I don't think it is good to copy this particular code over because we 
> are doing the same check twice at different places and the code can go out of 
> sync. 
> 
> May I get some suggestions to avoid duplicating the code? Thanks! 
This seems ok to me. You can also use `!= "single"` instead.

`getLastArgNoClaim` is probably slightly better.



Comment at: clang/test/Driver/fprofile-update.c:17
+// AIX specific tests
+// RUN: %if system-aix %{ %clang -### %s -fprofile-generate 
-fprofile-update=atomic 2>&1 | FileCheck %s --check-prefix=AIX %}
+// RUN: %if system-aix %{ %clang -### %s -fprofile-generate 
-fprofile-update=prefer-atomic 2>&1 | FileCheck %s --check-prefix=AIX %}

The idiom is to use `--target=...`. This driver decision is not dependent on 
what OS the host runs, so `%if system-aix` should be avoided.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159137/new/

https://reviews.llvm.org/D159137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/SizedDeallocation.h:23
+namespace clang {
+inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) {
+  switch (OS) {

wangpc wrote:
> MaskRay wrote:
> > Does this need to be in Basic/? It's only used by 
> > clang/lib/Driver/ToolChains/Darwin.cpp
> > 
> > 
> This file is just copied and changed from 
> `clang/include/clang/Basic/AlignedAllocation.h` actually, I don't know which 
> directory is more suitable.
If this is only used by clangDriver, I am not sure we want to expose it as a 
public header under include/.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112921/new/

https://reviews.llvm.org/D112921

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D158688#4624267 , @simon_tatham 
wrote:

> The change LGTM, and "agree with gcc" seems like a reasonable justification 
> in this case.

Thank you both!

> But I'm curious more generally about what options should / shouldn't be 
> covered by `-Wunused-command-line-argument`. Doesn't the same reasoning apply 
> to //most// options that C compilation uses and assembly doesn't? If you have 
> a command of the form `clang -someoption -c foo.c`, it's surely //always// 
> convenient for a user to be able to change the `.c` into a `.s`, or to put a 
> variable list of files on the end of the command line which might or might 
> not include any `.c` files.

`-Wunused-command-line-argument` does fire for most options when the only input 
kind is assembly without preprocessing.
It seems that the diagnostics are for `assembler` input, not 
`assembler-with-cpp`...

> Why is this option in particular different from others? Is there a documented 
> policy anywhere?

I am not aware of a documented policy anywhere, but I have some notes on 
https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument#assembler-input
 .

Let me ask on Discourse: 
https://discourse.llvm.org/t/wunused-command-line-argument-for-assembly-files/73111


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158688/new/

https://reviews.llvm.org/D158688

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159054: [Driver] Removal of C_INCLUDE_DIRS feature

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Technically there is some risk but I think the blast radius, if present, is 
extremely small.

`C_INCLUDE_DIRS` is not recognized by GCC.

If I use 
https://sourcegraph.com/search?q=context:global+%5CbC_INCLUDE_DIRS%3D+-f:clang/=regexp=yes=1=repo
 to find the use cases, one project has a use like add_opt 
C_INCLUDE_DIRS=$C_INCLUDES, but the search patch seems likely redundant.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159054/new/

https://reviews.llvm.org/D159054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158071: [clang] Remove rdar links; NFC

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158071/new/

https://reviews.llvm.org/D158071

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159010: [Driver,X86] Ignore -mfpmath= for assembler input

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG081afa3d04a4: [Driver,X86] Ignore -mfpmath= for assembler 
input (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D159010?vs=554010=554037#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159010/new/

https://reviews.llvm.org/D159010

Files:
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Arch/X86.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/x86-mfpmath.c


Index: clang/test/Driver/x86-mfpmath.c
===
--- /dev/null
+++ clang/test/Driver/x86-mfpmath.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
+// CHECK: "-mfpmath" "sse"
+
+/// Don't warn for assembler input.
+// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 
2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -528,7 +528,7 @@
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
-x86::getX86TargetFeatures(D, Triple, Args, Features);
+x86::getX86TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::hexagon:
 hexagon::getHexagonTargetFeatures(D, Triple, Args, Features);
Index: clang/lib/Driver/ToolChains/Arch/X86.h
===
--- clang/lib/Driver/ToolChains/Arch/X86.h
+++ clang/lib/Driver/ToolChains/Arch/X86.h
@@ -26,7 +26,7 @@
 
 void getX86TargetFeatures(const Driver , const llvm::Triple ,
   const llvm::opt::ArgList ,
-  std::vector );
+  std::vector , bool ForAS);
 
 } // end namespace x86
 } // end namespace target
Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -118,7 +118,13 @@
 
 void x86::getX86TargetFeatures(const Driver , const llvm::Triple ,
const ArgList ,
-   std::vector ) {
+   std::vector , bool ForAS) {
+  if (ForAS) {
+// Some target-specific options are only handled in AddX86TargetArgs, which
+// is not called by ClangAs::ConstructJob. Claim them here.
+Args.claimAllArgs(options::OPT_mfpmath_EQ);
+  }
+
   // Claim and report unsupported -mabi=. Note: we don't support "sysv_abi" or
   // "ms_abi" as default function attributes.
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_mabi_EQ)) {


Index: clang/test/Driver/x86-mfpmath.c
===
--- /dev/null
+++ clang/test/Driver/x86-mfpmath.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
+// CHECK: "-mfpmath" "sse"
+
+/// Don't warn for assembler input.
+// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -528,7 +528,7 @@
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
-x86::getX86TargetFeatures(D, Triple, Args, Features);
+x86::getX86TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::hexagon:
 hexagon::getHexagonTargetFeatures(D, Triple, Args, Features);
Index: clang/lib/Driver/ToolChains/Arch/X86.h
===
--- clang/lib/Driver/ToolChains/Arch/X86.h
+++ clang/lib/Driver/ToolChains/Arch/X86.h
@@ -26,7 +26,7 @@
 
 void getX86TargetFeatures(const Driver , const llvm::Triple ,
   const llvm::opt::ArgList ,
-  std::vector );
+  std::vector , bool ForAS);
 
 } // end namespace x86
 } // end namespace target
Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -118,7 +118,13 @@
 
 void x86::getX86TargetFeatures(const Driver , const llvm::Triple ,
const ArgList ,
-   std::vector ) {
+   std::vector , bool ForAS) {
+  if (ForAS) {
+// Some target-specific options are only handled in AddX86TargetArgs, which
+// is not called by 

[PATCH] D159010: [Driver,X86] Ignore -mfpmath= for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Tested by parona on Libera Chat IRC. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159010/new/

https://reviews.llvm.org/D159010

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: clang/include/clang/Basic/SizedDeallocation.h:23
+namespace clang {
+inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) {
+  switch (OS) {

Does this need to be in Basic/? It's only used by 
clang/lib/Driver/ToolChains/Darwin.cpp





Comment at: clang/include/clang/Basic/SizedDeallocation.h:36
+  case llvm::Triple::ZOS:
+return llvm::VersionTuple(); // All z/OS versions have no support.
+  }

This is major=minor=0, which is probably not desired.

We can just omit ZOS.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112921/new/

https://reviews.llvm.org/D112921

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159010: [Driver,X86] Ignore -mfpmath= for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:121
const ArgList ,
-   std::vector ) {
+   std::vector , bool ForAS) {
+  if (ForAS) {

Note: ClangAs::ConstructJob doesn't call `Clang::RenderTargetOptions` (and 
can't as the function is in `Clang::`). `bool ForAS` is a workaround used by 
ARM and will also be used by AArch64. I plan to clean this up at some point, 
but for release/17.x I intentionally make it simple.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159010/new/

https://reviews.llvm.org/D159010

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159010: [Driver,X86] Ignore -mfpmath= for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: mgorny, thesamesam.
Herald added subscribers: pengfei, kristof.beyls.
Herald added a reviewer: ctetreau.
Herald added a reviewer: ctetreau.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some options are only claimed in AddX86TargetArgs/etc (called by
Clang::RenderTargetOptions).
For assembler input, `Add*TargetArgs` is not called. If an option is
unclaimed, it either leads to a -Wunused-command-line-argument warning
or an error (if `TargetSpecific` is set)

  // clang '-###' --target=x86_64 -mfpmath=sse -c a.s
  clang: error: unsupported option '-mfpmath=sse' for target 'x86_64'

For -mfpmath=, it's actually claimed by RenderFloatingPointOptions,
which should be moved to AddARMTargetArgs/AddX86TargetArgs later
(non-AArch32-non-x86 targets give a frontend error).
This change is localized and similar to D153691 
, for release/17.x
backporting.

Fix https://github.com/llvm/llvm-project/issues/65023


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159010

Files:
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Arch/X86.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/x86-mfpmath.c


Index: clang/test/Driver/x86-mfpmath.c
===
--- /dev/null
+++ clang/test/Driver/x86-mfpmath.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
+// CHECK: "-mfpmath" "sse"
+
+/// Don't warn for assembler input.
+// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 
2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -528,7 +528,7 @@
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
-x86::getX86TargetFeatures(D, Triple, Args, Features);
+x86::getX86TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::hexagon:
 hexagon::getHexagonTargetFeatures(D, Triple, Args, Features);
Index: clang/lib/Driver/ToolChains/Arch/X86.h
===
--- clang/lib/Driver/ToolChains/Arch/X86.h
+++ clang/lib/Driver/ToolChains/Arch/X86.h
@@ -26,7 +26,7 @@
 
 void getX86TargetFeatures(const Driver , const llvm::Triple ,
   const llvm::opt::ArgList ,
-  std::vector );
+  std::vector , bool ForAs);
 
 } // end namespace x86
 } // end namespace target
Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -118,7 +118,13 @@
 
 void x86::getX86TargetFeatures(const Driver , const llvm::Triple ,
const ArgList ,
-   std::vector ) {
+   std::vector , bool ForAS) {
+  if (ForAS) {
+// Some target-specific options are only handled in AddX86TargetArgs, which
+// is not called by ClangAs::ConstructJob. Claim them here.
+Args.claimAllArgs(options::OPT_mfpmath_EQ);
+  }
+
   // Claim and report unsupported -mabi=. Note: we don't support "sysv_abi" or
   // "ms_abi" as default function attributes.
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_mabi_EQ)) {


Index: clang/test/Driver/x86-mfpmath.c
===
--- /dev/null
+++ clang/test/Driver/x86-mfpmath.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
+// CHECK: "-mfpmath" "sse"
+
+/// Don't warn for assembler input.
+// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -528,7 +528,7 @@
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
-x86::getX86TargetFeatures(D, Triple, Args, Features);
+x86::getX86TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::hexagon:
 hexagon::getHexagonTargetFeatures(D, Triple, Args, Features);
Index: clang/lib/Driver/ToolChains/Arch/X86.h
===
--- clang/lib/Driver/ToolChains/Arch/X86.h
+++ clang/lib/Driver/ToolChains/Arch/X86.h
@@ -26,7 +26,7 @@
 
 void getX86TargetFeatures(const Driver , const llvm::Triple ,
   

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> [VE][Clang] Change to enable VPU flag by default

For components like llvm/, clang/, we usually use a more specific tag. I'd pick 
`[Driver]` over `[Clang]`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157813/new/

https://reviews.llvm.org/D157813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/Arch/VE.cpp:22
+ std::vector ) {
+  // Defaults.
+  bool EnableVPU = true;

Delete the comment. The code speaks itself. 

```
if (Args.hasArg(options::OPT_mvevpu, options::OPT_mno_vevpu, true)
  Features.push_back("+vpu");
```



Comment at: clang/test/Driver/ve-features.c:1
+// RUN: %clang -target ve-unknown-linux-gnu -### %s -mvevpu 2>&1 | FileCheck 
%s -check-prefix=VEVPU
+// RUN: %clang -target ve-unknown-linux-gnu -### %s -mno-vevpu 2>&1 | 
FileCheck %s -check-prefix=NO-VEVPU

kaz7 wrote:
> MaskRay wrote:
> > `-target ` has been deprecated since Clang 3.4. Use `--target=`
> I didn't know that.  Thank you!
I think we typically spend just two RUN lines:

* one for the default
* one for `-mvevpu -mno-vevpu`

The additional coverage for having 3 RUN lines is probably not useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157813/new/

https://reviews.llvm.org/D157813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Ping:) This is probably a good candidate for `release/17.x` backporting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158688/new/

https://reviews.llvm.org/D158688

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158955: [Driver] Improve legibility of ld -z options on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158955/new/

https://reviews.llvm.org/D158955

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: erichkeane, ilinpv, pengfei, RKSimon.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For function multi-versioning using the target and target_clones
function attributes, currently we incorrectly set comdat for internal
linkage functions. This is problematic for ELF linkers
as GRP_COMDAT deduplication will kick in even with STB_LOCAL signature
(https://groups.google.com/g/generic-abi/c/2X6mR-s2zoc
"GRP_COMDAT group with STB_LOCAL signature").

In short, two `__attribute((target_clones(...))) static void foo()`
in two translation units will be deduplicated. Fix this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158963

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-target-clones.c
  clang/test/CodeGen/attr-target-mv.c


Index: clang/test/CodeGen/attr-target-mv.c
===
--- clang/test/CodeGen/attr-target-mv.c
+++ clang/test/CodeGen/attr-target-mv.c
@@ -32,6 +32,13 @@
   return foo();
 }
 
+static int __attribute__((target("arch=meteorlake"))) foo_internal(void) 
{return 15;}
+static int __attribute__((target("default"))) foo_internal(void) { return 2; }
+
+int bar1(void) {
+  return foo_internal();
+}
+
 inline int __attribute__((target("sse4.2"))) foo_inline(void) { return 0; }
 inline int __attribute__((target("arch=sandybridge"))) foo_inline(void);
 inline int __attribute__((target("arch=ivybridge"))) foo_inline(void) {return 
1;}
@@ -128,6 +135,7 @@
 
 
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
+// LINUX: @foo_internal.ifunc = internal ifunc i32 (), ptr 
@foo_internal.resolver
 // LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
 // LINUX: @foo_decls.ifunc = weak_odr ifunc void (), ptr @foo_decls.resolver
 // LINUX: @foo_multi.ifunc = weak_odr ifunc void (i32, double), ptr 
@foo_multi.resolver
@@ -254,6 +262,11 @@
 // WINDOWS: call i32 @foo.sse4.2
 // WINDOWS: call i32 @foo
 
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @foo_internal.resolver() {
+
+// WINDOWS: define internal i32 @foo_internal.resolver() {
+
 // LINUX: define{{.*}} i32 @bar2()
 // LINUX: call i32 @foo_inline.ifunc()
 
Index: clang/test/CodeGen/attr-target-clones.c
===
--- clang/test/CodeGen/attr-target-clones.c
+++ clang/test/CodeGen/attr-target-clones.c
@@ -16,6 +16,7 @@
 // LINUX: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
 // LINUX: @__cpu_features2 = external dso_local global [3 x i32]
 
+// LINUX: @internal.ifunc = internal ifunc i32 (), ptr @internal.resolver
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
 // LINUX: @foo_dupes.ifunc = weak_odr ifunc void (), ptr @foo_dupes.resolver
 // LINUX: @unused.ifunc = weak_odr ifunc void (), ptr @unused.resolver
@@ -23,6 +24,12 @@
 // LINUX: @foo_inline2.ifunc = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
 // LINUX: @foo_used_no_defn.ifunc = weak_odr ifunc i32 (), ptr 
@foo_used_no_defn.resolver
 
+static int __attribute__((target_clones("sse4.2, default"))) internal(void) { 
return 0; }
+int use(void) { return internal(); }
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @internal.resolver() {
+// WINDOWS: define internal i32 @internal() {
+
 int __attribute__((target_clones("sse4.2, default"))) foo(void) { return 0; }
 // LINUX: define {{.*}}i32 @foo.sse4.2.0()
 // LINUX: define {{.*}}i32 @foo.default.1()
@@ -192,7 +199,5 @@
 
 // CHECK: attributes #[[SSE42]] =
 // CHECK-SAME: 
"target-features"="+crc32,+cx8,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
-// CHECK: attributes #[[DEF]] =
-// Don't bother checking features, we verified it is the same as a normal 
function.
 // CHECK: attributes #[[SB]] =
 // CHECK-SAME: 
"target-features"="+avx,+cmov,+crc32,+cx16,+cx8,+fxsr,+mmx,+pclmul,+popcnt,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4059,7 +4059,7 @@
 
 ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
 
-if (supportsCOMDAT())
+if (!ResolverFunc->hasLocalLinkage() && supportsCOMDAT())
   ResolverFunc->setComdat(
   getModule().getOrInsertComdat(ResolverFunc->getName()));
 


Index: clang/test/CodeGen/attr-target-mv.c
===
--- clang/test/CodeGen/attr-target-mv.c
+++ clang/test/CodeGen/attr-target-mv.c
@@ -32,6 +32,13 @@
   return foo();
 }
 
+static int __attribute__((target("arch=meteorlake"))) foo_internal(void) {return 15;}
+static int __attribute__((target("default"))) 

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1379
 
+void init_cpu_features_resolver(unsigned long hwcap, const __ifunc_arg_t *arg) 
{
+  if (__aarch64_cpu_features.features)

It seems that we don't need the `_constructor` function. We can just move the 

```
#if defined(__ANDROID__)
  // ifunc resolvers don't have hwcaps in arguments on Android API lower
  // than 30. In this case set detection done and keep all CPU features
  // unsupported (zeros).
  if (android_get_device_api_level() < 30) {
setCPUFeature(FEAT_MAX);
return;
  }
```

logic to init_cpu_features_resolver


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158641/new/

https://reviews.llvm.org/D158641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158811: [X86] __builtin_cpu_supports: support x86-64{,-v2,-v3,-v4}

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/TargetParser/X86TargetParser.cpp:718
 ;
-FeaturesMask |= (1ULL << Feature);
+FeatureMask[Feature / 32] |= 1U << (Feature % 32);
   }

pengfei wrote:
> Should we use vector for future expansion, or add an assert to make sure it 
> won't exceed current limitation?
A resizable vector does not simplify code here. I've added an assert.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158811/new/

https://reviews.llvm.org/D158811

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158811: [X86] __builtin_cpu_supports: support x86-64{,-v2,-v3,-v4}

[Fangrui Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG27da15381cbe: [X86] __builtin_cpu_supports: support 
x86-64{,-v2,-v3,-v4} (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158811/new/

https://reviews.llvm.org/D158811

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/builtin-cpu-supports.c
  clang/test/Sema/builtin-cpu-supports.c
  llvm/include/llvm/TargetParser/X86TargetParser.def
  llvm/include/llvm/TargetParser/X86TargetParser.h
  llvm/lib/TargetParser/X86TargetParser.cpp

Index: llvm/lib/TargetParser/X86TargetParser.cpp
===
--- llvm/lib/TargetParser/X86TargetParser.cpp
+++ llvm/lib/TargetParser/X86TargetParser.cpp
@@ -703,18 +703,22 @@
   return I != std::end(Processors);
 }
 
-uint64_t llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
+std::array
+llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
   // Processor features and mapping to processor feature value.
-  uint64_t FeaturesMask = 0;
-  for (const StringRef  : FeatureStrs) {
+  std::array FeatureMask{};
+  for (StringRef FeatureStr : FeatureStrs) {
 unsigned Feature = StringSwitch(FeatureStr)
 #define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)\
   .Case(STR, llvm::X86::FEATURE_##ENUM)
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)   \
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
 #include "llvm/TargetParser/X86TargetParser.def"
 ;
-FeaturesMask |= (1ULL << Feature);
+assert(Feature / 32 < FeatureMask.size());
+FeatureMask[Feature / 32] |= 1U << (Feature % 32);
   }
-  return FeaturesMask;
+  return FeatureMask;
 }
 
 unsigned llvm::X86::getFeaturePriority(ProcessorFeatures Feat) {
Index: llvm/include/llvm/TargetParser/X86TargetParser.h
===
--- llvm/include/llvm/TargetParser/X86TargetParser.h
+++ llvm/include/llvm/TargetParser/X86TargetParser.h
@@ -15,6 +15,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
+#include 
 
 namespace llvm {
 template  class SmallVectorImpl;
@@ -57,7 +58,10 @@
 enum ProcessorFeatures {
 #define X86_FEATURE(ENUM, STRING) FEATURE_##ENUM,
 #include "llvm/TargetParser/X86TargetParser.def"
-  CPU_FEATURE_MAX
+  CPU_FEATURE_MAX,
+
+#define X86_MICROARCH_LEVEL(ENUM, STRING, PRIORITY) FEATURE_##ENUM = PRIORITY,
+#include "llvm/TargetParser/X86TargetParser.def"
 };
 
 enum CPUKind {
@@ -171,7 +175,7 @@
 
 char getCPUDispatchMangling(StringRef Name);
 bool validateCPUSpecificCPUDispatch(StringRef Name);
-uint64_t getCpuSupportsMask(ArrayRef FeatureStrs);
+std::array getCpuSupportsMask(ArrayRef FeatureStrs);
 unsigned getFeaturePriority(ProcessorFeatures Feat);
 
 } // namespace X86
Index: llvm/include/llvm/TargetParser/X86TargetParser.def
===
--- llvm/include/llvm/TargetParser/X86TargetParser.def
+++ llvm/include/llvm/TargetParser/X86TargetParser.def
@@ -128,6 +128,10 @@
 #define X86_FEATURE(ENUM, STR)
 #endif
 
+#ifndef X86_MICROARCH_LEVEL
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)
+#endif
+
 X86_FEATURE_COMPAT(CMOV,"cmov",  0)
 X86_FEATURE_COMPAT(MMX, "mmx",   1)
 X86_FEATURE_COMPAT(POPCNT,  "popcnt",9)
@@ -242,5 +246,11 @@
 X86_FEATURE   (RETPOLINE_INDIRECT_CALLS,"retpoline-indirect-calls")
 X86_FEATURE   (LVI_CFI, "lvi-cfi")
 X86_FEATURE   (LVI_LOAD_HARDENING,  "lvi-load-hardening")
+
+X86_MICROARCH_LEVEL(X86_64_BASELINE,"x86-64",   95)
+X86_MICROARCH_LEVEL(X86_64_V2,  "x86-64-v2",96)
+X86_MICROARCH_LEVEL(X86_64_V3,  "x86-64-v3",97)
+X86_MICROARCH_LEVEL(X86_64_V4,  "x86-64-v4",98)
 #undef X86_FEATURE_COMPAT
 #undef X86_FEATURE
+#undef X86_MICROARCH_LEVEL
Index: clang/test/Sema/builtin-cpu-supports.c
===
--- clang/test/Sema/builtin-cpu-supports.c
+++ clang/test/Sema/builtin-cpu-supports.c
@@ -20,6 +20,12 @@
   (void)__builtin_cpu_is("x86-64-v2"); // expected-error {{invalid cpu name for builtin}}
   (void)__builtin_cpu_is("x86-64-v3"); // expected-error {{invalid cpu name for builtin}}
   (void)__builtin_cpu_is("x86-64-v4"); // expected-error {{invalid cpu name for builtin}}
+
+  (void)__builtin_cpu_supports("x86-64");
+  (void)__builtin_cpu_supports("x86-64-v2");
+  (void)__builtin_cpu_supports("x86-64-v3");
+  (void)__builtin_cpu_supports("x86-64-v4");
+  (void)__builtin_cpu_supports("x86-64-v5"); // expected-error {{invalid cpu feature string for builtin}}
 #else
   if 

[PATCH] D158811: [X86] __builtin_cpu_supports: support x86-64{,-v2,-v3,-v4}

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 553698.
MaskRay edited the summary of this revision.
MaskRay added a comment.

rebase after precommitting `target("x86-64") and cpu_specific(x86_64) 
attributes` tests
add `__builtin_cpu_supports x86-64-v5` test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158811/new/

https://reviews.llvm.org/D158811

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/builtin-cpu-supports.c
  clang/test/Sema/builtin-cpu-supports.c
  llvm/include/llvm/TargetParser/X86TargetParser.def
  llvm/include/llvm/TargetParser/X86TargetParser.h
  llvm/lib/TargetParser/X86TargetParser.cpp

Index: llvm/lib/TargetParser/X86TargetParser.cpp
===
--- llvm/lib/TargetParser/X86TargetParser.cpp
+++ llvm/lib/TargetParser/X86TargetParser.cpp
@@ -703,18 +703,22 @@
   return I != std::end(Processors);
 }
 
-uint64_t llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
+std::array
+llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
   // Processor features and mapping to processor feature value.
-  uint64_t FeaturesMask = 0;
-  for (const StringRef  : FeatureStrs) {
+  std::array FeatureMask{};
+  for (StringRef FeatureStr : FeatureStrs) {
 unsigned Feature = StringSwitch(FeatureStr)
 #define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)\
   .Case(STR, llvm::X86::FEATURE_##ENUM)
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)   \
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
 #include "llvm/TargetParser/X86TargetParser.def"
 ;
-FeaturesMask |= (1ULL << Feature);
+assert(Feature / 32 < FeatureMask.size());
+FeatureMask[Feature / 32] |= 1U << (Feature % 32);
   }
-  return FeaturesMask;
+  return FeatureMask;
 }
 
 unsigned llvm::X86::getFeaturePriority(ProcessorFeatures Feat) {
Index: llvm/include/llvm/TargetParser/X86TargetParser.h
===
--- llvm/include/llvm/TargetParser/X86TargetParser.h
+++ llvm/include/llvm/TargetParser/X86TargetParser.h
@@ -15,6 +15,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
+#include 
 
 namespace llvm {
 template  class SmallVectorImpl;
@@ -57,7 +58,10 @@
 enum ProcessorFeatures {
 #define X86_FEATURE(ENUM, STRING) FEATURE_##ENUM,
 #include "llvm/TargetParser/X86TargetParser.def"
-  CPU_FEATURE_MAX
+  CPU_FEATURE_MAX,
+
+#define X86_MICROARCH_LEVEL(ENUM, STRING, PRIORITY) FEATURE_##ENUM = PRIORITY,
+#include "llvm/TargetParser/X86TargetParser.def"
 };
 
 enum CPUKind {
@@ -171,7 +175,7 @@
 
 char getCPUDispatchMangling(StringRef Name);
 bool validateCPUSpecificCPUDispatch(StringRef Name);
-uint64_t getCpuSupportsMask(ArrayRef FeatureStrs);
+std::array getCpuSupportsMask(ArrayRef FeatureStrs);
 unsigned getFeaturePriority(ProcessorFeatures Feat);
 
 } // namespace X86
Index: llvm/include/llvm/TargetParser/X86TargetParser.def
===
--- llvm/include/llvm/TargetParser/X86TargetParser.def
+++ llvm/include/llvm/TargetParser/X86TargetParser.def
@@ -128,6 +128,10 @@
 #define X86_FEATURE(ENUM, STR)
 #endif
 
+#ifndef X86_MICROARCH_LEVEL
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)
+#endif
+
 X86_FEATURE_COMPAT(CMOV,"cmov",  0)
 X86_FEATURE_COMPAT(MMX, "mmx",   1)
 X86_FEATURE_COMPAT(POPCNT,  "popcnt",9)
@@ -242,5 +246,11 @@
 X86_FEATURE   (RETPOLINE_INDIRECT_CALLS,"retpoline-indirect-calls")
 X86_FEATURE   (LVI_CFI, "lvi-cfi")
 X86_FEATURE   (LVI_LOAD_HARDENING,  "lvi-load-hardening")
+
+X86_MICROARCH_LEVEL(X86_64_BASELINE,"x86-64",   95)
+X86_MICROARCH_LEVEL(X86_64_V2,  "x86-64-v2",96)
+X86_MICROARCH_LEVEL(X86_64_V3,  "x86-64-v3",97)
+X86_MICROARCH_LEVEL(X86_64_V4,  "x86-64-v4",98)
 #undef X86_FEATURE_COMPAT
 #undef X86_FEATURE
+#undef X86_MICROARCH_LEVEL
Index: clang/test/Sema/builtin-cpu-supports.c
===
--- clang/test/Sema/builtin-cpu-supports.c
+++ clang/test/Sema/builtin-cpu-supports.c
@@ -20,6 +20,12 @@
   (void)__builtin_cpu_is("x86-64-v2"); // expected-error {{invalid cpu name for builtin}}
   (void)__builtin_cpu_is("x86-64-v3"); // expected-error {{invalid cpu name for builtin}}
   (void)__builtin_cpu_is("x86-64-v4"); // expected-error {{invalid cpu name for builtin}}
+
+  (void)__builtin_cpu_supports("x86-64");
+  (void)__builtin_cpu_supports("x86-64-v2");
+  (void)__builtin_cpu_supports("x86-64-v3");
+  (void)__builtin_cpu_supports("x86-64-v4");
+  (void)__builtin_cpu_supports("x86-64-v5"); // expected-error {{invalid cpu feature string for builtin}}
 #else
   if 

[PATCH] D158920: Delete CloudABI support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks for the cleanup.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158920/new/

https://reviews.llvm.org/D158920

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:326
 def err_unsupported_abi_for_opt : Error<"'%0' can only be used with the '%1' 
ABI">;
+def err_unsupported_opt_for_execute_only_target
+: Error<"unsupported option '%0' for the execute only target '%1'">;

MaggieYi wrote:
> MaskRay wrote:
> > We don't need this diagnostic as a common kind (we only use it in driver).
> > 
> > I think we can reuse `err_drv_argument_not_allowed_with` . Though for PS5 
> > you will get `... allowed with '-mexecute-only'` even if the user doesn't 
> > specify `-mexecute-only`, but I hope it is fine.
> Since `err_drv_argument_not_allowed_with` is an ARM-only option, We cannot 
> reuse it.
`err_drv_argument_not_allowed_with` is a generic diagnostic. My point is that 
we don't need an extra err_unsupported_opt_for_execute_only_target.



Comment at: clang/lib/Basic/Sanitizers.cpp:126
+  // execute-only output (no data access to code sections).
+  if (const llvm::opt::Arg *A =
+  Args.getLastArg(clang::driver::options::OPT_mexecute_only,

The idiom is `hasFlag(clang::driver::options::OPT_mexecute_only, 
clang::driver::options::OPT_mno_execute_only, false)`



Comment at: clang/lib/Basic/Sanitizers.cpp:130
+if (A->getOption().matches(clang::driver::options::OPT_mexecute_only) &&
+llvm::ARM::supportedExecuteOnly(Triple)) {
+  return true;

I don't think we need an extra `llvm::ARM::supportedExecuteOnly` check. We just 
return true when `-mexecute-only` is in effect.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:478
   }
+  // `-fsanitize=function` is silently discarded on an execute-only target
+  // if implicitly enabled through group expansion.

`-fsanitize=function => NotAllowedWithExecuteOnly

since we now handle kcfi as well.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:481
+  if (isExecuteOnlyTarget(Triple, Args)) {
+Add &= ~NotAllowedWithExecuteOnly;
+  }

omit braces



Comment at: clang/test/Driver/fsanitize.c:981
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined 
-fsanitize=function %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI

Drop `not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined 
-fsanitize=function`. 
Testing just `-fsanitize=function` for the negative test is sufficient.



Comment at: clang/test/Driver/fsanitize.c:983
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function 
-fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI 
--check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED

Drop `-fsanitize=function -fsanitize=kcfi` line. They already lead to an error.



Comment at: clang/test/Driver/fsanitize.c:984
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function 
-fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI 
--check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+

`--target=armv6t2-eabi -mexecute-only -fsanitize=undefined` needs a custom 
check prefix that checks `function` is not enabled.



Comment at: llvm/lib/TargetParser/ARMTargetParser.cpp:602
+
+bool ARM::supportedExecuteOnly(const Triple ) {
+  if (parseArchVersion(TT.getArchName()) < 7 &&

We don't need to extract the function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158614/new/

https://reviews.llvm.org/D158614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158811: [X86] __builtin_cpu_supports: support x86-64{,-v2,-v3,-v4}

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 553524.
MaskRay marked 2 inline comments as done.
MaskRay added a comment.

add assert


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158811/new/

https://reviews.llvm.org/D158811

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/builtin-cpu-supports.c
  clang/test/Sema/attr-cpuspecific.c
  clang/test/Sema/attr-target.c
  llvm/include/llvm/TargetParser/X86TargetParser.def
  llvm/include/llvm/TargetParser/X86TargetParser.h
  llvm/lib/TargetParser/X86TargetParser.cpp

Index: llvm/lib/TargetParser/X86TargetParser.cpp
===
--- llvm/lib/TargetParser/X86TargetParser.cpp
+++ llvm/lib/TargetParser/X86TargetParser.cpp
@@ -703,18 +703,22 @@
   return I != std::end(Processors);
 }
 
-uint64_t llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
+std::array
+llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
   // Processor features and mapping to processor feature value.
-  uint64_t FeaturesMask = 0;
-  for (const StringRef  : FeatureStrs) {
+  std::array FeatureMask{};
+  for (StringRef FeatureStr : FeatureStrs) {
 unsigned Feature = StringSwitch(FeatureStr)
 #define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)\
   .Case(STR, llvm::X86::FEATURE_##ENUM)
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)   \
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
 #include "llvm/TargetParser/X86TargetParser.def"
 ;
-FeaturesMask |= (1ULL << Feature);
+assert(Feature / 32 < FeatureMask.size());
+FeatureMask[Feature / 32] |= 1U << (Feature % 32);
   }
-  return FeaturesMask;
+  return FeatureMask;
 }
 
 unsigned llvm::X86::getFeaturePriority(ProcessorFeatures Feat) {
Index: llvm/include/llvm/TargetParser/X86TargetParser.h
===
--- llvm/include/llvm/TargetParser/X86TargetParser.h
+++ llvm/include/llvm/TargetParser/X86TargetParser.h
@@ -15,6 +15,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
+#include 
 
 namespace llvm {
 template  class SmallVectorImpl;
@@ -57,7 +58,10 @@
 enum ProcessorFeatures {
 #define X86_FEATURE(ENUM, STRING) FEATURE_##ENUM,
 #include "llvm/TargetParser/X86TargetParser.def"
-  CPU_FEATURE_MAX
+  CPU_FEATURE_MAX,
+
+#define X86_MICROARCH_LEVEL(ENUM, STRING, PRIORITY) FEATURE_##ENUM = PRIORITY,
+#include "llvm/TargetParser/X86TargetParser.def"
 };
 
 enum CPUKind {
@@ -171,7 +175,7 @@
 
 char getCPUDispatchMangling(StringRef Name);
 bool validateCPUSpecificCPUDispatch(StringRef Name);
-uint64_t getCpuSupportsMask(ArrayRef FeatureStrs);
+std::array getCpuSupportsMask(ArrayRef FeatureStrs);
 unsigned getFeaturePriority(ProcessorFeatures Feat);
 
 } // namespace X86
Index: llvm/include/llvm/TargetParser/X86TargetParser.def
===
--- llvm/include/llvm/TargetParser/X86TargetParser.def
+++ llvm/include/llvm/TargetParser/X86TargetParser.def
@@ -128,6 +128,10 @@
 #define X86_FEATURE(ENUM, STR)
 #endif
 
+#ifndef X86_MICROARCH_LEVEL
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)
+#endif
+
 X86_FEATURE_COMPAT(CMOV,"cmov",  0)
 X86_FEATURE_COMPAT(MMX, "mmx",   1)
 X86_FEATURE_COMPAT(POPCNT,  "popcnt",9)
@@ -242,5 +246,11 @@
 X86_FEATURE   (RETPOLINE_INDIRECT_CALLS,"retpoline-indirect-calls")
 X86_FEATURE   (LVI_CFI, "lvi-cfi")
 X86_FEATURE   (LVI_LOAD_HARDENING,  "lvi-load-hardening")
+
+X86_MICROARCH_LEVEL(X86_64_BASELINE,"x86-64",   95)
+X86_MICROARCH_LEVEL(X86_64_V2,  "x86-64-v2",96)
+X86_MICROARCH_LEVEL(X86_64_V3,  "x86-64-v3",97)
+X86_MICROARCH_LEVEL(X86_64_V4,  "x86-64-v4",98)
 #undef X86_FEATURE_COMPAT
 #undef X86_FEATURE
+#undef X86_MICROARCH_LEVEL
Index: clang/test/Sema/attr-target.c
===
--- clang/test/Sema/attr-target.c
+++ clang/test/Sema/attr-target.c
@@ -26,6 +26,11 @@
 //expected-warning@+1 {{unknown tune CPU 'hiss' in the 'target' attribute string; 'target' attribute ignored}}
 int __attribute__((target("tune=hiss,tune=woof"))) apple_tree(void) { return 4; }
 
+//expected-warning@+1 {{unsupported 'x86-64' in the 'target' attribute string}}
+void __attribute__((target("x86-64"))) baseline(void) {}
+//expected-warning@+1 {{unsupported 'x86-64-v2' in the 'target' attribute string}}
+void __attribute__((target("x86-64-v2"))) v2(void) {}
+
 #elifdef __aarch64__
 
 int __attribute__((target("sve,arch=armv8-a"))) foo(void) { return 4; }
Index: clang/test/Sema/attr-cpuspecific.c
===
--- 

[PATCH] D158385: [tsan] Respect !nosanitize metadata and remove gcov special case

[Fangrui Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5624e86ae0fb: [tsan] Respect !nosanitize metadata and remove 
gcov special case (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158385/new/

https://reviews.llvm.org/D158385

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/code-coverage-tsan.c
  clang/test/Driver/fprofile-update.c
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
  llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll

Index: llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll
===
--- llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll
+++ llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll
@@ -1,6 +1,6 @@
 ; This test checks that we are not instrumenting unwanted acesses to globals:
+; - Instructions with the !nosanitize metadata (e.g. -fprofile-arcs instrumented counter accesses)
 ; - Instruction profiler counter instrumentation has known intended races.
-; - The gcov counters array has a known intended race.
 ;
 ; RUN: opt < %s -passes='function(tsan),module(tsan-module)' -S | FileCheck %s
 
@@ -18,42 +18,44 @@
 
 define i32 @test_gep() sanitize_thread {
 entry:
-  %pgocount = load i64, ptr @__profc_test_gep
+  %pgocount = load i64, ptr @__profc_test_gep, !nosanitize !0
   %0 = add i64 %pgocount, 1
-  store i64 %0, ptr @__profc_test_gep
+  store i64 %0, ptr @__profc_test_gep, !nosanitize !0
 
-  %gcovcount = load i64, ptr @__llvm_gcov_ctr
+  %gcovcount = load i64, ptr @__llvm_gcov_ctr, !nosanitize !0
   %1 = add i64 %gcovcount, 1
-  store i64 %1, ptr @__llvm_gcov_ctr
+  store i64 %1, ptr @__llvm_gcov_ctr, !nosanitize !0
 
-  %gcovcount.1 = load i64, ptr @__llvm_gcov_ctr.1
+  %gcovcount.1 = load i64, ptr @__llvm_gcov_ctr.1, !nosanitize !0
   %2 = add i64 %gcovcount.1, 1
-  store i64 %2, ptr @__llvm_gcov_ctr.1
+  store i64 %2, ptr @__llvm_gcov_ctr.1, !nosanitize !0
 
   ret i32 1
 }
 
 define i32 @test_bitcast() sanitize_thread {
 entry:
-  %0 = load <2 x i64>, ptr @__profc_test_bitcast, align 8
-  %.promoted5 = load i64, ptr @__profc_test_bitcast_foo, align 8
+  %0 = load <2 x i64>, ptr @__profc_test_bitcast, align 8, !nosanitize !0
+  %.promoted5 = load i64, ptr @__profc_test_bitcast_foo, align 8, !nosanitize !0
   %1 = add i64 %.promoted5, 10
   %2 = add <2 x i64> %0, 
-  store <2 x i64> %2, ptr @__profc_test_bitcast, align 8
-  store i64 %1, ptr @__profc_test_bitcast_foo, align 8
+  store <2 x i64> %2, ptr @__profc_test_bitcast, align 8, !nosanitize !0
+  store i64 %1, ptr @__profc_test_bitcast_foo, align 8, !nosanitize !0
   ret i32 undef
 }
 
 define void @test_load() sanitize_thread {
 entry:
-  %0 = load i32, ptr @__llvm_gcov_global_state_pred
-  store i32 1, ptr @__llvm_gcov_global_state_pred
+  %0 = load i32, ptr @__llvm_gcov_global_state_pred, !nosanitize !0
+  store i32 1, ptr @__llvm_gcov_global_state_pred, !nosanitize !0
 
-  %1 = load i32, ptr @__llvm_gcda_foo
-  store i32 1, ptr @__llvm_gcda_foo
+  %1 = load i32, ptr @__llvm_gcda_foo, !nosanitize !0
+  store i32 1, ptr @__llvm_gcda_foo, !nosanitize !0
 
   ret void
 }
 
+!0 = !{}
+
 ; CHECK-NOT: {{call void @__tsan_write}}
 ; CHECK: __tsan_init
Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -364,11 +364,6 @@
   getInstrProfSectionName(IPSK_cnts, OF, /*AddSegmentInfo=*/false)))
 return false;
 }
-
-// Check if the global is private gcov data.
-if (GV->getName().startswith("__llvm_gcov") ||
-GV->getName().startswith("__llvm_gcda"))
-  return false;
   }
 
   // Do not instrument accesses from different address spaces; we cannot deal
@@ -522,6 +517,9 @@
   // Traverse all instructions, collect loads/stores/returns, check for calls.
   for (auto  : F) {
 for (auto  : BB) {
+  // Skip instructions inserted by another instrumentation.
+  if (Inst.hasMetadata(LLVMContext::MD_nosanitize))
+continue;
   if (isTsanAtomic())
 AtomicAccesses.push_back();
   else if (isa(Inst) || isa(Inst))
Index: clang/test/Driver/fprofile-update.c
===
--- clang/test/Driver/fprofile-update.c
+++ clang/test/Driver/fprofile-update.c
@@ -1,6 +1,5 @@
 /// For -fprofile-instr-generate and -fprofile-arcs, increment counters atomically
-/// if -fprofile-update={atomic,prefer-atomic} or -fsanitize=thread is specified.
-// RUN: %clang -### %s -c -target x86_64-linux -fsanitize=thread %s 2>&1 | FileCheck %s
+/// if -fprofile-update={atomic,prefer-atomic} is 

[PATCH] D158811: [X86] __builtin_cpu_supports: support x86-64{,-v2,-v3,-v4}

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: FreddyYe, pengfei, RKSimon, skan, erichkeane.
Herald added a subscriber: hiraditya.
Herald added a reviewer: ctetreau.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

GCC 12 (https://gcc.gnu.org/PR101696) allows
__builtin_cpu_supports("x86-64") (and -v2 -v3 -v4).
This patch ports the feature.

- Add `FEATURE_X86_64_{BASELINE,V2,V3,V4}` to enum ProcessorFeatures, but keep 
CPU_FEATURE_MAX unchanged to make FeatureInfos/FeatureInfos_WithPLUS happy.
- Change validateCpuSupports to allow `x86-64{,-v2,-v3,-v4}`
- Change getCpuSupportsMask to return `std::array where 
`x86-64{,-v2,-v3,-v4}` set bits `FEATURE_X86_64_{BASELINE,V2,V3,V4}`.
- `cpu_dispatch` feels legacy. It doesn't support `x86-64{,-v2,-v3,-v4}`. Add 
tests.

Close https://github.com/llvm/llvm-project/issues/55830


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158811

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/builtin-cpu-supports.c
  clang/test/Sema/attr-cpuspecific.c
  clang/test/Sema/attr-target.c
  llvm/include/llvm/TargetParser/X86TargetParser.def
  llvm/include/llvm/TargetParser/X86TargetParser.h
  llvm/lib/TargetParser/X86TargetParser.cpp

Index: llvm/lib/TargetParser/X86TargetParser.cpp
===
--- llvm/lib/TargetParser/X86TargetParser.cpp
+++ llvm/lib/TargetParser/X86TargetParser.cpp
@@ -703,18 +703,21 @@
   return I != std::end(Processors);
 }
 
-uint64_t llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
+std::array
+llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
   // Processor features and mapping to processor feature value.
-  uint64_t FeaturesMask = 0;
-  for (const StringRef  : FeatureStrs) {
+  std::array FeatureMask{};
+  for (StringRef FeatureStr : FeatureStrs) {
 unsigned Feature = StringSwitch(FeatureStr)
 #define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)\
   .Case(STR, llvm::X86::FEATURE_##ENUM)
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)   \
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
 #include "llvm/TargetParser/X86TargetParser.def"
 ;
-FeaturesMask |= (1ULL << Feature);
+FeatureMask[Feature / 32] |= 1U << (Feature % 32);
   }
-  return FeaturesMask;
+  return FeatureMask;
 }
 
 unsigned llvm::X86::getFeaturePriority(ProcessorFeatures Feat) {
Index: llvm/include/llvm/TargetParser/X86TargetParser.h
===
--- llvm/include/llvm/TargetParser/X86TargetParser.h
+++ llvm/include/llvm/TargetParser/X86TargetParser.h
@@ -15,6 +15,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
+#include 
 
 namespace llvm {
 template  class SmallVectorImpl;
@@ -57,7 +58,11 @@
 enum ProcessorFeatures {
 #define X86_FEATURE(ENUM, STRING) FEATURE_##ENUM,
 #include "llvm/TargetParser/X86TargetParser.def"
-  CPU_FEATURE_MAX
+  CPU_FEATURE_MAX,
+
+#define X86_FEATURE(ENUM, STRING)
+#define X86_MICROARCH_LEVEL(ENUM, STRING, PRIORITY) FEATURE_##ENUM = PRIORITY,
+#include "llvm/TargetParser/X86TargetParser.def"
 };
 
 enum CPUKind {
@@ -171,7 +176,7 @@
 
 char getCPUDispatchMangling(StringRef Name);
 bool validateCPUSpecificCPUDispatch(StringRef Name);
-uint64_t getCpuSupportsMask(ArrayRef FeatureStrs);
+std::array getCpuSupportsMask(ArrayRef FeatureStrs);
 unsigned getFeaturePriority(ProcessorFeatures Feat);
 
 } // namespace X86
Index: llvm/include/llvm/TargetParser/X86TargetParser.def
===
--- llvm/include/llvm/TargetParser/X86TargetParser.def
+++ llvm/include/llvm/TargetParser/X86TargetParser.def
@@ -128,6 +128,10 @@
 #define X86_FEATURE(ENUM, STR)
 #endif
 
+#ifndef X86_MICROARCH_LEVEL
+#define X86_MICROARCH_LEVEL(ENUM, STR, PRIORITY)
+#endif
+
 X86_FEATURE_COMPAT(CMOV,"cmov",  0)
 X86_FEATURE_COMPAT(MMX, "mmx",   1)
 X86_FEATURE_COMPAT(POPCNT,  "popcnt",9)
@@ -242,5 +246,11 @@
 X86_FEATURE   (RETPOLINE_INDIRECT_CALLS,"retpoline-indirect-calls")
 X86_FEATURE   (LVI_CFI, "lvi-cfi")
 X86_FEATURE   (LVI_LOAD_HARDENING,  "lvi-load-hardening")
+
+X86_MICROARCH_LEVEL(X86_64_BASELINE,"x86-64",   95)
+X86_MICROARCH_LEVEL(X86_64_V2,  "x86-64-v2",96)
+X86_MICROARCH_LEVEL(X86_64_V3,  "x86-64-v3",97)
+X86_MICROARCH_LEVEL(X86_64_V4,  "x86-64-v4",98)
 #undef X86_FEATURE_COMPAT
 #undef X86_FEATURE
+#undef X86_MICROARCH_LEVEL
Index: clang/test/Sema/attr-target.c
===
--- clang/test/Sema/attr-target.c
+++ 

[PATCH] D158706: [Driver] Remove Myriad.cpp

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2958
   default:
 return getTriple().getVendor() != llvm::Triple::Myriad;
   }

MaskRay wrote:
> brad wrote:
> > You might as well remove this.
> Will do!
There is some Myriad remaining in Sparc.cpp, which can be cleaned up later...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158706/new/

https://reviews.llvm.org/D158706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158706: [Driver] Remove Myriad.cpp

[Fangrui Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.
Closed by commit rGaa9d7d1cd0af: [Driver] Remove Myriad.cpp (authored by 
MaskRay).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D158706?vs=553001=553329#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158706/new/

https://reviews.llvm.org/D158706

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Myriad.cpp
  clang/lib/Driver/ToolChains/Myriad.h
  clang/test/Driver/Inputs/basic_myriad_tree/bin/.keep
  
clang/test/Driver/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/crtend.o
  
clang/test/Driver/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/crti.o
  
clang/test/Driver/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/crtn.o
  
clang/test/Driver/Inputs/basic_myriad_tree/sparc-myriad-rtems/include/c++/6.3.0/.keep
  clang/test/Driver/Inputs/basic_myriad_tree/sparc-myriad-rtems/lib/crt0.o
  clang/test/Driver/myriad-toolchain.c
  clang/test/Driver/sanitizer-ld.c
  llvm/utils/gn/secondary/clang/lib/Driver/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Driver/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Driver/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Driver/BUILD.gn
@@ -87,7 +87,6 @@
 "ToolChains/MSVC.cpp",
 "ToolChains/MinGW.cpp",
 "ToolChains/MipsLinux.cpp",
-"ToolChains/Myriad.cpp",
 "ToolChains/NaCl.cpp",
 "ToolChains/NetBSD.cpp",
 "ToolChains/OHOS.cpp",
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -259,15 +259,6 @@
 // CHECK-ASAN-ANDROID-SHARED-NOT: "-lpthread"
 // CHECK-ASAN-ANDROID-SHARED-NOT: "-lresolv"
 
-// RUN: %clang -### %s 2>&1 \
-// RUN: --target=sparcel-myriad-rtems-elf -fuse-ld=ld -fsanitize=address \
-// RUN: --sysroot=%S/Inputs/basic_myriad_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-ASAN-MYRIAD %s
-//
-// CHECK-ASAN-MYRIAD: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-ASAN-MYRIAD-NOT: "-lc"
-// CHECK-ASAN-MYRIAD: libclang_rt.asan-sparcel.a"
-
 // RUN: %clangxx -### %s 2>&1 \
 // RUN: --target=x86_64-unknown-linux -fuse-ld=ld -stdlib=platform -lstdc++ \
 // RUN: -fsanitize=thread \
Index: clang/test/Driver/myriad-toolchain.c
===
--- clang/test/Driver/myriad-toolchain.c
+++ /dev/null
@@ -1,90 +0,0 @@
-// RUN: %clang -### --target=sparc-myriad-rtems %s \
-// RUN: -ccc-install-dir %S/Inputs/basic_myriad_tree/bin \
-// RUN: --gcc-toolchain=%S/Inputs/basic_myriad_tree 2>&1 | FileCheck %s -check-prefix=LINK_WITH_RTEMS
-// LINK_WITH_RTEMS: Inputs{{.*}}crti.o
-// LINK_WITH_RTEMS: Inputs{{.*}}crtbegin.o
-// LINK_WITH_RTEMS: "-L{{.*}}Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0"
-// LINK_WITH_RTEMS: "-L{{.*}}Inputs/basic_myriad_tree/bin/../sparc-myriad-rtems/lib"
-// LINK_WITH_RTEMS: "--start-group" "-lc" "-lgcc" "-lrtemscpu" "-lrtemsbsp" "--end-group"
-// LINK_WITH_RTEMS: Inputs{{.*}}crtend.o
-// LINK_WITH_RTEMS: Inputs{{.*}}crtn.o
-
-// RUN: %clang -c -### --target=sparc-myriad-rtems -x c++ %s \
-// RUN: -stdlib=libstdc++ --gcc-toolchain=%S/Inputs/basic_myriad_tree 2>&1 | FileCheck %s -check-prefix=COMPILE_CXX
-// COMPILE_CXX: "-internal-isystem" "{{.*}}/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/../../../../sparc-myriad-rtems/include/c++/6.3.0"
-// COMPILE_CXX: "-internal-isystem" "{{.*}}/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/../../../../sparc-myriad-rtems/include/c++/6.3.0/sparc-myriad-rtems"
-// COMPILE_CXX: "-internal-isystem" "{{.*}}/Inputs/basic_myriad_tree/lib/gcc/sparc-myriad-rtems/6.3.0/../../../../sparc-myriad-rtems/include/c++/6.3.0/backward"
-
-// RUN: %clang -### -E --target=sparc-myriad --sysroot=/yow %s 2>&1 \
-// RUN:   | FileCheck %s -check-prefix=SLASH_INCLUDE
-// SLASH_INCLUDE: "-isysroot" "/yow" "-internal-isystem" "/yow/include"
-
-// RUN: %clang -### -E --target=sparc-myriad --sysroot=/yow %s -nostdinc 2>&1 \
-// RUN:   | FileCheck %s -check-prefix=NO_SLASH_INCLUDE
-// NO_SLASH_INCLUDE: "-isysroot" "/yow"
-// NO_SLASH_INCLUDE-NOT: "-internal-isystem" "/yow/include"
-
-// RUN: not %clang -### --target=what-myriad %s 2>&1 | FileCheck %s -check-prefix=BAD_ARCH
-// BAD_ARCH: the target architecture 'what' is not supported by the target 'myriad'
-
-// Ensure that '-target shave' picks a different compiler.
-// Also check that '-I' is turned into '-i:' for the 

[PATCH] D158706: [Driver] Remove Myriad.cpp

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2958
   default:
 return getTriple().getVendor() != llvm::Triple::Myriad;
   }

brad wrote:
> You might as well remove this.
Will do!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158706/new/

https://reviews.llvm.org/D158706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1382
+return;
+#if defined(__ANDROID__)
+  // ifunc resolvers don't have hwcaps in arguments on Android API lower

enh wrote:
> ilinpv wrote:
> > MaskRay wrote:
> > > I am unfamiliar with how Android ndk builds compiler-rt.
> > > 
> > > If `__ANDROID_API__ >= 30`, shall we use the regular Linux code path?
> > I think that leads to shipping different compile-rt libraries depend on 
> > ANDROID_API. If this is an option to consider than runtime check 
> > android_get_device_api_level() < 30 can be replaced by `__ANDROID_API__ < 
> > 30`
> depends what you mean... in 10 years or so, yes, no-one is likely to still 
> care about the older API levels and we can just delete this. but until then, 
> no, there's _one_ copy of compiler-rt that everyone uses, and although _OS 
> developers_ don't need to support anything more than a couple of years old, 
> most app developers are targeting far lower API levels than that (to maximize 
> the number of possible customers).
> 
> TL;DR: "you could add that condition to the `#if`, but no-one would use it 
> for a decade". (and i think the comment and `if` below should make it clear 
> enough to future archeologists when this code block can be removed :-) )
My thought was that people build Android with a specific `__ANDROID_API__`, and 
only systems >= this level are supported.
```
#If __ANDROID_API__ < 30
...
#endif
```

This code has a greater chance to be removed when it becomes obsoleted. The 
argument is similar to how we find obsoleted GCC workarounds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158641/new/

https://reviews.llvm.org/D158641

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits