[MergeICmps] Attach metadata to new created loads

[Allen]

Summary

Use clone to keep the metadata

[nikic]

I think this is right, but I'm not very confident about it. The problem is that mergeicmps can reorder loads, but I think these isolated comparisons will not get reordered before other ones (only possibly after), so I think it's fine. But maybe I just didn't manage to construct the right test case.

llvm/lib/Transforms/Scalar/MergeICmps.cpp

646

Maybe it would be better to clone the instructions, similar to the GEPs above?

[Allen]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

646

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());

[Allen]

hi @nikic

If you don't object, I still use the current solution with **new CreateLoad** temporarily?

[Allen]

I find https://reviews.llvm.org/D119319 also use this method ?

[Allen]

ping ?

[Allen]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

646

I use replaceUsesOfWith to update the PointerOperand, then clone according your comment works fine.

[Allen]

I updated with API clone

[Allen]

ping

llvm/lib/Transforms/Scalar/MergeICmps.cpp

 #include "llvm/Transforms/Scalar/MergeICmps.h"
 #include "llvm/Analysis/DomTreeUpdater.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/Instruction.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/BuildLibCalls.h"
 #include <algorithm>
 #include <numeric>
const auto ToSplit = llvm::find_if(
       Comparisons, [](const BCECmpBlock &B) { return B.RequireSplit; });
   if (ToSplit != Comparisons.end()) {
     LLVM_DEBUG(dbgs() << "Splitting non_BCE work to header\n");
     ToSplit->split(BB, AA);
   }
 
   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);
+    // Use clone to keep the metadata
+    Instruction *const LhsLoad = Builder.Insert(FirstCmp.Lhs().LoadI->clone());
+    Instruction *const RhsLoad = Builder.Insert(FirstCmp.Rhs().LoadI->clone());
+    LhsLoad->replaceUsesOfWith(LhsLoad->getOperand(0), Lhs);
+    RhsLoad->replaceUsesOfWith(RhsLoad->getOperand(0), Rhs);
     // There are no blocks to merge, just do the comparison.
     IsEqual = Builder.CreateICmpEQ(LhsLoad, RhsLoad);
   } else {
     const unsigned TotalSizeBits = std::accumulate(
         Comparisons.begin(), Comparisons.end(), 0u,
         [](int Size, const BCECmpBlock &C) { return Size + C.SizeBits(); });
 
     // memcmp expects a 'size_t' argument and returns 'int'.

llvm/test/Transforms/MergeICmps/X86/alias-merge-blocks.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -mtriple=x86_64-unknown-unknown -passes=mergeicmps -verify-dom-info -S | FileCheck %s --check-prefix=X86
 
-%S = type { i32, i32, i32, i32 }
+%S = type { i32, i32, i32, i32, i32}
 
 define zeroext i1 @opeq1(
 ; X86-LABEL: @opeq1(
 ; X86-NEXT:  "entry+land.rhs.i+land.rhs.i.2+land.rhs.i.3":
 ; X86-NEXT:    [[PTR:%.*]] = alloca i32, align 4
 ; X86-NEXT:    store i32 42, ptr [[PTR]], align 4
 ; X86-NEXT:    [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[A:%.*]], ptr [[B:%.*]], i64 16)
 ; X86-NEXT:    [[TMP2:%.*]] = icmp eq i32 [[MEMCMP]], 0
land.rhs.i.3:
   %7 = load i32, ptr %fourth2.i, align 4
   %cmp4.i = icmp eq i32 %6, %7
   br label %opeq1.exit
 
 opeq1.exit:
   %8 = phi i1 [ false, %entry ], [ false, %land.rhs.i] , [ false, %land.rhs.i.2 ], [ %cmp4.i, %land.rhs.i.3 ]
   ret i1 %8
 }
+
+define zeroext i1 @part_sequent_eq_with_metadata() {
+; X86-LABEL: @part_sequent_eq_with_metadata(
+; X86-NEXT:  bb01:
+; X86-NEXT:    [[A:%.*]] = alloca [[S:%.*]], align 8
+; X86-NEXT:    [[B:%.*]] = alloca [[S]], align 8
+; X86-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 4, !range [[RNG0:![0-9]+]], !noundef !1
+; X86-NEXT:    [[TMP1:%.*]] = load i32, ptr [[B]], align 4, !range [[RNG0]], !noundef !1
+; X86-NEXT:    [[TMP2:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+; X86-NEXT:    br i1 [[TMP2]], label %"bb1+bb2+bb3", label [[EXIT:%.*]]
+; X86:       "bb1+bb2+bb3":
+; X86-NEXT:    [[TMP3:%.*]] = getelementptr inbounds [[S]], ptr [[A]], i64 0, i32 2
+; X86-NEXT:    [[TMP4:%.*]] = getelementptr inbounds [[S]], ptr [[B]], i64 0, i32 2
+; X86-NEXT:    [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[TMP3]], ptr [[TMP4]], i64 12)
+; X86-NEXT:    [[TMP5:%.*]] = icmp eq i32 [[MEMCMP]], 0
+; X86-NEXT:    br label [[EXIT]]
+; X86:       exit:
+; X86-NEXT:    [[RET:%.*]] = phi i1 [ [[TMP5]], %"bb1+bb2+bb3" ], [ false, [[BB01:%.*]] ]
+; X86-NEXT:    ret i1 [[RET]]
+;
+bb0:
+  %a = alloca %S, align 8
+  %b = alloca %S, align 8
+  %value0 = load i32, ptr %a, align 4, !range !0, !noundef !1
+  %value1 = load i32, ptr %b, align 4, !range !0, !noundef !1
+  %cmp.i = icmp eq i32 %value0, %value1
+  br i1 %cmp.i, label %bb1, label %exit
+
+bb1:
+  %second.i = getelementptr inbounds %S, ptr %a, i64 0, i32 2
+  %value2 = load i32, ptr %second.i, align 4
+  %second2.i = getelementptr inbounds %S, ptr %b, i64 0, i32 2
+  %value3 = load i32, ptr %second2.i, align 4
+  %cmp2.i = icmp eq i32 %value2, %value3
+  br i1 %cmp2.i, label %bb2, label %exit
+
+bb2:
+  %third.i = getelementptr inbounds %S, ptr %a, i64 0, i32 3
+  %value4 = load i32, ptr %third.i, align 4
+  %third2.i = getelementptr inbounds %S, ptr %b, i64 0, i32 3
+  %value5 = load i32, ptr %third2.i, align 4
+  %cmp3.i = icmp eq i32 %value4, %value5
+  br i1 %cmp3.i, label %bb3, label %exit
+
+bb3:
+  %fourth.i = getelementptr inbounds %S, ptr %a, i64 0, i32 4
+  %value6 = load i32, ptr %fourth.i, align 4
+  %fourth2.i = getelementptr inbounds %S, ptr %b, i64 0, i32 4
+  %value7 = load i32, ptr %fourth2.i, align 4
+  %cmp4.i = icmp eq i32 %value6, %value7
+  br label %exit
+
+exit:
+  %ret = phi i1 [ false, %bb0 ], [ false, %bb1] , [ false, %bb2] ,[ %cmp4.i, %bb3 ]
+  ret i1 %ret
+}
+
+!0 = !{i32 0, i32 2}
+!1 = !{}

llvm/test/Transforms/MergeICmps/X86/entry-block-shuffled-2.ll

 ; CHECK-NEXT:    call void @init(ptr [[H]])
 ; CHECK-NEXT:    call void @init(ptr [[I]])
 ; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds %"struct.a::c", ptr [[H]], i64 0, i32 1
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds %"struct.a::c", ptr [[I]], i64 0, i32 1
 ; CHECK-NEXT:    [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[TMP0]], ptr [[TMP1]], i64 16)
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i32 [[MEMCMP]], 0
 ; CHECK-NEXT:    br i1 [[TMP2]], label [[LAND_RHS1:%.*]], label [[LAND_END:%.*]]
 ; CHECK:       land.rhs1:
-; CHECK-NEXT:    [[TMP5:%.*]] = load i32, ptr [[H]], align 4
-; CHECK-NEXT:    [[TMP6:%.*]] = load i32, ptr [[I]], align 4
-; CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i32 [[TMP5]], [[TMP6]]
+; CHECK-NEXT:    [[TMP3:%.*]] = load i32, ptr [[H]], align 8
+; CHECK-NEXT:    [[TMP4:%.*]] = load i32, ptr [[I]], align 8
+; CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i32 [[TMP3]], [[TMP4]]
 ; CHECK-NEXT:    br label [[LAND_END]]
 ; CHECK:       land.end:
-; CHECK-NEXT:    [[V9:%.*]] = phi i1 [ [[TMP7]], [[LAND_RHS1]] ], [ false, %"land.lhs.true+entry" ]
+; CHECK-NEXT:    [[V9:%.*]] = phi i1 [ [[TMP5]], [[LAND_RHS1]] ], [ false, %"land.lhs.true+entry" ]
 ; CHECK-NEXT:    ret i1 [[V9]]
 ;
 entry:
   %h = alloca %"struct.a::c", align 8
   %i = alloca %"struct.a::c", align 8
   call void @init(ptr %h)
   call void @init(ptr %i)
   %e = getelementptr inbounds %"struct.a::c", ptr %h, i64 0, i32 2