Index: lib/Analysis/MemorySSA.cpp =================================================================== --- lib/Analysis/MemorySSA.cpp +++ lib/Analysis/MemorySSA.cpp @@ -1778,7 +1778,6 @@ verifyOrdering(F); verifyDominationNumbers(F); Walker->verify(this); - verifyClobberSanity(F); } /// Check sanity of the clobbering instruction for access MA. @@ -1797,6 +1796,16 @@ } void MemorySSA::verifyClobberSanity(const Function &F) const { +// The original intention of this method is verify that the clobberingAccess +// cached by MemorySSA is the same as the clobberingAccess found at a later +// query to AA. This does not hold true in general due to the current fragility +// of BasicAA which has arbitrary caps on the things it analyzes before giving +// up. As a result, transformations that are correct, will lead to BasicAA +// returning different Alias answers before and after that transformation. +// Invalidating MemorySSA is not an option, as the results in BasicAA can be so +// random, in the worst case we'd need to rebuild MemorySSA from scratch after +// every transformation, which defeats the purpose of using it. For such an +// example, see test4 added in D51960. #if !defined(NDEBUG) && defined(EXPENSIVE_CHECKS) for (const BasicBlock &BB : F) { const AccessList *Accesses = getBlockAccesses(&BB); Index: test/Analysis/MemorySSA/pr40509.ll =================================================================== --- /dev/null +++ test/Analysis/MemorySSA/pr40509.ll @@ -0,0 +1,54 @@ +; REQUIRES: asserts +; RUN: opt -mtriple=systemz-unknown -march=z13 -O3 -enable-mssa-loop-dependency -disable-output %s + +; During transform to LCSSA, an access becomes obfuscated to: +; (2 = phi (phi(val), val)), which BasicAA fails to analyze. +; It's currently hard coded in BasicAA to return MayAlias for nested phis. +; This leads MemorySSA to finding a new (false) clobber for a previously +; optimized access. With verifyClobber included in verifyMemorySSA, such a +; transformation will cause MemorySSA verification to fail. +; If the verifyClobber is re-enabled, this test will crash. + +target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64" +target triple = "s390x-ibm-linux" + +%0 = type <{ i64, i8, i64, i16 }> + +@g_54 = external dso_local global i16, align 2 +@g_101 = external dso_local global <{ i64, i8, i64, i8, i8 }>, align 2 + +declare dso_local void @safe_lshift_func_int16_t_s_s() +declare dso_local i8 @safe_div_func_int8_t_s_s() + +define dso_local void @func_47(%0* %arg) { +bb: + %tmp = alloca i32, align 4 + br label %bb1 + +bb1: ; preds = %bb12, %bb + %tmp2 = getelementptr inbounds %0, %0* %arg, i32 0, i32 3 + store i16 undef, i16* %tmp2, align 1 + %tmp3 = call signext i8 @safe_div_func_int8_t_s_s() + %tmp7 = icmp ne i8 %tmp3, 0 + br i1 %tmp7, label %bb8, label %bb10 + +bb8: ; preds = %bb1 + %tmp9 = icmp eq i32 0, 0 + br i1 %tmp9, label %bb12, label %bb13 + +bb10: ; preds = %bb10, %bb1 + call void @safe_lshift_func_int16_t_s_s() + %tmp11 = getelementptr inbounds %0, %0* %arg, i32 0, i32 3 + store i16 0, i16* %tmp11, align 1 + store i8 0, i8* getelementptr inbounds (%0, %0* bitcast (<{ i64, i8, i64, i8, i8 }>* @g_101 to %0*), i32 0, i32 1), align 2 + br label %bb10 + +bb12: ; preds = %bb8 + store i16 0, i16* @g_54, align 2 + br label %bb1 + +bb13: ; preds = %bb8 + ret void +} + +