[llvm] [EarlyCSE] Fix improper icmp simplification (PR #122043)

[via llvm-commits]

https://github.com/llvmssh created https://github.com/llvm/llvm-project/pull/122043

When icmp's argument is a volatile instruction, we do not assume that icmp can be simplified.

>From 56e28a917cb74296917e77dc6dc6b03d32f6e256 Mon Sep 17 00:00:00 2001
From: shaosenhao <shaosenhao at huawei.com>
Date: Mon, 2 Dec 2024 17:42:35 +0800
Subject: [PATCH 1/3]     Use MapVector to fix lld thinLTO nondeterminism
 issue.

    When the ModuleSymbolTable is generated, the
    binary consistency problem occurs due to the
    unorder of the data structure when collecting
    asm symbols.
---
 llvm/lib/Object/RecordStreamer.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Object/RecordStreamer.h b/llvm/lib/Object/RecordStreamer.h
index 70b41f270720b4..73278dd67545b6 100644
--- a/llvm/lib/Object/RecordStreamer.h
+++ b/llvm/lib/Object/RecordStreamer.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/MC/MCDirectives.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/SMLoc.h"
@@ -32,7 +33,7 @@ class RecordStreamer : public MCStreamer {
   // Map of aliases created by .symver directives, saved so we can update
   // their symbol binding after parsing complete. This maps from each
   // aliasee to its list of aliases.
-  DenseMap<const MCSymbol *, std::vector<StringRef>> SymverAliasMap;
+  MapVector<const MCSymbol *, std::vector<StringRef>> SymverAliasMap;
 
   /// Get the state recorded for the given symbol.
   State getSymbolState(const MCSymbol *Sym);

>From 5afae5c46247ea13c4263543f534532e0c225e38 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at gmail.com>
Date: Mon, 2 Dec 2024 11:01:36 -0800
Subject: [PATCH 2/3] Order includes

---
 llvm/lib/Object/RecordStreamer.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Object/RecordStreamer.h b/llvm/lib/Object/RecordStreamer.h
index 73278dd67545b6..d9ef27c7901683 100644
--- a/llvm/lib/Object/RecordStreamer.h
+++ b/llvm/lib/Object/RecordStreamer.h
@@ -10,8 +10,8 @@
 #define LLVM_LIB_OBJECT_RECORDSTREAMER_H
 
 #include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/MC/MCDirectives.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/SMLoc.h"

>From dc7c72cef6a0f98d7361199ff4eed4864fb87323 Mon Sep 17 00:00:00 2001
From: shaosenhao <shaosenhao at huawei.com>
Date: Mon, 6 Jan 2025 20:20:05 +0800
Subject: [PATCH 3/3] [EarlyCSE] Fix improper icmp simplification

Check: the icmp parameter is volatile.
---
 llvm/lib/Analysis/InstructionSimplify.cpp     | 15 +++++++
 .../Transforms/EarlyCSE/icmp_with_volatile.ll | 40 +++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 llvm/test/Transforms/EarlyCSE/icmp_with_volatile.ll

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 515806428cbb29..db5156fcab9c1a 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3726,6 +3726,21 @@ static Value *simplifyICmpInst(CmpPredicate Pred, Value *LHS, Value *RHS,
                                const SimplifyQuery &Q, unsigned MaxRecurse) {
   assert(CmpInst::isIntPredicate(Pred) && "Not an integer compare!");
 
+  // volatile check
+  // %mark = load volatile ptr, ptr %1, align 8
+  // icmp eq ptr %mark, null
+  if (const auto LHSInstr = dyn_cast<Instruction>(LHS)) {
+     if (LHSInstr->isVolatile()) {
+       return nullptr;
+     }
+  }
+
+  if (const auto RHSInstr = dyn_cast<Instruction>(RHS)) { 
+     if (RHSInstr->isVolatile()) {
+        return nullptr;
+     }
+  }
+
   if (Constant *CLHS = dyn_cast<Constant>(LHS)) {
     if (Constant *CRHS = dyn_cast<Constant>(RHS))
       return ConstantFoldCompareInstOperands(Pred, CLHS, CRHS, Q.DL, Q.TLI);
diff --git a/llvm/test/Transforms/EarlyCSE/icmp_with_volatile.ll b/llvm/test/Transforms/EarlyCSE/icmp_with_volatile.ll
new file mode 100644
index 00000000000000..d85d56d50fb4a6
--- /dev/null
+++ b/llvm/test/Transforms/EarlyCSE/icmp_with_volatile.ll
@@ -0,0 +1,40 @@
+; RUN: opt < %s -S -passes=early-cse | FileCheck %s
+
+;When icmp's argument is a volatile instruction, we do not assume that icmp can
+;be simplified.
+define dso_local noundef i32 @_Z1funcv() {
+; CHECK-LABEL: @_Z1funcv(
+; CHECK-NEXT:    %1 = alloca ptr, align 8
+; CHECK-NEXT:    store volatile ptr null, ptr %1, align 8
+; CHECK-NEXT:    %2 = load volatile ptr, ptr %1, align 8
+; CHECK-NEXT:    %3 = ptrtoint ptr %2 to i64
+; CHECK-NEXT:    %4 = and i64 %3, 1
+; CHECK-NEXT:    %5 = icmp eq i64 %4, 0
+; CHECK-NEXT:    %6 = zext i1 %5 to i32
+; CHECK-NEXT:    ret i32 %6
+;
+  %1 = alloca ptr, align 8
+  store volatile ptr null, ptr %1, align 8
+  %2 = load volatile ptr, ptr %1, align 8
+  %3 = ptrtoint ptr %2 to i64
+  %4 = and i64 %3, 1
+  %5 = icmp eq i64 %4, 0
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define dso_local noundef i32 @_Z2funcv() {
+; CHECK-LABEL: @_Z2funcv(
+; CHECK-NEXT:    %1 = alloca ptr, align 8
+; CHECK-NEXT:    store ptr null, ptr %1, align 8
+; CHECK-NEXT:    ret i32 1
+;
+  %1 = alloca ptr, align 8
+  store ptr null, ptr %1, align 8
+  %2 = load ptr, ptr %1, align 8
+  %3 = ptrtoint ptr %2 to i64
+  %4 = and i64 %3, 1
+  %5 = icmp eq i64 %4, 0
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}