LLVM 3.6 and ConstantExpr::replaceUsesOfWithOnConstant(...) strange behavior

I wrote a pass in LLVM 3.5 and I’m trying to port it into LLVM 3.6.

Unfortunately, my pass replace the operand of some constants expressions using the function replaceUsesOfWithOnConstant(…) that have been modified in the 3.6 release…

In both LLVM 3.5 and 3.6, the function ConstantExpr::replaceUsesOfWithOnConstant(…) receives 3 arguments : two Value and one Use.

In LLVM 3.5, the Use object was not really used. So, I could easily replace an operand in a constant expression by giving the old and the new Value as arguments with nullptr for Use.

myConstExpr->replaceUsesOfWithOnConstant(oldOp, newOp, nullptr);

Since LLVM 3.6, the Use object is used in a strange manner and the function getWithOperands (used inside replaceUsesOfWithOnConstant(…)) take a new boolean argument OnlyIfReduced. The documentation says:

/// If \c OnlyIfReduced is \c true, nullptr will be returned unless something
/// gets constant-folded, the type changes, or the expression is otherwise
/// canonicalized. This parameter should almost always be \c false.

Strangely, the OnlyIfReduced is set to true as you can see in the source code below. Most of time, in my case, the getWithOperands(…) will return nullptr because of this new argument (in LLVM 3.5, the old getWithOperands(…) did the job).

if (Constant *C = getWithOperands(NewOps, getType(), true)) {
replaceUsesOfWithOnConstantImpl(C);
return;
}

// Update to the new value.
if (Constant *C = getContext().pImpl->ExprConstants.replaceOperandsInPlace(
NewOps, this, From, To, NumUpdated, U - OperandList))
replaceUsesOfWithOnConstantImpl(C);

Because it return nullptr, the new function replaceOperandsInPlace(…) should do the job instead. Unfortunately, there’s an “optimization” inside replaceOperandsInPlace(…) :

// Update to the new value. Optimize for the case when we have a single
// operand that we’re changing, but handle bulk updates efficiently.

remove(CP);
if (NumUpdated == 1) {
assert(OperandNo < CP->getNumOperands() && “Invalid index”);
assert(CP->getOperand(OperandNo) != To && “I didn’t contain From!”);
CP->setOperand(OperandNo, To);
} else {
for (unsigned I = 0, E = CP->getNumOperands(); I != E; ++I)
if (CP->getOperand(I) == From)
CP->setOperand(I, To);
}

Normally, the loop inside the else branch should work (iterate over the operands and change the old ones by the new ones) but when you have NumUpdated equal to 1 (means there’s only one operand to change), it actually use the Use object to identify the operand number to change (???)…

I see two differents situations:

  1. One want to replace an operand by another one in a constant expression. We don’t know how many operands are used by the constant expression nor the opcode of this constant expression. So, we call replaceUsesOfWithOnConstant(…) without Use (should be a default argument Use=nullptr) and the function should do the job (iterate over the operands, identify those that need to be replaced and replace them). If the replacement does not include a constant-folding/type change/canonicalized, the function replaceOperandsInPlace(…) is called and the modifications are done in place.
  2. One want to replace only one operand by another one in a constant expression. We identified the correct operand so we know in advance the Use object linked to the operand and the constant expression. In that case, we call replaceUsesOfWithOnConstant with the Use object and the function should use the “optimization” inside replaceOperandsInPlace(…) to avoid doing an unnecessary loop.

Furthemore, the function replaceOperandsInPlace(…) have defaults arguments for NumUpdated=0 and OperandNo=0 that means it can be explicitly called to avoid the “optimization” trick.

Maybe I’m wrong, but for me this implementation seems more correct:

Source Code

Because it does not do what I want. Basically, my algorithm is interested in some GlobalVariable that are used by constant expression. My goal is to replace the GlobalVariable reference inside the constant expression by another GlobalVariable. If I use replaceAllUsesWith(…) on the constant expression, I should pass as argument a constant expression recreated from scratch. In the other hand, if I call replaceAllUsesWith(…) on the GlobalVariable “a” with the new GlobalVariable “b”, it will replace all variables “a” with “b” in the whole module and I still need the “a” for my algorithm.

The best solution would be the first but, during my research, I found the function replaceUsesOfWithOnConstant(…) that does exactly the job (with some improvment like the “in-place” replacement) and started using it.

But now, it doesn’t work anymore in LLVM 3.6… So, I was interested in the change made between the two versions. And, apart for my algorithm, I think that maybe the new implementation was not very logical. Like I said, the replaceOperandsInPlace(…) seems designed to work with and without a given Use/NumUpdated argument (using an improvment for the first case). So, for me, it makes no sense to force using an Use argument for replaceUsesOfWithOnConstant(…) because it can handle both cases with minor changes.

Gael

AFK right now, but IIRC, this function is supposed to be internal to replaceAllUsesWith() and it's not really supposed to be called directly. Constants aren't supposed to change.

Why can't you just call replaceAllUsesWith()?

-- dpnes
Because it does not do what I want. Basically, my algorithm is interested in some GlobalVariable that are used by constant expression. My goal is to replace the GlobalVariable reference inside the constant expression by another GlobalVariable. If I use replaceAllUsesWith(...) on the constant expression, I should pass as argument a constant expression recreated from scratch. In the other hand, if I call replaceAllUsesWith(...) on the GlobalVariable "a" with the new GlobalVariable "b", it will replace all variables "a" with "b" in the whole module and I still need the "a" for my algorithm.

It seems strange to me that you need to replace constant expressions
but not other uses. For example, you'd be translating this code:

    ; This will not change.
    %ptr = getelementptr [2 x i8]* @a, i32 0, i32 0
    store i8 7, i8* %ptr

    ; This will change.
    store i8 7, i8* getelementptr ([2 x i8]* @a, i32 0, i32 0)

into:

    ; This still points at @a.
    %ptr = getelementptr [2 x i8]* @a, i32 0, i32 0
    store i8 7, i8* %ptr

    ; This points at @b now.
    store i8 7, i8* getelementptr ([2 x i8]* @b, i32 0, i32 0)

I don't understand how this is useful.

it will replace all variables "a" with "b" in the whole module and I still need the "a" for my algorithm.

Just to be sure: you know that this function affects the whole
module too, right?

The best solution would be the first but, during my research, I found the function replaceUsesOfWithOnConstant(...) that does exactly the job (with some improvment like the "in-place" replacement) and started using it.

But now, it doesn't work anymore in LLVM 3.6... So, I was interested in the change made between the two versions. And, apart for my algorithm, I think that maybe the new implementation was not very logical. Like I said, the replaceOperandsInPlace(...) seems designed to work with and without a given Use/NumUpdated argument (using an improvment for the first case). So, for me, it makes no sense to force using an Use argument for replaceUsesOfWithOnConstant(...) because it can handle both cases with minor changes.

I'm still skeptical that this functionality is generally useful. I
think you should find a valid `Use` to send in here (it's surprising
you don't have one already; how do you know `@a` is an operand?).

If we were to add this in-tree, I suppose the right place would be a
non-virtual overload of `Constant::replaceUsesOfWithOnConstant()`
that takes two arguments, searches operands for a `Use`, and then
calls the `virtual` 3-arg version that already exists.