lebedev.ri added subscribers: nlopes, regehr. lebedev.ri added a comment. In D138958#4054903 <https://reviews.llvm.org/D138958#4054903>, @jdoerfert wrote:
> In D138958#4045567 <https://reviews.llvm.org/D138958#4045567>, @efriedma > wrote: > >> From an IR semantics standpoint, I'm not sure the `memory(none)` marking is >> right if the function throws an exception. LangRef doesn't explicitly >> exclude the possibility, but take the following: >> >> void f() { throw 1; } >> >> It gets lowered to: >> >> define dso_local void @_Z1fv() local_unnamed_addr #0 { >> entry: >> %exception = tail call ptr @__cxa_allocate_exception(i64 4) #1 >> store i32 1, ptr %exception, align 16, !tbaa !5 >> tail call void @__cxa_throw(ptr nonnull %exception, ptr nonnull @_ZTIi, >> ptr null) #2 >> unreachable >> } > > If you mark this one as `readonly/none` we might, at some point, see the > mismatch in the body and declare it UB. > From the outside, it should almost be fine since it's not-`nounwind` and the > memory that is accessed is freshly allocated. > However, it still would be a problem waiting to happen to have escaping > memory being written in a `readonly/none` function. > > That said, I think we anyway want a `memory` category to express all accesses > are to freshly allocated memory that may escape the function. This is a > common pattern. > So `memory(allocated, inaccessiblemem)` would express that the allocation > does access some inaccesible memory and the function will also access the > newly allocated memory. > Its arguably not as good as `readnone` but I'm not sure how to get there. > The best idea I have off the top of my head is a dedicated `exception` > category for `memory` such that it won't interfere with anything but other > exceptions, which it already does due to `unwind`. > Thus, `pure/const` -> `memory(exception). Thank you for commenting! Is there a feeling that just dropping the attributes, until they can be reasonably represented, hinders optimizations too much, and we must retain the ongoing miscompile? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138958/new/ https://reviews.llvm.org/D138958 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
