[PATCH] D107667: [clang/test] Run thinlto-clang-diagnostic-handler-in-be.c on x86

steven_wu added a comment. Please don't use `x86_64` as target or triple since it might default to some weird platform. Can you spell out a valid triple/platform? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107667/new/ https://reviews.llvm.org/D

[PATCH] D107667: [clang/test] Run thinlto-clang-diagnostic-handler-in-be.c on x86

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. LGTM. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107667/new/ https://reviews.llvm.org/D107667 _

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

steven_wu added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:3002 +// The format of the stat cache is (pseudo-code): +// struct stat_cache { +//char Magic[4]; // "STAT" or "Stat" Few comments about stats representation. 1. Ca

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

steven_wu added a comment. > Yes, we can. I pondered doing it in the original patch but didn't see a > reason this would evolve in the future. Of course, I can't predict the > future, so maybe a version field is the safe approach. I think a version number should be good enough for now. For the

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

steven_wu added a comment. If we have to fix it, I slightly prefer just give it a proper option in CodeGenOption and a `cc1` and `cc1as` flag. Comment at: llvm/lib/MC/MCContext.cpp:62 -static cl::opt +static cl::opt AsSecureLogFileName("as-secure-log-file-name", ---

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. LGTM except the comments from @MaskRay CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136888/new/ https://reviews.llvm.org/D136888 __

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

steven_wu accepted this revision. steven_wu added a comment. LGTM. `RemoveDecl` does become more expensive but I don't have better solution. I am also wondering if as follow up we should add an option to `VerifyDiagnosticConsumer` to be location aware (so the diagnostics from a file is emitted

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

steven_wu added a comment. I don't know too much about this. I guess have a DiagReceiver to force the emitting order is a good thing but it is kind of weird to have this just for unused warning. I am guessing my suspect of removing decl from the scope is the cause of the slow down. Maybe we ju

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. Ok with me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135490/new/ https://reviews.llvm.org/D135490 ___

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu created this revision. steven_wu added reviewers: sammccall, akyrtzi, rymiel. Herald added subscribers: ributzka, kadircet, arphaman. Herald added a project: All. steven_wu requested review of this revision. Herald added projects: LLVM, clang-tools-extra. Herald added a subscriber: cfe-co

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu updated this revision to Diff 467215. steven_wu added a comment. Herald added a subscriber: bmahjour. Herald added a project: clang. Address review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135712/new/ https://reviews.llvm.or

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu updated this revision to Diff 467217. steven_wu added a comment. Was missing the latest feedback. Bump FindGRPC module to top layer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135712/new/ https://reviews.llvm.org/D135712 Files: clan

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu added inline comments. Comment at: cmake/Modules/FindGRPC.cmake:111 # They may be relative to the source dir or absolute (for generated protos). -function(generate_protos LibraryName ProtoFile) +macro(generate_protos_source GeneratedSource ProtoFile) cmake_parse_arg

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu updated this revision to Diff 467243. steven_wu added a comment. Minor rename Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135712/new/ https://reviews.llvm.org/D135712 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu added inline comments. Comment at: cmake/Modules/FindGRPC.cmake:111 # They may be relative to the source dir or absolute (for generated protos). -function(generate_protos LibraryName ProtoFile) +macro(generate_protos_source GeneratedSource ProtoFile) cmake_parse_arg

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

steven_wu updated this revision to Diff 467247. steven_wu added a comment. Turn generate_proto_sources back to function and forward argument more elegantly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135712/new/ https://reviews.llvm.org/D135712

[PATCH] D135712: [CMake] Fix FindGRPC cmake module to allow different layering

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4ba360d499f6: [CMake] Fix FindGRPC cmake module to allow different layering (authored by steven_wu). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137838: [Support] Move TargetParsers to new component

steven_wu added a comment. Also needed a follow up fix to completely fix module `9cd6fbee7ed8` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137838 ___ cfe-commits m

[PATCH] D139938: [clang] Don't spuriously pass -stdlib=libc++ to CC1 on Darwin

steven_wu added a comment. In D139938#4008232 , @hans wrote: > This seems to have broken the instrprof-darwin-exports.c test, see e.g. > https://green.lab.llvm.org/green/job/clang-stage1-RA/32351/ > > I'll revert for now. It seems you still need to clai

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

steven_wu updated this revision to Diff 487597. steven_wu added a comment. Address review feedback. Remove NFC from title. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ https://reviews.llvm.org/D139052 Files: clang/include/clang/Code

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4067225 , @dblaikie wrote: > In D141625#4066961 , @steven_wu > wrote: > >> No, reverse iteration will not break diff test for a small number of decls. >> Everything will be

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

steven_wu updated this revision to Diff 493724. steven_wu added a comment. Rebase to main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ https://reviews.llvm.org/D139052 Files: clang/include/clang/CodeGen/BackendUtil.h clang/lib/Co

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4094837 , @dblaikie wrote: > In D141625#4094742 , @steven_wu > wrote: > >> In D141625#4067225 , @dblaikie >> wrote: >> >>> In D1416

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

steven_wu updated this revision to Diff 493768. steven_wu added a comment. Try fix pre-merge build failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ https://reviews.llvm.org/D139052 Files: clang/include/clang/CodeGen/BackendUtil.

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. > Sorry, I'm still not really following - OK, sounds like you're saying this > test does fail at HEAD/without this patch in reverse iteration mode, and is a > bit larger than would be minimally necessary to achieve that, but also might > fail at HEAD without reverse i

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

This revision was automatically updated to reflect the committed changes. Closed by commit rG516e30175256: [NFC][Profile] Access profile through VirtualFileSystem (authored by steven_wu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ htt

[PATCH] D141632: [Module] Respect `-fno-pch-timestamps` when building modules

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5cff68fca0bc: [Module] Respect `-fno-pch-timestamps` when building modules (authored by steven_wu). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. I don't think it is feasible with current tool to write a test that check semantically the order of decls in clang module. (Let me know if that was wrong). The current test already unfortunately relies on the module layout assuming `op13` is the field for anonymous de

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4100790 , @dblaikie wrote: > In D141625#4100762 , @steven_wu > wrote: > >> I don't think it is feasible with current tool to write a test that check >> semantically the orde

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

This revision was automatically updated to reflect the committed changes. Closed by commit rG0480748ea672: [DeclContext] Sort the Decls before adding into DeclContext (authored by steven_wu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu created this revision. steven_wu added reviewers: jansvoboda11, akyrtzi, benlangmuir, vsapsai, rnk, dblaikie. Herald added subscribers: ributzka, mgrang. Herald added a project: All. steven_wu requested review of this revision. Herald added a project: clang. Fix a non-deterministic issu

[PATCH] D141632: [Module] Respect `-fno-pch-timestamps` when building modules

steven_wu created this revision. steven_wu added reviewers: benlangmuir, akyrtzi, jansvoboda11. Herald added a subscriber: ributzka. Herald added a project: All. steven_wu requested review of this revision. Herald added a project: clang. Always respect the FrontendOption to not include timestamps

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4052869 , @rsmith wrote: > In D141625#4052820 , @dblaikie > wrote: > >> Yeah, might be nice to document where the instability comes from - if it >> comes from a DenseMap or

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu updated this revision to Diff 489150. steven_wu added a comment. @akyrtzi has the good idea. It is really hard to control `Decl*` to get values to get an unstable iteration order from the small tests, going beyond 32 decls to get out of SmallPtrSet's small model is much consistent. Add

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu updated this revision to Diff 489587. steven_wu added a comment. Minor touch up on the test case. Also add some comments in test to explain what is being tested. I don't measure any performance regression from `-fsyntax-only` on Foundation.h but if there are some other performance ben

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. Actually, sorting in `numberAnonymousDeclsWithin` doesn't work for some reasons. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625 ___

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4056831 , @steven_wu wrote: > Actually, sorting in `numberAnonymousDeclsWithin` doesn't work for some > reasons. The reason for this doesn't work is `ASTWriter::WriteDeclContextLexicalBlock` also iterates on `DeclC

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. @dblaikie Do we have any bots running reverse iteration? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625 ___ cfe-commits mailing list

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4059540 , @tschuett wrote: > EXPANSIVE_CHECKS will reshuffle the llvm::sort input: > https://lists.llvm.org/pipermail/llvm-dev/2018-April/122576.html This is a slightly different concern. The problem to catch is ite

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4060460 , @dblaikie wrote: > In D141625#4059486 , @steven_wu > wrote: > >> @dblaikie Do we have any bots running reverse iteration? > > Hmm, not that I can see/find at a quic

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. @dblaikie Do you have any objection for committing the patch as it is? I don't think reverse iteration test is a proper way to test for this specific bug. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https:/

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. In D141625#4066466 , @dblaikie wrote: > In D141625#4066362 , @steven_wu > wrote: > >> @dblaikie Do you have any objection for committing the patch as it is? I >> don't think reverse it

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu updated this revision to Diff 490616. steven_wu added a comment. Update tests according to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625 Files: clang/lib/Parse/ParseDecl.cpp clang

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

steven_wu added a comment. This breaks macOS bot too: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/33839/ I think this changes might be more disruptive than expected. While it is encouraged to use `-isysroot` when building anything on Darwin platform, it is also perfe

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

steven_wu added a comment. No, reverse iteration will not break diff test for a small number of decls. Everything will be in reverse order so it is the same. Current test will fail early in reverse iteration and will fail in the end statistically for forward iteration. Will pass in all configur

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

steven_wu added a comment. In D136315#4067205 , @calebzulawski wrote: > One thing to throw into the mix: Apple's clang has a default sysroot > configured, so with the default system compiler, there is no way to replicate > this "build without a sysroot

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

steven_wu added a comment. In D136315#4069582 , @calebzulawski wrote: > That makes more sense, I thought perhaps it was using `DEFAULT_SYSROOT`. The > shim isn't smart enough to choose the sysroot from the target unfortunately. > > It looks like the on

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

steven_wu added a comment. This is not an apple platform problem. This is a cross compile problem where you have a SDK that is installed for cross compile target. In those cases, you just can't hard code the path to isysroot, and figuring out the sysroot is generally considered a problem for th

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

steven_wu added a comment. In D136315#4069849 , @calebzulawski wrote: > In my situation, at least, I am the vendor of the toolchain and my > configuration file contains `--sysroot=/../path/to/sysroot` with a > known relative path to where the sysroot i

[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. Thanks! LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125974/new/ https://reviews.llvm.org/D125974

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

steven_wu added a comment. I don't like this doesn't write to the output file by `-o` too. I also think the output should just match `llvm-dis` output, instead of splitting into multiple files. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153906/new/ https://reviews.llvm.org/D153906

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

steven_wu added a comment. This breaks Darwin bots: https://green.lab.llvm.org/green/job/clang-stage1-RA/35102/testReport/junit/Clang/AST/ast_print_method_decl_cpp/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156533/new/ https://reviews.llvm.org

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

steven_wu added a comment. error: aliases are not supported on darwin Maybe create a different testcase that is not supported on Darwin? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156533/new/ https://reviews.llvm.org/D156533

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

steven_wu added a comment. Reverted in 4098e13a7146 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156533/new/ https://reviews.llvm.org/D156533 _

[PATCH] D155824: [LoongArch] Support -march=native and -mtune=

steven_wu added inline comments. Comment at: llvm/lib/TargetParser/LoongArchTargetParser.cpp:19 +StringRef Arch; +StringRef TuneCPU; Why do we store `Arch` and `TuneCPU` as globals? We should not have as little global state in compiler as possible. Any reason

[PATCH] D155824: [LoongArch] Support -march=native and -mtune=

steven_wu added a comment. After another look, I will need to request to revert this because this implementation doesn't work with `-fno-integrated-cc1` at all. You can't setCPU/Tune in the driver into a global variable and expect that will work during compilation. You can reproduce with `-fno

[PATCH] D155824: [LoongArch] Support -march=native and -mtune=

steven_wu added a comment. In D155824#4548921 , @SixWeining wrote: > In D155824#4548677 , @steven_wu > wrote: > >> After another look, I will need to request to revert this because this >> implementation doesn'

[PATCH] D157119: cmake: add missing dependencies on ClangDriverOptions tablegen

steven_wu accepted this revision. steven_wu added subscribers: aprantl, dblaikie. steven_wu added a comment. This revision is now accepted and ready to land. LGTM. @aprantl @dblaikie who cares about module builds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D156014: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets

steven_wu added a comment. This breaks macOS bot: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/36900/testReport/junit/Clang/SemaCUDA/alias_cu/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156014/new/ https://reviews.llvm

[PATCH] D156014: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets

steven_wu added a comment. In D156014#4567446 , @jhuber6 wrote: > In D156014#4567363 , @steven_wu > wrote: > >> This breaks macOS bot: >> https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/36

[PATCH] D155824: [LoongArch] Support -march=native and -mtune=

steven_wu accepted this revision. steven_wu added a comment. In D155824#4568904 , @xen0n wrote: > This still LGTM, @steven_wu would you please take another look so this can > get re-landed if confirmed working? The driver part LGTM. Repository: rG L

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

steven_wu created this revision. steven_wu added reviewers: davidxl, tejohnson, xur, akyrtzi. Herald added subscribers: Enna1, ormris, wenlei, ributzka, pengfei, hiraditya. Herald added a project: All. steven_wu requested review of this revision. Herald added projects: clang, LLVM. Herald added a s

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

steven_wu added a comment. I forgot if there is reason to use the option by default at all time (I did ask that in the previous review but Alex might have given more context offline). You should definitely add test for this change. The fact that you change all `-mlinker-version=400` to `-mlinke

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

steven_wu added a comment. In D74784#1891750 , @dmajor wrote: > @steven_wu, ping, could you clarify about the tests please? You want to have testcase to cover all following 3 cases: - Default (version = 0): not using -platform_version - Old version (0 <

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

steven_wu added a comment. Thanks. LGTM. I will let @arphaman to approve this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74784/new/ https://reviews.llvm.org/D74784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

steven_wu added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:335 + .Case("bitcode", EmbedBitcodeKind::Embed_Bitcode) + .Case("marker", EmbedBitcodeKind::Embed_Marker) + .Default(~0U); t

[PATCH] D69893: libunwind: Evaluating DWARF operation DW_OP_pick is broken

steven_wu added a comment. The fix LGTM. Do you have a reproducer that can be used as a testcase? We should really add more tests for libunwind. Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69893/new/ https://reviews.llvm.org/D69893 ___

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

steven_wu added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:339 // Add the deployment target. - MachOTC.addMinVersionArgs(Args, CmdArgs); + if (!Version[0] || Version[0] >= 520) +MachOTC.addPlatformVersionArgs(Args, CmdArgs); Why

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

steven_wu added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2530 + return "ios"; +// FIXME: Add macCatalyst support here ("\"mac catalyst\""). +llvm_unreachable("macCatalyst isn't yet supported"); arphaman wrote: > steven_wu w

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. SGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71579/new/ https://reviews.llvm.org/D71579

[PATCH] D69893: libunwind: Evaluating DWARF operation DW_OP_pick is broken

steven_wu accepted this revision. steven_wu added a comment. In D69893#1786222 , @kamleshbhalui wrote: > In D69893#1786202 , @steven_wu wrote: > > > The fix LGTM. Do you have a reproducer that can be used as a test

[PATCH] D69893: libunwind: Evaluating DWARF operation DW_OP_pick is broken

This revision was automatically updated to reflect the committed changes. Closed by commit rG9366397f057d (authored by steven_wu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69893/new/ https://reviews.llvm.org/D69893 Files: libunwind/src/DwarfI

[PATCH] D69893: libunwind: Evaluating DWARF operation DW_OP_pick is broken

steven_wu added a comment. Commited in: 9366397f057d18401e680b2cb28a0ee17c59d4a6 Phabriactor might not update this because the patch was created on libunwind repo, not the monorepo. Repository: rUNW libunwind CHANGES SIN

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

steven_wu added a comment. @rsmith This also breaks macOS SDK. Can we revert this as we discuss a less aggressive option? Here is an example in macOS SDK (from /usr/include/simd/logic.h): static inline SIMD_CFUNC simd_bool simd_any(simd_int2 x) { return (_mm_movemask_ps(simd_make_int4_und

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

steven_wu added a subscriber: dexonsmith. steven_wu added a comment. In D67678#1834957 , @rsmith wrote: > In D67678#1834542 , @steven_wu wrote: > > > @rsmith This also breaks macOS SDK. Can we revert this as we disc

[PATCH] D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros

steven_wu added a comment. I disagree. I think "target" is the correct name, even for cross compiling. For something compiled with -target foo, we are consistent calling it "target" during compile time. It only becomes appropriate to call it "host" during the runtime of the executable. There is

[PATCH] D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros

steven_wu added a comment. It is not about matching command line name to builtin marco name. "target" is the platform we are compiling for, whether it is host or device or something else. It is a different concept when you talking about cross-compiling, which "target" is strictly not host and "

[PATCH] D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros

steven_wu added a comment. I am not trying to discuss which english word is best here. My point is simply: 1. macros are evaluated during compile time 2. "host"means either the platform you compiled on during compile time or the platform you run on during the runtime 3. __is_host_* is not a good

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu created this revision. This is basically reverting r261774 with a tweak for clang-cl. UNIX standard states: When c99 encounters a compilation error that causes an object file not to be created, it shall write a diagnostic to standard error and continue to compile other source code operan

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu added a comment. More background here that doesn't fit in the commit message. The tweak for cl mode is intended for r262420. I don't see any test breakage on my machine without that hack but I leave it in there just in case. A little reasoning behind clang driver for CUDA. This is wha

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 121403. steven_wu added a comment. Address review feedback. Also split out the testcase for UNIX conformance. That test is not related to output file cleanup. Split it out to make it clear that is for UNIX conformance. https://reviews.llvm.org/D39502 Fil

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 121410. steven_wu added a comment. Fix testcase. My test was passing because files left over from previous run. Now it should work with a clean build. https://reviews.llvm.org/D39502 Files: lib/Driver/Compilation.cpp test/Driver/output-file-cleanup.c

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu added a comment. In https://reviews.llvm.org/D39502#916409, @jlebar wrote: > > Note the host clang side has the inputs "test.cu" and "test-c3378c.fatbin". > > Which means if the device side compilation failed, the host side will not > > compile because InputOk will return false in thi

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 121779. steven_wu added a comment. This seems to be the cleanest solution I can find for CUDA without touching the job configuration for CUDA. My proposed fix is to bail out of the jobs that are parts of CUDA offload if some command failed before that. Now i

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 121795. steven_wu added a comment. Add testcase for CUDA driver https://reviews.llvm.org/D39502 Files: lib/Driver/Compilation.cpp test/Driver/cuda-bail-out.cu test/Driver/output-file-cleanup.c test/Driver/unix-conformance.c Index: test/Driver/uni

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu added a comment. In https://reviews.llvm.org/D39502#917132, @tra wrote: > In https://reviews.llvm.org/D39502#917097, @steven_wu wrote: > > > Also, the reason I don't know how to craft a testcase is not because I have > > trouble with CUDA driver, but how to write a test to check when d

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 121807. steven_wu added a comment. Improve testcase according to review feedback. In order to let compilation to be successful for one input, I need to use -fsyntax-only because I don't have nvptx assembler. Enable -fsyntax-only changes the order of the com

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu added inline comments. Comment at: test/Driver/cuda-bail-out.cu:47 +// CHECK-HOST-ERROR: Error during compilation for host +// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35 tra wrote: > To make it more robust, I'd add another copy of the last

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu added inline comments. Comment at: test/Driver/cuda-bail-out.cu:47 +// CHECK-HOST-ERROR: Error during compilation for host +// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35 tra wrote: > steven_wu wrote: > > tra wrote: > > > To make it more ro

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 121822. steven_wu added a comment. Update testcase for CUDA driver https://reviews.llvm.org/D39502 Files: lib/Driver/Compilation.cpp test/Driver/cuda-bail-out.cu test/Driver/output-file-cleanup.c test/Driver/unix-conformance.c Index: test/Driver/

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu added inline comments. Comment at: test/Driver/cuda-bail-out.cu:47 +// CHECK-HOST-ERROR: Error during compilation for host +// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35 tra wrote: > steven_wu wrote: > > tra wrote: > > > steven_wu wrote: >

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 122372. steven_wu added a comment. I think I understand what you mean now. You want some cases when the compilation doesn't failed on the first source it process. I add a testcase which compiles for host + sm_35 + sim_60. I run the test 3 times and each tim

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

steven_wu updated this revision to Diff 122376. steven_wu added a comment. Add more tests! The new tests can trigger two errors during the compilation but only one should be emitted. https://reviews.llvm.org/D39502 Files: lib/Driver/Compilation.cpp test/Driver/cuda-bail-out.cu test/Driver

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

This revision was automatically updated to reflect the committed changes. Closed by commit rL317860: [Driver] Make clang/cc conforms to UNIX standard (authored by steven_wu). Repository: rL LLVM https://reviews.llvm.org/D39502 Files: cfe/trunk/lib/Driver/Compilation.cpp cfe/trunk/test/Dri

[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

steven_wu added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:3016 for (const auto &I : *State) { -if (I.second.isPragma() || IncludeNonPragmaStates) { - Record.push_back(I.first); - Record.push_back(I.second.serialize()); -

[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154016/new/ https://reviews.llvm.org/D154016 ___ cfe-commits mailing list cfe-com

[PATCH] D154191: [LTO] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. This is generally fine. I don't see any error that should fall into `consumeError` condition that won't just error out anyway (like out of disk space, etc.) and I don't see those are spe

[PATCH] D152924: [libLTO][AIX] Respect `-f[no]-integrated-as`

steven_wu added a comment. `lld` does not use LTOCodeGenerator. I am also not sure how disable integrated assembler suppose to work? Do you have an end to end design for how to invoke assembler during linking? For current libLTO API, this is probably going to produce assembler file buffer to l

[PATCH] D152924: [libLTO][AIX] Respect `-f[no]-integrated-as` on AIX

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. I see. Sounds fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152924/new/ https://reviews.llvm.org/D152924

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

steven_wu updated this revision to Diff 480990. steven_wu marked 13 inline comments as done. steven_wu added a comment. Address review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ https://reviews.llvm.org/D139052 Files: cl

<    1   2   3   >