[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

lebedev.ri added a comment. In D76572#2472256 , @Quuxplusone wrote: > In D76572#2472221 , @lebedev.ri > wrote: > >> Do you plan to follow-up with the diagnostic itself, in some form? > > No, I don't. My diagnostic

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri added a comment. Thank you for taking a look! In D93822#2474236 , @dblaikie wrote: > Haven't reviewed the code in detail, but the idea seems sound. That's reassuring! > Is there any prior art in other compilers you can draw data/experiences f

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 314103. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. NFC, rebased, added test with multiplication of `short`s. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93822/new/ https://rev

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri added inline comments. Comment at: clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c:24 +void t1(char *base, int a, int b) { + // FIXME: test `[a * b]base` pattern? +} dblaikie wrote: > lebedev.ri wrote: > > dblaikie

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 314188. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. - Add test for inverse array subscript expression - Support it by skipping paren expressions - Actually ensure that we only diagnose only truly implicit casts, but not implici

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri added inline comments. Comment at: clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c:24 +void t1(char *base, int a, int b) { + // FIXME: test `[a * b]base` pattern? +} dblaikie wrote: > lebedev.ri wrote: > > dblaikie

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

lebedev.ri added a comment. In D94367#2489090 , @fhahn wrote: > Thanks for putting up the patch! > > I think the code here and D94366 could be > simplified if we would introduce 2 codegen options to distinguish the > di

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri added a comment. Ping In D93822#2474475 , @dblaikie wrote: > In D93822#2474355 , @lebedev.ri > wrote: > >> Thank you for taking a look! >> >> In D93822#2474236 ,

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri added a comment. @aaron.ballman thank you for taking a look! @NoQ thank you for commenting. Indeed, i very much don't want to diagnose just the cases where the overflow can be proven to happen, doing that i believe, would remove most of the true-positive reports. So indeed, this is be

[PATCH] D90448: [clang] Add type check for explicit instantiation of static data members

lebedev.ri added a comment. Tests missing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90448/new/ https://reviews.llvm.org/D90448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

lebedev.ri created this revision. lebedev.ri added reviewers: fhahn, jdoerfert, arsenm, mkazantsev, kuhar, brzycki, nikic. lebedev.ri added projects: LLVM, AMDGPU. Herald added subscribers: wenlei, kerbowa, steven_wu, hiraditya, nhaehnle, jvesely. lebedev.ri requested review of this revision. Her

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

lebedev.ri added a comment. Wow. So much polarization here. In D94827#2502435 , @kuhar wrote: > Wow, this is fantastic. When I first started working on the domtree updater > back in 2017, SimplifyGFG seemed like one of the most difficult passes to > han

[PATCH] D94979: [CGExpr] Honor getCharWidth() in ConstantAggregateBuilderUtils

lebedev.ri added subscribers: jfb, lebedev.ri. lebedev.ri added a comment. As far as i recall, all RFC's about LLVM support for non-8-bit char targets didn't end in favor of the proposal I'd say that until there's general consensus/buy-in (including testing path), cleaning stuff up will only con

[PATCH] D94979: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC

lebedev.ri added a comment. I don't particularly want/like to straight-up derail this, but my opinion still hasn't changed, and it is not unsubstantiated, see https://lists.llvm.org/pipermail/llvm-dev/2019-May/132080.html, https://lists.llvm.org/pipermail/llvm-dev/2019-October/136115.html, etc,

[PATCH] D106191: [clang] Option control afn flag

lebedev.ri added reviewers: aaron.ballman, erichkeane. lebedev.ri added subscribers: erichkeane, aaron.ballman. lebedev.ri added a comment. Looks reasonable to me. @aaron.ballman @erichkeane does it seem complete or are we missing some infra piece? Comment at: clang/include/cl

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

lebedev.ri added a comment. Question: should this really diagnose classes that are marked as `final`? Surely such a class can never be a base class? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102325/new/ https://reviews.llvm.org/D102325 ___

[PATCH] D108138: [WIP] Remove switch statements before vectorization

lebedev.ri added a comment. :( I'm sorry for derailing this. I still think proper switch handling for loops would be nice. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108138/new/ https://reviews.llvm.org/D108138 ___ cfe-commits mailing list

[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

lebedev.ri added a reviewer: rsmith. lebedev.ri added a comment. Doesn't this make AST non-representable of the reality, shouldn't the lowering happen in codegen, not in sema? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71016/new/ https://reviews

[PATCH] D112292: [Clang][OpenMP] Allow loop iteration var with threadprivate directive

lebedev.ri added a comment. In which standard version was that rule introduced, is this missing some checks for the OpenMP version? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112292/new/ https://reviews.llvm.org/D112292 ___ cfe-commits ma

[PATCH] D104854: Introduce intrinsic llvm.isnan

lebedev.ri added a comment. In D104854#2957471 , @sepavloff wrote: > In D104854#2957423 , @spatel wrote: > >> Is it intentional that we are not canonicalizing the intrinsic call back to >> `fcmp uno` in the defau

[PATCH] D104854: Introduce intrinsic llvm.isnan

lebedev.ri added a comment. In D104854#2957529 , @spatel wrote: > In D104854#2957471 , @sepavloff > wrote: > >> In D104854#2957423 , @spatel wrote: >> >>> Is it intention

[PATCH] D108421: Mark openmp internal global dso_local

lebedev.ri added a comment. In D108421#2958742 , @kamleshbhalui wrote: > assume dso local if relocation model static I think these are two separate fixes. `TargetMachine::shouldAssumeDSOLocal()` change might make sense, but as the comment there notes,

[PATCH] D108422: [NFC][clang] Move remaining part of X86Target.def to llvm/Support/X86TargetParser.def

lebedev.ri added a comment. This should be autogenerated from the main `X86.td`, otherwise this is, unfortunately only partially, duplicates that. E.g. there isn't a single AMD CPU in there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108422/new/

[PATCH] D104854: Introduce intrinsic llvm.isnan

lebedev.ri added a subscriber: hans. lebedev.ri added a comment. In D104854#2959680 , @thopre wrote: > In D104854#2957735 , @kpn wrote: > >> In D104854#2957490 , @lebedev.

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

lebedev.ri added a comment. In D101960#2960622 , @jdoerfert wrote: > To summarize the conversation, can we do LD_LIBRARY_PATH overwrites after > this patch or not? If so, I feel everyone is in favor, if not, we need a > different solution. +1 Reposit

[PATCH] D108421: Mark openmp internal global dso_local

lebedev.ri added a comment. In D108421#2961611 , @kamleshbhalui wrote: > updated test and make changes local to auto generated global vars for lock. I do not understand why the most original diff here was wrong. Is is the wrong default for `getOrCreateI

[PATCH] D108421: Mark openmp internal global dso_local

lebedev.ri added a comment. In D108421#2962160 , @ABataev wrote: > In D108421#2962151 , @lebedev.ri > wrote: > >> In D108421#2961611 , >> @kamleshbhalui wrote: >> >>> up

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri created this revision. lebedev.ri added reviewers: jdoerfert, efriedma, aaron.ballman, nikic, MaskRay. lebedev.ri added a project: LLVM. Herald added subscribers: dexonsmith, okura, kuter, pengfei, jfb. lebedev.ri requested review of this revision. Herald added a reviewer: sstefan1. Hera

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri updated this revision to Diff 368583. lebedev.ri added a comment. In D108661#2963726 , @arsenm wrote: > Could use some MIR tests to make sure that parser doesn't explode Added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri updated this revision to Diff 368719. lebedev.ri added a comment. @arsenm i do not understand how i can avoid having IR there, but addressed other nits. thanks for taking a look! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108661/new/

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri updated this revision to Diff 368722. lebedev.ri added a comment. Address rest of nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108661/new/ https://reviews.llvm.org/D108661 Files: clang/include/clang/Sema/Sema.h clang/test/CXX

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri added a comment. @arsenm are you happy with the test coverage here? In D108661#2963519 , @craig.topper wrote: > Wild speculation. This may be a historical artifact of LoadInst and StoreInst > having their getAlignment() function written like

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri updated this revision to Diff 368744. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Add `-march=x86-64` that i forgot :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108661/new/ https://reviews.llvm.org/D10866

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

lebedev.ri added a comment. Thanks everyone! I don't expect that there will be any further feedback, so i plan on landing this tomorrow, unless there is further feedback before then. Comment at: llvm/test/CodeGen/MIR/X86/load-with-1gb-alignment.mir:1 +# RUN: llc -run-pass=none

[PATCH] D108661: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG564d85e090af: The maximal representable alignment in LLVM IR is 1GiB, not 512MiB (authored by lebedev.ri). Changed prior

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

lebedev.ri added a comment. IMO anything other than enhancing LV is wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108138/new/ https://reviews.llvm.org/D108138 ___ cfe-commits mailing list cfe-comm

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

lebedev.ri added a comment. How is it conceptually different to break apart IR in LV itself, or do the same in a special pass just before that? If we want to go this road, we need to completely make `switch`es illegal/non-canonical before LV. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

lebedev.ri added a comment. In D108138#2967156 , @david-arm wrote: > In D108138#2967133 , @lebedev.ri > wrote: > >> How is it conceptually different to break apart IR in LV itself, or do the >> same in a special

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

lebedev.ri added a comment. Do we *really* need `-enable-approx-func-fp-math`? I'm pretty sure we are moving away from such global options, onto relying only on the per-instruction fast-math flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1017

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

lebedev.ri added a reviewer: spatel. lebedev.ri added a comment. In D101759#2967331 , @masoud.ataei wrote: > In D101759#2967250 , @lebedev.ri > wrote: > >> Do we *really* need `-enable-approx-func-fp-math`? >> I

[PATCH] D106191: [clang] Option control afn flag

lebedev.ri added a comment. FWIW i think this part is fine in principle. Not sure about errno. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106191/new/ https://reviews.llvm.org/D106191 ___ cfe-commits m

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

lebedev.ri added subscribers: spatel, lebedev.ri. lebedev.ri added a comment. I think there is something really wrong with vectorzer passes in LTO pipelines. Can you say whether the problem you are observing is in ThinLTO, Full LTO, or both? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

lebedev.ri added a comment. In D108826#2969547 , @ABataev wrote: > In D108826#2969471 , @lebedev.ri > wrote: > >> I think there is something really wrong with vectorzer passes in LTO >> pipelines. >> Can you say

[PATCH] D108625: [Test][Time profiler] Fix test time checking

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Everything timing-based is pretty much guaranteed to be flaky. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108625/new/ https://reviews

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

lebedev.ri added a comment. In D109175#2980333 , @weiwang wrote: > Our internal codebase never uses the target directive. Once the deferred > diags is bypassed, we observed 18% e2e build time improvement. Is that with `-fopenmp` or without? That seems,

[PATCH] D109321: [clang][OpenMP] Fix the bug in codegen for ordered directive

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D109321#2985244 , @peixin wrote: > The following test case fails after > https://reviews.llvm.org/rGaf000197c4214926bd7d0862d86f8

[PATCH] D109321: [clang][OpenMP] Fix the bug in codegen for ordered directive

lebedev.ri added a comment. Aha. But i don't think this is the right fix, the fact that the inlining manifests the miscompile is a symptom. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109321/new/ https://reviews.llvm.org/D109321

[PATCH] D109338: [clang][OpenMP] Make IR function names for OpenMP-outlined blocks slightly less awful

lebedev.ri created this revision. lebedev.ri added reviewers: jdoerfert, ABataev, rjmccall. lebedev.ri added projects: OpenMP, clang. Herald added subscribers: zzheng, guansong, yaxunl. lebedev.ri requested review of this revision. Herald added a subscriber: sstefan1. I've recently had to stare at

[PATCH] D109338: [clang][OpenMP] Make IR function names for OpenMP-outlined blocks slightly less awful

lebedev.ri added a comment. In D109338#2985731 , @ABataev wrote: > Maybe use something close to target entries format? Encode function name and > line number of the region. This would help if there are several omp regions > in a function. Sure, i can d

[PATCH] D108138: [WIP] Remove switch statements before vectorization

lebedev.ri added a comment. Hi. I'm personally still not very okay with the approach as it currently is. Do you need to run LoopRotate after lowering switches? Anything else? But then you don't actually know that after spending all this compile time, the vectorization will actually happen, and y

<    18   19   20   21   22   23