Backstory:
Yet another time I tracked down an issue due to special casing of intrinsics.
I really think we need a change our mentality when it comes to them.
To be fair, some special handling predates the attributes to describe desired properties.
It is also OK to special case based on the intrinsic ID, but, as mentioned below, blanket statements about intrinsics are going to be wrong eventually.
It started with GlobalsAA (again) but this time I put the blame on the CallGraph.
The CallGraph (1) does not contain dbg intrinsics, and (2) does not contain call edges to most other intrinsics.
Together, I find this rather odd. Why add the latter in the first place if we don’t actually connect them?
Then at some point we realized some intrinsics can actually do interesting things, e.g., call into the module again.
However, a list that includes “special intrinsics” is not the right approach here; especially since there are no rules what an intrinsic should not be allowed to do in the lang ref (at least as far as I know).
Remember, any atomic intrinsic can effectively call into the module and have any effect (via another thread), so if we want to support any atomic intrinsic, all bets are off (by default).
This brings us to the TLDR; Intrinsics are just functions for which we know the semantics.
But if you don’t use the intrinsic ID, you cannot conclude anything about the (intrinsic) function (other than the attributes).
For the actual bug:
It turns out that GlobalsAA tries to cope with the lack of intrinsic call edges in the CallGraph by checking the function body. However, it only updates the ModRef info and not the potential effects on globals. This is similar to the CallGraph assumption that intrinsics, except the 3 special ones, could not call back into the module.
If this sounds familiar you might remember the last time this came up but this time it has nothing to do with attributes.
The fun thing about the bug is that GVN knows intrinsics can clobber globals but it trusts GlobalsAA if there is a call to a function definition. (Maybe we should simplify inline all calls 
)
To trigger a miscompilation we need a transformation, and GVN is well suited again (godbold):
@G1 = internal global i32 1
@G2 = internal global i32 1
define i32 @good(i1 %c) {
br i1 %c, label %init, label %check
init:
store i32 0, ptr @G1
br label %check
check:
call void @llvm.unknown()
%v = load i32, ptr @G1
ret i32 %v
}
define i32 @bad(i1 %c) {
br i1 %c, label %init, label %check
init:
store i32 0, ptr @G2
br label %check
check:
call void @definition()
%v = load i32, ptr @G2
ret i32 %v
}
define fastcc void @definition() {
tail call void @llvm.unknown()
ret void
}
declare void @llvm.unknown()
In @bad we will hoist the load from @G2 over the call to @definition which is wrong. As mentioned, GVN won’t do it in @good.
My proposal is to remove all special handling for intrinsics in the CallGraph and replace it with a nocallback check. Most intrinsics are nowadays annotated with this (default) attribute so the impact there should be small. We can then also remove the (wrong) special handling in GlobalsAA.
Here is how this would look like: âš™ D141190 [CallGraph][FIX] Ensure generic intrinsics are represented in the CG
Feel free to comment here or on the review.