[PATCH] D157513: [X86] Fix aliasing check between a TargetFrameIndex and a FrameIndex

[Nabeel Omer via Phabricator via llvm-commits]

n-omer created this revision.
n-omer added reviewers: RKSimon, pengfei.
Herald added subscribers: jeroen.dobbelaere, steven.zhang, arphaman, hiraditya.
Herald added a project: All.
n-omer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fixes https://github.com/llvm/llvm-project/issues/63645.

In the bug above, a store is moved up past an aliasing `lifetime.start` because of the comparison being between a TargetFrameIndex and a FrameIndex resulting in invalid code generation. This can be seen in the DAG below, t15 clearly aliases t24 but the chain argument of t24 will be moved up past t15 by DAGCombine because the comparison is between a TargetFrameIndex and a FrameIndex which have different pointer values in the compiler process but point to the exact same stack slot in the target function.

    ...
      t15: ch = lifetime.start<0 to 8> t14, TargetFrameIndex:i64<0>
    t18: ch = store<(store (s64) into @p, !tbaa !13)> t15, Constant:i64<64>, GlobalAddress:i64<ptr @p> 0, undef:i64
  t19: i32,ch = load<(dereferenceable load (s32) from @c, !tbaa !6)> t18, GlobalAddress:i64<ptr @c> 0, undef:i64
          t20: i64 = zero_extend t19
        t22: i64 = shl t20, Constant:i64<2>
      t23: i64 = add FrameIndex:i64<0>, t22
    t24: ch = store<(store (s32) into %ir.arrayidx.1.i, !tbaa !6)> t19:1, Constant:i32<50009>, t23, undef:i64


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157513

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
  llvm/test/CodeGen/X86/pr63645.ll


Index: llvm/test/CodeGen/X86/pr63645.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/pr63645.ll
@@ -0,0 +1,62 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+ at p = dso_local local_unnamed_addr global i64 0, align 8
+ at l = dso_local local_unnamed_addr global ptr null, align 8
+ at c = dso_local local_unnamed_addr global i32 0, align 4
+
+; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(argmem: readwrite)
+declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture)
+
+; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(argmem: readwrite)
+declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture)
+
+; Function Attrs: nofree nosync nounwind memory(readwrite, inaccessiblemem: none) uwtable
+define i32 @main() #1 {
+; CHECK-LABEL: main:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    movl $0, c(%rip)
+; CHECK-NEXT:    movq $64, p(%rip)
+; CHECK-NEXT:    movabsq $214787019555673, %rax # imm = 0xC3590000C359
+; CHECK-NEXT:    movq %rax, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    cmpl $42828, -{{[0-9]+}}(%rsp) # imm = 0xA74C
+; CHECK-NEXT:    jg .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %if.then.1.i
+; CHECK-NEXT:    movq l(%rip), %rax
+; CHECK-NEXT:    movb $0, (%rax)
+; CHECK-NEXT:  .LBB0_2: # %t.exit
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    retq
+entry:
+  %a.i = alloca [2 x i32], align 4
+  store i32 0, ptr @c, align 4
+  %arrayidx15.i = getelementptr inbounds [2 x i32], ptr %a.i, i64 0, i64 1
+  call void @llvm.lifetime.start.p0(i64 8, ptr nonnull %a.i)
+  store i32 50009, ptr %a.i, align 4
+  %arrayidx.i.1 = getelementptr inbounds [2 x i32], ptr %a.i, i64 0, i64 1
+  store i32 50009, ptr %arrayidx.i.1, align 4
+  call void @llvm.lifetime.end.p0(i64 8, ptr nonnull %a.i)
+  call void @llvm.lifetime.start.p0(i64 8, ptr nonnull %a.i)
+  store i64 64, ptr @p, align 8
+  %0 = load i32, ptr @c, align 4
+  %idxprom.1.i = zext i32 %0 to i64
+  %arrayidx.1.i = getelementptr inbounds [2 x i32], ptr %a.i, i64 0, i64 %idxprom.1.i
+  store i32 50009, ptr %arrayidx.1.i, align 4
+  %add14.1.i.1 = add i32 %0, 1
+  %idxprom.1.i.1 = zext i32 %add14.1.i.1 to i64
+  %arrayidx.1.i.1 = getelementptr inbounds [2 x i32], ptr %a.i, i64 0, i64 %idxprom.1.i.1
+  store i32 50009, ptr %arrayidx.1.i.1, align 4
+  %1 = load i32, ptr %arrayidx15.i, align 4
+  %cmp16.1.i = icmp slt i32 %1, 42829
+  br i1 %cmp16.1.i, label %if.then.1.i, label %t.exit
+
+if.then.1.i:                                      ; preds = %entry
+  %2 = load ptr, ptr @l, align 8
+  store i8 0, ptr %2, align 1
+  br label %t.exit
+
+t.exit:                                           ; preds = %entry, %if.then.1.i
+  call void @llvm.lifetime.end.p0(i64 8, ptr nonnull %a.i)
+  ret i32 0
+}
+
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
@@ -130,7 +130,7 @@
       MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
       // If the base are the same frame index but the we couldn't find a
       // constant offset, (indices are different) be conservative.
-      if (A != B && (!MFI.isFixedObjectIndex(A->getIndex()) ||
+      if (A->getIndex() != B->getIndex() && (!MFI.isFixedObjectIndex(A->getIndex()) ||
                      !MFI.isFixedObjectIndex(B->getIndex()))) {
         IsAlias = false;
         return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157513.548619.patch
Type: text/x-patch
Size: 3681 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230809/257dcead/attachment.bin>