[PATCH] D28385: Add a cc1 option to force disabling lifetime-markers emission from clang

mehdi_amini created this revision. mehdi_amini added reviewers: chandlerc, rsmith. mehdi_amini added a subscriber: cfe-commits. This intended as a debugging/development flag only. https://reviews.llvm.org/D28385 Files: clang/include/clang/Driver/CC1Options.td clang/include/clang/Frontend/Co

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini created this revision. mehdi_amini added reviewers: chandlerc, rsmith. mehdi_amini added subscribers: cfe-commits, dexonsmith. Herald added a reviewer: tstellarAMD. Herald added a subscriber: nhaehnle. Amongst other, this will help LTO to correctly handle/honor files compiled with O0,

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini updated this revision to Diff 83391. mehdi_amini added a comment. Herald added a subscriber: wdng. Remove spurious change https://reviews.llvm.org/D28404 Files: clang/include/clang/Driver/CC1Options.td clang/include/clang/Frontend/CodeGenOptions.def clang/lib/CodeGen/CodeGenMo

[PATCH] D28409: Use CodegenOpts::less when creating a TargetMachine for clang `-O1`

mehdi_amini created this revision. mehdi_amini added reviewers: echristo, chandlerc. mehdi_amini added a subscriber: cfe-commits. Clang was initializing the TargetMachine with CodeGenOpt::Default for https://reviews.llvm.org/owners/package/1/. This change is aligning it on llc: -O0: OptLevel = C

[PATCH] D28409: Use CodegenOpts::less when creating a TargetMachine for clang `-O1`

mehdi_amini updated this revision to Diff 83404. mehdi_amini added a comment. clang-format https://reviews.llvm.org/D28409 Files: clang/lib/CodeGen/BackendUtil.cpp Index: clang/lib/CodeGen/BackendUtil.cpp === --- clang/lib/Code

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D28362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D28409: Use CodegenOpts::less when creating a TargetMachine for clang `-O1`

This revision was automatically updated to reflect the committed changes. Closed by commit rL291276: Use CodegenOpts::less when creating a TargetMachine for clang `-O1` (authored by mehdi_amini). Changed prior to commit: https://reviews.llvm.org/D28409?vs=83404&id=83408#toc Repository: rL LL

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#638217, @probinson wrote: > Maybe instead, pass a flag to enable setting optnone on everything when the > driver sees `-O0 -flto`? I'm not fond of this: limiting discrepancy between LTO and non-LTO reduces the LTO specific bugs a

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

mehdi_amini added a comment. > In order to simplify distributed build system integration, where actions may be scheduled before the Thin Link which determines the list of objects selected by the linker. The gold plugin currently will emit 0-sized index files for objects not selected by the link,

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#638299, @probinson wrote: > In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote: > > > In https://reviews.llvm.org/D28404#638217, @probinson wrote: > > > > > The patch as-is obviously has a massive testing cost, and it's ea

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini updated this revision to Diff 83433. mehdi_amini added a comment. Herald added a subscriber: jholewinski. Fix minsize issue (conditional was reversed) https://reviews.llvm.org/D28404 Files: clang/include/clang/Driver/CC1Options.td clang/include/clang/Frontend/CodeGenOptions.def

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini updated this revision to Diff 83441. mehdi_amini added a comment. Herald added subscribers: dschuff, jfb. Fix one more conflicts with always_inline, and change some test check lines https://reviews.llvm.org/D28404 Files: clang/include/clang/Driver/CC1Options.td clang/include/cla

[PATCH] D28385: Add a cc1 option to force disabling lifetime-markers emission from clang

This revision was automatically updated to reflect the committed changes. Closed by commit rL291300: Add a cc1 option to force disabling lifetime-markers emission from clang (authored by mehdi_amini). Changed prior to commit: https://reviews.llvm.org/D28385?vs=83321&id=83443#toc Repository:

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini updated this revision to Diff 83459. mehdi_amini added a comment. Address comments: reorganize the way ShouldAddOptNone is handled, hopefully make it more easy to track. Also after talking with Chandler on IRC, the source attribute "cold" does not add the LLVM IR attribute "optsize"

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini marked 6 inline comments as done. mehdi_amini added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:760-762 Fn->removeFnAttr(llvm::Attribute::NoInline); + Fn->removeFnAttr(llvm::Attribute::OptimizeNone); Fn->addFnAttr(llvm::Attribute::AlwaysI

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini marked 2 inline comments as done. mehdi_amini added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:900 + ShouldAddOptNone &= !D->hasAttr(); + if (ShouldAddOptNone) { +B.addAttribute(llvm::Attribute::OptimizeNone); probinson wro

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini updated this revision to Diff 83468. mehdi_amini added a comment. Address Paul's comment (remove useless block and add period to end comment) https://reviews.llvm.org/D28404 Files: clang/include/clang/Driver/CC1Options.td clang/include/clang/Frontend/CodeGenOptions.def clang/l

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912 // OptimizeNone wins over OptimizeForSize and MinSize. F->removeFnAttr(llvm::Attribute::OptimizeForSize); F->removeFnAttr(llvm::Attribute::MinSize); probinso

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini updated this revision to Diff 83480. mehdi_amini added a comment. Forgot to update test/CodeGen/attr-naked.c https://reviews.llvm.org/D28404 Files: clang/include/clang/Driver/CC1Options.td clang/include/clang/Frontend/CodeGenOptions.def clang/lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912 // OptimizeNone wins over OptimizeForSize and MinSize. F->removeFnAttr(llvm::Attribute::OptimizeForSize); F->removeFnAttr(llvm::Attribute::MinSize); chandler

[PATCH] D24688: Introduce "hosted" module flag.

mehdi_amini updated this revision to Diff 83510. mehdi_amini added a comment. Rebase https://reviews.llvm.org/D24688 Files: clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/test/CodeGen/nobuiltin.c clang/test/CodeGenCUDA/flush-denormals.cu clang/test/CodeGen

[PATCH] D24688: Introduce "hosted" module flag.

mehdi_amini added a comment. In https://reviews.llvm.org/D24688#638835, @pcc wrote: > Didn't we figure out in the end that this could be a function attribute > instead? We did? You wrote in PR30403: "I had a brief look at what it would take to have a per-function TLI, and I'm not convinced th

[PATCH] D24688: Introduce "hosted" module flag.

mehdi_amini added a comment. In https://reviews.llvm.org/D24688#639158, @jyknight wrote: > Since this requires everyone generating llvm IR to add this module attribute > for optimal codegen, Not sure yet how it'll end up, I'll revamp this. See also: http://lists.llvm.org/pipermail/cfe-dev/2017

[PATCH] D28461: [clang] Enable using --section-ordering-file option of ld.gold

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM Comment at: CMakeLists.txt:113 +check_linker_flag("-Wl,--section-ordering-file,${CLANG_ORDER_FILE}" + LINKER_ORDER_FILE_WORKS) + endif()

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#639874, @probinson wrote: > Over the weekend I had a thought: Why is -O0 so special here? That is, > after going to all this trouble to propagate -O0 to LTO, how does this > generalize to propagating -O1 or any other specific -O

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#640046, @probinson wrote: > In https://reviews.llvm.org/D28404#639887, @mehdi_amini wrote: > > > In https://reviews.llvm.org/D28404#639874, @probinson wrote: > > > > > Over the weekend I had a thought: Why is -O0 so special here? T

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#640170, @probinson wrote: > In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote: > > > In https://reviews.llvm.org/D28404#640046, @probinson wrote: > > > > > "I don't care" doesn't seem like much of a principle. > > > > > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > Also, that's not practicable: what if I have an LTO static library for which > I don't have the source, now if I build my own file with -O0 -flto I can't > link anymore. Also: LTO is required for som

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#640284, @probinson wrote: > In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > > > In https://reviews.llvm.org/D28404#640170, @probinson wrote: > > > > > In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote: >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#640297, @probinson wrote: > Sorry, you lost me. CFI is part of DWARF and we do DWARF perfectly well > without LTO (and at O0). This CFI: http://clang.llvm.org/docs/ControlFlowIntegrity.html https://reviews.llvm.org/D28404 __

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. Actually, as mentioned before, I could be fine with making `O0` incompatible with LTO, however security features like CFI (or other sort of whole-program analyses/instrumentations) requires LTO. https://reviews.llvm.org/D28404 ___

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. > I'm now thinking along the lines of a `-foptimize-off` flag (bikesheds > welcome) which would set the default for the pragma to 'off'. How is that > different than what you wanted for `-O0`? It is defined in terms of an > existing pragma, which is WAY easier t

[PATCH] D24688: Introduce "hosted" module flag.

mehdi_amini added a comment. Thanks, I'll investigate function attributes then! https://reviews.llvm.org/D24688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#641538, @probinson wrote: > > - optnone isn't *really* no optimizations: clearly this is true, but then > > neither is -O0. We run the always inliner, a couple of other passes, and we > > run several parts of the code generators op

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#641597, @probinson wrote: > In https://reviews.llvm.org/D28404#641557, @mehdi_amini wrote: > > > As I stand right now, there hasn't been any correction. > > I still consider the fact that `optnone` wouldn't produce the "same" > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

mehdi_amini added a comment. In https://reviews.llvm.org/D28404#641632, @probinson wrote: > In https://reviews.llvm.org/D28404#641606, @mehdi_amini wrote: > > > If we want to support `-O0 -flto` and `optnone` it the way to convey this > > to the optimizer, I don't see the alternative. > > > opts

[PATCH] D26649: [CMake] Support lld with LTO bootstrap

mehdi_amini added inline comments. Comment at: cfe/trunk/CMakeLists.txt:531 + if(BOOTSTRAP_LLVM_ENABLE_LLD) +add_dependencies(clang-bootstrap-deps lld) + elseif(LLVM_BINUTILS_INCDIR) I come back to this a bit late, sorry, but I'm not sure I unde

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

mehdi_amini added inline comments. Comment at: docs/ReleaseNotes.rst:69 + called from cold callsite. + We need to mention ThinLTO in the Release Notes, so thanks for initiating this. We likely should go with a paragraph summarizing the improvements, I don't k

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM, thanks. Comment at: docs/ReleaseNotes.rst:70 + + - Integration with profile data. When available, profile data enables mo

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

mehdi_amini added a comment. Oh I didn't notice that we were in the clang Release Notes, actually I've only updated the LLVM ones in the past. So yes, I think this should be in LLVM as well, I don't know if it should differ in some way? https://reviews.llvm.org/D28746 _

[PATCH] D28821: [CMake] Separate LLVM_ENABLE_LTO and LLVM_ENABLE_LLD

mehdi_amini added a comment. Yes, thanks! Repository: rL LLVM https://reviews.llvm.org/D28821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini added a comment. What is the justification for a platform specific default change here? Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini added a comment. In https://reviews.llvm.org/D27163#606744, @arphaman wrote: > In https://reviews.llvm.org/D27163#606695, @mehdi_amini wrote: > > > What is the justification for a platform specific default change here? > > > The flag itself is platform agnostic, however, the default v

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:530 + // Don't need to mark Objective-C methods or blocks since the undefined + // behaviour optimization isn't used for them. +} Quuxplusone wrote: > This seems like a trap waiti

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini added inline comments. Comment at: test/CodeGenCXX/return.cpp:21 + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } rjmccall wrote: > mehdi_amini wrote: > > Quuxplusone wrote: > > > Can you explain why a lo

[PATCH] D27298: [Frontend] Fix an issue where a quoted search path is incorrectly removed as a duplicate header search path

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D27298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D27332: With LTO and profile-use, enable hotness info in opt remarks

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D27332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D27332: With LTO and profile-use, enable hotness info in opt remarks

mehdi_amini added a comment. (You have a typo in the decription, you may want to fix it before commit) https://reviews.llvm.org/D27332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26376: Undef stdatomic.h macro definitions that are defining functions provided in libc++

mehdi_amini added a comment. In https://reviews.llvm.org/D26376#597614, @mclow.lists wrote: > More info - The following code: > > #include > int main () {} > > > fails to compile on either gcc 6.2 (locally), gcc 7 head (online compiler) or > MSVC (online compiler). Interesting, that le

[PATCH] D26376: Undef stdatomic.h macro definitions that are defining functions provided in libc++

mehdi_amini abandoned this revision. mehdi_amini added a comment. Thanks for the information! Closing this. https://reviews.llvm.org/D26376 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D27604: [Driver] Add compiler option to generate a reproducer

mehdi_amini added inline comments. Comment at: include/clang/Basic/DiagnosticDriverKinds.td:95 def err_drv_force_crash : Error< - "failing because environment variable '%0' is set">; + "failing because %select{environment variable|option}0 '%1' is set">; def err_drv_invalid_m

[PATCH] D27796: Fix printf specifier handling: invalid specifier should not be marked as "consuming data arguments"

mehdi_amini created this revision. mehdi_amini added reviewers: rsmith, bruno, dexonsmith. mehdi_amini added a subscriber: cfe-commits. https://reviews.llvm.org/D27796 Files: clang/include/clang/Analysis/Analyses/FormatString.h clang/test/CodeGen/builtins.c clang/test/Sema/format-strings.c

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/CodeGen/CodeGenFunction.cpp:1086 + } + return true; +} Just a nit: reversing the if condition allows early exit. Reposito

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini added a comment. (Wait a little in case @Quuxplusone still has comments.) Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

mehdi_amini added inline comments. Comment at: include/clang/Driver/Options.td:1324 + HelpText<"Use C++ undefined behaviour optimization for control flow paths" + "that reach the end of the function without executing a required return">; +def fno_strict_return : Flag<["-"],

[PATCH] D27796: Fix printf specifier handling: invalid specifier should not be marked as "consuming data arguments"

This revision was automatically updated to reflect the committed changes. Closed by commit rL289850: Fix printf specifier handling: invalid specifier should not be marked as… (authored by mehdi_amini). Changed prior to commit: https://reviews.llvm.org/D27796?vs=81532&id=81621#toc Repository:

[PATCH] D27832: Add -plugin-opt=sample-profile for thinLTO build.

mehdi_amini added a comment. Is this covered by a driver test usually? https://reviews.llvm.org/D27832 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95766: [Branch-Rename] Fix some links

mehdi_amini accepted this revision. mehdi_amini added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h:20 // Check for underscores in the names of googletest tests, per -// https://github.com/google/googletest/blob/master/

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

mehdi_amini added a comment. Overall that would likely work for XLA. Something I'd like to mention though in response to: > The veclib type is also tied to the accepted values for -fveclib, which is a > list of supported lib, `-fveclib` is a Clang thing, it shouldn't limit what LLVM does. Of c

[PATCH] D85596: [Docs] Fix --print-supported-cpus option rendering

This revision was automatically updated to reflect the committed changes. Closed by commit rG0fb2203cd6c2: [Docs] Fix --print-supported-cpus option rendering (authored by tmfink, committed by mehdi_amini). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D83088: Introduce CfgTraits abstraction

mehdi_amini added a comment. In D83088#2345540 , @nhaehnle wrote: > David, I don't think this is appropriate here. Let's take the discussion to > llvm-dev. Seems like David asked to revert in the meantime? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D83088: Introduce CfgTraits abstraction

mehdi_amini added a comment. In D83088#2347111 , @arsenm wrote: > In D83088#2346322 , @mehdi_amini > wrote: > >> In D83088#2345540 , @nhaehnle wrote: >> >>> David, I don't t

[PATCH] D83088: Introduce CfgTraits abstraction

mehdi_amini added a comment. In D83088#2350287 , @nhaehnle wrote: > Hi Mehdi, this is not an appropriate place for this discussion. Yes, we have > a general rule that patches can be reverted if they're obviously broken (e.g. > build bot problems) or clea

[PATCH] D82860: Port ObjCMTAction to new option parsing system

mehdi_amini added a comment. Reverted in f917356f9ce0 ; I suspect a `static constexpr` in a class missing a definition out of class (required pre-c++17). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D84293: Add an assertion in SmallVector::push_back()

mehdi_amini updated this revision to Diff 305279. mehdi_amini added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix one clang instance failing this assertion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84293/n

[PATCH] D84293: Add an assertion in SmallVector::push_back()

mehdi_amini added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10811-10813 +Ops.reserve(Ops.size() + 1); Ops.push_back(Ops[0]); Ops.erase(Ops.begin()); njames93 wrote: > Probably a little off topic, but shouldn't this be refactored a

[PATCH] D84293: Add an assertion in SmallVector::push_back()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2c196bbc6bd8: Add an assertion in SmallVector::push_back() (authored by mehdi_amini). Changed prior to commit: https://reviews.llvm.org/D84293?vs=

[PATCH] D87981: [X86] AMX programming model.

mehdi_amini added a comment. (Drive by comments) Comment at: llvm/include/llvm/CodeGen/TileShapeInfo.h:27 + +class ShapeT { +public: Can you document the class? Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:30 + +class X86PreTileConfig

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

mehdi_amini added a comment. Clang is crashing after this change when bootstrapping and building lib/Support/CMakeFiles/LLVMSupport.dir/Hashing.cpp.o (our log if it helps: https://buildkite.com/mlir/mlir-core/builds/10003#80a5b6a9-d8d9-4420-ab2a-c24f398bc5d4 ) Repository: rG LLVM Github Mon

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

mehdi_amini added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:101 +#include "llvm/Frontend/OpenMP/OMPKinds.def" +}; + This is broken on gcc-5: ``` llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:101:1: error: could not convert

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

mehdi_amini added a comment. That's interesting! I'm not sure how to get there though: a complete solution should be able to "degrade" to global constructor when the platform-specific logic isn't available (unless we're confident that no such environment can exist?). Repository: rG LLVM Git

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

mehdi_amini added a comment. Something else I'm not sure about is how it works across DSOs: when each LLVM library is linked into its own shared library, is the dynamic linker creating this array when loading the libraries? (I'm starting to doubt about how this would even work on windows) Repo

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

mehdi_amini added inline comments. Comment at: llvm/lib/Support/CMakeLists.txt:7 +# ManagedStatic can be used to enable lazy-initialization of globals. +add_flag_if_supported("-Werror=global-constructors" WERROR_GLOBAL_CONSTRUCTOR) + MaskRay wrote: > Perhaps move

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

mehdi_amini updated this revision to Diff 359191. mehdi_amini marked 7 inline comments as done. mehdi_amini added a comment. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman. Herald added projects: clang, clang-tools-extra. Address comment and fix build errors Repository: r

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

mehdi_amini updated this revision to Diff 359196. mehdi_amini added a comment. Fix windows build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105959/new/ https://reviews.llvm.org/D105959 Files: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

mehdi_amini added a comment. In D105959#2882099 , @bondhugula wrote: > This is a really welcome change! Multiple registration issues were really an > inconvenience - I had no clue this was the pattern to use to fix it. Thanks! To be fair: we can do it

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG42f588f39c5c: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it… (authored by mehdi_amini). Changed prior to commit: h

[PATCH] D107349: [Matrix] Overload stride arg in matrix.columnwise.load/store.

mehdi_amini added a comment. Revert to unbreak bots (like this one : https://lab.llvm.org/buildbot/#/builders/13/builds/10930 ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107349/new/ https://reviews.llvm.org/D107349 ___

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

mehdi_amini added a comment. +1 on arguments from @dblaikie : I'm not sure I understand the motivation for this change right now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://reviews.llvm.org/D108265 _

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

mehdi_amini added a comment. > the original discussion that spawned the (not-yet-accepted, as it says in its > opening sentence) linked proposal around variable naming. +1 with David again: if the proposal gets accepted, then this patch makes sense to me. Repository: rG LLVM Github Monorepo

[PATCH] D69498: IR: Invert convergent attribute handling

mehdi_amini added a comment. What matter isn't "convergent" in itself, it is how it restricts transformations: if you know that all the control flow is always uniform, are there still restriction on any transformation in presence of convergent instructions? If not, then the "target approach" s

[PATCH] D121076: [MLIR][Python] Add SCFIfOp Python binding

mehdi_amini added a comment. Can you rebase? Your patch does not apply apparently Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121076/new/ https://reviews.llvm.org/D121076 ___ cfe-commits mailing list c

[PATCH] D121076: [MLIR][Python] Add SCFIfOp Python binding

mehdi_amini added a comment. Seems like there is a local base commit in your repository: you're not uploading a diff that applies on HEAD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121076/new/ https://reviews.llvm.org/D121076

[PATCH] D121076: [MLIR][Python] Add SCFIfOp Python binding

This revision was automatically updated to reflect the committed changes. Closed by commit rG036088fd6ea2: [MLIR][Python] Add SCFIfOp Python binding (authored by chhzh123, committed by mehdi_amini). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12107

[PATCH] D121549: Define ABI breaking class members correctly

mehdi_amini added inline comments. Comment at: mlir/lib/Target/SPIRV/Deserialization/Deserializer.h:601 + // TODO: place logger under #if LLVM_ENABLE_ABI_BREAKING_CHECKS #ifndef NDEBUG This is a private header (you're in the `lib` directory and not in the `i

[PATCH] D121549: Define ABI breaking class members correctly

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

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. For this to be a usable canonicalization, it is really the case where the operands are already sorted (no-op) that needs to be heavily optimized (that is no complex data structure to populate, etc.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. Herald added a subscriber: JDevlieghere. Nice, LGTM, thanks for driving this! > Remember that if we want to adopt some new feature in a bigger way it should > be discussed and added to the CodingStandard document. What does it mean exactly? We can't use **anything**

[PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. In D130689#3684333 , @thieta wrote: > In D130689#3684330 , @mehdi_amini > wrote: > >> What does it mean exactly? We can't use **anything** C++17 without writing >> it in the coding s

[PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. In D130689#3686760 , @h-vetinari wrote: > My point boils down to: "written using standard C++17 > code" does not sound at all like "core language, no stdlib", but very much > like "core+stdlib". We're allowing C++17 library

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. > . I haven't thought too hard about the performance of that while loop but it > seems good enough to land for now. What's the finality of it? That is: outside of canonicalization what is its purpose? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D123297: [flang][driver] Add support for -mmlir

mehdi_amini added a comment. `-mmlir` is fine with me, but note that MLIR has much less global options than LLVM: you will only get access to context and passmanager options and not individual passes flags. That's not a criticism :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

mehdi_amini added a comment. Seems like this is breaking clang bootstrapping? llvm/include/llvm/CodeGen/LiveInterval.h:630:53: error: captured variable 'Idx' cannot appear here [=](std::remove_reference_t V, ^ Repository:

[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

mehdi_amini added a comment. In D119136#3449325 , @cor3ntin wrote: > Yes, working on a fix as we speak. > The meaning of that code changed (in all c++ language modes), I'm > currently trying to find if we have any other issue of that sort and will > commi

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. Seems like this should be added to canonicalization? The "push constants to the right hand side" is there already. I also don't understand the complexity of the implementation, I may need an example to understand why you're recursively operating on the producer ops

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. In D124750#3502228 , @srishti-pm wrote: > In D124750#3500748 , @mehdi_amini > wrote: > >> Seems like this should be added to canonicalization? The "push constants to >> the right ha

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. In D124750#3502401 , @srishti-pm wrote: > In D124750#3502295 , @mehdi_amini > wrote: > >> You're telling me "what" while I'm actually more interested in the "why" >> here? > > I'm n

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. That was the sense of my question about canonicalization indeed :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124750/new/ https://reviews.llvm.org/D124750 ___ cfe-commits

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. Thanks for improving the doc! Are you moving this to be used in canonicalization next? I think a first good step would be to make it a pattern and test it with a pass that applies it in the greedy rewriter. I would land this first and then try to enable this in the

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

mehdi_amini added a comment. I hit a config failure after f8c8bda965dd0f622de1ad3863b728661af6eb72 CMake Error at /var/lib/buildkite-agent/builds/buildkite-588bd64db9-mk2vq-1/mlir/mlir-core/libcxx/CMakeLists.txt:225 (messa

<    1   2   3   4   >