release/19.x: [MachineLICM] Don't allow hoisting invariant loads across mem barrier. (#116987)

[llvmbot]

Backport a9b3ec1 ef102b4

Requested by: @fhahn

[llvmbot]

@david-arm What do you think about merging this PR to the release branch?

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: None (llvmbot)

Changes

Backport a9b3ec1 ef102b4

Requested by: @fhahn

---

Full diff: https://github.com/llvm/llvm-project/pull/117154.diff

3 Files Affected:

• (modified) llvm/lib/CodeGen/MachineLICM.cpp (+1-1)
• (modified) llvm/test/CodeGen/AArch64/machine-licm-hoist-load.ll (+29)
• (modified) llvm/test/CodeGen/Mips/lcb5.ll (+2-2)

diff --git a/llvm/lib/CodeGen/MachineLICM.cpp b/llvm/lib/CodeGen/MachineLICM.cpp
index f24ab187ef4005..21a02a6f094784 100644
--- a/llvm/lib/CodeGen/MachineLICM.cpp
+++ b/llvm/lib/CodeGen/MachineLICM.cpp
@@ -1474,7 +1474,7 @@ void MachineLICMBase::InitializeLoadsHoistableLoops() {
       if (!AllowedToHoistLoads[Loop])
         continue;
       for (auto &MI : *MBB) {
-        if (!MI.mayStore() && !MI.isCall() &&
+        if (!MI.isLoadFoldBarrier() && !MI.mayStore() && !MI.isCall() &&
             !(MI.mayLoad() && MI.hasOrderedMemoryRef()))
           continue;
         for (MachineLoop *L = Loop; L != nullptr; L = L->getParentLoop())
diff --git a/llvm/test/CodeGen/AArch64/machine-licm-hoist-load.ll b/llvm/test/CodeGen/AArch64/machine-licm-hoist-load.ll
index e8dafd5e8fbabe..17f8263560430d 100644
--- a/llvm/test/CodeGen/AArch64/machine-licm-hoist-load.ll
+++ b/llvm/test/CodeGen/AArch64/machine-licm-hoist-load.ll
@@ -497,6 +497,35 @@ for.exit:                                 ; preds = %for.body
   ret i64 %spec.select
 }
 
+@a = external local_unnamed_addr global i32, align 4
+
+; Make sure the load is not hoisted out of the loop across memory barriers.
+define i32 @load_between_memory_barriers() {
+; CHECK-LABEL: load_between_memory_barriers:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    adrp x8, :got:a
+; CHECK-NEXT:    ldr x8, [x8, :got_lo12:a]
+; CHECK-NEXT:  .LBB8_1: // %loop
+; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    //MEMBARRIER
+; CHECK-NEXT:    ldr w0, [x8]
+; CHECK-NEXT:    //MEMBARRIER
+; CHECK-NEXT:    cbz w0, .LBB8_1
+; CHECK-NEXT:  // %bb.2: // %exit
+; CHECK-NEXT:    ret
+  br label %loop
+
+loop:
+  fence syncscope("singlethread") acq_rel
+  %l = load i32, ptr @a, align 4
+  fence syncscope("singlethread") acq_rel
+  %c = icmp eq i32 %l, 0
+  br i1 %c, label %loop, label %exit
+
+exit:
+  ret i32 %l
+}
+
 declare i32 @bcmp(ptr, ptr, i64)
 declare i32 @memcmp(ptr, ptr, i64)
 declare void @func()
diff --git a/llvm/test/CodeGen/Mips/lcb5.ll b/llvm/test/CodeGen/Mips/lcb5.ll
index f320f6fc5660ce..bb059f1ee8453e 100644
--- a/llvm/test/CodeGen/Mips/lcb5.ll
+++ b/llvm/test/CodeGen/Mips/lcb5.ll
@@ -186,7 +186,7 @@ if.end:                                           ; preds = %if.then, %entry
 }
 
 ; ci:	.ent	z3
-; ci:	bteqz	$BB6_3
+; ci:	bteqz	$BB6_2
 ; ci:	.end	z3
 
 ; Function Attrs: nounwind optsize
@@ -210,7 +210,7 @@ if.end:                                           ; preds = %if.then, %entry
 
 ; ci:	.ent	z4
 ; ci:	btnez	$BB7_1  # 16 bit inst
-; ci:	jal	$BB7_3	# branch
+; ci:	jal	$BB7_2	# branch
 ; ci:	nop
 ; ci: $BB7_1:
 ; ci:	.p2align	2

[tru]

@david-arm Should this be merged?

[david-arm]

@david-arm Should this be merged?

Hi yes I think it should be merged. It's a fairly serious bug fix.

[github-actions]

@fhahn (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.