[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

aqjune added a comment. In D83360#2154584 , @echristo wrote: > It's that even before the msan instrumentation the IR doesn't look correct - > thus a miscompile. I'll share a prototype of the InstSimplify patch on Phabricator, in a day or two; it would

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

aqjune added a comment. In D83360#2162898 , @craig.topper wrote: > @aqjune did you put a patch for InstSimplify doing distribution over undef > yet? Sorry, making InstSimplify to safely distribute undef was a nontrivial job - other than updating InstS

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

aqjune added a comment. Made a patch at https://reviews.llvm.org/D84250 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https://reviews.llvm.org/D83360 ___ cfe-commits mailing list cfe-commi

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

aqjune added a comment. Hi, this is really interesting. I was interested in statically analyzing whether a value is undef/poison, so I also thought about the existence of this attribute, but I never imagined that MSan would benefit from this attribute as well. > The partialinit attribute is, i

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

aqjune added a comment. In D81678#2091089 , @eugenis wrote: > Positive attribute sounds good to me (frozen is not a bad name), but the > tests update change is going to be huge. Any concerns about IR size bloat? > The attribute will apply to the majority

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

aqjune added a comment. Seems like a bug in instsimplify: define i1 @f(i32 %x, i32 %y) { %cmp9.not.1 = icmp eq i32 %x, %y %cmp15 = icmp slt i32 %x, %y %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15 %spec.select40 = xor i1 %cmp9.not.1, 1 %spec.select = and

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

aqjune added a comment. (renaming variables for readability) %a = select i1 %s, i1 undef, i1 %t %b = xor i1 %s, 1 %c = and i1 %a, %b This series of reasoning happened from a single SimplifyAndInst call: c = a & (s ^ 1) = (a & s) ^ (a & 1); ExpandBinOp = ((sel

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

aqjune accepted this revision. aqjune added a comment. This revision is now accepted and ready to land. For the LangRef part, LGTM modulo the suggested change. Could anyone review clang and LLVM's updated part? Comment at: llvm/docs/LangRef.rst:1644 +and return values. Whe

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

aqjune added a comment. In D81678#2099444 , @efriedma wrote: > So I guess we've discussed the following alternatives so far: > > (snip) > > Maybe (1) is the least-bad; all the others compromise by making LLVM harder > to understand. We can make porting t

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

aqjune added a comment. In D81678#2105077 , @eugenis wrote: > > Could you explicitly state that if it is aggregate or vector type all > > elements and paddings should be frozen too? > > If an aggregate is passed as an aggregate at IR level, we should not

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

aqjune updated this revision to Diff 284246. aqjune added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Update PR3589-freestanding-libcalls.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85345/new/ https://revie

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGef018cb65c98: [BuildLibCalls] Add noundef to standard I/O functions (authored by aqjune). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

aqjune added a comment. If things go well, I'll add noundef to other library functions such as utime, setenv, unsetenv, as well. For malloc/calloc/realloc/free, I'll think more about them. I guess it is safe to tag noundef to free's operand, but for malloc I'm not 100% sure... Repository:

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

aqjune added a comment. What about undef or poison is given to malloc? If it should raise UB, the size argument and returned pointer should be noundef. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85345/new/ https://reviews.llvm.org/D85345 _

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

aqjune added a comment. In D85345#2205745 , @jdoerfert wrote: > In D85345#2205712 , @aqjune wrote: > >> What about undef or poison is given to malloc? If it should raise UB, the >> size argument and returned pointe

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

aqjune added a comment. I have a minor question: > a call of a readnone function with a byval argument is not classified as > readnone (which it is today: https://godbolt.org/z/dDfQ5r) %0 at caller has readnone attribute - is it related with the propagation of readnone attribute from %0 of emp

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune added a comment. In D103874#3594617 , @aqjune wrote: > It seems llvm/lib/Target/X86/X86ISelLowering.cpp's LowerAVXCONCAT_VECTORS is > relevant to efficient lowering of `shufflevector %x, freeze(poison), mask`. After patching `LowerAVXCONCAT_VECTO

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune added a comment. In D103874#3611483 , @nikic wrote: > Which intrinsic are you working on here? If this is about the mm_undefined > intrinsics, why do we need to change those from the current status quo of > using a zero value instead of undef? I

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune added a comment. PR31524 (https://github.com/llvm/llvm-project/issues/31524) discusses about the lowering of such intrinsics. According to the PR, it seems users consider undefined intrinsics as returning unknown but consistent bits. If it works, my updates in the draft (https://github.

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

aqjune added a comment. Herald added a subscriber: frasercrmck. In D85788#2335838 , @eugenis wrote: > I wonder it can be avoided by > > - disable noundef analysis by default in cc1 > - always add -enable-noundef-analysis in the driver when invoking cc1 Wo

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

aqjune added a comment. In D81678#2438264 , @eugenis wrote: > This change looks fine to me, but I'm slightly concerned about > https://reviews.llvm.org/D85788 - see my last comment on that revision. Thank you for the link! I left a comment there. Repos

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

aqjune added a comment. In D85788#2444136 , @guiand wrote: > IMO it's better to just one-and-done programatically add `-Xclang > -disable-noundef-analysis` to all the tests' RUN lines. That way there are no > hidden flags. If a script for this can be wr

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. In D93586#2464841 , @spatel wrote: > Besides changing IRBuilder and shufflevector's definition, I think we'll also > need updates in the vectorizers, InstSimplify, and other places in > InstCombine that use UndefValue for InsertEl

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. In D93586#2466284 , @aqjune wrote: > The transition will be swiftly done (if there's no other issue hopefully) by > the next weekend. Oops, I meant this weekend hopefully. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. > There are 792 files in llvm/test, most of which are in test/Transform (119) > and test/CodeGen(655). > The transition will be swiftly done (if there's no other issue hopefully) by > the next weekend. Thinking about these again, do we need to make a poison copy for test

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. In D93586#2467844 , @RKSimon wrote: > I'm sorry I've only just started looking at this - are you saying that you > want to handle all "insertelement undef" cases in one go and not just a > series of patcches after this one? It wi

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. In D93586#2468350 , @spatel wrote: > It would be good to update those for consistency; the codegen tests are > supposed to be representative of what comes out of the IR optimizer. IIUC, we > could do the substitution on those file

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

aqjune created this revision. aqjune added reviewers: spatel, lebedev.ri, efriedma, nlopes, regehr, RKSimon, zhengyangl, nikic, hfinkel. Herald added subscribers: dmgreen, kbarton, hiraditya, nemanjai. aqjune requested review of this revision. Herald added projects: clang, LLVM. Herald added subsc

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1021 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); } nlopes wrote: > while at it, don't you want to c

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. I'll make two more patches - the instsimplify/vectorizer/ changes that make insertelement poison, and the langref update to shufflevector. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93586/new/ https://reviews.llvm.or

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

aqjune added a comment. Hi, Naming is a hard thing... I have no special preference. :/ However, I'd like to understand the details of this attribute. Would LTO be affected because `leaf` is guaranteed to untouch the current translation unit only? // a.c int x; void f1() { f2(); }

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

aqjune added a comment. In D90275#2371813 , @jdoerfert wrote: > As noted by the GCC docs, it doesn't mean anything on a definition so that > you can safely merge TUs. I want us to forbid `leaf` on IR function > definitions for that reason, it would not m

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

aqjune added a comment. In D90275#2374648 , @gulfem wrote: > Leaf attribute is specifically intended for library functions and I think all > the existing usage of leaf attribute is in the library function declarations. > For ex, it is only used in syscall

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

aqjune added a comment. Herald added a subscriber: frasercrmck. In D82317#2201215 , @rjmccall wrote: > second, it's yet another contribution towards the giant pile of attributes > that seem to have become necessary to do any work in LLVM I don't think th

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

aqjune added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:801 + getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 || + getLangOpts().CPlusPlus17 || getLangOpts().C2x) { +LoopMustProgress = true; A silly question: does

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

aqjune added a comment. I think it makes sense - IIUC, for most of the clang tests, noundef won't be the attribute of interest. For brevity of tests, I think the change is fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://revi

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

aqjune added a comment. +1 for this patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88979/new/ https://reviews.llvm.org/D88979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

aqjune added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst:12 +if you got that integral value via a pointer-to-integer cast originally, +the new pointer will lack the provenance information from the original pointer. +

[PATCH] D92270: [ConstantFold] Fold more operations to poison

aqjune updated this revision to Diff 308200. aqjune added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. update clang/test/Frontend/fixed_point_unary.c It seems unsigned _Fract type can only represent [0.0, 1.0)? I tried to find a relevant sentence from http:/

[PATCH] D92270: [ConstantFold] Fold more operations to poison

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG53040a968dc2: [ConstantFold] Fold more operations to poison (authored by aqjune). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D92270: [ConstantFold] Fold more operations to poison

aqjune added a comment. Hi, It seems it is related to two optimizations: (1) `select i1 x, true, y` -> `or i1 x, y` (2) `or i1 x, poison` -> `poison` Semantically, the first one is broken. It needs to freeze y. But, it will introduce a lot of freeze instructions. The clang patches that introduc

[PATCH] D92270: [ConstantFold] Fold more operations to poison

aqjune added a comment. In D92270#2422661 , @uabelho wrote: > Should langref also be updated to describe this? Or does it already? > I just found this > "An instruction that depends on a poison value, produces a poison value > itself." > here > http://l

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

aqjune added a comment. Hello all, What is the status of this patch? Do we need someone who look into the lit change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678

[PATCH] D92270: [ConstantFold] Fold more operations to poison

aqjune added a comment. In D92270#2538713 , @RKSimon wrote: > https://bugs.llvm.org/show_bug.cgi?id=49005 seems to be due to this (either > directly or it has unearthed an existing problem) I reverted this commit; I'll reland this after the underlying pr

[PATCH] D93817: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (2/2)

aqjune abandoned this revision. aqjune added a comment. I abandon this patch because (1) shufflevector parts are covered in D110226 , D110227 , D110230 , and (2) insertelement parts will be cov

[PATCH] D94380: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (1/2)

aqjune abandoned this revision. aqjune added a comment. I abandon this patch because (1) shufflevector parts are covered in D110226 , D110227 , D110230 , and (2) insertelement parts will be cov

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune updated this revision to Diff 375094. aqjune added a comment. Herald added a subscriber: arphaman. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103874/new/ https://reviews.llvm.org/D103874 Files: clang/test/CodeGen/X86/avx-builtin

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune updated this revision to Diff 375095. aqjune added a comment. Resurrect mistakenly removed test statements Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103874/new/ https://reviews.llvm.org/D103874 Files: clang/test/CodeGen/X86/avx-builti

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune marked an inline comment as done. aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-builtins.c:182 // CHECK-LABEL: test_mm256_castsi128_si256 - // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32> + // CHECK: shufflevector <2 x i

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

aqjune added a comment. I'm fine with any direction that helps people land test updates/implementations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678 ___ cfe-c

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune created this revision. aqjune added reviewers: nikic, efriedma, spatel, fhahn, lebedev.ri, RKSimon. Herald added a reviewer: deadalnix. Herald added subscribers: dexonsmith, kerbowa, pengfei, dmgreen, zzheng, kbarton, hiraditya, nhaehnle, jvesely, nemanjai. aqjune requested review of this r

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune added a comment. In D103874#2806519 , @efriedma wrote: > I noted the cases where it looks like the undef->poison change might actually > impact code using compiler intrinsic functions that have external > specifications. The relevant specificati

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune created this revision. aqjune added reviewers: efriedma, spatel, craig.topper, RKSimon. Herald added a subscriber: pengfei. aqjune requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes lowering of `mm*_undefined*` intrinsics to

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune added a comment. I couldn't find end-to-end tests for checking assembly generation. To check whether this is working ok, which tests should I write and how would it look like? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ https:

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune updated this revision to Diff 354675. aqjune added a comment. Herald added subscribers: llvm-commits, ecnelises, hiraditya. Herald added a project: LLVM. - Update llvm's fast-isels tests for undefined intrinsics to compile freeze(poison) - Update X86ISelLowering's getAVX2GatherNode and get

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune updated this revision to Diff 354678. aqjune added a comment. Minor fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ https://reviews.llvm.org/D104790 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/test/CodeGen/X86/avx-built

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune updated this revision to Diff 354682. aqjune added a comment. - Update llvm/test/CodeGen/X86/sse-intrinsics-fast-isel.ll as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ https://reviews.llvm.org/D104790 Files: clang/lib/C

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune added a comment. In D104790#2836463 , @craig.topper wrote: > In D104790#2836253 , @aqjune wrote: > >> I couldn't find end-to-end tests for checking assembly generation. >> To check whether this is working

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

aqjune added a comment. In D104790#2842523 , @nikic wrote: > Is this actually better in any meaningful way? InstCombine will turn `freeze > poison` into `zeroinitializer`, and until then this is just a completely > opaque value. I think to correctly em

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

aqjune added a comment. I asked @hyeongyukim (who is a participant of google summer of code) to rebase this patch and make an updated version. I thought it was a good goal for him to target, as the validity of the enable-noundef flag is already reviewed and the changes are syntactic updates to

[PATCH] D92270: [ConstantFold] Fold more operations to poison

aqjune added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } neildhar wrote: > spatel wrote: > > MatzeB wrote: > > > I believ

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

aqjune added a comment. I am not familiar with inline assembly, but it seems the output variable (`%0`) is not updated because it does not appear in the code. 1382 __asm__ __volatile__( 1383"mov x0,x2\n" /* flags */ 1384"mov x2,x4\n" /* pt

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

aqjune added a comment. In D105169#3104262 , @eugenis wrote: > You are absolutely right. X86 variant uses an "=a" constraint (rax register), > others pin the output variable to a specific register with __asm__ > declaration. It appears we've missed it i

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

aqjune created this revision. Herald added subscribers: steakhal, martong, arphaman. Herald added a reviewer: shafik. Herald added a reviewer: NoQ. Herald added a project: All. aqjune requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This pat

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

aqjune added a comment. The updates are analogous to how `GNUNullExprClass` is processed because `UnspecifiedValueExprClass` and it is similar (have no operand). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136737/new/ https://reviews.llvm.org/D1

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-builtins.c:182 // CHECK-LABEL: test_mm256_castsi128_si256 - // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32> + // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32>

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

aqjune added a comment. Herald added subscribers: jsji, kosarev. Herald added a project: All. It seems llvm/lib/Target/X86/X86ISelLowering.cpp's LowerAVXCONCAT_VECTORS is relevant to efficient lowering of `shufflevector %x, freeze(poison), mask`. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D152658: [InstCombine] Change SimplifyDemandedVectorElts to use PoisonElts instead of UndefElts

aqjune added a comment. Hi, thanks for your hard work! Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1653 - if (ShMask[I] >= 0) { -assert(ShMask[I] < (int)NumElts && "Not expecting narrowing shuffle"); Constant *NewCElt = NewVecC[ShM

[PATCH] D152658: [InstCombine] Change SimplifyDemandedVectorElts to use PoisonElts instead of UndefElts

aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1653 - if (ShMask[I] >= 0) { -assert(ShMask[I] < (int)NumElts && "Not expecting narrowing shuffle"); Constant *NewCElt = NewVecC[ShMask[I]]; a

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

aqjune added a comment. Hi, The term 'object' seems ambiguous to me. For example, void f(i32* noalias x, i32* noalias y) { *x = 1; *y = 2; } int x[2]; f(&x[0], &x[1]); If object means an allocation, this should be UB, because x and y are pointing to the same object. Memory lo

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

aqjune added a comment. Never mind, I see that the keyword object comes from the alias analysis document. The alias analysis document is using the term object consistently. I was confused because the term is used at llvm.objectsize intrinsic's LangRef documentation, which seems to refer an alloc

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

aqjune added a comment. I agree readnone functions can read constant memory; at least LangRef doesn't restrict readnone functions from accessing memory (unless it changes a state visible to caller). I see that readnone is also attached to a function that loads from/stores to alloca: https://god

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

aqjune added a comment. H, is D104790 superseded by this patch? I wonder what is the status of this patch as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143287/new/ https://reviews.llvm.org/D143287 _

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

aqjune added a comment. In D143287#4179344 , @ManuelJBrito wrote: > In D143287#4179020 , @aqjune wrote: > >> H, is D104790 superseded by this patch? > > I don't think so we stil

[PATCH] D144903: [X86] Drop single use check for freeze(undef) in LowerAVXCONCAT_VECTORS

aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-cast-builtins.c:1 // RUN: %clang_cc1 %s -O3 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-unknown -target-feature +avx -target-feature +avx512f -target-feature +avx512fp16 -S -o - |

[PATCH] D144903: [X86] Drop single use check for freeze(undef) in LowerAVXCONCAT_VECTORS

aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-cast-builtins.c:1 // RUN: %clang_cc1 %s -O3 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-unknown -target-feature +avx -target-feature +avx512f -target-feature +avx512fp16 -S -o - |

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

aqjune abandoned this revision. aqjune added a comment. Closing this as D142388 added a function that can be used instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136737/new/ https://reviews.llvm.org/D136737

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; nikic wrote: > It looks like this fold could be salvaged, i

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2680 +// select c, (select a, true, b), false -> select c, a, false +// if c implies that b is false. +if (match(CondVal, m_Select(m_Value(A), m_One(), m_Value(B))) && -

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; nikic wrote: > aqjune wrote: > > nikic wrote: > > > It look

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { aqjune wrote: > spatel wrote: > > Any ideas ab

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { nikic wrote: > spatel wrote: > > aqjune wrote:

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. I found that there are a few more patterns that can be salvaged. Will create a patch for them. Comment at: llvm/test/Transforms/InstCombine/and.ll:898 ; CHECK-NEXT:[[Y:%.*]] = icmp ugt i72 [[C:%.*]], 42 -; CHECK-NEXT:[[AND:%.*]] = and i1 [[X]],

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. I made D101720 to cover the remaining cases except `Transforms/InstCombine/and2.ll`. Supporting `and2.ll` doesn't seem super-straightforward, but maybe some minor tweaks can make it. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. (BTW, if the patch is reviewed, then I believe we are finally ready to land this patch.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/ https://reviews.llvm.org/D101191 __

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/or.ll:1102 +; CHECK-NEXT:[[AND:%.*]] = select i1 [[Y]], i1 [[X]], i1 false +; CHECK-NEXT:[[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[AND]] ; CHECK-NEXT:ret i1 [[OR]] nikic

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/or.ll:1135 ; CHECK-NEXT:ret i1 [[OR]] ; %x = icmp sge i16 %a, %b This can be salvaged as well: https://alive2.llvm.org/ce/z/yXF96T But I think there are more patterns that are m

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. I think this patch is ready to go: running the test-suite on an ARM machine didn't complain anything. Well, but one thing that I'm concerned about is that from tomorrow I'll not be online for about three weeks. :( I'd like to find someone who reverts this patch if there

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. In D101191#2738130 , @nikic wrote: > LGTM. I can take care of reverting if there are issues. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/ https://reviews.llvm.

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

aqjune added a comment. In D93793#3221414 , @Allen wrote: > I have a babyism question, why poison is preferred to the undef in the > pattern ConstantVector::getSplat ? Hi Allen, It is because folding poison is easier than folding undef. :) For example, a

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

aqjune added a comment. This sounds like a great fix to me! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115804/new/ https://reviews.llvm.org/D115804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

aqjune added a comment. Herald added a subscriber: lxfind. Hello all, Is there a plan to enable `-enable-noundef-analysis` by default? It seems this patch is already addressing a lot of test cases. Another good news is that DeadArgElim is also fixed by D98899 to

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

aqjune added a comment. Thank you all! Incremental change makes sense to me as well. It will help smooth landing without buildbot failures. I'll start writing patches soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82317/new/ https://reviews.l

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. There are 3 more patches: https://reviews.llvm.org/D93793 (IRBuilder's CreateVectorSplat) https://reviews.llvm.org/D93817 (Other transformations) https://reviews.llvm.org/D93818 (LangRef) Would it be desirable if I land all of these at once as well as this (93586) when

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1021 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); } nikic wrote: > aqjune wrote: > > nlopes wrote: >

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

aqjune added a comment. Okay, I'll gently land this. I was just wondering whether insertelements having heterogenous placeholder would be problematic (this patch makes some of insertelement use poison, but not all), but it may not matter very much. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1021 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); } aqjune wrote: > nikic wrote: > > aqjune wrote: >

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

aqjune updated this revision to Diff 313945. aqjune added a comment. Herald added subscribers: kerbowa, nhaehnle, jvesely. Update IRBuilder to fill poison at shufflevector's empty operand Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93793/new/ htt

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

aqjune added inline comments. Comment at: clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c:84 vd = vec_splat(vd, 1); // CHECK: shufflevector <2 x double> %{{.*}}, <2 x double> undef, <2 x i32> // CHECK-ASM: vrepg These SystemZ zvector

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

aqjune updated this revision to Diff 313989. aqjune added a comment. Herald added a reviewer: bollu. Update comments Call unary CreateShuffleVector Update polly tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93793/new/ https://reviews.llvm.org

  1   2   >