[PATCH] D157331: [clang] Implement C23

xbolva00 added a comment. Please use update_cc_test_checks.py Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://reviews.llvm.org/D157331 ___ cfe-commits mailing list

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

xbolva00 added a comment. Yes, cc1 flag would be useful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74094/new/ https://reviews.llvm.org/D74094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

xbolva00 added a comment. >> So we can start by giving these objects full-expression lifetime, chase down >> any program bugs that that uncovers (which will all *unquestionably* be >> program bugs under the standard), and then gradually work on landing the >> more aggressive rule (perhaps even

[PATCH] D148381: [Clang] Add counted_by attribute

xbolva00 added a comment. Will gcc use counted_by or element_count ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148381/new/ https://reviews.llvm.org/D148381 ___ cfe-commits mailing list

[PATCH] D157331: [clang] Implement C23

xbolva00 added inline comments. Comment at: clang/lib/Headers/stdckdint.h:13 + +#if defined(__GNUC__) +#define ckd_add(R, A, B) __builtin_add_overflow((A), (B), (R)) enh wrote: > enh wrote: > > enh wrote: > > > ZijunZhao wrote: > > > > enh wrote: > > > > > is

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xbolva00 added a comment. I my opinion it makes sense to adjust that kernel code based on this warning in the current inplementation state. @aaron.ballman ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609

[PATCH] D155457: [clang] Skip tautological comparison if the comparison involves the 'size_t' type

xbolva00 added a comment. You should add a testcase which uses “expected no diagnostics”. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155457/new/ https://reviews.llvm.org/D155457 ___ cfe-commits

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xbolva00 added a comment. We should not warn for macros. if (ENABLE_XYZ && x) {} } This pattern is used in real world codebases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 ___ cfe-commits

[PATCH] D86993: Document Clang's expectations of the C standard library.

xbolva00 added a comment. In D86993#4497744 , @aaron.ballman wrote: > In D86993#4477744 , @RalfJung wrote: > >> It would probably be worth including all string functions that take a length >> in such a DR. In

[PATCH] D86993: Document Clang's expectations of the C standard library.

xbolva00 added a comment. In D86993#4474403 , @RalfJung wrote: > Yeah, agreed. Though I hear that GCC makes similar assumptions, so this > should count as something I think. (I don't have a source for this, so it > should probably be verified.) glibc's

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. >> If all you want is (1), that doesn't require any attribute (the optimizer >> will do it automatically). Not entirely true, as compiler can do anything. In some cases you really need *musttail* (in llvm IR) to get or preserve tail call. CHANGES SINCE LAST ACTION

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. In D147714#4446809 , @haberman wrote: >> Such a definition doesn't make much sense because for some targets tail call >> optimization isn't available at all. >> Eg. Wasm w/o tail call proposal, xtensa windowed abi. > > That is

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. In D147714#4446780 , @yamt wrote: >> musttail provide assurances that the tail call can be optimized on all >> targets. > > Such a definition doesn't make much sense because for some targets tail call > optimization isn't

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

xbolva00 added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3087 + +``__builtin_nondeterministic_value`` returns a valid nondeterministic value of the same type as the provided argument. + This text really sells this new builtin as a random

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

xbolva00 added a comment. https://reviews.llvm.org/D136737 used 'builtin_unspecified_value' which could be okay. Are we sure we want to expose this intrinsic as it can be easily misused (as mentioned, as random generator) and/or cause security issues? Maybe only allow it in system headers

[PATCH] D150450: Add C++26 compile flags.

xbolva00 added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:458 +else if (LangOpts.CPlusPlus23) + Builder.defineMacro("__cplusplus", "202302L"); // [C++20] The integer literal 202002L. Separate patch / commit? CHANGES

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

xbolva00 added a comment. @fhahn do you plan to continue with this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122573/new/ https://reviews.llvm.org/D122573 ___ cfe-commits mailing list

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. >> I'm curious if folks have pursued @efriedma 's suggestion #2 from >> https://github.com/llvm/llvm-project/issues/54964#issuecomment-1101612886? This is something we mentioned here, perform target specific checks in Sema (but still it is not possible to catch

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. So to make some conclusion - there is a need for less strict version of musttail in LLVM, right? Ideally platform specific, like x86_musttail? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147714/new/ https://reviews.llvm.org/D147714

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. >> Will the diagnostic be emitted to the correct location if there's no debug >> info? I did some experiments and with no debug info, I got atleast the location of the caller. If we consider than currently there is no location, anything is improvement and this is

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a subscriber: jacobsa. xbolva00 added a comment. >> Could we instead encode the platform specific tail-call rules in Sema? We >> definitely have attributes that differ in behavior between platforms, and >> this seems like a perfect candidate. But then you break promise of

[PATCH] D146376: Update static_assert message for redundant cases

xbolva00 added a comment. So.. new diagnostic is: :4:1: error: static assertion failed due to requirement 'is_gitlab' static_assert(is_gitlab and is_weekend); ^ ~ Okay, a dev changes is_gitlab to true, compiles it again and boom, new error: static assertion failed due

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. Possibly verifier ignores it? Because I dont see any verifier error compiling this testcase: void NoParams(); void TestParamArityMismatchNonPortable(int x) { [[clang::nonportable_musttail]] return NoParams(); } But yeah, this is something we need to

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. In D147714#4249357 , @efriedma wrote: > An error message pointing at the call in the source code would be good > enough, I think. Adding a "reason" might be nice, but not strictly necessary. Good idea. So now for: short

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 updated this revision to Diff 511498. xbolva00 added a comment. Herald added subscribers: llvm-commits, pengfei, hiraditya. Herald added a project: LLVM. Added location when emiting diagnostic about failed TCO in the backend. CHANGES SINCE LAST ACTION

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 added a comment. In D147714#4249274 , @efriedma wrote: > Any thoughts on diagnostics here? If I'm not mistaken, with this patch, if > you request an impossible tail call, you get a crash with very little useful > information. (Although, see

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 updated this revision to Diff 511427. xbolva00 added a comment. Mention it in attribute documentation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147714/new/ https://reviews.llvm.org/D147714 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

xbolva00 created this revision. xbolva00 added reviewers: aaron.ballman, efriedma, haberman. Herald added a project: All. xbolva00 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As noted in

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xbolva00 requested changes to this revision. xbolva00 added a comment. This revision now requires changes to proceed. Huge code duplication. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

xbolva00 added a comment. Please add a release note. Thanks for this new builtin, as I was working on something very very similar - the implementation looks fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary)

xbolva00 added a comment. > I assume with this patch landed, many such cases may change code behavior. So > we will need to update msan to have a tool to detect cases like this anyway. I believe that updated msan should come first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

xbolva00 added inline comments. Comment at: clang/docs/ReleaseNotes.rst:452 +-- +- ``-fsanitize-memory-param-retval`` is turned on by default. With + ``-fsanitize=memory``, passing uninitialized variables to functions and Probably I would mention that

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary)

xbolva00 added a comment. In D134410#3816918 , @vitalybuka wrote: > I tried to hook this patch into MemorySanitizer and it reduces instrumented > code by ~30% ! Wow, amazing results! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D133634: [clang] Allow vector of BitInt

xbolva00 added a comment. Missing IR tests. No issues in codegen? Very poor description. Why it was disabled? Why we can enable it now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133634/new/ https://reviews.llvm.org/D133634

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

xbolva00 added a comment. In D125142#3782244 , @nickdesaulniers wrote: > @rsmith can we get some guidance here? Has your opinion changed in the time > since GCC has been shipping this? Maybe we should now ask @aaron.ballman as he is now main code

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

xbolva00 added a comment. This commit caused following regression https://github.com/llvm/llvm-project/issues/57449#issuecomment-1232102039 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120305/new/ https://reviews.llvm.org/D120305

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

xbolva00 added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1408 +// Intrinsic to wrap a thread local variable. +def int_threadlocal_address : DefaultAttrsIntrinsic<[llvm_anyptr_ty], [LLVMMatchType<0>], + [IntrNoMem,

[PATCH] D131683: "Diagnosing the Future Keywords"

xbolva00 added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.def:395 CXX11_KEYWORD(noexcept , 0) CXX11_KEYWORD(nullptr , 0) +CXX11_KEYWORD(static_assert , KEYMSCOMPAT|KEYC23) nullptr is now in C23, right?

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. Btw, https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 ___

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

xbolva00 added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:57 + +* GCC >= 7.1 +* Clang >= 5.0 Do we have bots which uses last supported compiler versions? GCC 7.1, Clang 5.0 and MSVC 16.7. It is bad to promise something and then breakages here and

[PATCH] D90275: [clang][IR] Add support for leaf attribute

xbolva00 added a comment. Was there any follow up? It seems that this hint (attribute) is currently not used by LLVM passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. In D130894#3715124 , @mstorsjo wrote: > This broke building with GCC (also noted by buildbot mails): > > ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void > clang::Sema::DiagnoseStaticAssertDetails(const

[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

xbolva00 added a comment. Any progress? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121593/new/ https://reviews.llvm.org/D121593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. Any tests with macros? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. or test.cpp:25:17: note: evaluated expression: '0' > 'a' CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. looks great CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. personally I dont see huge value to print “ left-hand side of operator '==' evaluates to …” I would like to see the full expression. +1 to just copy gcc’s approach CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894

[PATCH] D130894: [clang] Print more information about failed static assertions

xbolva00 added a comment. In D130894#3696534 , @tbaeder wrote: > FWIW, complex numbers are already covered it seems: > > test.cpp:35:1: error: static assertion failed due to requirement '__real c > == __imag c' > static_assert(__real c == __imag c);

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

xbolva00 added a comment. In D125142#3505767 , @jfb wrote: > I think the most relevant post from @rsmith is: > https://discourse.llvm.org/t/making-ftrivial-auto-var-init-zero-a-first-class-option/55143/40 > > He has a prototype:

[PATCH] D128645: Update developer policy.

xbolva00 added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:88 +#. Patches should be unified diffs with "infinte context" (i.e. using something + like `git diff -U 99 main`). + vext01 wrote: > xbolva00 wrote: > > vext01 wrote: > > > xbolva00

[PATCH] D128645: Update developer policy.

xbolva00 added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:88 +#. Patches should be unified diffs with "infinte context" (i.e. using something + like `git diff -U 99 main`). + vext01 wrote: > xbolva00 wrote: > > main is weird example here

[PATCH] D128645: Update developer policy.

xbolva00 added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:87 -#. Patches should be made with ``git format-patch``, or similar (see special - commands for `Requesting Phabricator review via the web interface - `_ ). If you use a - different tool, make sure

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

xbolva00 added a comment. >> -fsanitize=array-bounds workaround for size-1 array as the last member of a >> structure Could you provide option for that (to enable stricker bound checks introduced with this patch) ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

xbolva00 added a comment. In D127460#3601152 , @GuillaumeGomez wrote: > This is my first LLVM contribution so I don't think I do? If I do have a > commit access anyway, do you have a link to the documentation where it > explains what I'm supposed to

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

xbolva00 requested changes to this revision. xbolva00 added a comment. This revision now requires changes to proceed. Patch description needs to be added, especially answer the question why we should rename it, why we want it.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D127812: [AArch64] Function multiversioning support added.

xbolva00 added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:1086 +.. option:: -mno-fmv + ilinpv wrote: > MaskRay wrote: > > This file is auto-generated. Don't touch it. > It looked out of sync with options td files: > > ``` > +..

[PATCH] D125723: [MSVC] Add initial support for MSVC pragma optimize

xbolva00 added a comment. Thanks Comment at: clang/docs/LanguageExtensions.rst:3800 +``g``, ``t``, and ``y``. Clang's current implementation of the pragma behaves in +the same way as the clang pragma. All functions between ``off`` and ``on`` will +be decorated with the

[PATCH] D125723: [MSVC] Add initial support for MSVC pragma optimize

xbolva00 added inline comments. Comment at: clang/docs/ReleaseNotes.rst:304 + all functions following the pragma. At the moment, only an empty list is + supported. xbolva00 wrote: > So to sum up.. #pragma optimize("", off) disables all optimizations, right?

[PATCH] D125723: [MSVC] Add initial support for MSVC pragma optimize

xbolva00 added a comment. In D125723#3585726 , @aaron.ballman wrote: > LGTM! But given the number of people who've expressed opinions on this, > please wait a day or two before landing in case other reviewers have > additional feedback. In term of

[PATCH] D125723: [MSVC] Add initial support for MSVC pragma optimize

xbolva00 added inline comments. Comment at: clang/docs/ReleaseNotes.rst:304 + all functions following the pragma. At the moment, only an empty list is + supported. So to sum up.. #pragma optimize("", off) disables all optimizations, right? #pragma

[PATCH] D126984: [clang] Add support for optimize function attribute

xbolva00 added a comment. Also note that there is a '#pragma GCC optimize' pragma. After this patch, it should be hard to implement it. https://gcc.gnu.org/onlinedocs/gcc/Function-Specific-Option-Pragmas.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125723: [MSVC] Add support for MSVC pragma optimize

xbolva00 added a comment. >> I would like to hear quite strong motivating words. From https://reviews.llvm.org/D127565, Aaron mentioned "we have > 400 semantic attributes already and the support matrix for their combinations is pretty bad. ". Yes, this is a real good motivation why to rework

[PATCH] D126984: [clang] Add support for optimize function attribute

xbolva00 added a comment. >> That's not stopped us from exposing attributes like minsize (and you >> proposed optsize as well) which get described in the same terms as -Os/-Oz >> in our documentation, so this ship has somewhat sailed. Well, there is no promise that attribute matches -O flag in

[PATCH] D125723: [MSVC] Add support for MSVC pragma optimize

xbolva00 added a comment. In D125723#3547789 , @rnk wrote: > I think it's fine to implement this, but I wanted to share some of the > motivation for the current state of things. In our experience, the majority > of uses of pragma optimize were to work

[PATCH] D126984: [clang] Add support for optimize function attribute

xbolva00 added a comment. In D126984#3581708 , @aaron.ballman wrote: > In D126984#3579842 , @jdoerfert > wrote: > >> In D126984#3571573 , @aeubanks >> wrote: >> >>>

[PATCH] D126984: [clang] Add support for optimize function attribute

xbolva00 added a comment. If you compile file with -Ofast and use optimise(-Os) for F - I would expect no fast math flags for function F but I am worried a bit that only “optsize” is added. Plesse verify and/or add such test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. >> Would that work for you (and you as well @aeubanks)? yes :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://reviews.llvm.org/D126984 ___ cfe-commits

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added inline comments. Comment at: clang/test/CodeGen/attr-optimize.c:16 + +__attribute__((optimize("Oz"))) void f4(void) {} +// O2: @f4{{.*}}[[ATTR_OPTSIZE]] For -Os, clang adds optsize. For -Oz, clang adds optsize and minsize. So tests should check

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added inline comments. Comment at: clang/test/CodeGen/attr-optimize.c:4 + +__attribute__((optimize("O0"))) void f1(void) {} +// O2: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]] No support for ``` __attribute__ ((__optimize__ (0))) ``` ? GCC supports it

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. But where I think this feature could be very useful in following case from gcc test suite where there is some FP computation.. Imagine you compile you program with -ffast-math and then you have function: __attribute__ ((optimize ("no-associative-math"))) double fn3

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. >> Are you opposed to exposing #pragma optimize? >> (https://docs.microsoft.com/en-us/cpp/preprocessor/optimize?view=msvc-170) >> If yes, I think Stephen should run an RFC on Discourse to see if there's >> general agreement. No, I like it, seems more useful and

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. "The optimize attribute should be used for debugging purposes only. It is not suitable in production code." Until they (users) start and any change in pipeline may surprise them. Personally I am bigger fan of more targeted attributes like we have noinline / noipa

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. In D126984#3574071 , @xbolva00 wrote: > I agree with @aeubanks , this feature requires major changes to pass manager > and I see no value to land this currently. > > I see that somebody may prefer “opt for size”, but this is

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. [[gnu::optimize("O0")]] is okay but [[gnu::optimize("O3 ")]] is not gonna work without major changes. Not sure why we should deliver half-broken new attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

xbolva00 added a comment. I agree with @aeubanks , this feature requires major changes to pass manager and I see no value to land this currently. I see that somebody may prefer “opt for size”, but this is exposed via “minsize” attribute so I see no strong need for optimize(“-Os”) Repository:

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

xbolva00 added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2624 +.. option:: -fstrict-flex-array + arrays Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/

[PATCH] D72386: [ThinLTO] pass UnrollLoops/VectorizeLoop/VectorizeSLP in CGOpts down to pass builder in ltobackend

xbolva00 added inline comments. Herald added a reviewer: MaskRay. Herald added subscribers: vporpo, pmatos, asb. Herald added a project: All. Comment at: lld/ELF/LTO.cpp:96 + c.PTO.LoopVectorization = c.OptLevel > 1; + c.PTO.SLPVectorization = c.OptLevel > 1;

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

xbolva00 added a comment. In D125078#3503921 , @cjdb wrote: > I support turning it on by default. I'm simply passing on the message. +1, I support on by default as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

xbolva00 added a comment. In D125078#3501061 , @cjdb wrote: > I'm in favour of this, but @rsmith warned me a little while ago that we > mightn't be able to have stuff like this on by default because scripts > probably depend on specific compiler

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

xbolva00 added a comment. Strong +1. It would be great to backport it to llvm 14.0.x as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142 ___ cfe-commits

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added a comment. In D122983#3484064 , @manojgupta wrote: > We are finding a lot of failures in our ToT builds with this change. here is > an example for a configure script: > > $ cat tent.c > int main () > { > tgetent(0,0); > return 0; > }

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

xbolva00 added a comment. >> The intent is to allow adding arbitrary annotations to types for use in >> static analysis tools This patch should not land until we see some real use cases to justify new hundreds of lines of code. We should more careful and take maintanence cost really into

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added a comment. In D122983#3464347 , @aaron.ballman wrote: > In D122983#3464342 , @xbolva00 > wrote: > >> https://github.com/llvm/llvm-test-suite/commit/79f2b03c5111392d647fa542e120f70c8f39a991 > >

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added a comment. https://github.com/llvm/llvm-test-suite/commit/79f2b03c5111392d647fa542e120f70c8f39a991 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122983/new/ https://reviews.llvm.org/D122983

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added a subscriber: fhahn. xbolva00 added a comment. In D122983#3464315 , @aaron.ballman wrote: > In D122983#3463600 , @nemanjai > wrote: > >> In D122983#3463569

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added a comment. In D122983#3463521 , @nemanjai wrote: > This is still causing failures when building `test-suite`: > https://lab.llvm.org/buildbot/#/builders/105/builds/24292 > > Why don't we just turn off this warning for the entire

[PATCH] D123957: Update the developer policy to mention release notes

xbolva00 added a comment. Can you please add link to RFC to commit message? Otherwise looks good CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957 ___ cfe-commits mailing list

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

xbolva00 added inline comments. Comment at: clang/test/Sema/annotate-type.c:6 +void foo(float * [[clang::annotate_type("foo")]] a) { + int [[clang::annotate_type("bar")]] x1; + int * [[clang::annotate_type("bar")]] x2; Is it possible to typedef int

[PATCH] D123642: [clang codegen] Assume arguments of __atomic_* are aligned.

xbolva00 added subscribers: jwakely, jakubjelinek, msebor. xbolva00 added a comment. >> IMO, it is basically a historical bug in GCC that it ignores the type >> alignment Do you have any comments related to this issue by gcc devs that this is a "known" bug? cc @jakubjelinek @msebor @jwakely

[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

xbolva00 added inline comments. Comment at: clang/docs/CommandGuide/clang.rst:264 + +.. option:: -fno-builtin-std- + -fno-builtin=xxx,yyy,zzz wdyt? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123345/new/

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

xbolva00 added a comment. >> passing one of the seed flags Yeah, then this is fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123544/new/ https://reviews.llvm.org/D123544 ___ cfe-commits mailing

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

xbolva00 added a comment. >> If we had error diagnostics when the user is about to shoot their foot off, >> I'd be more comfortable with the automatic hardening behavior. This feature IMHO should be enabled with simple easy to understand flag, not silently and automatically do something which

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

xbolva00 added a comment. >> Because this feature requires a feature flag to enable it, this behavior >> *is* a conforming extension (the user has to take an action to get the new >> behavior). Well, then we should have proper C test (ok if linux only or so) to show case behaviour of this

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

xbolva00 added a comment. >> While I agree with the security aspects of this in principle, it is not a >> conforming behavior in C and it runs significant risk of breaking existing >> code such that it introduces new security issues. I agree strongly. This could happily can do more harm than

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

xbolva00 added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17982 + +// Maybe randomize the field order. We automatically randomize a structure +// of function pointers, unless it has the "no_randomize_layout" attribute. Is this allowed by C

[PATCH] D123405: [dllexport] odr-use constexpr default args for constructor closures

xbolva00 added a comment. No, I mean just second look. I believe @erichkeane is working in this area as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123405/new/ https://reviews.llvm.org/D123405

[PATCH] D123405: [dllexport] odr-use constexpr default args for constructor closures

xbolva00 added subscribers: aaron.ballman, rjmccall, rsmith, xbolva00. xbolva00 added a comment. Please wait for some proper clang reviewer @rsmith @aaron.ballman @rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123405/new/

[PATCH] D121556: [randstruct] Add randomize structure layout support

xbolva00 added a comment. Release note is missing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121556/new/ https://reviews.llvm.org/D121556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added a comment. Could you please check that https://github.com/llvm/llvm-test-suite is buildable with your patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122983/new/ https://reviews.llvm.org/D122983 ___ cfe-commits mailing

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

xbolva00 added inline comments. Comment at: clang/docs/ReleaseNotes.rst:114 +- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to + emitting an error (which can be downgraded with ``-Wno-error``) in C11 and + C17 mode. This is because the feature was

  1   2   3   4   5   6   7   8   9   10   >