[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 230494. serge-sans-paille added a comment. Added extra test for arch support (warns if the flag is used on unsupported targets, and checks it's effectively unused), cc @sylvestre.ledru Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @Meinersbur to make your reviewer job easier, I've setup validation for that patch, see https://github.com/serge-sans-paille/llvm-project/pull/2/checks It build and validates fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. In D61446#1681841 , @Meinersbur wrote: > Keep in mind that for static linking you will need something that pulls-in a > symbol from the pass static library. In this patch, `NewPMDriver.cpp` does it > for `opt` by

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 230415. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Take into account most of the reviews. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @rnk : up ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 229127. serge-sans-paille added a comment. First rework pass, sevral comments have been taken into account but not all of them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @sylvestre.ledru did the testing and benchmarking on firefox (see https://bugzilla.mozilla.org/show_bug.cgi?id=1588710#c12), everything seems ok, let's move forward? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @sylvestre.ledru : quick backport for llvm9: https://sergesanspaille.fedorapeople.org/0001-Stack-clash-mir-attempt.patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @rnk what's your take on that version? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 227194. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Moved to a simpler, easier to review, less error prone implementation. Validates just fines on llvm bootstrap an cpython code base. @sylvestre.ledru

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 226109. serge-sans-paille added a comment. Temporarily comment out call support as free probe, everything else passes validation but a call may have some stack effect I don't handle (yet). CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Some benchmark / instrumentation report: due to the way memory moves are ordered in the entry block, there tend to be relatively few free probes between two stack growth within a function, and a large number after the last stack growth. When recompiling llc,

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 226043. serge-sans-paille added a comment. Update documentation as suggested by @sylvestre.ledru Corner case when a write was touching memory beyond the allocated stack. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 226002. serge-sans-paille added a comment. Better integration with MachineInstr description Handle stacks with multiple stack objects More test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 225580. serge-sans-paille added a comment. Explicilty prefer existing mechanism for windows. Split loop/block allocation strategy to different functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Extra note: an older version of the patch has been tested by the firefox team without much performance impact, (and no test failure), see https://bugzilla.mozilla.org/show_bug.cgi?id=1588710 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 225462. serge-sans-paille added a comment. Moved the implementation to a specialization of `emitStackProbeInline` that used to be Windows-centric. Provide a generic implementation that generates inline assembly instead. that way we don't clutter

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 225049. serge-sans-paille added a comment. Get rid of static mapping + update test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Another test run on an x264 encoder (source: https://openbenchmarking.org/test/pts/x264) Compiled with -O2 and with or without -fstack-clash-protection; Run without threads (`x265 --pools 1 -F 1 ./Bosphorus_1920x1080_120fps_420_8bit_YUV.y4m /dev/null`)

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. > For maintenance reasons, I'd really prefer it if we could find a way to reuse > the existing code that calls an external stack probe function. What do you > think about taking a look at X86RetpolineThunks.cpp and doing something > similar to that?

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Some benchmarks, running the python performance suite from recompiled cpython build, one built with `-O2` (baseline) and one built with `-O2 -fstack-clash-protection` (protection). Surprisingly enough, the stack protection makes the code faster in various

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224606. serge-sans-paille added a comment. Ensure the distance between two probes is at max PAGE_SIZE. Use Calls as free probes. Fix alignment for dynamic alloca This passes the llvm-test suite, and thanks to the use of calls, no inserted probe

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Some early stats: on the sqlite amalgamation [0], the free probe reuse allows to skip 123 out of the 474 probes needed during frame lowering. [0] https://www.sqlite.org/download.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224431. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Added test case, statistics and refactor interactions with existing stack probing mechanism. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille marked 3 inline comments as done. serge-sans-paille added inline comments. Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:400 +!(STI.isOSWindows() && !STI.isTargetMachO()); +if (InlineStackClashProtector && !InEpilogue) { + const uint64_t

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a subscriber: eli.friedman. serge-sans-paille added a comment. @efriedma the free probe algorithm requires more testing, and I'd like to take into account memset and memcpy as free probes too. To showcase this algorithm, consider the following LLVM bitcode: define

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224254. serge-sans-paille added a comment. Added documentation + release note entry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @efriedma alos compared to `probe-stack` with a function, this version has the ability to use existing MOV operations to avoid generating probes, which looks like a big plus to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224196. serge-sans-paille added a comment. Move to `stack-probe` compatibility, using a dedicated name to trigger inline assembly. It looks better to me because 1. it leverage existing mechanics 2. it has a finer grain Repository: rG LLVM

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @efriedma : there's indeed an intersection with the `probe-stack` attribute. The `probe-stack` attribute (a) forces a function call, and (b) this function call only happens **before** the stack gets expanded. (a) is probably a performance issue in several

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille created this revision. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, dschuff. Herald added projects: clang, LLVM. Implement protection against the stack clash attack [0]. Probe stack allocation every PAGE_SIZE during frame lowering or dynamic allocation to

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 221321. serge-sans-paille added a comment. Updates: - fix typo in documentation - take into account @andwar advices - improve shared/static build automation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @Meinersbur any feedback on this update? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. Up :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 217604. serge-sans-paille added a comment. @Meinersbur patch rebased. I removed the linker trick (which only work for global variables, not function, anyway), as it's no longer needed: target_compile_definitions(${name} PRIVATE

[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. Sorry for the long delay, this LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56343/new/ https://reviews.llvm.org/D56343

[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

serge-sans-paille added a reviewer: hfinkel. serge-sans-paille added a subscriber: hfinkel. serge-sans-paille added a comment. Adding @hfinkel to the thread to confirm it's okay to add this behavior, as he authored an RFC on the subject (see

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @Meinersbur up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65616/new/ https://reviews.llvm.org/D65616

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 213176. serge-sans-paille added a comment. Also fix symbol redefinition during static builds on Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @Meinersbur nop, forgot that one, I'll have a look, thanks for pointing that out. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 212567. serge-sans-paille added a comment. Make validation more resilient depending on shared/static build. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 Files:

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @Meinersbur I've updated the test case to test extension point if the extension is linked in, this is not so intrusive, I'm happy with the solution. Also fixed some linkage options when using llvm dynlib, validation looks nice on my side. Repository: rG

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 212509. serge-sans-paille added a comment. Herald added subscribers: dexonsmith, steven_wu. - Test extension point registration when extension is built-in - Correctly handle -DLLVM_LINK_LLVM_DYLIB=0 - Fix a few details Repository: rG LLVM Github

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 210273. serge-sans-paille marked 5 inline comments as done and 2 inline comments as done. serge-sans-paille added a comment. Added a `Bye` project in examples/ Fixed linking of plugins into core tools Fixed dependency issue Repository: rG LLVM

[PATCH] D64129: [clang-scan-view] Force utf-8 when handling report (python2 only)

serge-sans-paille created this revision. serge-sans-paille added a reviewer: michaelplatings. Herald added a project: clang. Herald added a subscriber: cfe-commits. If the report file contains unicode characters, under Python2, opening the file does not decode it to utf-8 by default.

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille marked 10 inline comments as done. serge-sans-paille added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:808 +if(NOT llvm-pass-plugins) +# Target used to hold global properties referencable from generator-expression

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 206648. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. @Meinersbur your comment and my devs crossed, but this should be fine. This update enables new PM static plugin support for clang, something that was

[PATCH] D63613: [clang-tidy] Fail gracefully upon empty database fields

serge-sans-paille added a comment. Thanks @jkorous for the fast review o/ Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63613/new/ https://reviews.llvm.org/D63613 ___ cfe-commits mailing list

[PATCH] D63613: [clang-tidy] Fail gracefully upon empty database fields

serge-sans-paille created this revision. serge-sans-paille added a reviewer: alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. fix https://bugs.llvm.org/show_bug.cgi?id=42281 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D63613 Files:

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 205298. serge-sans-paille added a comment. Mostly documentation update + helper function renaming. @meinersbur, I've tested the setup with static and dynamic builds on Linux, please let me know your thoughts on this. CHANGES SINCE LAST ACTION

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added inline comments. Comment at: polly/lib/Support/RegisterPasses.cpp:727 +extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK +llvmGetPassPluginInfo() { + return getPassPluginInfo(); Meinersbur wrote: > [serious] Unfortunately, the

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 204073. serge-sans-paille marked 3 inline comments as done. serge-sans-paille added a comment. @beanz : In addition to your suggested changes, I've renamed the cmake utility into `add_llvm_pass_plugin` to match other functions from `AddLLVM.cmake`

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 203697. serge-sans-paille added a comment. Using @beanz idea, provide an option to `add_llvm_executable` to declare it as receiving plugins, which provides lower coupling with actual plugins Tested with four configurations: `BUILD_SHARED_LIBS`

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. In D61446#1532848 , @Meinersbur wrote: > Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in > opt/bugpoint/clang by adding Polly.cpp as a source file or object library to > the executables. This

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @Meinersbur Itested with the following configuration cmake3 ../llvm -DLLVM_EXTERNAL_PROJECTS=bye -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 cmake3 ../llvm -DLLVM_EXTERNAL_PROJECTS=bye -DBUILD_SHARED_LIBS=ON

[PATCH] D62623: Reduce memory consumption of coverage dumps

This revision was automatically updated to reflect the committed changes. Closed by commit rG4cd07dbeec98: Reduce memory consumption of coverage dumps (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62623/new/

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 203086. serge-sans-paille added a comment. - get Rid of the constraint on initializePasses : it can be registered suing a static constructor, no need to clutter the API with it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 203029. serge-sans-paille added a comment. - make it possible for client code to speicify the registration function for new PM, thanks @philip.pfaffe for the hint. @Meinersbur this should be ok now. CHANGES SINCE LAST ACTION

[PATCH] D62623: Reduce memory consumption of coverage dumps

serge-sans-paille added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1393 + size_t CoverageMappingSize = 0; + for (auto : CoverageMappings) { +CoverageMappingSize += S.size(); vsk wrote: > It doesn't look like the CoverageMappings

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 202950. serge-sans-paille added a comment. Take into account @philip.pfaffe remarks and improve support for new PM. - harmonize static/dynamic loading, although two functions are still needed, one for dynamic loading and one for static loading

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @beanz turns out the depdendency problem was just a matter of moving the `register_llvm_extension` call at the top of the file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille added a comment. @philip.pfaffe : NewPM integeration actually works thanks to ``Register${Name}Passes(llvm::PassBuilder )``. It's already used by Polly for NewPM integration and we mimic that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 202908. serge-sans-paille added a comment. - reorder registration to make sure options are correctly taken into account - make LINK_POLLY_INTO_TOOLS obsolete in favor of just using LLVM_LINK_POLLY_INTO_TOOLS Repository: rG LLVM Github Monorepo

[PATCH] D62623: Reduce memory consumption of coverage dumps

serge-sans-paille updated this revision to Diff 202896. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Update comment + force reduced memory consumption. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

serge-sans-paille updated this revision to Diff 202795. serge-sans-paille added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. - provide minimal documentation - fix testing configuration - validate locally with both shared and static libraries, using monorepo or

[PATCH] D62623: Reduce memory consumption of coverage dumps

serge-sans-paille added a comment. @kwk: looks like you're still compiling with clang-7, this patch is to be applied on the master version of LLVM/clang, then you can install it and use it to recompile llvm/lld with coverage info. Does it make sense to you? Repository: rG LLVM Github

[PATCH] D62623: Reduce memory consumption of coverage dumps

serge-sans-paille created this revision. serge-sans-paille added reviewers: vsk, arphaman. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. serge-sans-paille edited the summary of this revision. Avoiding an intermediate join operation, which in turns removes the

[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

serge-sans-paille added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105 // which is just that always inlining occurs. - MPM.addPass(AlwaysInlinerPass()); + // We always pass false here since according to the legacy PM logic for + //

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

serge-sans-paille updated this revision to Diff 199585. serge-sans-paille added a comment. Updated with clang-formated diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59413/new/ https://reviews.llvm.org/D59413 Files: clang/include/clang/Basic/SourceManager.h

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

serge-sans-paille added a comment. @rsmith : up :-) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59413/new/ https://reviews.llvm.org/D59413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

serge-sans-paille added a comment. @rsmith : up :-) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59413/new/ https://reviews.llvm.org/D59413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

serge-sans-paille added a comment. @rsmith up! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59413/new/ https://reviews.llvm.org/D59413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59734: [clang-tidy] Handle missing yaml module in run-clang-tidy.py

serge-sans-paille added inline comments. Comment at: clang-tidy/tool/run-clang-tidy.py:210 +help='Create a yaml file to store suggested fixes in, ' +'which can be applied with clang-apply-replacements.')

[PATCH] D59734: [clang-tidy] Handle missing yaml module in run-clang-tidy.py

serge-sans-paille added inline comments. Comment at: clang-tidy/tool/run-clang-tidy.py:51 + +yaml_imported = True +try: Could be ``` try: import yaml except ImportError: yaml = None ``` and later ``` if yaml: ... ``` CHANGES SINCE LAST ACTION

[PATCH] D59440: add steps to preprocess file and reduce command line args

serge-sans-paille added inline comments. Comment at: clang/utils/creduce-clang-crash.py:45 + result = [] + for i, arg in enumerate(cmd): +if arg.startswith('$'): Useless enumerate here. You could even use a list comprehension here ``` return ' '.join(arg

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

serge-sans-paille created this revision. serge-sans-paille added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. When a warning is raised from the expansion of a system macro that involves pasted token, there was still situations were they were not

[PATCH] D57523: Fix uninitialized value in ABIArgInfo

serge-sans-paille added a comment. @hans agreed; Thanks for taking the time to try to reproduce the original issue o/ Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57523/new/ https://reviews.llvm.org/D57523 ___

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

serge-sans-paille added a comment. @MarcoFalke it's in, thanks for the patch o/ Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57528/new/ https://reviews.llvm.org/D57528 ___ cfe-commits mailing list

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. Herald added a project: clang. LGTM, do you have commit right? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57528/new/

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Herald added a subscriber: llvm-commits. It's in, thanks @rsmith for the review. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55782/new/ https://reviews.llvm.org/D55782

[PATCH] D57523: Fix uninitialized value in ABIArgInfo

serge-sans-paille created this revision. serge-sans-paille added a reviewer: hans. Herald added a subscriber: cfe-commits. GCC-9 takes advantage of this uninitialized values to optimize stuff, which ends up in failing validation when compiling clang. This fixes

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: include/clang/Basic/SourceManager.h:1458-1460 + // This happens when the macro is the result of a paste, in that case + // its spelling is the scratch

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

serge-sans-paille updated this revision to Diff 184275. serge-sans-paille added a comment. Patch updated to take into account @rsmith remarks. Test case added as an illustration. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55782/new/

[PATCH] D56429: fix python3 compability issue

serge-sans-paille added a comment. Thanks @roxma for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D56429: fix python3 compability issue

serge-sans-paille added a comment. @roxma: do you want me to commit this on your behalf? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

serge-sans-paille added a comment. @rsmith up! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55782/new/ https://reviews.llvm.org/D55782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D56429: fix python3 compability issue

serge-sans-paille accepted this revision. serge-sans-paille added inline comments. This revision is now accepted and ready to land. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

serge-sans-paille updated this revision to Diff 181498. Herald added a subscriber: cfe-commits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55782/new/ https://reviews.llvm.org/D55782 Files: include/clang/Basic/SourceManager.h

[PATCH] D56429: fix python3 compability issue

serge-sans-paille added inline comments. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if hasattr(contents, "read"): +contents = contents.read()

[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54071/new/ https://reviews.llvm.org/D54071

[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

serge-sans-paille added a comment. @Romain-Geissler-1A sure thing. Can you rebase your patch on master first? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54071/new/ https://reviews.llvm.org/D54071 ___ cfe-commits

[PATCH] D56429: fix python3 compability issue

serge-sans-paille added inline comments. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if hasattr(contents, "read"): +contents = contents.read() Why did you

[PATCH] D56343: [clang-tidy] Refactor: Compose Method on check_clang_tidy.py

serge-sans-paille added inline comments. Comment at: test/clang-tidy/check_clang_tidy.py:103 + return (has_check_fixes, has_check_messages, has_check_notes, \ +check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes) Such a long list of

[PATCH] D56341: [python] Make the collections import future-proof

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56341/new/ https://reviews.llvm.org/D56341

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

serge-sans-paille resigned from this revision. serge-sans-paille added a comment. Just wanted to review the change to the Python script to ensure it remains compatible with Python2 and Python3, which is indeed the case; so LGTM on that aspect. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

serge-sans-paille updated this revision to Diff 176330. serge-sans-paille edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55121/new/ https://reviews.llvm.org/D55121 Files: bindings/python/clang/cindex.py docs/tools/dump_format_style.py

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

serge-sans-paille updated this revision to Diff 176141. serge-sans-paille added a comment. Use generic ``from __future__ import `` line whenever it makes sense CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55121/new/ https://reviews.llvm.org/D55121 Files:

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

serge-sans-paille added a comment. Conversion using the ``futurize`` script, then manual review. For the sake of reviewer sanity, I've left all the difficult part to further separate commits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55121/new/

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

serge-sans-paille created this revision. serge-sans-paille added reviewers: ddunbar, djasper. Herald added subscribers: cfe-commits, kadircet, arphaman, whisperity. Mostly: - portability of print through from __future__ import print_function - xrange -> range - range(...) ->

<    3   4   5   6   7   8   9   >