[PATCH] D95299: Fix truncated __OPENMP_NVPTX__ preprocessor condition

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc49142e4f5c8: Fix truncated __OPENMP_NVPTX__ preprocessor condition (authored by r-burns, committed by mehdi_amini). Repository: rG LLVM Github Mo

[PATCH] D95299: Fix truncated __OPENMP_NVPTX__ preprocessor condition

mehdi_amini added a comment. Herald added a subscriber: wangpc. @r-burns : what should I use for the authorship (name/pseudo and email)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95299/new/ https://reviews.llvm.org/D95299 _

[PATCH] D155509: Revert "Remove rdar links; NFC"

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe0ac46e69d7a: Revert "Remove rdar links; NFC" (authored by mehdi_amini). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D155509: Revert "Remove rdar links; NFC"

mehdi_amini created this revision. mehdi_amini added reviewers: lattner, aaron.ballman. Herald added subscribers: steakhal, jdoerfert, martong, pengfei, arphaman. Herald added a reviewer: NoQ. Herald added a project: All. mehdi_amini requested review of this revision. Herald added subscribers: cfe-

[PATCH] D155081: Specify the developer policy around links to external resources

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. Approving in support, but don't consider this enough as-is. I suspect you should get @lattner approval here :) Comment at: llvm/docs/DeveloperPolicy.rst:357 -* I

[PATCH] D151923: [APFloat] Add APFloat semantic support for TF32

This revision was automatically updated to reflect the committed changes. Closed by commit rG55c2211a233e: [APFloat] Add APFloat semantic support for TF32 (authored by jfurtek, committed by mehdi_amini). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D151923: [APFloat] Add APFloat semantic support for TF32

mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. Ok, sorry I missed the mangle update, seeing how I missed the MLIR mention as well: I need more sleep!! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D151923: [APFloat] Add APFloat semantic support for TF32

mehdi_amini added a comment. > Also do you have plans for MLIR? (nothing required for this patch, just > curious). Actually it's the last sentence of the patch description (/facepalm) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151923/new/ http

[PATCH] D151923: [APFloat] Add APFloat semantic support for TF32

mehdi_amini added a comment. I think this LG, but David had to update clang/lib/AST/MicrosoftMangle.cpp when some other float format was added, you don't need this here? Also do you have plans for MLIR? (nothing required for this patch, just curious). Repository: rG LLVM Github Monorepo CH

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

mehdi_amini added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1491 +mapTypes = exitDataOp.getMapTypes(); +mapperFunc = false; +return success(); This line is not needed afte

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

mehdi_amini added inline comments. Herald added a subscriber: bviyer. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1533 +ompLoc, builder.saveIP(), mapTypeFlags, mapNames, mapperAllocas, +mapperFunc, deviceID, ifCond, processMapO

[PATCH] D146041: initial commit

mehdi_amini added a comment. Herald added a subscriber: JDevlieghere. You should look into the title and description of the commit: https://cbea.ms/git-commit/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146041/new/ https://reviews.llvm.org/D146

[PATCH] D141098: [clang-format][NFC] Set LineEnding to LF in config files

mehdi_amini added a comment. In D141098#4111809 , @owenpan wrote: > Can we just fix the buildbot so that it runs git-clang-format with > `--binary`? Using clang-format-15 to format a patch to clang-format-16 source > still looks wrong to me. The genera

[PATCH] D141298: Move from llvm::makeArrayRef to ArrayRef deduction guides - last part

mehdi_amini added a comment. Seems mechanical, and if it build everywhere LGTM :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141298/new/ https://reviews.llvm.org/D141298 ___ cfe-commits mailing list c

[PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

mehdi_amini added inline comments. Herald added a subscriber: Michael137. Comment at: llvm/include/llvm/ADT/ArrayRef.h:502 + /// Deduction guide to construct an ArrayRef from a C array. + template ArrayRef(const T (&Arr)[N]) -> ArrayRef; Can we keep the make

[PATCH] D134878: Update developer policy on potentially breaking changes

mehdi_amini added a comment. From what I saw when you posted the discourse thread initially, I understand you're targeting user-visible features, and mostly from the "toolchain" side of the project. However I find the language here to be potentially confusing for the API surface of the librarie

[PATCH] D131172: [clang][llvm][doc] Add more information for the ABI change in FP16

mehdi_amini added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:206 +When you find failures with ``half`` type, check the calling conversion of the +code and switch it to the new ABI. + (same here: users reading "switch it to the new ABI" will be confus

[PATCH] D131172: [clang][llvm][doc] Add more information for the ABI change in FP16

mehdi_amini added inline comments. Comment at: clang/docs/ReleaseNotes.rst:633 +- If you are using downstream runtimes that provide FP16 conversions, update +them with the new ABI. + Can you add a link to a documentation that describe the new ABI? Repository:

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

mehdi_amini added a comment. > There should be a better way than this. Comprehensive pre-merge testing of > all PRs etc. We already have pre-commit tests on Phabricator on Windows and Linux, but that's not exhaustive and for many reasons I don't believe this is realistic in any way: we will al

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

mehdi_amini added a comment. > Agreed, I think we need to update the protocol for changing the C++ standard > in the future to account for more testing beforehand. The way I would do it is: wait for a Sunday so that the bots aren't loaded, land the change, wait for a couple of hours to ensure t

[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] 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] 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. 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] 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] D124750: [MLIR] Add a utility to sort the operands of commutative ops

mehdi_amini added a comment. Right now I'm not yet understanding all of the algorithm (haven't spent enough time on it), but I'm mostly concerned by the runtime cost of this normalization. Comment at: mlir/lib/Transforms/Utils/CommutativityUtils.cpp:371 +// assigned a sort

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

mehdi_amini added inline comments. Comment at: mlir/include/mlir/Transforms/CommutativityUtils.h:211 +template +class SortCommutativeOperands : public OpRewritePattern { + using OpRewritePattern::OpRewritePattern; Can we make this not a template? This will be a

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

mehdi_amini added a comment. This broke the bot here: https://lab.llvm.org/buildbot/#/builders/61/builds/27616 The cmake invocation includes some GPU specific options that you can omit (`-DMLIR_ENABLE_CUDA_RUNNER=1` , `-DCMAKE_CUDA_COMPILER=/usr/local/cuda/bin/nvcc`, `-DMLIR_ENABLE_VULKAN_RUN

[PATCH] D126158: [MLIR][GPU] Replace fdiv on fp16 with promoted (fp32) multiplication with reciprocal plus one (conditional) Newton iteration.

mehdi_amini added a comment. The shared library build was broken, I had to revert: https://lab.llvm.org/buildbot/#/builders/61/builds/27377 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126158/new/ https://reviews.llvm.org/D126158 ___

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

mehdi_amini added a comment. Forgot to mention, we configure this build with: cmake -G Ninja ${src_dir}/llvm -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU" -DLLVM_ENABLE_PROJECTS="lld;clang;mlir" -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=Off -DLIBCXX_CXX_ABI=default -DLLVM_ENABLE_RUNTI

[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

[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] 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. 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. 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. 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] 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] 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] 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] 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] 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] 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] 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

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] D120375: Trim unnecessary component/library dependencies.

mehdi_amini added a comment. Please expand the description and include how you figured this out and how do we know it is actually correct. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120375/new/ https://reviews.llvm.org/D120375

[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

mehdi_amini added a comment. In D72404#3307623 , @aykevl wrote: > So, should all passes just look at the `optsize` and `minsize` attributes > instead of the `SizeLevel`? In other words, should > `PassManagerBuilder.SizeLevel` be removed and should passes

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

mehdi_amini added a comment. Yes, I saw these, thanks a lot @aganea !! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119257/new/ https://reviews.llvm.org/D119257 ___ cfe-commits mailing list cfe-commits@

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

mehdi_amini added a comment. (The discussion seems to be happening in two places: https://github.com/llvm/llvm-project/issues/53475 ) You can break downstream users, the relationship inside the monorepo is different as far as I know: I believe we will have to live with revert like the one abov

[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

mehdi_amini added a comment. In D72404#3304205 , @aykevl wrote: > I would be very interested in this patch, because for me to use ThinLTO > without size regressions I need to set the optimization size level in the > linker (`PassManagerBuilder.SizeLevel`

[PATCH] D118652: Cleanup header dependencies in LLVMCore

mehdi_amini added a comment. I'm not sure how it'd help clients of the released version of LLVM to delay, the development branch and the release seems fairly disconnected to me from this point of view. (what can help are deprecating APIs with instructions about how to update so that they can up

[PATCH] D116512: [clang-tidy] Limit non-Strict mode to public functions

mehdi_amini abandoned this revision. mehdi_amini added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:43 + a human reader, and there's basically no place for a bug to hide. On the other + hand for non-public functions, all t

[PATCH] D116513: [clang-tidy] Fix bugs in misc-unused-parameters for Constructors calls site

mehdi_amini added a comment. Thanks @aaron.ballman for the review! Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159 // CHECK-FIXES: C() {} C(int i) {} // CHECK-MESSAGES: :[[@LINE-1]]:9: warning aaron.ballman wrote:

[PATCH] D116512: [clang-tidy] Limit non-Strict mode to public functions

mehdi_amini added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:43 + a human reader, and there's basically no place for a bug to hide. On the other + hand for non-public functions, all the call-sites are visible and the pa

[PATCH] D116513: [clang-tidy] Fix bugs in misc-unused-parameters for Constructors calls site

mehdi_amini created this revision. mehdi_amini added a reviewer: Eugene.Zelenko. Herald added subscribers: carlosgalvezp, xazax.hun. mehdi_amini requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. These weren't tracked and so weren'

[PATCH] D116512: [clang-tidy] Limit non-Strict mode to public functions

mehdi_amini created this revision. mehdi_amini added a reviewer: Eugene.Zelenko. Herald added subscribers: carlosgalvezp, xazax.hun. mehdi_amini requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. The rational to avoid applying the

[PATCH] D116488: Add a misc-unused-parameters.CommentOutUnusedParameters to clang-tidy

mehdi_amini added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:25 + + void a(int ) { /*some code that doesn't use `i`*/ } + jpienaar wrote: > OOC why the extra space after the type? The space is in the origina

[PATCH] D116488: Add a misc-unused-parameters.CommentOutUnusedParameters to clang-tidy

mehdi_amini created this revision. mehdi_amini added reviewers: rriddle, jpienaar, Mogball. Herald added a subscriber: carlosgalvezp. mehdi_amini requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This option allows to eliminate pa

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

mehdi_amini added a comment. In D114639#3162141 , @erichkeane wrote: >> Right, but last time we did the motivation was specifically to get to c++14, >> while here the motivation is to drop an old MSVC according to the >> MSVC-specific support we intend

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

mehdi_amini added a comment. In D114639#3162069 , @erichkeane wrote: > In D114639#3162032 , @RKSimon wrote: > >> In D114639#3162000 , @mehdi_amini >> wrote: >> >>> In D1

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

mehdi_amini added a comment. In D114639#3161303 , @erichkeane wrote: > IMO, if we're updating the MSVC versions, we should do the same for the > GCC/Clang/AppleClang versions too. For example, GCC 5.1 is from 2015, and > Clang 3.5 is from 2014. We've

[PATCH] D114699: Fixed a memory leak in the PDLToPDLInterp RootOrderingTest.

mehdi_amini added inline comments. Comment at: mlir/unittests/Conversion/PDLToPDLInterp/RootOrderingTest.cpp:37 + ~RootOrderingTest() { +for (int i = 0; i < 4; ++i) + v[i].getDefiningOp()->destroy(); bondhugula wrote: > bondhugula wrote: > > You need to

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

mehdi_amini added a comment. In D114639#3158401 , @RKSimon wrote: >> Have you checked whether there are any bots in the lab that will need to be >> updated? > > I did find a number of bots that I think need addressing, I am intending to > privately emai

[PATCH] D114502: File Reorganization changes

mehdi_amini added inline comments. Comment at: mlir/lib/ExecutionEngine/CMakeLists.txt:153 + set(CMAKE_MODULE_PATH "${HIP_PATH}/lib/cmake/hip" ${CMAKE_MODULE_PATH}) find_package(HIP) if (NOT HIP_FOUND) mehdi_amini wrote: > Is MLIR really coupled to Clang h

[PATCH] D114502: File Reorganization changes

mehdi_amini added inline comments. Comment at: mlir/lib/ExecutionEngine/CMakeLists.txt:153 + set(CMAKE_MODULE_PATH "${HIP_PATH}/lib/cmake/hip" ${CMAKE_MODULE_PATH}) find_package(HIP) if (NOT HIP_FOUND) Is MLIR really coupled to Clang here? That seems suspi

[PATCH] D114394: Compile-time computation of string attribute hashes

mehdi_amini added inline comments. Comment at: llvm/lib/IR/Attributes.cpp:125 FoldingSetNodeID ID; - ID.AddString(Kind); + ID.AddString(Kind.value()); if (!Val.empty()) ID.AddString(Val); Carrying over my comment from the original revision: it seem that y

[PATCH] D114082: [WIP] Normalize String Attributes

mehdi_amini added a comment. The patch is pretty large, any chance you could split in two? First the change introducing the AttributeKey and changing the API, and then updating all the callees separately as a follow up? Comment at: llvm/include/llvm/IR/Attributes.h:84 + } +}

[PATCH] D109977: LLVM Driver Multicall tool

mehdi_amini added a comment. That's pretty nice! Have you thought about looking into a lit option (triggered by a cmake flag maybe) that would change the substitution for the tools that are enabled by llvm-driver to use the latter instead when running the tests? Repository: rG LLVM Github Mo

[PATCH] D109167: Try to unbreak Win build differently after 973519826edb76

mehdi_amini added a comment. > @goncharov @mehdi_amini is there a way to change the way the phab UI or the > pre-merge checks work here to make failing pre-merge checks clearer? This > isn't the first time I've seen someone be very confused about this UI I asked the same thing a few days ago an

[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] 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] 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] 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 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

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] 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

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 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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#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] 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] 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] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

mehdi_amini added a comment. I rather have a non closed list of veclib: if you just have a map keyed by string instead of an enum it should just work out? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77925/new/ https://reviews.llvm.org/D77925 __

[PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

mehdi_amini added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84691/new/ https://reviews.llvm.org/D84691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

mehdi_amini added inline comments. Comment at: mlir/examples/standalone/CMakeLists.txt:35 +endif() + include(TableGen) mehdi_amini wrote: > I am a bit unsure that it is desirable that it is desirable to need this > change? > This requires every single user of L

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

mehdi_amini added inline comments. Comment at: mlir/examples/standalone/CMakeLists.txt:35 +endif() + include(TableGen) I am a bit unsure that it is desirable that it is desirable to need this change? This requires every single user of LLVM to do this. Is there a

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

mehdi_amini added a comment. It seems like this pass was added in lib/Transforms but does not have any unit-tests (lit tests) provided. It isn't even linked into `opt`. As it is an LLVM IR pass it should be tested as such I believe. Can you provide tests for this? Repository: rG LLVM Github

[PATCH] D81422: Change filecheck default to dump input on failure

mehdi_amini marked an inline comment as done. mehdi_amini added a comment. In D81422#2096643 , @arsenm wrote: > I would also expect a simple command line flag to llvm-lit to be able to > control this, rather than having to set an environment variable Wo

[PATCH] D81422: Change filecheck default to dump input on failure

mehdi_amini added a comment. In D81422#2093360 , @arsenm wrote: > The amount of context printed previously was good enough for development use. > If I ever can't figure it out from the diff, I want to look at the output > completely separate from the ter

[PATCH] D81045: [LLVM] Change isa<> to a variadic function template

This revision was automatically updated to reflect the committed changes. Closed by commit rG72d20b9604f6: [LLVM] Change isa<> to a variadic function template (authored by jurahul, committed by mehdi_amini). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

  1   2   3   4   >