Dead 2+ level ConstantExprs

IR & Optimizations
1

Hi,

After the recent patch to ConstantFolder concerning folding of gep of gep ([ConstantFold] Drop gep of gep fold entirely by nikic · Pull Request #95126 · llvm/llvm-project · GitHub) I’ve noticed an issue in our downstream compiler.

We have an analysis that among other things goes over users of global variables and checks them for validity for some particular transformation. The problem the aforementioned patch caused is some globals which were used in at least one ConstantExpr of ‘gep of gep’ type kept their original user in the user list, even though InstCombine folded those original expressions into something else.

I’d rather demonstrate this on a simple example:

%t = type {ptr, i64, i64, i64, i64, i64, [3 x { i64, i64, i64 }] }

@a = global %t { ptr null, i64 0, i64 0, i64 1073741952, i64 3, i64 0, [3 x { i64, i64, i64 }] zeroinitializer }

define void @foo() {
    store i64 1, ptr getelementptr inbounds ({ i64, i64, i64 }, ptr getelementptr inbounds (%t, ptr @a, i32 0, i32 6, i32 0), i32 0, i32 1)
    ret void
}

InstCombine transforms the store into

store i64 32, ptr getelementptr inbounds (i8, ptr @a, i64 56)

so the original ConstantExprs are effectively dead. But the resulting user list of @a contains both the original ones and the intermediate folding results, which are itself not dead because they are used in the enclosing ConstantExprs (which are dead):

U: ptr getelementptr inbounds (i8, ptr @a, i64 56), uses = 1
U: ptr getelementptr inbounds (i8, ptr @a, i64 48), uses = 1
        UU: ptr getelementptr inbounds ({ i64, i64, i64 }, ptr getelementptr                inbounds (i8, ptr @a, i64 48), i64 0, i32 1), uses = 0
U: ptr getelementptr inbounds (%t, ptr @a, i64 0, i32 6, i64 0), uses = 0
U: ptr getelementptr inbounds (%t, ptr @a, i32 0, i32 6, i32 0), uses = 1
        UU: ptr getelementptr inbounds ({ i64, i64, i64 }, ptr getelementptr                inbounds (%t, ptr @a, i32 0, i32 6, i32 0), i32 0, i32 1), uses = 0

One workaround I figured out is to use Constant::hasZeroLiveUses in our analysis to check if the expression is dead, but that still does seem hacky. Is there any nicer existing way to clean up (or ignore) such users before analyzing the IR?

Per my understanding the problem is caused by the fact that constant folder performs the folding bottom-up, i.e. when the enclosing expression is folded and the original one declared dead the inner ones have already been processed and there’s no way to call removeDeadConstantUsers for them. A separate cleanup utility looks like an overkill too.

Would appreciate any advice!

2

If you’re doing an analysis rooted at globals, I think doing a G->removeDeadConstantUsers() call before the analysis would be the cleanest way to avoid this.

Unfortunately, the existence of dead constant users is pretty much “expected” in LLVM.

3

Yeah, but there’s another problem - removeDeadConstantUsers() doesn’t remove dead 2+ level users when the 1st level user is alive. E.g. if I extend my example:

define void @foo() {
    store i64 1, ptr getelementptr inbounds ({ i64, i64, i64 }, ptr getelementptr inbounds (%t, ptr @a, i32 0, i32 6, i32 0), i32 0, i32 1)
    %p = ptrtoint ptr getelementptr inbounds (%t, ptr @a, i32 0, i32 6, i32 0) to i64
    store i64 %p, ptr getelementptr inbounds ({ i64, i64, i64 }, ptr getelementptr inbounds (%t, ptr @a, i32 0, i32 6, i32 0), i32 0, i32 2)
    ret void
}

That will be combined into

define void @foo() {
  store i64 1, ptr getelementptr inbounds (i8, ptr @a, i64 56), align 4
  store i64 ptrtoint (ptr getelementptr inbounds (i8, ptr @a, i64 48) to i64), ptr getelementptr inbounds (i8, ptr @a, i64 64), align 4
  ret void
}

but dead 2nd level constants will be kept:

U: ptr getelementptr inbounds (i8, ptr @a, i64 64), uses = 1
U: ptr getelementptr inbounds (i8, ptr @a, i64 56), uses = 1
U: ptr getelementptr inbounds (i8, ptr @a, i64 48), uses = 2
        UU: i64 ptrtoint (ptr getelementptr inbounds (i8, ptr @a, i64 48) to i64), uses = 1
        UU: ptr getelementptr inbounds ({ i64, i64, i64 }, ptr getelementptr inbounds (i8, ptr @a, i64 48), i64 0, i32 1), uses = 0

I suppose that was never the intention of that routine to handle cases like that?

4

I see. I just checked what we do inside GlobalOpt to deal with this, and it looks like for ConstantExpr uses that we don’t know about, we check isSafeToDestroyConstant() and skip the use. See e.g. analyzeGlobalAux or collectSRATypes. So this would be similar to your suggestion of using hasZeroLiveUses().