[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

2023-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:4218-4223 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath", LangOpts<"OpenMP">, - DefaultTrue, + DefaultFalse, PosFlag, NegFlag, BothFlags<[NoArgumentUnused]>>;

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

2023-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143306#4144396 , @JonChesterfield wrote: > So what is this configuration file? Joseph found a Gentoo blog post > https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/ > and I

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

2023-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143306#4144518 , @JonChesterfield wrote: > Marking this as "no" because as far as I can tell it'll stop anyone running > openmp built from source which constitutes a severe regression and I am > struggling to find informati

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

2023-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143306#4144541 , @jdoerfert wrote: > I'm worried this makes use of LLVM on HPC machines even harder. That said, > I'm open to suggestions and I am not well versed in all the ways we can make > it work. > Our problem is that

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

2023-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LG Comment at: clang/test/CodeGenCXX/pr60668.cpp:1 +// RUN: %clang --target=aarch64-unknown-linux-gnu -O1 -fvisibility=hidden \ +// RUN: -fsanitize=hwaddress -fsanitize=undefined -std=c++17 -fno-exceptions \ We try avoiding .cc to as

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

2023-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Full context at https://github.com/llvm/llvm-project/issues/60668. Best to leave a simplified description in the summary so that a reader doesn't have to go to the external link and summarize the discussions themselves. This SCEV function is invoked by `FunctionPass M

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

2023-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. > `[llvm] ` `[SCEV]` is a more common tag for ScalarEvolution patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144035/new/ https://reviews.llvm.org/D144035 ___

[PATCH] D144620: [clang][driver] Handle '-mrelax' and '-mno-relax' for AVR

2023-02-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:549 +// '-mrelax' is default unless '-mno-relax' is specified. +if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true)) + CmdArgs.push_back("--relax"); If the

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

2023-02-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This looks like introducing a footgun (when something behaves differently from an upstream Clang, it would be difficult for toolchain maintainers to know why). Why can't your user specify `CLANG_CONFIG_FILE_SYSTEM_DIR`? Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D144620: [clang][driver] Handle '-mrelax' and '-mno-relax' for AVR

2023-02-23 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/AVR.cpp:548 +// '-mrelax' is default unless '-mno-relax' is specified. +if (Args.hasFlag(options::OPT_mrelax, options:

[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

2023-05-04 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 rG053bf8640aa8: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr) (authored by MaskRay). Repository: rG LLVM Github Mono

[PATCH] D149920: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address

2023-05-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 519734. MaskRay retitled this revision from "ms inline asm: recognize "jmp" as TargetLowering::C_Address" to "ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address". MaskRay edited the summary of this revision. MaskRay added a co

[PATCH] D149920: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address

2023-05-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 519744. MaskRay marked an inline comment as done. MaskRay added a comment. fix a typo in a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149920/new/ https://reviews.llvm.org/D149920 Files: clang/test

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/MC/X86/intel-syntax-branch.s:61-67 + // FIXME: MASM does not accept this syntax and GAS assembles this as a direct + //call/jump instead of indirect. Consider making this syntax an error? + call [offset fn_ref] + j

[PATCH] D149920: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address

2023-05-05 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8afd831b456a: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-05 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149579/new/ https://reviews.llvm.org/D149579 __

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2662 +// TODO: Consider making the `call [offset fn_ref]` syntax an error? +#if 0 +if (!PtrInOperand && SM.isOffsetOperator()) I think this patch can be pushed now.

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/MC/X86/intel-syntax-branch-fail.s:1 +// RUN: not llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s 2>&1 | FileCheck %s + Don't create a separate test for negative tests. Use the `--defsym=ERR=1` an

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/MC/X86/intel-syntax-branch-fail.s:5 +// call [offset fn_ref] +// // TODO-CHECK: {{.*}}intel-syntax-branch-fail.s:[[#@LINE+-1]]:1: error: `OFFSET` operator cannot be used in an unconditional branch +// jmp [offset fn_ref] -

[PATCH] D148094: [clang][CodeGen] Break up TargetInfo.cpp [6/6]

2023-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/ABIInfo.h:98 +/// isHomogeneousAggregate - Return true if a type is an ELFv2 homogeneous +/// aggregate. Base is set to the base element type, and Members is set Remove "isHomogeneousAggregat

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

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

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 3 inline comments as done. MaskRay added a comment. In D149193#4328452 , @dblaikie wrote: >> I agree that for most(all?) split DWARF users will not see any difference >> since they always use -c -o and don't combine compilation and linking

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 520573. MaskRay added a comment. use `llvm::sys::path::stem(getDefaultImageName())` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149193/new/ https://reviews.llvm.org/D149193 Files: clang/docs/ReleaseNotes.r

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

2023-05-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:546 addLinkerCompressDebugSectionsOption(getToolChain(), Args, CmdArgs); AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA); + Args.AddAllArgs(CmdArgs, options::OPT_L); --

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

2023-05-09 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. > The linker is just lld so it should be the same conceptually. I'm just > figuring that even if the user is cross compiling we should respect -L passed > on the command line. Should I chang

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

2023-05-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. GCC has -fkeep-static-consts (which Clang supports) but not -fkeep-static-variables. > This could be useful in cases where the presence of all static variables as > symbols in the object file are required. Can you give a more compelling reason that this option is neede

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

2023-05-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148665#4316310 , @peter.smith wrote: > My apologies for not responding. If I've got this right there are 4 related > patches: > D148573 (approved) > D148785

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-09 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 rGdbedcfdc2007: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking (authored by MaskRay). Changed prior to commit: https://review

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D149193#4331015 , @dyung wrote: > Hi @MaskRay, the test you modified clang/test/Driver/split-debug.c is failing > on the PS5 Windows bot, can you take a look? > > https://lab.llvm.org/buildbot/#/builders/216/builds/20981 > >

[PATCH] D137397: [Sanitizer] [Scudo] Add riscv64 support for scudo

2023-05-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: asb. 2538e550420fed5036d224fd93e9145c845f0ffe contains `Reviewers: cryptoad, eugenis, vitalybuka, luismarques, hiraditya`. The reviewer list isn't useful. Just drop it for fu

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, jamieschmeiser, Whitney, Maetveis, dblaikie, scott.linder. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Inspired by D133662

[PATCH] D148490: [AIX] use system assembler for assembly files

2023-05-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/aix-assembler.s:23 + +// RUN: %clang %s -### -c 2>&1 -fno-integrated-as \ +// RUN: --target=powerpc-ibm-aix7.1.0.0 \ I am not sure you need 6 RUN lines to test this. Whether a target uses integ

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added a comment. In D150282#4334927 , @Maetveis wrote: > LGTM, but I don't feel like I have the experience to "formally" approve. I > left a nice-to have suggestion too, but feel free to ignore, if you fe

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 521379. MaskRay marked an inline comment as done. MaskRay edited the summary of this revision. MaskRay added a comment. Add a warning test. Report -Wunused-command-line-argument warning for -ftime-trace-granularity=0 if unused as well. Repository: rG LLVM

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 521427. MaskRay marked an inline comment as done. MaskRay edited the summary of this revision. MaskRay added a comment. add an assert to addTimeTraceFile. improve a -dumpdir test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D150282#4335568 , @scott.linder wrote: > Does GCC have the same `-ftime-trace=` option? It seems like it doesn't, as > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92396 is still open? You found it! I am trying to collect o

[PATCH] D148490: [AIX] use system assembler for assembly files

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148490#4334476 , @shchenz wrote: >> I am not sure you need 6 RUN lines to test this. Whether a target uses >> integrated assembler has an existing test file and you can reuse that. > > I don't have strong prefer which one we

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/asan_abi/asan_abi_shim.cpp:14 +extern "C" { +// Globals +void __asan_register_image_globals(uptr *flag) { Comments are usually a complete sentence with a period. There are exceptions, but a "Globals" nee

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143675#4336365 , @rsundahl wrote: > In D143675#4310903 , @eugenis wrote: > >> I'm fine with it in general. Is asan_abi.cpp meant as a temporary stub? It's >> not even link anywhere in

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

2023-05-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143675#4310904 , @vitalybuka wrote: > In D143675#4310734 , @rsundahl > wrote: > >> @kcc @eugenis @MaskRay @vitalybuka Ok to go with this? All new functionality >> is under the added

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-12 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 rG49b87b05726b: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir (authored by MaskRay). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D150450: Add C++26 compile flags.

2023-05-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:114 +^ +- Compiler flags -std=c++2c and -std=gnu++2c have been added for experimental C++26 implementation work. + cor3ntin wrote: > This s

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

2023-05-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D146054#4337866 , @kito-cheng wrote: > GCC ins't implement yet, but planed, so add it later I think? > > @4vtomat already drop -march=help, @MaskRay did you mind take a look again? Done. But this update still doesn't look qui

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ftime-trace.cpp:54 +// RUN: %clang -### -ftime-trace=e -ftime-trace-granularity=0 d/a.cpp d/b.c -o f/x -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=LINK3 +// LINK3: -cc1{{.*}} "-ftime-trace=e/a-{{[^.]*}}.json" "-fti

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

2023-05-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:530 + Triple.isAArch64() || Triple.isPS() || Triple.isVE() || + (Triple.isAndroid() && (Triple.getArch() == llvm::Triple::riscv64))); if (NoOmitFP || mustUseNo

[PATCH] D150524: [cmake] Disable GCC lifetime DSE

2023-05-14 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. I think the correct fix for `trivially-destructible.cpp` is to append `--`. No `-p` or comment is needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150524/new/ https:

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

2023-05-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D150282#4341060 , @Jake-Egan wrote: > Hi, this fails on AIX. It looks like it still produces the > `/tmp/lit-tmp-*/[ab]-*.json` format. Can you take a look please? > > https://lab.llvm.org/buildbot/#/builders/214/builds/7429/s

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-05-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 2 inline comments as done. MaskRay added a comment. In D148785#4343112 , @peter.smith wrote: > Should `HANDLER(__ubsan_handle_function_type_mismatch,"function")` be added > to ubsan_minimal_runtime if this is supported in the minimal runt

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-05-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 522286. MaskRay marked 2 inline comments as done. MaskRay added a comment. rename a variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148785/new/ https://reviews.llvm.org/D148785 Files: clang/docs/Undef

[PATCH] D148827: -fsanitize=function: support C

2023-05-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 522287. MaskRay added a comment. rebase the final patch in the series Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148827/new/ https://reviews.llvm.org/D148827 Files: clang/docs/UndefinedBehaviorSanitizer.r

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

2023-05-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D150490#4343442 , @craig.topper wrote: > In D150490#4343145 , @enh wrote: > >> In D150490#4343128 , @hiraditya >> wrote: >> Is there mor

[PATCH] D150549: Move SubtargetFeature.h from MC to TargetParser

2023-05-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This seems fine, but please consider making `MC/SubtargetFeature.h` a forwarding header temporarily. Our official build system CMake doesn't do layering checking for headers (https://llvm.org/docs/CodingStandards.html#library-layering). Our unofficial build system Baze

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

2023-05-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Is this the new GCC behavior? > `clang -target mipsel-linux-gnu -mabi=64` Use `--target=` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146269/new/ https://reviews.llvm.org/D146269 ___ cfe-commits mailing list cfe-c

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

2023-05-16 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 rG7983f8aca82e: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple (authored by wzssyqa, committed by MaskRay). Repository: rG LLVM Git

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-05-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148785#4348053 , @vitalybuka wrote: > Is possible to split the patch into smaller ones? The Clang CodeGen side needs to match `llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp` and `compiler-rt/lib/ubsan`, so no, I think this is

[PATCH] D150632: [IR] Adds Instruction::setNoSanitizeMetadata()

2023-05-16 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. LGTM. Comment at: llvm/include/llvm/IR/Instruction.h:353 + /// Sets the nosanitize metadata on this instruction. + void setNoSanitizeMetadata(); `Set`.

[PATCH] D150549: Move SubtargetFeature.h from MC to TargetParser

2023-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. In D150549#4349531 , @jobnoorman wrote: > In D150549#4347056 , @MaskRay wrote: > >> This seems fine, but please consider making `MC/SubtargetFeature.h` a

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll:92 ; CHECK-NEXT: .Ltmp{{.*}}: ; CHECK-NEXT: nop ; CHECK-NEXT: .word 3238382334 // 0xc105cafe peter.smith wrote: > samitolvanen wrote: > > peter.smith wr

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/test/CodeGenCXX/catch-undef-behavior.cpp:408 // RTTI pointer check + // CHECK: [[CalleeTypeHashPtr:%.+]] = getelementptr <{ i32, i32 }>, <{ i32, i32 }>* [[PTR]], i32 -1, i32 1 --

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll:92 ; CHECK-NEXT: .Ltmp{{.*}}: ; CHECK-NEXT: nop ; CHECK-NEXT: .word 3238382334 // 0xc105cafe MaskRay wrote: > peter.smith wrote: > > samitolvanen wrote:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

2023-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/docs/ASanABI.rst:29 + + Following are some examples of reasonable responses to such changes: + How does the 2-space indentation render in the built HTML? It may look good, I ask just in case.

[PATCH] D150841: [IR] Make stack protector symbol dso_local according to -f[no-]direct-access-external-data

2023-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: ardb, nickdesaulniers, pengfei, xiangzhangllvm. Herald added subscribers: bd1976llvm, hiraditya, krytarowski, arichardson, nemanjai, emaste. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang,

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

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

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

2023-05-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Please also mark resolved comments as done, otherwise reviewers can presume that some comments are unresolved and don't necessarily give another round of reviews. Comment at: clang/test/Driver/print-supported-extensions.c:6 +// RUN: %clang --target=ri

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

2023-05-18 Thread 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/test/Driver/print-supported-extensions.c:1 +// Test that --print-supported-extensions lists supported extensions. + Some ne

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

2023-05-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:59 +- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16 + converted this to a warning-as-default-error as well as a SFINAE error. Mention `-Wvoid-ptr-dereference

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

2023-05-18 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. I object to parse and ignore this GCC option, which appears obscure. Clang traditionally added many `clang_ignored_gcc_optimization_f_Group` and `IgnoredGCCCompat` options to suppor

[PATCH] D148573: Port -fsanitize=function to AArch64

2023-04-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: dmgreen, lenary, pcc, peter.smith. Herald added subscribers: Enna1, hiraditya, kristof.beyls, emaste. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscrib

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

2023-04-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: dmgreen, lenary, pcc, peter.smith. Herald added subscribers: Enna1, hiraditya, kristof.beyls. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers: llv

[PATCH] D148573: Port -fsanitize=function to AArch64

2023-04-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148573#4277573 , @peter.smith wrote: > As it stands I think this may have problems with -mbranch-protection. In that > case we'll need a `BTI c` to be the target of the indirect branch. I'm > guessing something like: > >

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

2023-04-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 514761. MaskRay added a comment. fix clang/test/CodeGen Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148665/new/ https://reviews.llvm.org/D148665 Files: clang/lib/CodeGen/CGExpr.cpp clang/test/CodeGen/ubs

[PATCH] D148573: Port -fsanitize=function to AArch64

2023-04-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 514768. MaskRay edited the summary of this revision. MaskRay added a comment. Make -mbranch-protection=bti -fsanitize=function work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148573/new/ https://reviews.llv

[PATCH] D148671: [Driver] Make -fsanitize=kcfi,function incompatible

2023-04-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: nickdesaulniers, samitolvanen, peter.smith. Herald added subscribers: yaneury, supersymetrie, Chia-hungDuan, cryptoad. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscr

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

2023-04-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: llvm/lib/Target/Mips/MipsSubtarget.cpp:107 // Check if Architecture and ABI are compatible. - assert(((!isGP64bit() && isABI_O32()) || - (isGP6

[PATCH] D148671: [Driver] Make -fsanitize=kcfi,function incompatible

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515063. MaskRay edited the summary of this revision. MaskRay added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148671/new/ https://reviews.llvm.org/D148671 Files: clang/lib/Driver/Sanitiz

[PATCH] D148671: [Driver] Make -fsanitize=kcfi,function incompatible

2023-04-19 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 rGadbdef6a9f39: [Driver] Make -fsanitize=kcfi,function incompatible (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D146987#4281068 , @Orlando wrote: >> I think we may have an issue due to this. I'm basing that on the blamelist, >> but I'm still bisecting our build, which may take some time to confirm. Can >> you take a look, and revert if

[PATCH] D148556: [libcxxabi][demangle] create helper for std::string_view::starts_with

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. > This was added to ADT in https://reviews.llvm.org/D148367 then reverted in > https://reviews.llvm.org/D148547 because it was a layering violation pointed > out in https://reviews.llvm.org/D148384#4270356. This description requires reade

[PATCH] D148556: [libcxxabi][demangle] create helper for std::string_view::starts_with

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. A member from #libc_abi needs to review this as well to make this differential green :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148556/new/ https://reviews.llvm.org/D148556 _

[PATCH] D148387: remove Demangle/StringView.h

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. LGTM, but this can only be accepted from a member of #libc_abi Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148387/new/ https://reviews.llvm.org/D14

[PATCH] D148573: Port -fsanitize=function to AArch64

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148573#4280335 , @peter.smith wrote: > With moving the signature before the function entry this looks good to me. > I'm not so familiar with the code in https://reviews.llvm.org/D148665 would > ideally find someone a bit mo

[PATCH] D148757: [clang] Follow user-provided order for prefix map

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Can you add example options in the description and say how this patch is going to change it? I think `std::map` is intended so that for `-f*-prefix-map` values, if both `a/b=...` and `a/b/c=...` match an input path, the longest wins, not the latest wins. Repository:

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I hope that we add just one option, instead of aliases. Aliases are usually added for compatibility when something is deprecated or changed, not for new options. My "Request Changes" status still holds. The patch hasn't fixed something obvious: the description mentions

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515157. MaskRay edited the summary of this revision. MaskRay added a comment. remove an assert to make this feature available to all targets Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148665/new/ https://rev

[PATCH] D148573: Allow -fsanitize=function on all targets

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515161. MaskRay retitled this revision from "Port -fsanitize=function to AArch64" to "Allow -fsanitize=function on all targets". MaskRay edited the summary of this revision. MaskRay added a comment. Herald added subscribers: kadircet, ilya-biryukov. allow all

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515166. MaskRay added a comment. update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148665/new/ https://reviews.llvm.org/D148665 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/TargetInfo.cpp

[PATCH] D148779: [Sema] Fix spurious warning for printf("%lb", (long)10)

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aaron.ballman, dim, enh. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix https://github.com/llvm/llvm-project/issues/62247 D131057

[PATCH] D148779: [Sema] Fix spurious warning for printf("%lb", (long)10)

2023-04-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515205. MaskRay added a comment. update test/Sema/format-strings-fixit.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148779/new/ https://reviews.llvm.org/D148779 Files: clang/lib/AST/FormatString.cpp cla

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: Sanitizers, pcc, peter.smith, sberg, samitolvanen. 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: ll

[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515243. MaskRay added a comment. Herald added a subscriber: pengfei. update llvm/test/CodeGen Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148785/new/ https://reviews.llvm.org/D148785 Files: clang/docs/Unde

[PATCH] D148779: [Sema] -Wformat: recognize %lb for the ``printf``/``scanf`` family of functions

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 515357. MaskRay retitled this revision from "[Sema] Fix spurious warning for printf("%lb", (long)10)" to "[Sema] -Wformat: recognize %lb for the ``printf``/``scanf`` family of functions". MaskRay edited the summary of this revision. MaskRay added a comment.

[PATCH] D148779: [Sema] -Wformat: recognize %lb for the printf/scanf family of functions

2023-04-20 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 rG5cf37d8bd5c0: [Sema] -Wformat: recognize %lb for the printf/scanf family of functions (authored by MaskRay). Repository: rG LLVM Github Monorepo

[PATCH] D148827: -fsanitize=function: support C

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: Sanitizers, efriedma, pcc, peter.smith, sberg, samitolvanen. Herald added a subscriber: Enna1. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, Sanitizers. Herald added a subscriber: cfe-co

[PATCH] D148827: -fsanitize=function: support C

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This patch is also available https://github.com/MaskRay/llvm-project/tree/function-c , which may make inspecting stacked patches easier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148827/new/ https://reviews.llvm.org/D

[PATCH] D148596: [KMSAN] Enable on SystemZ

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll:12 + +; CHECK-LABEL: @Store1 +; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8 `; CHECK-LABEL: define {{[^@]+}}@Store1(` to match the `@St

[PATCH] D148596: [KMSAN] Enable on SystemZ

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1498 +if (MS.TargetTriple.getArch() == Triple::systemz) + MS.MsanMetadataAlloca = IRB.CreateAlloca(MS.MsanMetadata, (unsigned)0); } Just use 0. Even

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

2023-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148851#4285583 , @shafik wrote: > Do you know why these started timing out? I saw this locally the other day > but could not figure out the root cause. D86170 provides some information about

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

2023-04-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Related: D122444 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148945/new/ https://reviews.llvm.org/D148945 ___ cfe-commits mailing list cfe-co

[PATCH] D148958: Size and element numbers are often swapped when calling calloc

2023-04-21 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. This is an unnecessary change. In musl, there are at least 2 places where the parameters could be swapped, but I not unsure this warrants a commit. Repository: rG LLVM Github Mo

[PATCH] D148958: Size and element numbers are often swapped when calling calloc

2023-04-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D148958#4288782 , @vzakhari wrote: > In D148958#4288728 , @MaskRay wrote: > >> This is an unnecessary change. The arguments are interchangeable. >> In musl, there are at least 2 places

<    28   29   30   31   32   33   34   35   36   37   >