[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

nlopes added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1541 +CGF.Builder.CreateBr(CastFail); +return llvm::UndefValue::get(CGF.VoidPtrTy); + } Please use PoisonValue as a placeholder whenever possible. We are trying to get rid of

[PATCH] D147732: [AMDGPU] Add type mangling for {read, write, readfirst, perm}lane intrinsics

nlopes added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp:213 + +Value *Result = UndefValue::get(Ty); +for (int i = 0; i < EC; i += 1 + is16Bit) { Please use poison wherever possible. In this case it seems it's just a

[PATCH] D149548: [IR] Update to use new shufflevector semantics

nlopes added a comment. In D149548#4413598 , @uweigand wrote: > So the semantics of the `vec_promote(a, b)` intrinsic is defined as: > >> Returns a vector with a in element position b. The result is a vector with a >> in element position b. [...] The oth

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

nlopes added inline comments. Comment at: llvm/lib/Target/X86/X86InstCombineIntrinsic.cpp:3102-3105 case Intrinsic::x86_sse4a_extrqi: case Intrinsic::x86_sse4a_insertq: case Intrinsic::x86_sse4a_insertqi: +PoisonElts.setHighBits(VWidth / 2); this cha

[PATCH] D148654: Modify BoundsSan to improve debuggability

nlopes added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp:189 auto GetTrapBB = [&TrapBB](BuilderTy &IRB) { -if (TrapBB && SingleTrapBB) - return TrapBB; - -Function *Fn = IRB.GetInsertBlock()->getParent(); -// FIXME: This

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

nlopes accepted this revision. nlopes added a comment. This revision is now accepted and ready to land. LGTM, but please wait for another reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143287/new/ https://reviews.llvm.org/D143287

[PATCH] D147572: [Clang][OpenMP] Fix failure with team-wide allocated variable

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:3355 +llvm::GlobalValue::InternalLinkage, +CGM.getTriple().isAMDGCN() ? llvm::UndefValue::get(VarTy) + : llvm::Constant::getNullValue(VarTy), -

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

nlopes added a comment. In D143287#4202174 , @ManuelJBrito wrote: > Implementing the 128 to 512 casts by filling the rest of the vector with the > same definition of a nondeterministic_value is not correct because : > > a = freeze poison > v = > > is

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

nlopes added a comment. In D144903#4182498 , @efriedma wrote: > I think the point of the hasOneUse check is to avoid a possible miscompile; > if a FREEZE has more than one use, all users need to see the same value. So > not sure dropping the check is c

[PATCH] D144590: Fix shared memory allocation on AMDGPU

nlopes added a comment. Please use poison values as place holders instead of undef values as we're trying to get rid of Undef. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144590/new/ https://reviews.llvm.org/D144590 _

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

nlopes added a comment. The issue is the function call, not the stores. Stores are valid as long as they are to local memory (stack or allocated by the function). Because of the call, you can't tag it as `memory(none)`. But you may be able to tag it with inaccessiblemem if the call is also mark

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

nlopes added a comment. In D134410#3983684 , @nikic wrote: > The other problem I see here is that we still need to do something about the > memcpy -> load/store fold. As soon as we have poison from uninit values > (either directly or via `!uninit_is_poi

[PATCH] D140538: [Clang][CodeGen] Use poison instead of undef for dummy values in CGExpr [NFC]

nlopes accepted this revision. nlopes added a comment. LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140538/new/ https://reviews.llvm.org/D140538 ___ cfe-commits mailing list cfe-commits

[PATCH] D139745: [Clang]Use poison instead of undef where its used as placeholder[NFC]

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2482dbff461e: [Clang] Use poison instead of undef where its used as placeholder [NFC] (authored by ManuelJBrito, committed by nlopes). Repository:

[PATCH] D139745: [Clang]Use poison instead of undef where its used as placeholder[NFC]

nlopes accepted this revision. nlopes added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139745/new/ https://reviews.llvm.org/D139745 ___

[PATCH] D138755: [Clang][CodeGen] Use poison instead of undef for extra argument in __builtin_amdgcn_mov_dpp [NFC]

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG481170cb5511: [Clang][CodeGen] Use poison instead of undef for extra argument in… (authored by ManuelJBrito, committed by nlopes). Repository: rG

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

nlopes added a comment. In D134410#3898563 , @nikic wrote: > In D134410#3895253 , @nlopes wrote: > >> In D134410#3895077 , @nikic wrote: >> >>> In D134410#3894995

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

nlopes added a comment. In D134410#3895077 , @nikic wrote: > In D134410#3894995 , @nlopes wrote: > >> We wanted this patch to make us switch uninitialized loads to poison at >> will, since they become UB. In prac

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

nlopes added a comment. In D134410#3893918 , @nikic wrote: > I think adding this under a default-disabled flag is fine for evaluation > purposes, but I have doubts that we will ever be able to enable this by > default. There is a lot of code out there a

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGClass.cpp:3080 + if (I.getName().equals("this")) { +VMap[&I] = llvm::UndefValue::get(I.getType()); + } Please use PoisonValue instead whenever possible, as we are trying to remove un

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

nlopes added a comment. In D136284#3874755 , @jamieschmeiser wrote: > In D136284#3874614 , @nlopes wrote: > >> In D136284#3874596 , >> @jamieschmeiser wrote: >> >>> In D

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

nlopes added a comment. In D136284#3874596 , @jamieschmeiser wrote: > In D136284#3874492 , @nikic wrote: > >> At least in C++, working with uninitialized memory is pretty much always >> immediate undefined behav

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

nlopes requested changes to this revision. nlopes added a comment. > However, multiple loads of the same memory must be equal This premise is incorrect w.r.t. with current semantics, which say that loads return undef for uninit memory. As Nikita mentioned we are working towards changing this se

[PATCH] D135091: Create storage for the `_cmd` argument to the helper function for generated getters/setters of `direct` Objective-C properties.

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:1125 +llvm::Type *selType = CGF.ConvertType(CGF.getContext().getObjCSelType()); +return llvm::UndefValue::get(selType); + } mwyman wrote: > nlopes wrote: > > Please consider using Poi

[PATCH] D135392: Use PoisonValue in vector BIs [NFC]

This revision was automatically updated to reflect the committed changes. Closed by commit rG14e2592ff611: [clang][CodeGen] Use poison instead of undef as placeholder in ARM builtins… (authored by ManuelJBrito, committed by nlopes). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135392: Use PoisonValue in vector BIs [NFC]

nlopes accepted this revision. nlopes added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135392/new/ https://reviews.llvm.org/D135392 ___

[PATCH] D135091: Create storage for the `_cmd` argument to the helper function for generated getters/setters of `direct` Objective-C properties.

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:1125 +llvm::Type *selType = CGF.ConvertType(CGF.getContext().getObjCSelType()); +return llvm::UndefValue::get(selType); + } Please consider using PoisonValue here instead (if appropri

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

nlopes added a comment. In D90275#3712969 , @gulfem wrote: > In D90275#3712880 , @nlopes wrote: > >> In D90275#3707793 , @nlopes wrote: >> >>> In D90275#3671983

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

nlopes added a comment. In D90275#3707793 , @nlopes wrote: > In D90275#3671983 , @gulfem wrote: > >> In D90275#3671019 , @nlopes wrote: >> >>> Could you please add a descript

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9114 + unsigned int MinElts = SrcTy->getElementCount().getKnownMinValue(); + Value *Call = llvm::UndefValue::get(VTy); + for (unsigned int I = 0; I < Ops.size(); I++) { Please use Poison

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

nlopes added a comment. In D90275#3671983 , @gulfem wrote: > In D90275#3671019 , @nlopes wrote: > >> Could you please add a description of this attribute to LangRef? >> We need the semantics of this. >> >> Thank you

[PATCH] D126745: [RISCV][Clang] Support policy functions for vmerge, vfmerge and vcompress.

nlopes added a comment. In D126745#3693790 , @khchen wrote: > @nlopes we will update all undef to poison in follow up patches. Thank you! The benefit is that we want to remove undef altogether, so I'm trying to avoid having more undefs introduced, and i

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

nlopes added a comment. In D130224#3687351 , @arsenm wrote: > In D130224#3686756 , @nlopes wrote: > >> Would it be possible to implement this by dropping the `noundef` attribute >> rather than emitting a freeze?

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

nlopes added a comment. Would it be possible to implement this by dropping the `noundef` attribute rather than emitting a freeze? Seems like a much better option to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130224/new/ https://reviews.llvm

[PATCH] D126745: [RISCV][Clang] Support policy functions for vmerge, vfmerge and vcompress.

nlopes added a comment. While at it, could you switch those UndefValue with PoisonValue if possible? Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126745/new/ https://reviews.llvm.org/D126745 __

[PATCH] D126748: [RISCV][Clang] Support policy functions for Vector Reduction Instructions.

nlopes added a comment. While at it, could you switch those UndefValue with PoisonValue if possible? Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126748/new/ https://reviews.llvm.org/D126748 __

[PATCH] D126750: [RISCV][Clang] Support policy function for all vector segment load.

nlopes added a comment. While at it, could you switch those UndefValue with PoisonValue if possible? Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126750/new/ https://reviews.llvm.org/D126750 __

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

nlopes added a comment. Herald added a subscriber: ormris. Herald added a project: All. Could you please add a description of this attribute to LangRef? We need the semantics of this. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/n

[PATCH] D128501: [CodeGen] Make uninitialized Lvalue bit-field stores poison compatible

nlopes added a comment. In D128501#3613420 , @efriedma wrote: >> No, you can still link those. There's no ABI change nor any interference at >> IR level. > > The scenario I was thinking of with -ffine-grained-bitfield-accesses is > something like the fo

[PATCH] D128501: [CodeGen] Make uninitialized Lvalue bit-field stores poison compatible

nlopes added a comment. In D128501#3608846 , @efriedma wrote: > Under this scheme, is it illegal to link together object files built with > -ffine-grained-bitfield-accesses and object files built with > -fno-fine-grained-bitfield-accesses? No, you can

[PATCH] D125983: [DeadArgElim] Use poison instead of undef as placeholder for dead arguments

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5fc9449c962a: [DeadArgElim] Use poison instead of undef as placeholder for dead arguments (authored by nlopes). Herald added subscribers: cfe-commits

[PATCH] D98152: [InstCombine] Canonicalize SPF to min/max intrinsics

nlopes added a comment. This fixed 9 bugs reports by Alive2: - Transforms/InstCombine/max-of-nots.ll - Transforms/InstCombine/max_known_bits.ll - Transforms/InstCombine/minmax-fold.ll - Transforms/InstCombine/saturating-add-sub.ll - Transforms/InstCombine/select-pr39595.ll - Transforms/InstCombin

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

nlopes added a comment. In D119816#3332414 , @ztong0001 wrote: > In D119816#3331658 , @nlopes wrote: > >> Well, this patch is just a band-aid and a disaster waiting to happen. >> If kmalloc is tagged with an `__at

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

nlopes added a comment. Well, this patch is just a band-aid and a disaster waiting to happen. If kmalloc is tagged with an __attribute__ stating the allocation size, then you can't dereference beyond that limit or risk the alias analysis do something you don't want. You are triggering UB like th

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

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2535 ArgAttrs[FirstIRArg + i] = llvm::AttributeSet::get(getLLVMContext(), Attrs); } sammccall wrote: > fhahn wrote: > > nlopes wrote: > > > fhahn wrote: > > > > nlop

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

nlopes added a comment. In D105169#3257849 , @sdesmalen wrote: > In D105169#3254757 , @nlopes wrote: > >> In D105169#3254692 , >> @DavidSpickett wrote: >> >>> Hi, this pa

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

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2535 ArgAttrs[FirstIRArg + i] = llvm::AttributeSet::get(getLLVMContext(), Attrs); } fhahn wrote: > nlopes wrote: > > ab wrote: > > > Hmm, if I'm reading this right,

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

nlopes added a comment. In D105169#3254692 , @DavidSpickett wrote: > Hi, this patch has caused a gcc test suite failure on our SVE bots: > https://lab.llvm.org/buildbot/#/builders/184/builds/1941 > (test source is > https://github.com/llvm/llvm-test-su

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

nlopes added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2535 ArgAttrs[FirstIRArg + i] = llvm::AttributeSet::get(getLLVMContext(), Attrs); } ab wrote: > Hmm, if I'm reading this right, this overwrites the `nonnull derefere

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

nlopes added a comment. In D105169#3244624 , @nathanchance wrote: > I can reduce all of these down for you and/or I can start an email thread > with the `objtool` maintainers to see if there is a way to fix or avoid these > warnings on the `objtool` si

[PATCH] D116887: [SROA] Switch replacement of dead/UB/unreachable ops from undef to poison

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7b1cb72ad944: [SROA] Switch replacement of dead/UB/unreachable ops from undef to poison (authored by nlopes). Herald added a project: clang. Herald a

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

nlopes accepted this revision. nlopes added a comment. This revision is now accepted and ready to land. LGTM. We have discussed this before. It's important to fix the reference types in C++. Comment at: llvm/test/Analysis/BasicAA/dereferenceable.ll:66 ; CHECK: Function: l

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

nlopes added a comment. (repost my message from llvm-dev) I can add one thought regarding why this direction makes sense and why it doesn't constrain optimizations. Traditionally we don't want to mark too many things as UB as it restricts code movement and thus limits optimizations. That's why

[PATCH] D99121: [IR][InstCombine] IntToPtr Produces Typeless Pointer To Byte

nlopes added a comment. Given that it gives a decent speedup for some important workload, and provided it doesn't regress others, I think this should go in then. It's easy to revert this once opaque pointers arrive. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D99121: [IR][InstCombine] IntToPtr Produces Typeless Pointer To Byte

nlopes added a comment. In D99121#2645593 , @dblaikie wrote: > In D99121#2644362 , @lebedev.ri > wrote: > >> In D99121#2644223 , @nlopes wrote: >> >>> The pointee type in LL

[PATCH] D99121: [IR][InstCombine] IntToPtr Produces Typeless Pointer To Byte

nlopes added a comment. The pointee type in LLVM doesn't really matter. It's even supposed to disappear one day after the migration is completed. E.g., i8* and i64* are exactly the same thing: they are pointers to data. So, I don't understand the motivation for this patch. It doesn't solve the ro

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

nlopes added a comment. @rsmith already gave his blessing, so please go ahead. I hope you guys have a plan to enable this by default at some point as features behind a flag get rotten and no one uses them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

nlopes added a comment. In D87188#2476230 , @thakis wrote: > In D87188#2470401 , @lebedev.ri > wrote: > >> In D87188#2470392 , @thakis wrote: >> >>> Heads up: Breaks a test

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

nlopes 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"); } while at it, don't you want to change this one t

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

nlopes added a comment. This patch regressed the following test of Transforms/InstCombine/abs-1.ll: (need to drop NSW in this case). define i8 @nabs_canonical_3(i8 %x) { %0: %cmp = icmp slt i8 %x, 0 %neg = sub nsw i8 0, %x %abs = select i1 %cmp, i8 %x, i8 %neg ret i8 %abs }

[PATCH] D78938: Make LLVM build in C++20 mode

nlopes added a comment. In D78938#2459973 , @RKSimon wrote: > @BRevzin @nlopes This is causing MSVC build failure please can you take a > look? > > E:\llvm\llvm-project\llvm\include\llvm/DebugInfo/DWARF/DWARFDie.h(405): > note: see declaration of 'std:

[PATCH] D78938: Make LLVM build in C++20 mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG92310454bf0f: Make LLVM build in C++20 mode (authored by BRevzin, committed by nlopes). Changed prior to commit: https://reviews.llvm.org/D78938?v

[PATCH] D78938: Make LLVM build in C++20 mode

nlopes added a comment. Thanks @lebedev.ri for the pointer! I started working on exactly the same thing as I was trying to link a C++20 project with LLVM. @BRevzin is there anything missing in this patch? Do you have commit access or do you need help to land this? Repository: rG LLVM Github

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

nlopes added a comment. Nice! BTW, another popular idiom is to store data in the last few bits of the pointer (e.g., LLVM's own PointerIntPair). I guess that one can also be implement by casting the ptr to char* and doing operations over that. Repository: rG LLVM Github Monorepo CHANGES SIN

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

nlopes accepted this revision. nlopes added a comment. In D88979#2323940 , @lebedev.ri wrote: > In D88979#2323935 , @nikic wrote: > >> LGTM > > @nlopes does this look good to you? > >> Looking through other uses of

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

nlopes added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:563 if (auto* CI = dyn_cast(LI.user_back())) - if (CI->isNoopCast(DL)) + if (CI->isNoopCast(DL) && LI.getType()->isPtrOrPtrVectorTy() == +

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

nlopes added a comment. FYI InstSimplify doing distribution of undef is a known bug: https://bugs.llvm.org/show_bug.cgi?id=33165 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https://reviews.llvm.org/D83360 __

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

nlopes added a subscriber: tgt. nlopes added a comment. In D83360#2142457 , @efriedma wrote: > > that's fine but I still don't understand why the counterexample to my > > version says %x2 in @src can be undef > > If I'm understanding correctly, this reduc

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

nlopes added a comment. In D83360#2140241 , @craig.topper wrote: > Alive does like this https://alive2.llvm.org/ce/z/yhibbe which is what I was > going to implement. right. There's a guaranteedNonPoison (or similar name) in ValueTracking that can be u

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

nlopes added a comment. I'm a bit concerned with this patch as it increases the amount of UB that LLVM exploits without any study of the impact. For example, right now it's ok do this with clang (not with constants; make it less trivial so clang doesn't fold it right away): int f() { return I

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

nlopes added a comment. I agree with @aqjune that stating clearly the definition of object in this context. See this example in the C spec: For example, the second call of f in g has undefined behavior because each of d[1] through d[49] is accessed through both p and q. void g(void) {

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

nlopes added a comment. @lebedev.ri I agree with you that the semantics of these alignment builtins should only return a pointer that is of the same object as the one given as input. Otherwise, these builtins would be even worst that ptr2int/int2ptr, since their result could alias with any othe

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

nlopes added a comment. Let me give just 2 more Z3-related suggestions: - instead of re-creating the solver, it might be faster to do Z3_solver_reset - "once in a while" it might be helpful to delete everything (all solvers, asts, context) and call Z3_reset_memory. Z3's small object pool is not

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

nlopes added a comment. Regarding incremental solving with Z3 (or with most SMT solvers in general), let me just lower the expectations a bit: In Z3, when you do push(), there are a few things that change immediately: 1) it uses a different SAT solver (one that supports incremental reasoning), a

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

nlopes added a comment. Cool work Dominic! Just a few random comments from the peanut gallery regarding the usage of Z3: - I would definitely split the Z3Expr into expr/solver/model classes. Several advantages: plugging in new solvers becomes easier and reference counting can be handled more sa