[llvm] 1896fb2 - [SelectionDAG] Assume that a GlobalAlias may alias other global values
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 5 03:17:01 PDT 2021
Author: Bjorn Pettersson
Date: 2021-10-05T12:15:55+02:00
New Revision: 1896fb2cfffcf120eb28cefb67ac3d56035adc43
URL: https://github.com/llvm/llvm-project/commit/1896fb2cfffcf120eb28cefb67ac3d56035adc43
DIFF: https://github.com/llvm/llvm-project/commit/1896fb2cfffcf120eb28cefb67ac3d56035adc43.diff
LOG: [SelectionDAG] Assume that a GlobalAlias may alias other global values
This fixes a bug detected in DAGCombiner when using global alias
variables. Here is an example:
@foo = global i16 0, align 1
@aliasFoo = alias i16, i16 * @foo
define i16 @bar() {
...
store i16 7, i16 * @foo, align 1
store i16 8, i16 * @aliasFoo, align 1
...
}
BaseIndexOffset::computeAliasing would incorrectly derive NoAlias
for the two accesses in the example above, resulting in DAGCombiner
miscompiles.
This patch fixes the problem by a defensive approach letting
BaseIndexOffset::computeAliasing return false, i.e. that the aliasing
couldn't be determined, when comparing two global values and at least
one is a GlobalAlias. In the future we might improve this with a
deeper analysis to look at the aliasee for the GlobalAlias etc. But
that is a bit more complicated considering that we could have
'local_unnamed_addr' and situations with several 'alias' variables.
Fixes PR51878.
Differential Revision: https://reviews.llvm.org/D110064
Added:
llvm/test/CodeGen/X86/pr51878_computeAliasing.ll
Modified:
llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
index 20c7d771bfb61..2f9a93c56d6ba 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
@@ -14,6 +14,7 @@
#include "llvm/CodeGen/SelectionDAG.h"
#include "llvm/CodeGen/SelectionDAGNodes.h"
#include "llvm/CodeGen/TargetLowering.h"
+#include "llvm/IR/GlobalAlias.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Debug.h"
#include <cstdint>
@@ -143,13 +144,31 @@ bool BaseIndexOffset::computeAliasing(const SDNode *Op0,
bool IsCV0 = isa<ConstantPoolSDNode>(BasePtr0.getBase());
bool IsCV1 = isa<ConstantPoolSDNode>(BasePtr1.getBase());
- // If of mismatched base types or checkable indices we can check
- // they do not alias.
- if ((BasePtr0.getIndex() == BasePtr1.getIndex() || (IsFI0 != IsFI1) ||
- (IsGV0 != IsGV1) || (IsCV0 != IsCV1)) &&
- (IsFI0 || IsGV0 || IsCV0) && (IsFI1 || IsGV1 || IsCV1)) {
- IsAlias = false;
- return true;
+ if ((IsFI0 || IsGV0 || IsCV0) && (IsFI1 || IsGV1 || IsCV1)) {
+ // We can derive NoAlias In case of mismatched base types.
+ if (IsFI0 != IsFI1 || IsGV0 != IsGV1 || IsCV0 != IsCV1) {
+ IsAlias = false;
+ return true;
+ }
+ // We cannot safely determine whether the pointers alias if we compare two
+ // global values and at least one is a GlobalAlias.
+ if (IsGV0 && IsGV1 &&
+ (isa<GlobalAlias>(
+ cast<GlobalAddressSDNode>(BasePtr0.getBase())->getGlobal()) ||
+ isa<GlobalAlias>(
+ cast<GlobalAddressSDNode>(BasePtr1.getBase())->getGlobal())))
+ return false;
+ // If checkable indices we can check they do not alias.
+ // FIXME: Please describe this a bit better. Looks weird to say that there
+ // is no alias if the indices are the same. Is this code assuming that
+ // someone has checked that the base isn't the same as a precondition? And
+ // what about offsets? And what about NumBytes0 and NumBytest1 (can we
+ // really derive NoAlias here if we do not even know how many bytes that are
+ // dereferenced)?
+ if (BasePtr0.getIndex() == BasePtr1.getIndex()) {
+ IsAlias = false;
+ return true;
+ }
}
return false; // Cannot determine whether the pointers alias.
}
diff --git a/llvm/test/CodeGen/X86/pr51878_computeAliasing.ll b/llvm/test/CodeGen/X86/pr51878_computeAliasing.ll
new file mode 100644
index 0000000000000..4ad1bb2ec4927
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr51878_computeAliasing.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O1 -mtriple i686-unknown-linux-gnu -o - %s | FileCheck %s
+
+ at foo = global i16 0, align 1
+ at aliasFoo = alias i16, i16 * @foo
+ at bar = global i16 0, align 1
+
+; This used to miscompile due to not realizing that the store to @aliasFoo
+; clobbered @foo (see PR51878).
+;
+; With some improvements to codegen it should be possible to detect that @foo
+; and @aliasFoo are aliases, and that @aliasFoo isn't aliasing with @bar. So
+; ideally we would end up with three movw instructions here. Running opt
+; before llc on this test case would take care of that, but llc is not smart
+; enough to deduce that itself yet.
+define i16 @main() {
+; CHECK-LABEL: main:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: movw $1, foo
+; CHECK-NEXT: movw $2, bar
+; CHECK-NEXT: movw $4, aliasFoo
+; CHECK-NEXT: movzwl foo, %eax
+; CHECK-NEXT: addw bar, %ax
+; CHECK-NEXT: retl
+entry:
+ store i16 1, i16 * @foo
+ store i16 2, i16 * @bar
+ store i16 4, i16 * @aliasFoo
+ %foo = load i16, i16 * @foo
+ %bar = load i16, i16 * @bar
+ %res = add i16 %foo, %bar
+ ret i16 %res
+}
diff --git a/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp b/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
index b7c519971589a..28485a34d9e49 100644
--- a/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
+++ b/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
@@ -30,6 +30,7 @@ class SelectionDAGAddressAnalysisTest : public testing::Test {
void SetUp() override {
StringRef Assembly = "@g = global i32 0\n"
+ "@g_alias = alias i32, i32* @g\n"
"define i32 @f() {\n"
" %1 = load i32, i32* @g\n"
" ret i32 %1\n"
@@ -63,6 +64,9 @@ class SelectionDAGAddressAnalysisTest : public testing::Test {
G = M->getGlobalVariable("g");
if (!G)
report_fatal_error("G?");
+ AliasedG = M->getNamedAlias("g_alias");
+ if (!AliasedG)
+ report_fatal_error("AliasedG?");
MachineModuleInfo MMI(TM.get());
@@ -89,6 +93,7 @@ class SelectionDAGAddressAnalysisTest : public testing::Test {
std::unique_ptr<Module> M;
Function *F;
GlobalVariable *G;
+ GlobalAlias *AliasedG;
std::unique_ptr<MachineFunction> MF;
std::unique_ptr<SelectionDAG> DAG;
};
@@ -209,6 +214,35 @@ TEST_F(SelectionDAGAddressAnalysisTest, globalWithFrameObject) {
EXPECT_FALSE(IsAlias);
}
+TEST_F(SelectionDAGAddressAnalysisTest, globalWithAliasedGlobal) {
+ SDLoc Loc;
+
+ 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));
+ Optional<int64_t> GNumBytes = MemoryLocation::getSizeOrUnknown(
+ cast<StoreSDNode>(GStore)->getMemoryVT().getStoreSize());
+
+ SDValue AliasedGValue = DAG->getConstant(1, Loc, GTy);
+ SDValue AliasedGAddr = DAG->getGlobalAddress(AliasedG, Loc, GTy);
+ SDValue AliasedGStore =
+ DAG->getStore(DAG->getEntryNode(), Loc, AliasedGValue, AliasedGAddr,
+ MachinePointerInfo(AliasedG, 0));
+
+ bool IsAlias;
+ bool IsValid = BaseIndexOffset::computeAliasing(GStore.getNode(), GNumBytes,
+ AliasedGStore.getNode(),
+ 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
+ // GlobalAlias might alias with any global variable.
+ EXPECT_FALSE(IsValid);
+}
+
TEST_F(SelectionDAGAddressAnalysisTest, fixedSizeFrameObjectsWithinDiff) {
SDLoc Loc;
auto Int8VT = EVT::getIntegerVT(Context, 8);
More information about the llvm-commits
mailing list