[InstCombine] Avoid combining the bitcast of a var that is used as both address and result of the same load instruction

[Carrot]

Summary

This patch fixes https://llvm.org/bugs/show_bug.cgi?id=27703.

In the testcase, the result of a load instruction is used as the address of the same instruction, so a bitcast is necessary to change the value type to address type, we should avoid to optimize out the bitcast.

[majnemer]

Please add a testcase.

[Carrot]

Testcase added.

[majnemer]

Can add more context to this diff?

test/Transforms/InstCombine/pr27703.ll

2

please pipe this into FileCheck.

[Carrot]

Add more contents.

[majnemer]

LGTM with nits addressed.

test/Transforms/InstCombine/pr27703.ll

16

Can you check the load and the phi too?

[Carrot]

The original patch considers only one load instruction. There may be multiple load instructions, each loaded value is used as address of later load instruction, just as Caroline's testcase, it still causes problem. This updated patch solves the problem. Detecting the exact pattern is too complex. For simplicity the code gives up if the load address comes from another load instruction.

Please review it again.

[majnemer]

LGTM

[Eugene.Zelenko]

Looks like patch was not committed.

lib/Transforms/InstCombine/InstCombineCasts.cpp

   while (!PhiWorklist.empty()) {
     auto *OldPN = PhiWorklist.pop_back_val();
     for (Value *IncValue : OldPN->incoming_values()) {
       if (isa<Constant>(IncValue))
         continue;
 
       auto *LI = dyn_cast<LoadInst>(IncValue);
       if (LI) {
+        // If there is a sequence of one or more load instructions, each loaded
+        // value is used as address of later load instruction, bitcast is
+        // necessary to change the value type, don't optimize it. For
+        // simplicity we give up if the load address comes from another load.
+        Value *Addr = LI->getOperand(0);
+        if (Addr == &CI || isa<LoadInst>(Addr))
+          return nullptr;
         if (LI->hasOneUse() && LI->isSimple())
           continue;
         // If a LoadInst has more than one use, changing the type of loaded
         // value may create another bitcast.
         return nullptr;
       }
 
       auto *PNode = dyn_cast<PHINode>(IncValue);

test/Transforms/InstCombine/pr27703.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+define void @mem() {
+bb:
+  br label %bb6
+
+bb6:
+  %.0 = phi i8** [ undef, %bb ], [ %t2, %bb6 ]
+  %tmp = load i8*, i8** %.0, align 8
+  %bc = bitcast i8* %tmp to i8**
+  %t1 = load i8*, i8** %bc, align 8
+  %t2 = bitcast i8* %t1 to i8**
+  br label %bb6
+
+bb206:
+  ret void
+; CHECK: phi
+; CHECK: bitcast
+; CHECK: load
+}