GVN fails with bitcasts

Hi,

i have the following code:
define internal %“struct.dexter::ConditionConstant”* @_ZN6dexter18BinaryConditionAdd8evaluateEv5(%“class.dexter::BinaryConditionAdd”) {
entry:
%1 = getelementptr inbounds %“class.dexter::BinaryConditionAdd”
%0, i32 0, i32 0, i32 1
%2 = load %“class.dexter::BaseCondition”** %1, align 8
%3 = bitcast %“class.dexter::BaseCondition”* %2 to %“class.dexter::BinaryConditionAdd”*
%4 = getelementptr inbounds %“class.dexter::BinaryConditionAdd”* %3, i32 0, i32 0, i32 0, i32 1, i32 0
%5 = bitcast %union.anon* %4 to i64*
store i64 4, i64* %5, align 8
%6 = getelementptr inbounds %“class.dexter::BinaryConditionAdd”* %3, i32 0, i32 0, i32 0, i32 1
%7 = bitcast %“struct.dexter::ConditionConstant”* %6 to i64*
%8 = load i64* %7, align 8
%9 = add nsw i64 %8, 2
%10 = getelementptr inbounds %“class.dexter::BinaryConditionAdd”* %0, i32 0, i32 0, i32 0, i32 1, i32 0
%11 = bitcast %union.anon* %10 to i64*
store i64 %9, i64* %11, align 8
%12 = getelementptr inbounds %“class.dexter::BinaryConditionAdd”* %0, i32 0, i32 0, i32 0, i32 1
ret %“struct.dexter::ConditionConstant”* %12
}

%5 and %7 point to the same memory location. This is not detected because %4 and %6 look different. But they have the same offset and the bitcast creates. Do you have an idea how to fix that?

I created the following pass that fixes the issue:

class bitcastCombinePass: public llvm::BasicBlockPass {
private:
JITEngine *engine;
static char ID;

public:
bitcastCombinePass(JITEngine *engine): BasicBlockPass(ID), engine(engine) {}

virtual bool runOnBasicBlock(llvm::BasicBlock &BB) {
bool changed = false;
for(auto i = BB.begin(); i != BB.end(); i++) {
llvm::BitCastInst *bitcast = dynamic_castllvm::BitCastInst*(&*i);
if(bitcast) {
// find Twin-Bitcast
for(auto i2 = BB.begin(); i2 != i; i2++) {
llvm::BitCastInst *bitcast2 = dynamic_castllvm::BitCastInst*(&*i2);
if(bitcast2) {
// two bitcasts … same type?
if(bitcast->getType() == bitcast2->getType()) {
llvm::GetElementPtrInst *gep1 = dynamic_castllvm::GetElementPtrInst*(bitcast->getOperand(0));
llvm::GetElementPtrInst *gep2 = dynamic_castllvm::GetElementPtrInst*(bitcast2->getOperand(0));
// both get GEP from same operand
if(gep1 && gep2 && gep1->getPointerOperand() == gep2->getPointerOperand()) {
llvm::APInt offset1(sizeof(int *) * 8, 0);
llvm::APInt offset2(sizeof(int *) * 8, 0);
if(gep1->accumulateConstantOffset(*engine->executionEngine->getDataLayout(), offset1)
&& gep2->accumulateConstantOffset(*engine->executionEngine->getDataLayout(), offset2)
&& offset1 == offset2) {
// bitcasts point to same value => replace them
bitcast->replaceAllUsesWith(bitcast2);
}
}
}
}
}
}
}
return changed;
}
};
char bitcastCombinePass::ID;

This should become part of either InstCombine or GVN I think.

Carl-Philip Hänsch wrote:

Hi,

i have the following code:
define internal %"struct.dexter::ConditionConstant"*
@_ZN6dexter18BinaryConditionAdd8evaluateEv5(%"class.dexter::BinaryConditionAdd"*)
{
entry:
   %1 = getelementptr inbounds %"class.dexter::BinaryConditionAdd"* %0,
i32 0, i32 0, i32 1
   %2 = load %"class.dexter::BaseCondition"** %1, align 8
   %3 = bitcast %"class.dexter::BaseCondition"* %2 to
%"class.dexter::BinaryConditionAdd"*
   %4 = getelementptr inbounds %"class.dexter::BinaryConditionAdd"* %3,
i32 0, i32 0, i32 0, i32 1, i32 0
   %5 = bitcast %union.anon* %4 to i64*
   store i64 4, i64* %5, align 8
   %6 = getelementptr inbounds %"class.dexter::BinaryConditionAdd"* %3,
i32 0, i32 0, i32 0, i32 1
   %7 = bitcast %"struct.dexter::ConditionConstant"* %6 to i64*
   %8 = load i64* %7, align 8
   %9 = add nsw i64 %8, 2
   %10 = getelementptr inbounds %"class.dexter::BinaryConditionAdd"* %0,
i32 0, i32 0, i32 0, i32 1, i32 0
   %11 = bitcast %union.anon* %10 to i64*
   store i64 %9, i64* %11, align 8
   %12 = getelementptr inbounds %"class.dexter::BinaryConditionAdd"* %0,
i32 0, i32 0, i32 0, i32 1
   ret %"struct.dexter::ConditionConstant"* %12
}

%5 and %7 point to the same memory location. This is not detected
because %4 and %6 look different. But they have the same offset and the
bitcast creates. Do you have an idea how to fix that?

Without seeing the definition of class.dexter::BinaryConditionAdd it's hard to tell. Is the element it's pointing to actually an i64? If it is, then we could canonicalize the gep+bitcast to just a gep. If not, then we need to have a GEP+bitcast situation, but it's possible that when we see a GEP with a single use bitcast, we could canonicalize by stripping trailing zeroes off the GEP?

If instcombine did that, would GVN then be able to solve this?

Nick