https://github.com/llvmbot created
https://github.com/llvm/llvm-project/pull/83856
Backport 6e41d60a717132fadac74abe61ac6a9b1ca98778
63725ab1196ac50509ad382fc12c56f6d8b5d874
Requested by: @davemgreen
>From cb7eeae523e271cbc83512ed6f4c98b2161b155f Mon Sep 17 00:00:00 2001
From: David Green <david.gr...@arm.com>
Date: Wed, 28 Feb 2024 09:43:05 +0000
Subject: [PATCH 1/2] [SelectionDAG] Change computeAliasing signature from
optional<uint64> to LocationSize. (#83017)
This is another smaller step of #70452, changing the signature of
computeAliasing() from optional<uint64_t> to LocationSize, and follow-up
changes in DAGCombiner::mayAlias(). There are some test change due to
the previous AA->isNoAlias call incorrectly using an unknown size
(~UINT64_T(0)). This should then be improved again in #70452 when the
types are known to be scalable.
(cherry picked from commit 6e41d60a717132fadac74abe61ac6a9b1ca98778)
---
.../CodeGen/SelectionDAGAddressAnalysis.h | 7 +-
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 37 +++++----
.../SelectionDAGAddressAnalysis.cpp | 20 +++--
.../alloca-load-store-scalable-array.ll | 36 ++++-----
.../alloca-load-store-scalable-struct.ll | 12 +--
.../rvv/alloca-load-store-scalable-array.ll | 12 +--
.../rvv/alloca-load-store-scalable-struct.ll | 8 +-
.../SelectionDAGAddressAnalysisTest.cpp | 80 ++++++++-----------
8 files changed, 102 insertions(+), 110 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
b/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
index 3d0f836b0c7578..29de6bd8685e06 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
@@ -9,6 +9,7 @@
#ifndef LLVM_CODEGEN_SELECTIONDAGADDRESSANALYSIS_H
#define LLVM_CODEGEN_SELECTIONDAGADDRESSANALYSIS_H
+#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/CodeGen/SelectionDAGNodes.h"
#include <cstdint>
@@ -81,10 +82,8 @@ class BaseIndexOffset {
// Returns true `Op0` and `Op1` can be proven to alias/not alias, in
// which case `IsAlias` is set to true/false.
- static bool computeAliasing(const SDNode *Op0,
- const std::optional<int64_t> NumBytes0,
- const SDNode *Op1,
- const std::optional<int64_t> NumBytes1,
+ static bool computeAliasing(const SDNode *Op0, const LocationSize NumBytes0,
+ const SDNode *Op1, const LocationSize NumBytes1,
const SelectionDAG &DAG, bool &IsAlias);
/// Parses tree in N for base, index, offset addresses.
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 3135ec73a99e76..eb912bff4094ce 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -27849,7 +27849,7 @@ bool DAGCombiner::mayAlias(SDNode *Op0, SDNode *Op1)
const {
bool IsAtomic;
SDValue BasePtr;
int64_t Offset;
- std::optional<int64_t> NumBytes;
+ LocationSize NumBytes;
MachineMemOperand *MMO;
};
@@ -27868,7 +27868,8 @@ bool DAGCombiner::mayAlias(SDNode *Op0, SDNode *Op1)
const {
LSN->isAtomic(),
LSN->getBasePtr(),
Offset /*base offset*/,
- std::optional<int64_t>(Size),
+ Size != ~UINT64_C(0) ? LocationSize::precise(Size)
+ : LocationSize::beforeOrAfterPointer(),
LSN->getMemOperand()};
}
if (const auto *LN = cast<LifetimeSDNode>(N))
@@ -27876,13 +27877,15 @@ bool DAGCombiner::mayAlias(SDNode *Op0, SDNode *Op1)
const {
/*isAtomic*/ false,
LN->getOperand(1),
(LN->hasOffset()) ? LN->getOffset() : 0,
- (LN->hasOffset()) ? std::optional<int64_t>(LN->getSize())
- : std::optional<int64_t>(),
+ (LN->hasOffset()) ? LocationSize::precise(LN->getSize())
+ : LocationSize::beforeOrAfterPointer(),
(MachineMemOperand *)nullptr};
// Default.
return {false /*isvolatile*/,
- /*isAtomic*/ false, SDValue(),
- (int64_t)0 /*offset*/, std::optional<int64_t>() /*size*/,
+ /*isAtomic*/ false,
+ SDValue(),
+ (int64_t)0 /*offset*/,
+ LocationSize::beforeOrAfterPointer() /*size*/,
(MachineMemOperand *)nullptr};
};
@@ -27937,18 +27940,20 @@ bool DAGCombiner::mayAlias(SDNode *Op0, SDNode *Op1)
const {
int64_t SrcValOffset1 = MUC1.MMO->getOffset();
Align OrigAlignment0 = MUC0.MMO->getBaseAlign();
Align OrigAlignment1 = MUC1.MMO->getBaseAlign();
- auto &Size0 = MUC0.NumBytes;
- auto &Size1 = MUC1.NumBytes;
+ LocationSize Size0 = MUC0.NumBytes;
+ LocationSize Size1 = MUC1.NumBytes;
if (OrigAlignment0 == OrigAlignment1 && SrcValOffset0 != SrcValOffset1 &&
- Size0.has_value() && Size1.has_value() && *Size0 == *Size1 &&
- OrigAlignment0 > *Size0 && SrcValOffset0 % *Size0 == 0 &&
- SrcValOffset1 % *Size1 == 0) {
+ Size0.hasValue() && Size1.hasValue() && Size0 == Size1 &&
+ OrigAlignment0 > Size0.getValue() &&
+ SrcValOffset0 % Size0.getValue() == 0 &&
+ SrcValOffset1 % Size1.getValue() == 0) {
int64_t OffAlign0 = SrcValOffset0 % OrigAlignment0.value();
int64_t OffAlign1 = SrcValOffset1 % OrigAlignment1.value();
// There is no overlap between these relatively aligned accesses of
// similar size. Return no alias.
- if ((OffAlign0 + *Size0) <= OffAlign1 || (OffAlign1 + *Size1) <= OffAlign0)
+ if ((OffAlign0 + (int64_t)Size0.getValue()) <= OffAlign1 ||
+ (OffAlign1 + (int64_t)Size1.getValue()) <= OffAlign0)
return false;
}
@@ -27961,12 +27966,12 @@ bool DAGCombiner::mayAlias(SDNode *Op0, SDNode *Op1)
const {
UseAA = false;
#endif
- if (UseAA && AA && MUC0.MMO->getValue() && MUC1.MMO->getValue() && Size0 &&
- Size1) {
+ if (UseAA && AA && MUC0.MMO->getValue() && MUC1.MMO->getValue() &&
+ Size0.hasValue() && Size1.hasValue()) {
// Use alias analysis information.
int64_t MinOffset = std::min(SrcValOffset0, SrcValOffset1);
- int64_t Overlap0 = *Size0 + SrcValOffset0 - MinOffset;
- int64_t Overlap1 = *Size1 + SrcValOffset1 - MinOffset;
+ int64_t Overlap0 = Size0.getValue() + SrcValOffset0 - MinOffset;
+ int64_t Overlap1 = Size1.getValue() + SrcValOffset1 - MinOffset;
if (AA->isNoAlias(
MemoryLocation(MUC0.MMO->getValue(), Overlap0,
UseTBAA ? MUC0.MMO->getAAInfo() : AAMDNodes()),
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
index 66825d845c1910..9670c3ac8430eb 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
@@ -91,11 +91,10 @@ bool BaseIndexOffset::equalBaseIndex(const BaseIndexOffset
&Other,
}
bool BaseIndexOffset::computeAliasing(const SDNode *Op0,
- const std::optional<int64_t> NumBytes0,
+ const LocationSize NumBytes0,
const SDNode *Op1,
- const std::optional<int64_t> NumBytes1,
+ const LocationSize NumBytes1,
const SelectionDAG &DAG, bool &IsAlias) {
-
BaseIndexOffset BasePtr0 = match(Op0, DAG);
if (!BasePtr0.getBase().getNode())
return false;
@@ -105,27 +104,26 @@ bool BaseIndexOffset::computeAliasing(const SDNode *Op0,
return false;
int64_t PtrDiff;
- if (NumBytes0 && NumBytes1 &&
- BasePtr0.equalBaseIndex(BasePtr1, DAG, PtrDiff)) {
+ if (BasePtr0.equalBaseIndex(BasePtr1, DAG, PtrDiff)) {
// If the size of memory access is unknown, do not use it to analysis.
// One example of unknown size memory access is to load/store scalable
// vector objects on the stack.
// BasePtr1 is PtrDiff away from BasePtr0. They alias if none of the
// following situations arise:
- if (PtrDiff >= 0 &&
- *NumBytes0 != static_cast<int64_t>(MemoryLocation::UnknownSize)) {
+ if (PtrDiff >= 0 && NumBytes0.hasValue() && !NumBytes0.isScalable()) {
// [----BasePtr0----]
// [---BasePtr1--]
// ========PtrDiff========>
- IsAlias = !(*NumBytes0 <= PtrDiff);
+ IsAlias = !(static_cast<int64_t>(NumBytes0.getValue().getFixedValue()) <=
+ PtrDiff);
return true;
}
- if (PtrDiff < 0 &&
- *NumBytes1 != static_cast<int64_t>(MemoryLocation::UnknownSize)) {
+ if (PtrDiff < 0 && NumBytes1.hasValue() && !NumBytes1.isScalable()) {
// [----BasePtr0----]
// [---BasePtr1--]
// =====(-PtrDiff)====>
- IsAlias = !((PtrDiff + *NumBytes1) <= 0);
+ IsAlias = !((PtrDiff + static_cast<int64_t>(
+ NumBytes1.getValue().getFixedValue())) <= 0);
return true;
}
return false;
diff --git a/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-array.ll
b/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-array.ll
index 7244ac949ab88c..9a4e01a29ecb6d 100644
--- a/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-array.ll
+++ b/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-array.ll
@@ -14,12 +14,12 @@ define void @array_1D(ptr %addr) #0 {
; CHECK-NEXT: .cfi_escape 0x0f, 0x0c, 0x8f, 0x00, 0x11, 0x10, 0x22, 0x11,
0x18, 0x92, 0x2e, 0x00, 0x1e, 0x22 // sp + 16 + 24 * VG
; CHECK-NEXT: .cfi_offset w29, -16
; CHECK-NEXT: ptrue p0.d
-; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0]
-; CHECK-NEXT: ld1d { z1.d }, p0/z, [x0, #2, mul vl]
-; CHECK-NEXT: ld1d { z2.d }, p0/z, [x0, #1, mul vl]
-; CHECK-NEXT: st1d { z0.d }, p0, [sp]
-; CHECK-NEXT: st1d { z1.d }, p0, [sp, #2, mul vl]
-; CHECK-NEXT: st1d { z2.d }, p0, [sp, #1, mul vl]
+; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0, #2, mul vl]
+; CHECK-NEXT: ld1d { z1.d }, p0/z, [x0, #1, mul vl]
+; CHECK-NEXT: ld1d { z2.d }, p0/z, [x0]
+; CHECK-NEXT: st1d { z0.d }, p0, [sp, #2, mul vl]
+; CHECK-NEXT: st1d { z1.d }, p0, [sp, #1, mul vl]
+; CHECK-NEXT: st1d { z2.d }, p0, [sp]
; CHECK-NEXT: addvl sp, sp, #3
; CHECK-NEXT: ldr x29, [sp], #16 // 8-byte Folded Reload
; CHECK-NEXT: ret
@@ -81,18 +81,18 @@ define void @array_2D(ptr %addr) #0 {
; CHECK-NEXT: .cfi_escape 0x0f, 0x0c, 0x8f, 0x00, 0x11, 0x10, 0x22, 0x11,
0x30, 0x92, 0x2e, 0x00, 0x1e, 0x22 // sp + 16 + 48 * VG
; CHECK-NEXT: .cfi_offset w29, -16
; CHECK-NEXT: ptrue p0.d
-; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0]
-; CHECK-NEXT: ld1d { z1.d }, p0/z, [x0, #5, mul vl]
-; CHECK-NEXT: ld1d { z2.d }, p0/z, [x0, #1, mul vl]
-; CHECK-NEXT: ld1d { z3.d }, p0/z, [x0, #4, mul vl]
-; CHECK-NEXT: ld1d { z4.d }, p0/z, [x0, #2, mul vl]
-; CHECK-NEXT: ld1d { z5.d }, p0/z, [x0, #3, mul vl]
-; CHECK-NEXT: st1d { z0.d }, p0, [sp]
-; CHECK-NEXT: st1d { z1.d }, p0, [sp, #5, mul vl]
-; CHECK-NEXT: st1d { z3.d }, p0, [sp, #4, mul vl]
-; CHECK-NEXT: st1d { z5.d }, p0, [sp, #3, mul vl]
-; CHECK-NEXT: st1d { z4.d }, p0, [sp, #2, mul vl]
-; CHECK-NEXT: st1d { z2.d }, p0, [sp, #1, mul vl]
+; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0, #5, mul vl]
+; CHECK-NEXT: ld1d { z1.d }, p0/z, [x0, #4, mul vl]
+; CHECK-NEXT: ld1d { z2.d }, p0/z, [x0]
+; CHECK-NEXT: ld1d { z3.d }, p0/z, [x0, #3, mul vl]
+; CHECK-NEXT: ld1d { z4.d }, p0/z, [x0, #1, mul vl]
+; CHECK-NEXT: ld1d { z5.d }, p0/z, [x0, #2, mul vl]
+; CHECK-NEXT: st1d { z0.d }, p0, [sp, #5, mul vl]
+; CHECK-NEXT: st1d { z1.d }, p0, [sp, #4, mul vl]
+; CHECK-NEXT: st1d { z3.d }, p0, [sp, #3, mul vl]
+; CHECK-NEXT: st1d { z5.d }, p0, [sp, #2, mul vl]
+; CHECK-NEXT: st1d { z4.d }, p0, [sp, #1, mul vl]
+; CHECK-NEXT: st1d { z2.d }, p0, [sp]
; CHECK-NEXT: addvl sp, sp, #6
; CHECK-NEXT: ldr x29, [sp], #16 // 8-byte Folded Reload
; CHECK-NEXT: ret
diff --git a/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-struct.ll
b/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-struct.ll
index f03a6f018d34d0..7292d52aaf4765 100644
--- a/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-struct.ll
+++ b/llvm/test/CodeGen/AArch64/alloca-load-store-scalable-struct.ll
@@ -13,12 +13,12 @@ define void @test(ptr %addr) #0 {
; CHECK-NEXT: .cfi_escape 0x0f, 0x0c, 0x8f, 0x00, 0x11, 0x10, 0x22, 0x11,
0x18, 0x92, 0x2e, 0x00, 0x1e, 0x22 // sp + 16 + 24 * VG
; CHECK-NEXT: .cfi_offset w29, -16
; CHECK-NEXT: ptrue p0.d
-; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0]
-; CHECK-NEXT: ld1d { z1.d }, p0/z, [x0, #2, mul vl]
-; CHECK-NEXT: ld1d { z2.d }, p0/z, [x0, #1, mul vl]
-; CHECK-NEXT: st1d { z0.d }, p0, [sp]
-; CHECK-NEXT: st1d { z1.d }, p0, [sp, #2, mul vl]
-; CHECK-NEXT: st1d { z2.d }, p0, [sp, #1, mul vl]
+; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0, #2, mul vl]
+; CHECK-NEXT: ld1d { z1.d }, p0/z, [x0, #1, mul vl]
+; CHECK-NEXT: ld1d { z2.d }, p0/z, [x0]
+; CHECK-NEXT: st1d { z0.d }, p0, [sp, #2, mul vl]
+; CHECK-NEXT: st1d { z1.d }, p0, [sp, #1, mul vl]
+; CHECK-NEXT: st1d { z2.d }, p0, [sp]
; CHECK-NEXT: addvl sp, sp, #3
; CHECK-NEXT: ldr x29, [sp], #16 // 8-byte Folded Reload
; CHECK-NEXT: ret
diff --git a/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-array.ll
b/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-array.ll
index 1fe91c721f4dd2..1d025a2f776f82 100644
--- a/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-array.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-array.ll
@@ -18,15 +18,15 @@ define void @test(ptr %addr) {
; CHECK-NEXT: add a2, a0, a1
; CHECK-NEXT: vl1re64.v v8, (a2)
; CHECK-NEXT: slli a2, a1, 1
-; CHECK-NEXT: vl1re64.v v9, (a0)
-; CHECK-NEXT: add a0, a0, a2
+; CHECK-NEXT: add a3, a0, a2
+; CHECK-NEXT: vl1re64.v v9, (a3)
; CHECK-NEXT: vl1re64.v v10, (a0)
; CHECK-NEXT: addi a0, sp, 16
-; CHECK-NEXT: vs1r.v v9, (a0)
; CHECK-NEXT: add a2, a0, a2
-; CHECK-NEXT: vs1r.v v10, (a2)
-; CHECK-NEXT: add a0, a0, a1
-; CHECK-NEXT: vs1r.v v8, (a0)
+; CHECK-NEXT: vs1r.v v9, (a2)
+; CHECK-NEXT: add a1, a0, a1
+; CHECK-NEXT: vs1r.v v8, (a1)
+; CHECK-NEXT: vs1r.v v10, (a0)
; CHECK-NEXT: csrrs a0, vlenb, zero
; CHECK-NEXT: slli a0, a0, 2
; CHECK-NEXT: add sp, sp, a0
diff --git a/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-struct.ll
b/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-struct.ll
index 4143ea25f2bba9..90adbc24178633 100644
--- a/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-struct.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/alloca-load-store-scalable-struct.ll
@@ -16,13 +16,13 @@ define <vscale x 1 x double> @test(%struct.test* %addr, i64
%vl) {
; CHECK-NEXT: sub sp, sp, a2
; CHECK-NEXT: .cfi_escape 0x0f, 0x0d, 0x72, 0x00, 0x11, 0x10, 0x22, 0x11,
0x02, 0x92, 0xa2, 0x38, 0x00, 0x1e, 0x22 # sp + 16 + 2 * vlenb
; CHECK-NEXT: csrrs a2, vlenb, zero
-; CHECK-NEXT: vl1re64.v v8, (a0)
-; CHECK-NEXT: add a0, a0, a2
+; CHECK-NEXT: add a3, a0, a2
+; CHECK-NEXT: vl1re64.v v8, (a3)
; CHECK-NEXT: vl1re64.v v9, (a0)
; CHECK-NEXT: addi a0, sp, 16
-; CHECK-NEXT: vs1r.v v8, (a0)
; CHECK-NEXT: add a2, a0, a2
-; CHECK-NEXT: vs1r.v v9, (a2)
+; CHECK-NEXT: vs1r.v v8, (a2)
+; CHECK-NEXT: vs1r.v v9, (a0)
; CHECK-NEXT: vl1re64.v v8, (a2)
; CHECK-NEXT: vl1re64.v v9, (a0)
; CHECK-NEXT: vsetvli zero, a1, e64, m1, ta, ma
diff --git a/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
b/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
index 7426884217a08e..1f2b8c1754f6ef 100644
--- a/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
+++ b/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
@@ -110,12 +110,12 @@ TEST_F(SelectionDAGAddressAnalysisTest, sameFrameObject) {
SDValue Index = DAG->getMemBasePlusOffset(FIPtr, Offset, Loc);
SDValue Store = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index,
PtrInfo.getWithOffset(Offset));
- std::optional<int64_t> NumBytes = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes = cast<StoreSDNode>(Store)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store.getNode(), NumBytes, Store.getNode(), NumBytes, *DAG, IsAlias);
+ Store.getNode(), LocationSize::precise(NumBytes), Store.getNode(),
+ LocationSize::precise(NumBytes), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_TRUE(IsAlias);
@@ -134,14 +134,10 @@ TEST_F(SelectionDAGAddressAnalysisTest,
sameFrameObjectUnknownSize) {
SDValue Store = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index,
PtrInfo.getWithOffset(Offset));
- // Maybe unlikely that BaseIndexOffset::computeAliasing is used with the
- // optional NumBytes being unset like in this test, but it would be confusing
- // if that function determined IsAlias=false here.
- std::optional<int64_t> NumBytes;
-
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store.getNode(), NumBytes, Store.getNode(), NumBytes, *DAG, IsAlias);
+ Store.getNode(), LocationSize::beforeOrAfterPointer(), Store.getNode(),
+ LocationSize::beforeOrAfterPointer(), *DAG, IsAlias);
EXPECT_FALSE(IsValid);
}
@@ -165,14 +161,13 @@ TEST_F(SelectionDAGAddressAnalysisTest,
noAliasingFrameObjects) {
PtrInfo.getWithOffset(Offset0));
SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index1,
PtrInfo.getWithOffset(Offset1));
- std::optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
- std::optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes0 = cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize();
+ TypeSize NumBytes1 = cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store0.getNode(), NumBytes0, Store1.getNode(), NumBytes1, *DAG, IsAlias);
+ Store0.getNode(), LocationSize::precise(NumBytes0), Store1.getNode(),
+ LocationSize::precise(NumBytes1), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_FALSE(IsAlias);
@@ -195,14 +190,13 @@ TEST_F(SelectionDAGAddressAnalysisTest,
unknownSizeFrameObjects) {
DAG->getStore(DAG->getEntryNode(), Loc, Value, FIPtr, PtrInfo);
SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index1,
MachinePointerInfo(PtrInfo.getAddrSpace()));
- std::optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
- std::optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes0 = cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize();
+ TypeSize NumBytes1 = cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store0.getNode(), NumBytes0, Store1.getNode(), NumBytes1, *DAG, IsAlias);
+ Store0.getNode(), LocationSize::precise(NumBytes0), Store1.getNode(),
+ LocationSize::precise(NumBytes1), *DAG, IsAlias);
EXPECT_FALSE(IsValid);
}
@@ -220,20 +214,19 @@ TEST_F(SelectionDAGAddressAnalysisTest,
globalWithFrameObject) {
SDValue Index = DAG->getMemBasePlusOffset(FIPtr, Offset, Loc);
SDValue Store = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index,
PtrInfo.getWithOffset(Offset));
- std::optional<int64_t> NumBytes = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes = cast<StoreSDNode>(Store)->getMemoryVT().getStoreSize();
EVT GTy = DAG->getTargetLoweringInfo().getValueType(DAG->getDataLayout(),
G->getType());
SDValue GValue = DAG->getConstant(0, Loc, GTy);
SDValue GAddr = DAG->getGlobalAddress(G, Loc, GTy);
SDValue GStore = DAG->getStore(DAG->getEntryNode(), Loc, GValue, GAddr,
MachinePointerInfo(G, 0));
- std::optional<int64_t> GNumBytes = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(GStore)->getMemoryVT().getStoreSize());
+ TypeSize GNumBytes = cast<StoreSDNode>(GStore)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store.getNode(), NumBytes, GStore.getNode(), GNumBytes, *DAG, IsAlias);
+ Store.getNode(), LocationSize::precise(NumBytes), GStore.getNode(),
+ LocationSize::precise(GNumBytes), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_FALSE(IsAlias);
@@ -248,8 +241,7 @@ TEST_F(SelectionDAGAddressAnalysisTest,
globalWithAliasedGlobal) {
SDValue GAddr = DAG->getGlobalAddress(G, Loc, GTy);
SDValue GStore = DAG->getStore(DAG->getEntryNode(), Loc, GValue, GAddr,
MachinePointerInfo(G, 0));
- std::optional<int64_t> GNumBytes = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(GStore)->getMemoryVT().getStoreSize());
+ TypeSize GNumBytes = cast<StoreSDNode>(GStore)->getMemoryVT().getStoreSize();
SDValue AliasedGValue = DAG->getConstant(1, Loc, GTy);
SDValue AliasedGAddr = DAG->getGlobalAddress(AliasedG, Loc, GTy);
@@ -258,9 +250,9 @@ TEST_F(SelectionDAGAddressAnalysisTest,
globalWithAliasedGlobal) {
MachinePointerInfo(AliasedG, 0));
bool IsAlias;
- bool IsValid = BaseIndexOffset::computeAliasing(GStore.getNode(), GNumBytes,
- AliasedGStore.getNode(),
- GNumBytes, *DAG, IsAlias);
+ bool IsValid = BaseIndexOffset::computeAliasing(
+ GStore.getNode(), LocationSize::precise(GNumBytes),
+ AliasedGStore.getNode(), LocationSize::precise(GNumBytes), *DAG,
IsAlias);
// With some deeper analysis we could detect if G and AliasedG is aliasing or
// not. But computeAliasing is currently defensive and assumes that a
@@ -290,19 +282,19 @@ TEST_F(SelectionDAGAddressAnalysisTest,
fixedSizeFrameObjectsWithinDiff) {
PtrInfo.getWithOffset(Offset0));
SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value1, Index1,
PtrInfo.getWithOffset(Offset1));
- std::optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
- std::optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes0 = cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize();
+ TypeSize NumBytes1 = cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store0.getNode(), NumBytes0, Store1.getNode(), NumBytes1, *DAG, IsAlias);
+ Store0.getNode(), LocationSize::precise(NumBytes0), Store1.getNode(),
+ LocationSize::precise(NumBytes1), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_FALSE(IsAlias);
IsValid = BaseIndexOffset::computeAliasing(
- Store1.getNode(), NumBytes1, Store0.getNode(), NumBytes0, *DAG, IsAlias);
+ Store1.getNode(), LocationSize::precise(NumBytes1), Store0.getNode(),
+ LocationSize::precise(NumBytes0), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_FALSE(IsAlias);
}
@@ -331,14 +323,13 @@ TEST_F(SelectionDAGAddressAnalysisTest,
fixedSizeFrameObjectsOutOfDiff) {
PtrInfo.getWithOffset(Offset0));
SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value1, Index1,
PtrInfo.getWithOffset(Offset1));
- std::optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
- std::optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes0 = cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize();
+ TypeSize NumBytes1 = cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store0.getNode(), NumBytes0, Store1.getNode(), NumBytes1, *DAG, IsAlias);
+ Store0.getNode(), LocationSize::precise(NumBytes0), Store1.getNode(),
+ LocationSize::precise(NumBytes1), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_TRUE(IsAlias);
}
@@ -365,14 +356,13 @@ TEST_F(SelectionDAGAddressAnalysisTest,
twoFixedStackObjects) {
PtrInfo0.getWithOffset(Offset0));
SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value1, Index1,
PtrInfo1.getWithOffset(Offset0));
- std::optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
- std::optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
- cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize());
+ TypeSize NumBytes0 = cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize();
+ TypeSize NumBytes1 = cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize();
bool IsAlias;
bool IsValid = BaseIndexOffset::computeAliasing(
- Store0.getNode(), NumBytes0, Store1.getNode(), NumBytes1, *DAG, IsAlias);
+ Store0.getNode(), LocationSize::precise(NumBytes0), Store1.getNode(),
+ LocationSize::precise(NumBytes1), *DAG, IsAlias);
EXPECT_TRUE(IsValid);
EXPECT_FALSE(IsAlias);
}
>From 03aed0842f86c8f829512269699009e5348a2737 Mon Sep 17 00:00:00 2001
From: Luke Lau <l...@igalia.com>
Date: Mon, 4 Mar 2024 15:35:57 +0800
Subject: [PATCH 2/2] [RISCV] Add test for aliasing miscompile fixed by #83017.
NFC
Previously we incorrectly removed the scalar load store pair here assuming it
was dead, when it actually aliased with the memset. This showed up as a
miscompile on SPEC CPU 2017 when compiling with -mrvv-vector-bits, and was only
triggered by the changes in #75531. This was fixed in #83017, but this patch
adds a test case for this specific miscompile.
For reference, the incorrect codegen was:
vsetvli a1, zero, e8, m4, ta, ma
vmv.v.i v8, 0
vs4r.v v8, (a0)
addi a1, a0, 80
vsetivli zero, 16, e8, m1, ta, ma
vmv.v.i v8, 0
vs1r.v v8, (a1)
addi a0, a0, 64
vs1r.v v8, (a0)
(cherry picked from commit 63725ab1196ac50509ad382fc12c56f6d8b5d874)
---
llvm/test/CodeGen/RISCV/rvv/pr83017.ll | 50 ++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 llvm/test/CodeGen/RISCV/rvv/pr83017.ll
diff --git a/llvm/test/CodeGen/RISCV/rvv/pr83017.ll
b/llvm/test/CodeGen/RISCV/rvv/pr83017.ll
new file mode 100644
index 00000000000000..3719a2ad994d6f
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/rvv/pr83017.ll
@@ -0,0 +1,50 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
UTC_ARGS: --version 4
+; RUN: llc < %s -mtriple=riscv64 -mattr=+v -riscv-v-vector-bits-max=128
-verify-machineinstrs | FileCheck %s
+
+; This showcases a miscompile that was fixed in #83107:
+; - The memset will be type-legalized to a 512 bit store + 2 x 128 bit stores.
+; - the load and store of q aliases the upper 128 bits store of p.
+; - The aliasing 128 bit store will be between the chain of the scalar
+; load/store:
+;
+; t54: ch = store<(store (s512) into %ir.p, align 1)> t0, ...
+; t51: ch = store<(store (s128) into %ir.p + 64, align 1)> t0, ...
+;
+; t44: i64,ch = load<(load (s32) from %ir.q), sext from i32> t0, ...
+; t50: ch = store<(store (s128) into %ir.p + 80, align 1)> t44:1, ...
+; t46: ch = store<(store (s32) into %ir.q), trunc to i32> t50, ...
+;
+; Previously, the scalar load/store was incorrectly combined away:
+;
+; t54: ch = store<(store (s512) into %ir.p, align 1)> t0, ...
+; t51: ch = store<(store (s128) into %ir.p + 64, align 1)> t0, ...
+;
+; // MISSING
+; t50: ch = store<(store (s128) into %ir.p + 80, align 1)> t44:1, ...
+; // MISSING
+;
+; - We need to compile with an exact VLEN so that we select an ISD::STORE node
+; which triggers the combine
+; - The miscompile doesn't happen if we use separate GEPs as we need the stores
+; to share the same MachinePointerInfo
+define void @aliasing(ptr %p) {
+; CHECK-LABEL: aliasing:
+; CHECK: # %bb.0:
+; CHECK-NEXT: lw a1, 84(a0)
+; CHECK-NEXT: addi a2, a0, 80
+; CHECK-NEXT: vsetivli zero, 16, e8, m1, ta, ma
+; CHECK-NEXT: vmv.v.i v8, 0
+; CHECK-NEXT: vs1r.v v8, (a2)
+; CHECK-NEXT: vsetvli a2, zero, e8, m4, ta, ma
+; CHECK-NEXT: vmv.v.i v12, 0
+; CHECK-NEXT: vs4r.v v12, (a0)
+; CHECK-NEXT: addi a2, a0, 64
+; CHECK-NEXT: vs1r.v v8, (a2)
+; CHECK-NEXT: sw a1, 84(a0)
+; CHECK-NEXT: ret
+ %q = getelementptr inbounds i8, ptr %p, i64 84
+ %tmp = load i32, ptr %q
+ tail call void @llvm.memset.p0.i64(ptr %p, i8 0, i64 96, i1 false)
+ store i32 %tmp, ptr %q
+ ret void
+}
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
