[PATCH] D146702: [MergeICmps] Attach metadata to new created loads

[Allen zhong via Phabricator via llvm-commits]

Allen added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MergeICmps.cpp:646
+    LoadInst *const RhsLoad =
         Builder.CreateLoad(FirstCmp.Rhs().LoadI->getType(), Rhs);
+    // Copy metadata
----------------
nikic wrote:
> Maybe it would be better to clone the instructions, similar to the GEPs above?
Thanks for your suggestion. 
I tried with following change, and find the case  **Transforms/MergeICmps/X86/entry-block-shuffled.ll** will fail.
this is because we only clone the load instrunction itself, and its PointerOperand will be deleted with following **DeleteDeadBlocks(DeadBlocks, &DTU)** in function BCECmpChain::simplify, then **%4 = load i32, ptr %first.i, align 4** will become  invalid instruction **%4 = load i32, ptr poison, align 4**. Did I miss something ?
```
@@ -639,10 +640,9 @@ static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
 
   if (Comparisons.size() == 1) {
     LLVM_DEBUG(dbgs() << "Only one comparison, updating branches\n");
-    Value *const LhsLoad =
-        Builder.CreateLoad(FirstCmp.Lhs().LoadI->getType(), Lhs);
-    Value *const RhsLoad =
-        Builder.CreateLoad(FirstCmp.Rhs().LoadI->getType(), Rhs);
+    Instruction *const LhsLoad = Builder.Insert(FirstCmp.Lhs().LoadI->clone());
+    Instruction *const RhsLoad = Builder.Insert(FirstCmp.Rhs().LoadI->clone());
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146702/new/

https://reviews.llvm.org/D146702