[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Make "-fno-split-machine-functions" a valid flag for all archs. Driver changes usually are named `[Driver] ...`. The popular convention is to omit the full stop at the end of the subject. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. With a test nit Comment at: clang/test/Driver/fsplit-machine-functions.c:19 +// RUN: not %clang -### -x cuda -S -nocudainc -nocudalib -fsplit-machine-functions %s 2>&1 |

[PATCH] D158411: [clang] [MinGW] Pass LTO options to the linker

2023-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158411/new/ https://reviews.llvm.org/D158411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/fsplit-machine-functions.c:19 +// RUN: not %clang -### -x cuda -S -nocudainc -nocudalib -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2 +// ERR2: clang: error: unsupported option

[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for the patch. We need a test. I cleaned up `fsplit-machine-functions.c` a bit. You can rebase and modify the `not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions` RUN line in

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp and Contiki.cpp

2023-08-24 Thread 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 rG3515409012fa: [Driver] Remove unlikely-working Minix.cpp and Contiki.cpp (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES

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

2023-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: brad, jyknight, waltl. Herald added subscribers: jrtc27, fedor.sergeev. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I am trying to clean up

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp and Contiki.cpp

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @brad :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158461/new/ https://reviews.llvm.org/D158461 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-23 Thread 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 rG7a41af86041b: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute (authored by MaskRay). Changed prior to commit:

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: chill, peter.smith, simon_tatham, stuij. Herald added a subscriber: kristof.beyls. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Some options

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. 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

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. 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. Repository: rG

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

2023-08-23 Thread 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

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158641#4611731 , @rprichard wrote: > [...] > > I suspect Bionic ought to apply relocations to libraries in a bottom-up > fashion, so that libc.so is relocated before the executable or shared > objects, but I _think_ it's

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/aarch64-features.c:10 // Check Function Multi Versioning option and rtlib dependency. -// RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt \ +// RUN: %clang --target=aarch64-linux-android23

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D158612: [flang][driver] Ensure negative flags have the same visibility as positive

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: flang/test/Driver/fintegrated-as.f90:15 +! +! Without `-fno-integrated-as` (default) / With `-fintegrated-as`

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. (I think there is no action item on my side and this patch is good for review) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158329/new/ https://reviews.llvm.org/D158329 ___

[PATCH] D158252: Fix regression of D157680

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/x86-no-gather-no-scatter.cpp:2 /// Tests -mno-gather and -mno-scatter -// RUN: %clang -c -mno-gather -### %s 2>&1 | FileCheck --check-prefix=NOGATHER %s -// RUN: %clang_cl -c /Qgather- -### %s 2>&1 | FileCheck

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/hipstdpar.c:1 +// RUN: not %clang -### -hipstdpar --compile %s 2>&1 | \ +// RUN: FileCheck --check-prefix=HIPSTDPAR-MISSING-LIB %s Better to use `--hipstdpar` instead of `-hipstdpar`. In GCC, `-h`

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

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

2023-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Context not available See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment at: compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake:83 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64} ${ARM32} ${ARM64} ${MIPS32}

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13319 +Value *Features = Builder.CreateAlignedLoad( +Int32Ty, Builder.CreateGEP(ATy, CpuFeatures2, Idxs), +CharUnits::fromQuantity(4)); FreddyYe wrote: > MaskRay

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp and Contiki.cpp

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 552204. MaskRay added a comment. update gn Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158461/new/ https://reviews.llvm.org/D158461 Files: clang/lib/Basic/Targets.cpp clang/lib/Basic/Targets/OSTargets.h

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

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > // Android target triples contain a target version. If we don't have > libraries for the exact target version, we should fall back to the next > newest version or a versionless path, if any. An Android maintainer can override my opinion but my feeling is that this

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp and Contiki.cpp

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 552171. MaskRay retitled this revision from "[Driver] Remove unlikely-working Minix.cpp" to "[Driver] Remove unlikely-working Minix.cpp and Contiki.cpp". MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a project: LLVM.

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158461#4605084 , @brad wrote: > Isn't there more than this to do a full proper removal? > > Looking around it looks like the Ananas and Contiki support can be removed. > Ananas used > to use Clang but switched back to GCC

[PATCH] D158378: [Driver] move Minix header search path management to the driver

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Created https://reviews.llvm.org/D158461 to remove it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158378/new/ https://reviews.llvm.org/D158378 ___ cfe-commits mailing list

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: brad. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This ToolChain was added back in 2010 but has been unmaintained with no test. The

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13317 + continue; +Value *Idxs[] = {Builder.getInt32(0), Builder.getInt32(i - 1)}; +Value *Features = Builder.CreateAlignedLoad( FreddyYe wrote: > Don't quite understand why

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 552078. MaskRay marked 3 inline comments as done. MaskRay added a comment. <4 x i32> => <3 x i32> (though no difference at runtime) remove a redundant `= 57` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9 + +// Check that -fsplit-machine-functions is passed to both x86 and cuda compilation and does not cause driver error. +// MFS2: -fsplit-machine-functions

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-21 Thread 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 rGf0bbda00bd06: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin (authored by artem, committed by MaskRay). Changed prior to

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13318-13320 +Value *Idxs[] = {Builder.getInt32(0), Builder.getInt32(i - 1)}; +Value *Features = Builder.CreateAlignedLoad( +Int32Ty,

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13318-13320 +Value *Idxs[] = {Builder.getInt32(0), Builder.getInt32(i - 1)}; +Value *Features = Builder.CreateAlignedLoad( +Int32Ty, Builder.CreateGEP(ATy, CpuFeatures2, Idxs),

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/CMakeLists.txt:179-183 +if(DEFAULT_SYSROOT) + message(WARNING "DEFAULT_SYSROOT is deprecated and will be removed. Use " +"configuration files (https://clang.llvm.org/docs/UsersManual.html#configuration-files)" +"to

[PATCH] D158378: [Driver] move Minix header search path management to the driver

2023-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158378#4602375 , @brad wrote: > In D158378#4602289 , @MaskRay wrote: > >> Thank you for driving the migration! >> >> case llvm::Triple::Minix: >>

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

2023-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: Sanitizers, dvyukov, melver, vitalybuka, Enna1. 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,

[PATCH] D158378: [Driver] move Minix header search path management to the driver

2023-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you for driving the migration! case llvm::Triple::Minix: AddGnuCPlusPlusIncludePaths("/usr/gnu/include/c++/4.4.3", "", "", "", triple); The GCC 4.4.3 thing is from 3e2ee147d0ddb23592b2ec8294381b5e1801cc62 (2010).

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D156821#4600605 , @craig.topper wrote: > In D156821#4600550 , @MaskRay wrote: > >>> Currenly both Clang and GCC support the following set of flags that control >> >> code gen of

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay resigned from this revision. MaskRay added a comment. This revision now requires review to proceed. Sorry, after reading through D157750 , I think the patch should be reverted. Comment at:

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13278 +llvm::Value * +CodeGenFunction::EmitX86CpuSupports(std::array FeatureMask) { Value *Result = Builder.getTrue(); erichkeane wrote: > Hmm... I guess size-wise this is on the edge

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 551787. MaskRay marked 4 inline comments as done. MaskRay added a comment. Use Lo_32/Hi_32 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158329/new/ https://reviews.llvm.org/D158329 Files:

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9 + +// Check that -fsplit-machine-functions is passed to both x86 and cuda compilation and does not cause driver error. +// MFS2: -fsplit-machine-functions

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Sorry, but I think this change should be reverted. (a) `-fsplit-machine-functions` on an unsupported target now emits a warning instead of an error. This diverges from the regular expectation for target-specific features. % fclang --target=riscv64

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13272 + uint64_t Mask = llvm::X86::getCpuSupportsMask(FeatureStrs); + uint32_t FeaturesMask[4] = {uint32_t(Mask), uint32_t(Mask >> 32), 0, 0}; + return EmitX86CpuSupports(FeaturesMask);

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 551773. MaskRay marked 6 inline comments as done. MaskRay added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158329/new/ https://reviews.llvm.org/D158329 Files:

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +//

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target/target_clones attributes

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: craig.topper, FreddyYe, pengfei, RKSimon, skan. Herald added subscribers: Enna1, hiraditya. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers:

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Currenly both Clang and GCC support the following set of flags that control code gen of signed overflow: > [...] > Howerver, clang ignores these flags for __builtin_abs(int) and its > higher-width versions, so passing minimum integer value always causes poison.

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1787 +if (!VCI->isMinSignedValue()) { + return EmitAbs(CGF, ArgValue, true); +} MaskRay wrote: > nit: we delete braces in this cascading case > >

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-18 Thread 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: compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:11 +int main() { + // ABORT: abs.cpp:[[@LINE+3]]:17: runtime error: negation of -{{[0-9]+}} cannot be

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D156821#4599471 , @thurston wrote: > This patch might have broke the buildbots, starting with when it was first > built in https://lab.llvm.org/buildbot/#/builders/85/builds/18390 > > >

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay reopened this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1787 +if (!VCI->isMinSignedValue()) { + return EmitAbs(CGF, ArgValue, true); +} nit: we

[PATCH] D157157: [clang] Add clang support for Machine Function Splitting on AArch64

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5852 options::OPT_fno_split_machine_functions)) { -// This codegen pass is only available on x86-elf targets. -if (Triple.isX86() &&

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: sbc100. MaskRay added inline comments. Comment at: clang/CMakeLists.txt:179-183 +if(DEFAULT_SYSROOT) + message(WARNING "DEFAULT_SYSROOT is deprecated and will be removed. Use " +"configuration files

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/CMakeLists.txt:179-183 +if(DEFAULT_SYSROOT) + message(WARNING "DEFAULT_SYSROOT is deprecated and will be removed. Use " +"configuration files (https://clang.llvm.org/docs/UsersManual.html#configuration-files)" +"to

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fsplit-machine-functions.c:1 +// REQUIRES: system-linux +// REQUIRES: x86-registered-target Any reason `system-linux` is needed? Comment at:

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/CMakeLists.txt:179-183 +if(DEFAULT_SYSROOT) + message(WARNING "DEFAULT_SYSROOT is deprecated and will be removed. Use " +"configuration files (https://clang.llvm.org/docs/UsersManual.html#configuration-files)" +"to

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, arphaman, aaron.ballman, andrew.w.kaylor, hans, skan, zahiraam, dexonsmith. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This restores

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4599461 , @dblaikie wrote: > In D158137#4597491 , @dexonsmith > wrote: > >> In D158137#4597009 , @MaskRay >> wrote: >> >>> In

[PATCH] D158206: [Driver] Add PIE support on Solaris

2023-08-18 Thread 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/Solaris.cpp:136 +const char *crtbegin = nullptr; +if (Args.hasArg(options::OPT_shared) || IsPIE) + crtbegin =

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-18 Thread 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 rG1c66d08b0137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to… (authored by MaskRay). Changed prior to commit:

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 551542. MaskRay added a comment. add a release note about the renamed -W option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158137/new/ https://reviews.llvm.org/D158137 Files: clang/docs/ReleaseNotes.rst

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4597491 , @dexonsmith wrote: > In D158137#4597009 , @MaskRay wrote: > >> In D158137#4596948 , @dexonsmith >> wrote: >> >>> Can you

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fsplit-machine-functions.c:41 + +// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions %s \ +// RUN: 2>&1 | FileCheck -check-prefix=MFS5 %s shenhan wrote: > MaskRay wrote:

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fsplit-machine-functions.c:41 + +// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions %s \ +// RUN: 2>&1 | FileCheck -check-prefix=MFS5 %s `-target ` has been deprecated

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4596948 , @dexonsmith wrote: > Can you explain the downside of leaving behind an alias? Two minor ones. (a) Existing `-Wno-overriding-t-option` will not notice that they need to migrate and (b) Clang has accrued

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: DavidSpickett, phosek, smeenai, tra, tstellar. Herald added subscribers: ekilmer, s.egerton, abidh, PkmX, atanasyan, simoncook, fedor.sergeev, kristof.beyls, krytarowski, arichardson, sdardis, emaste. Herald added a project: All. MaskRay

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

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: sunshaoce. In D146054#4587210 , @4vtomat wrote: > In D146054#4586067 , @MaskRay wrote: > >> I think the best place to test `RISCVISAInfo.cpp` is >>

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16 +// causes a warning. +// RUN: %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \ +// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s

[PATCH] D158206: [Driver] Add PIE support on Solaris

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > I'll submit a follow-up patch to make use of crtbeginS.o and crtendS.o > shortly. I think this patch should make this change, so that the change is correct on itself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158206: [Driver] Add PIE support on Solaris

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:55 const char *LinkingOutput) const { + const bool IsPIE = + !Args.hasArg(options::OPT_shared) && On Linux, `clang -r` `-static` also

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4596111 , @MaskRay wrote: > In D158137#4595927 , @dexonsmith > wrote: > >> This seems to drop `-Woverriding-t-option` entirely. Could that break builds >> if someone has

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4595927 , @dexonsmith wrote: > This seems to drop `-Woverriding-t-option` entirely. Could that break builds > if someone has (e.g.) `-Werror -Wno-overriding-t-option` in their build > settings? Yes. We could also

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 551156. MaskRay retitled this revision from "Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option" to "Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option

[PATCH] D158137: Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4595234 , @hans wrote: >> Thanks! I agree. d9ad0681fad9a98f43d9baddb95d505b37153c48 (2013) renamed >> `warn_drv_overriding_t_option` to `warn_drv_overriding_flag_option`. >> Perhaps the original name

[PATCH] D158137: Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4594181 , @dexonsmith wrote: > LGTM. > > Perhaps as a follow-up, rename warn_drv_overriding_flag_option to have “t” in > it? Thanks! I agree. d9ad0681fad9a98f43d9baddb95d505b37153c48 (2013) renamed

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 550963. MaskRay added a comment. rebase we may not go with this approach, but update to show the test difference Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153835/new/ https://reviews.llvm.org/D153835

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

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 550961. MaskRay added a comment. rebase. this does not address rjmccall's comment, but just to show the difference to other tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154774/new/

[PATCH] D157485: [X86][RFC] Support new feature AVX10

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:261 +if (AVXVecSize == 256) + D.Diag(diag::warn_drv_overriding_flag_option) << "AVX10-256" +<< "AVX10-512";

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Herald added a subscriber: ributzka. Herald added a project: All. Comment at: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp:1545 +std::string TargetArg = OSTarget->getAsString(Args, Opts); +

[PATCH] D158137: Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, aaron.ballman, andrew.w.kaylor, hans, skan, zahiraam. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. warn_drv_overriding_flag_option was

[PATCH] D157485: [X86][RFC] Support new feature AVX10

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/X86.cpp:739 + if (HasAVX10_512BIT) +Builder.defineMacro("__AVX10_512BIT__"); + This is untested? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129061: [Lex] Diagnose macro in command lines

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:664 HelpText<"Define to (or 1 if omitted)">; +def DriverDefine : JoinedOrSeparate<["-"], "driver-define">, Group, +Flags<[CC1Option, FlangOption, FC1Option]>, MetaVarName<"=">,

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/x86-no-gather-no-scatter.cpp:4 +// RUN: | FileCheck --check-prefix=NOGATHER %s +// RUN: %clangxx -c -mno-gather -### %s 2>&1 \ +// RUN: | FileCheck --check-prefix=NOGATHER %s Each RUN line makes

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:4218 + Outputs.push_back(Out.get()); + Arg++; +} https://llvm.org/docs/CodingStandards.html#prefer-preincrement Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292 + std::string NewFieldName = + PackedField->getName().str() + "@" + std::to_string(Arg); + PackedField->setDeclName((NewFieldName)); cjdb

[PATCH] D78441: Delete NaCl support

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D78441#4074689 , @MaskRay wrote: > In D78441#4067440 , @dschuff wrote: > >> Sorry, no :) > > Is there a timeline for the removal? @dschuff :) Repository: rG LLVM Github Monorepo

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/include/llvm/Object/ArchiveWriter.h:43 +enum SymtabWritingMode { + NoSymtab, // Do not write symbol table. Below you use `SymtabWritingMode::` for all members, so just make this enum scoped.

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `# REQUIRES: system-aix` in llvm/test/tools/llvm-ranlib/aix-X-option.test unfortunately makes most -X tests only work on AIX, which most contributors don't have access to. Ideally `-X` is usable when the user specifies `--format=bigarchive`. The environment variable

[PATCH] D157445: [CodeGen][UBSan] Add support for handling attributed functions in getUBSanFunctionTypeHash.

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/CodeGen/ubsan-function-attributed.c:3 + +// CHECK: .long248076293 +void __attribute__((ms_abi)) f(void) {} It's useful to add `// CHECK-LABEL: f:` interleaving with

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Thanks for the patch, but there are two issues that should be fixed: (1) stat => fstat (2) change the subject to mean that this is not AIX specific (`[LLVM][Support] Report EISDIR

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

2023-08-15 Thread 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/Driver/ToolChains/Gnu.cpp:2258 - Prefixes.push_back(CandidatePrefix); + SolarisPrefixes.push_back(CandidatePrefix); }

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

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:5169 + HelpText<"Emit VPU instructions for VE">; +def mno_vevpu : Flag<["-"], "mno-vevpu">, Group, + HelpText<"Do not emit VPU instructions for VE">; In general, we just need

<    1   2   3   4   5   6   7   8   9   10   >