Date: Wed, 29 Jul 2015 13:07:19 +0000 (UTC) From: Dimitry Andric <dim@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r286008 - stable/10/contrib/llvm/patches Message-ID: <201507291307.t6TD7Ju7039251@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: dim Date: Wed Jul 29 13:07:18 2015 New Revision: 286008 URL: https://svnweb.freebsd.org/changeset/base/286008 Log: Add llvm patch corresponding to r286007. Added: stable/10/contrib/llvm/patches/patch-r286007-llvm-r219009-x86-codegen-crash.diff Added: stable/10/contrib/llvm/patches/patch-r286007-llvm-r219009-x86-codegen-crash.diff ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ stable/10/contrib/llvm/patches/patch-r286007-llvm-r219009-x86-codegen-crash.diff Wed Jul 29 13:07:18 2015 (r286008) @@ -0,0 +1,214 @@ +Pull in r219009 from upstream llvm trunk (by Adam Nemet): + + [ISel] Keep matching state consistent when folding during X86 address match + + In the X86 backend, matching an address is initiated by the 'addr' complex + pattern and its friends. During this process we may reassociate and-of-shift + into shift-of-and (FoldMaskedShiftToScaledMask) to allow folding of the + shift into the scale of the address. + + However as demonstrated by the testcase, this can trigger CSE of not only the + shift and the AND which the code is prepared for but also the underlying load + node. In the testcase this node is sitting in the RecordedNode and MatchScope + data structures of the matcher and becomes a deleted node upon CSE. Returning + from the complex pattern function, we try to access it again hitting an assert + because the node is no longer a load even though this was checked before. + + Now obviously changing the DAG this late is bending the rules but I think it + makes sense somewhat. Outside of addresses we prefer and-of-shift because it + may lead to smaller immediates (FoldMaskAndShiftToScale is an even better + example because it create a non-canonical node). We currently don't recognize + addresses during DAGCombiner where arguably this canonicalization should be + performed. On the other hand, having this in the matcher allows us to cover + all the cases where an address can be used in an instruction. + + I've also talked a little bit to Dan Gohman on llvm-dev who added the RAUW for + the new shift node in FoldMaskedShiftToScaledMask. This RAUW is responsible + for initiating the recursive CSE on users + (http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/076903.html) but it + is not strictly necessary since the shift is hooked into the visited user. Of + course it's safer to keep the DAG consistent at all times (e.g. for accurate + number of uses, etc.). + + So rather than changing the fundamentals, I've decided to continue along the + previous patches and detect the CSE. This patch installs a very targeted + DAGUpdateListener for the duration of a complex-pattern match and updates the + matching state accordingly. (Previous patches used HandleSDNode to detect the + CSE but that's not practical here). The listener is only installed on X86. + + I tested that there is no measurable overhead due to this while running + through the spec2k BC files with llc. The only thing we pay for is the + creation of the listener. The callback never ever triggers in spec2k since + this is a corner case. + + Fixes rdar://problem/18206171 + +This fixes a possible crash in x86 code generation when compiling recent +llvm/clang trunk sources. + +Introduced here: http://svnweb.freebsd.org/changeset/base/286007 + +Index: include/llvm/CodeGen/SelectionDAGISel.h +=================================================================== +--- include/llvm/CodeGen/SelectionDAGISel.h ++++ include/llvm/CodeGen/SelectionDAGISel.h +@@ -238,6 +238,12 @@ class SelectionDAGISel : public MachineFunctionPas + const unsigned char *MatcherTable, + unsigned TableSize); + ++ /// \brief Return true if complex patterns for this target can mutate the ++ /// DAG. ++ virtual bool ComplexPatternFuncMutatesDAG() const { ++ return false; ++ } ++ + private: + + // Calls to these functions are generated by tblgen. +Index: lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +=================================================================== +--- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp ++++ lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +@@ -2345,6 +2345,42 @@ struct MatchScope { + bool HasChainNodesMatched, HasGlueResultNodesMatched; + }; + ++/// \\brief A DAG update listener to keep the matching state ++/// (i.e. RecordedNodes and MatchScope) uptodate if the target is allowed to ++/// change the DAG while matching. X86 addressing mode matcher is an example ++/// for this. ++class MatchStateUpdater : public SelectionDAG::DAGUpdateListener ++{ ++ SmallVectorImpl<std::pair<SDValue, SDNode*> > &RecordedNodes; ++ SmallVectorImpl<MatchScope> &MatchScopes; ++public: ++ MatchStateUpdater(SelectionDAG &DAG, ++ SmallVectorImpl<std::pair<SDValue, SDNode*> > &RN, ++ SmallVectorImpl<MatchScope> &MS) : ++ SelectionDAG::DAGUpdateListener(DAG), ++ RecordedNodes(RN), MatchScopes(MS) { } ++ ++ void NodeDeleted(SDNode *N, SDNode *E) { ++ // Some early-returns here to avoid the search if we deleted the node or ++ // if the update comes from MorphNodeTo (MorphNodeTo is the last thing we ++ // do, so it's unnecessary to update matching state at that point). ++ // Neither of these can occur currently because we only install this ++ // update listener during matching a complex patterns. ++ if (!E || E->isMachineOpcode()) ++ return; ++ // Performing linear search here does not matter because we almost never ++ // run this code. You'd have to have a CSE during complex pattern ++ // matching. ++ for (auto &I : RecordedNodes) ++ if (I.first.getNode() == N) ++ I.first.setNode(E); ++ ++ for (auto &I : MatchScopes) ++ for (auto &J : I.NodeStack) ++ if (J.getNode() == N) ++ J.setNode(E); ++ } ++}; + } + + SDNode *SelectionDAGISel:: +@@ -2599,6 +2635,14 @@ SelectCodeCommon(SDNode *NodeToMatch, const unsign + unsigned CPNum = MatcherTable[MatcherIndex++]; + unsigned RecNo = MatcherTable[MatcherIndex++]; + assert(RecNo < RecordedNodes.size() && "Invalid CheckComplexPat"); ++ ++ // If target can modify DAG during matching, keep the matching state ++ // consistent. ++ std::unique_ptr<MatchStateUpdater> MSU; ++ if (ComplexPatternFuncMutatesDAG()) ++ MSU.reset(new MatchStateUpdater(*CurDAG, RecordedNodes, ++ MatchScopes)); ++ + if (!CheckComplexPattern(NodeToMatch, RecordedNodes[RecNo].second, + RecordedNodes[RecNo].first, CPNum, + RecordedNodes)) +Index: lib/Target/X86/X86ISelDAGToDAG.cpp +=================================================================== +--- lib/Target/X86/X86ISelDAGToDAG.cpp ++++ lib/Target/X86/X86ISelDAGToDAG.cpp +@@ -290,6 +290,13 @@ namespace { + const X86InstrInfo *getInstrInfo() const { + return getTargetMachine().getInstrInfo(); + } ++ ++ /// \brief Address-mode matching performs shift-of-and to and-of-shift ++ /// reassociation in order to expose more scaled addressing ++ /// opportunities. ++ bool ComplexPatternFuncMutatesDAG() const override { ++ return true; ++ } + }; + } + +Index: test/CodeGen/X86/addr-mode-matcher.ll +=================================================================== +--- test/CodeGen/X86/addr-mode-matcher.ll ++++ test/CodeGen/X86/addr-mode-matcher.ll +@@ -0,0 +1,62 @@ ++; RUN: llc < %s | FileCheck %s ++ ++; This testcase used to hit an assert during ISel. For details, see the big ++; comment inside the function. ++ ++; CHECK-LABEL: foo: ++; The AND should be turned into a subreg access. ++; CHECK-NOT: and ++; The shift (leal) should be folded into the scale of the address in the load. ++; CHECK-NOT: leal ++; CHECK: movl {{.*}},4), ++ ++target datalayout = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128" ++target triple = "i386-apple-macosx10.6.0" ++ ++define void @foo(i32 %a) { ++bb: ++ br label %bb1692 ++ ++bb1692: ++ %tmp1694 = phi i32 [ 0, %bb ], [ %tmp1745, %bb1692 ] ++ %xor = xor i32 0, %tmp1694 ++ ++; %load1 = (load (and (shl %xor, 2), 1020)) ++ %tmp1701 = shl i32 %xor, 2 ++ %tmp1702 = and i32 %tmp1701, 1020 ++ %tmp1703 = getelementptr inbounds [1028 x i8]* null, i32 0, i32 %tmp1702 ++ %tmp1704 = bitcast i8* %tmp1703 to i32* ++ %load1 = load i32* %tmp1704, align 4 ++ ++; %load2 = (load (shl (and %xor, 255), 2)) ++ %tmp1698 = and i32 %xor, 255 ++ %tmp1706 = shl i32 %tmp1698, 2 ++ %tmp1707 = getelementptr inbounds [1028 x i8]* null, i32 0, i32 %tmp1706 ++ %tmp1708 = bitcast i8* %tmp1707 to i32* ++ %load2 = load i32* %tmp1708, align 4 ++ ++ %tmp1710 = or i32 %load2, %a ++ ++; While matching xor we address-match %load1. The and-of-shift reassocication ++; in address matching transform this into into a shift-of-and and the resuting ++; node becomes identical to %load2. CSE replaces %load1 which leaves its ++; references in MatchScope and RecordedNodes stale. ++ %tmp1711 = xor i32 %load1, %tmp1710 ++ ++ %tmp1744 = getelementptr inbounds [256 x i32]* null, i32 0, i32 %tmp1711 ++ store i32 0, i32* %tmp1744, align 4 ++ %tmp1745 = add i32 %tmp1694, 1 ++ indirectbr i8* undef, [label %bb1756, label %bb1692] ++ ++bb1756: ++ br label %bb2705 ++ ++bb2705: ++ indirectbr i8* undef, [label %bb5721, label %bb5736] ++ ++bb5721: ++ br label %bb2705 ++ ++bb5736: ++ ret void ++}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201507291307.t6TD7Ju7039251>