[PATCH] D93039: Introduce llvm.noalias.decl intrinsic

[Jeroen Dobbelaere via Phabricator via llvm-commits]

jeroen.dobbelaere added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:121
+    cl::desc("Ensure that llvm.experimental.noalias.scope.decl for identical "
+             "scopes are not dominating"));
+
----------------
fhahn wrote:
> jeroen.dobbelaere wrote:
> > nikic wrote:
> > > fhahn wrote:
> > > > jeroen.dobbelaere wrote:
> > > > > nikic wrote:
> > > > > > Can we flip this flag now, or are you aware of any further issues?
> > > > > I just did a test-suite run and a stage2 release build using an assertion build with the flag flipped, and both went fine. IMHO it makes sense to try it out...
> > > > > 
> > > > What's the impact on compile-time? If it is potentially expensive, it should probably be `true` for `EXPENSIVE_CHECKS` builds.
> > > I see no reason for it to be expensive at least.
> > @fhahn, @nikic, It's O(N log(N)) (sorting of N = number of scope declarations)
> > Next to that, it is roughly k * O((M-1)^2) * O(dominance checking) with M the the number of identical scope declarations, and k the number if different scopes.
> > In practice M should be pretty low (mostly 1) and O(N log(N)) should be the main part.
> > 
> > Is that considered to be too expensive ?
> > 
> > 
> IIUC we need to potentially check whether any `llvm.experimental.noalias.scope.decl` dominates any other `llvm.experimental.noalias.scope.decl`? 
> 
> 
> Is it possible to have a function with `N` basic blocks which do not dominate each other, each with `N` `llvm.experimental.noalias.scope.decl` for the same scope?
> Is it possible to have a function with `N` basic blocks which do not dominate each other, each with `N` `llvm.experimental.noalias.scope.decl` for the same scope?
It is allowed to construct such llvm-ir manually.
I am not aware on how to trigger this by an optimization pass, just based on declarations introduced by inlining.
My understanding is that most optimizations will not duplicate the intrinsic. The extra check is there to catch those optimizations that do and do it wrongly.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93039/new/

https://reviews.llvm.org/D93039