diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp --- 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 @@ -143,13 +144,31 @@ bool IsCV0 = isa(BasePtr0.getBase()); bool IsCV1 = isa(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( + cast(BasePtr0.getBase())->getGlobal()) || + isa( + cast(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 --- /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 + +@foo = global i16 0, align 1 +@aliasFoo = alias i16, i16 * @foo +@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 --- a/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp +++ b/llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp @@ -30,6 +30,7 @@ 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 @@ 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 @@ std::unique_ptr M; Function *F; GlobalVariable *G; + GlobalAlias *AliasedG; std::unique_ptr MF; std::unique_ptr DAG; }; @@ -209,6 +214,35 @@ 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 GNumBytes = MemoryLocation::getSizeOrUnknown( + cast(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);