<html><body><p><font size="2">Hi Mikael,</font><br><br><font size="2">Could you please try your fix in the target with the problem and submit a patch?</font><br><font size="2"><br>-----<br>Hiroshi Inoue <inouehrs@jp.ibm.com><br>IBM Research - Tokyo<br></font><br><br><tt><font size="2">Mikael Holmén <mikael.holmen@ericsson.com> wrote on 2017/10/05 16:37:46:<br><br>> From: Mikael Holmén <mikael.holmen@ericsson.com></font></tt><br><tt><font size="2">> To: Hiroshi 7 Inoue <INOUEHRS@jp.ibm.com></font></tt><br><tt><font size="2">> Cc: Hal Finkel <hfinkel@anl.gov>, <llvm-commits@lists.llvm.org>, <br>> Mattias Eriksson V <mattias.v.eriksson@ericsson.com>, Matthias Braun<br>> <matze@braunis.de></font></tt><br><tt><font size="2">> Date: 2017/10/05 16:38</font></tt><br><tt><font size="2">> Subject: Re: [llvm] r309651 - [StackColoring] Update AliasAnalysis <br>> information in stack coloring pass</font></tt><br><tt><font size="2">> <br>> Hi,<br>> <br>> On 10/03/2017 05:47 PM, Hiroshi 7 Inoue wrote:<br>> > Hi Mikael, Mattias,<br>> > <br>> > It looks like a bug in my patch. I do not intend to change that behavior.<br>> > Maybe we need `return false` depending on the results of <br>> > getUnderlyingObjectsForCodeGen.<br>> <br>> Alright. Will you fix?<br>> <br>> Perhaps<br>> <br>> if (Objs.empty())<br>> return false;<br>> <br>> after the getUnderlyingObjectsForCodeGen() call in <br>> getUnderlyingObjectsForInstr() in ScheduleDAGInstrs.cpp would do the trick?<br>> <br>> Regards,<br>> Mikael<br>> <br>> > <br>> > I am sorry for bothering you.<br>> > <br>> > -----<br>> > Hiroshi Inoue <inouehrs@jp.ibm.com><br>> > IBM Research - Tokyo<br>> > <br>> > <br>> > Mikael Holmén <mikael.holmen@ericsson.com> wrote on 2017/10/03 14:22:19:<br>> > <br>> > > From: Mikael Holmén <mikael.holmen@ericsson.com><br>> > > To: Hiroshi 7 Inoue <INOUEHRS@jp.ibm.com><br>> > > Cc: Mattias Eriksson V <mattias.v.eriksson@ericsson.com>, <llvm-<br>> > > commits@lists.llvm.org>, Hal Finkel <hfinkel@anl.gov>, Matthias<br>> > > Braun <matze@braunis.de><br>> > > Date: 2017/10/03 14:22<br>> > > Subject: Re: [llvm] r309651 - [StackColoring] Update AliasAnalysis<br>> > > information in stack coloring pass<br>> > ><br>> > > Hi Hiroshi,<br>> > ><br>> > > On 10/02/2017 06:37 PM, Mattias Eriksson V wrote:<br>> > > > On 10/02/2017 06:20 PM, Hiroshi 7 Inoue wrote:<br>> > > >> Hi Mikael,<br>> > > >><br>> > > >> Thank you for sharing your problem. I will investigate it.<br>> > > >> Do you have this problem with the latest code base? I have updated my<br>> > > >> first patch (r309651) in <a href="https://urldefense.proofpoint.com/v2/">https://urldefense.proofpoint.com/v2/</a><br>> > > url?u=https-3A__reviews.llvm.org_rL309849&d=DwIF-g&c=jf_iaSHvJObTbx-<br>> > > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > <br>> > <br>> YkVI&m=LpTapfEiXCfaCXpeIggFCkGTbgEzp0_1t3xivK5Dkqg&s=FNJP45aHFjcZvjKgUenRJIUj7zz4zJUeAG5kNqCAsTE&e=<br>> > > .<br>> > > ><br>> > > > Hi, Hiroshi!<br>> > > ><br>> > > > Yes, we see the issue also with part 2 applied.<br>> > > ><br>> > ><br>> > > Yes as Mattias said part 2 suffers from the same problem since it also<br>> > > happily continues with memop #2 even after memop #1 was found to be<br>> > > non-identified, and fails to inform buildSchedGraph about it.<br>> > ><br>> > > Regards,<br>> > > Mikael<br>> > ><br>> > > > --<br>> > > > Mattias<br>> > > ><br>> > > >><br>> > > >> Regards,<br>> > > >> Hiroshi<br>> > > >><br>> > > >> -----<br>> > > >> Hiroshi Inoue <inouehrs@jp.ibm.com><br>> > > >> IBM Research - Tokyo<br>> > > >><br>> > > >><br>> > > >> Mikael Holmén <mikael.holmen@ericsson.com> wrote on 2017/10/02 <br>> > 20:34:01:<br>> > > >><br>> > > >> > From: Mikael Holmén <mikael.holmen@ericsson.com><br>> > > >> > To: Hiroshi Inoue <inouehrs@jp.ibm.com>, <llvm-<br>> > > >> > commits@lists.llvm.org>, "Hal Finkel" <hfinkel@anl.gov>, Matthias<br>> > > >> > Braun <matze@braunis.de><br>> > > >> > Cc: Mattias Eriksson V <mattias.v.eriksson@ericsson.com><br>> > > >> > Date: 2017/10/02 20:35<br>> > > >> > Subject: Re: [llvm] r309651 - [StackColoring] Update AliasAnalysis<br>> > > >> > information in stack coloring pass<br>> > > >> ><br>> > > >> > Hi Hiroshi, Matthias and Hal,<br>> > > >> ><br>> > > >> > (CC my colleague Mattias)<br>> > > >> ><br>> > > >> > I think I've found a problem with this patch.<br>> > > >> ><br>> > > >> > Found it on my out-of-tree target, but I'll try to describe what's<br>> > > >> > happening.<br>> > > >> ><br>> > > >> > The problem is during scheduling where we fail to identify a<br>> > > >> dependency<br>> > > >> > between a store and a following load. The store _might_ store <br>> > at the<br>> > > >> > same address as the load loads from, but it doesn't always do <br>> > it. It<br>> > > >> > depends on earlier code flow.<br>> > > >> ><br>> > > >> > The store looks like:<br>> > > >> ><br>> > > >> > mv_a32_r16_rmod1 %a0_32, %r1; mem:ST2[%_tmp9] ST2[@g_290]<br>> > > >> ><br>> > > >> > I.e. it has two memops describing the two different memory<br>> > > >> locations it<br>> > > >> > might store to (is this weird?). They memory locations are <br>> > completely<br>> > > >> > unrelated and describe the two locations the pointer %r1 can <br>> > point to.<br>> > > >> ><br>> > > >> > Before this patch, when ScheduleDAGInstrs::buildSchedGraph called<br>> > > >> ><br>> > > >> > UnderlyingObjectsVector Objs;<br>> > > >> > getUnderlyingObjectsForInstr(&MI, MFI, Objs, <br>> > MF.getDataLayout());<br>> > > >> ><br>> > > >> > it got an empty Objs vector back since getUnderlyingObjectsForInstr<br>> > > >> > found one non-identified object (%_tmp9) and then cleared the <br>> > whole<br>> > > >> vector.<br>> > > >> ><br>> > > >> > With this patch, I instead get a non-empty Objs, containing only<br>> > > >> @g_290<br>> > > >> > and because of that we don't add a chain dependency towards the <br>> > load<br>> > > >> > following the store, and we get wrong code as a result.<br>> > > >> ><br>> > > >> > I know there is code in the method <br>> > getUnderlyingObjectsForCodeGen that<br>> > > >> > looks for non-identified objects and if found it clears the Objects<br>> > > >> > arrays, but the way it's implemented it doesn't help!<br>> > > >> ><br>> > > >> > In my current case where there are two memops and the first one is<br>> > > >> > non-identified, we still continue with the second memop <br>> > afterwards and<br>> > > >> > add that underlying object to the vector, and leave that to the<br>> > > >> > schedgraph builder where we don't see any traces of any <br>> > non-identified<br>> > > >> > object, and we build a graph with missing dependencies<br>> > > >> ><br>> > > >> > I suppose this difference is not intended?<br>> > > >> ><br>> > > >> > Regards,<br>> > > >> > Mikael<br>> > > >> ><br>> > > >> > On 08/01/2017 05:32 AM, Hiroshi Inoue via llvm-commits wrote:<br>> > > >> > > Author: inouehrs<br>> > > >> > > Date: Mon Jul 31 20:32:15 2017<br>> > > >> > > New Revision: 309651<br>> > > >> > ><br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D309651-26view-3Drev&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=e3Q5lQrWHghbIG_b8_wJ0PPVh5IF218IDKqkTOkGGV0&e=<br>> > > >><br>> > > >> > > Log:<br>> > > >> > > [StackColoring] Update AliasAnalysis information in stack<br>> > > >> coloring pass<br>> > > >> > ><br>> > > >> > > Stack coloring pass need to maintain AliasAnalysis information<br>> > > >> > when merging stack slots of different types.<br>> > > >> > > Actually, there is a FIXME comment in StackColoring.cpp<br>> > > >> > ><br>> > > >> > > // FIXME: In order to enable the use of TBAA when using AA in<br>> > > >> CodeGen,<br>> > > >> > > // we'll also need to update the TBAA nodes in MMOs with values<br>> > > >> > > // derived from the merged allocas.<br>> > > >> > ><br>> > > >> > > But, TBAA has been already enabled in CodeGen without fixing <br>> > this<br>> > > >> pass.<br>> > > >> > > The incorrect TBAA metadata results in recent failures in<br>> > > >> > bootstrap test on ppc64le (PR33928) by allowing unsafe instruction<br>> > > >> scheduling.<br>> > > >> > > Although we observed the problem on ppc64le, this is a platform<br>> > > >> > neutral issue.<br>> > > >> > ><br>> > > >> > > This patch makes the stack coloring pass maintains AliasAnalysis<br>> > > >> > information when merging multiple stack slots.<br>> > > >> > ><br>> > > >> > ><br>> > > >> > > Modified:<br>> > > >> > > llvm/trunk/include/llvm/Analysis/ValueTracking.h<br>> > > >> > > llvm/trunk/include/llvm/CodeGen/MachineFunction.h<br>> > > >> > > llvm/trunk/include/llvm/CodeGen/MachineInstr.h<br>> > > >> > > llvm/trunk/lib/Analysis/ValueTracking.cpp<br>> > > >> > > llvm/trunk/lib/CodeGen/MachineFunction.cpp<br>> > > >> > > llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp<br>> > > >> > > llvm/trunk/lib/CodeGen/StackColoring.cpp<br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/include/llvm/Analysis/ValueTracking.h<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_Analysis_ValueTracking.h-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=KsDAR0RUJBPSTneO3i2jMCIKtjKOSAi1260oq3nZfkA&e=<br>> > > >><br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/include/llvm/Analysis/ValueTracking.h (original)<br>> > > >> > > +++ llvm/trunk/include/llvm/Analysis/ValueTracking.h Mon Jul 31<br>> > > >> > 20:32:15 2017<br>> > > >> > > @@ -312,6 +312,12 @@ template <typename T> class ArrayRef;<br>> > > >> > > const DataLayout &DL, LoopInfo *LI<br>> > > >> = nullptr,<br>> > > >> > > unsigned MaxLookup = 6);<br>> > > >> > ><br>> > > >> > > + /// This is a wrapper around GetUnderlyingObjects and adds<br>> > > >> > support for basic<br>> > > >> > > + /// ptrtoint+arithmetic+inttoptr sequences.<br>> > > >> > > + void getUnderlyingObjectsForCodeGen(const Value *V,<br>> > > >> > > + SmallVectorImpl<Value *> &Objects,<br>> > > >> > > + const DataLayout &DL);<br>> > > >> > > +<br>> > > >> > > /// Return true if the only users of this pointer are<br>> > > >> lifetime markers.<br>> > > >> > > bool onlyUsedByLifetimeMarkers(const Value *V);<br>> > > >> > ><br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/include/llvm/CodeGen/MachineFunction.h<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_CodeGen_MachineFunction.h-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=WJpDpSXhK9W0O2Y7ry9udwfGJezS0aC1GhsuQrPJ2a8&e=<br>> > > >><br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/include/llvm/CodeGen/MachineFunction.h (original)<br>> > > >> > > +++ llvm/trunk/include/llvm/CodeGen/MachineFunction.h Mon Jul 31<br>> > > >> > 20:32:15 2017<br>> > > >> > > @@ -661,6 +661,12 @@ public:<br>> > > >> > > MachineMemOperand *getMachineMemOperand(const<br>> > > >> MachineMemOperand *MMO,<br>> > > >> > > int64_t Offset,<br>> > > >> uint64_t Size);<br>> > > >> > ><br>> > > >> > > + /// Allocate a new MachineMemOperand by copying an <br>> > existing one,<br>> > > >> > > + /// replacing only AliasAnalysis information.<br>> > > >> > MachineMemOperands are owned<br>> > > >> > > + /// by the MachineFunction and need not be explicitly<br>> > > >> deallocated.<br>> > > >> > > + MachineMemOperand *getMachineMemOperand(const<br>> > > >> MachineMemOperand *MMO,<br>> > > >> > > + const AAMDNodes <br>> > &AAInfo);<br>> > > >> > > +<br>> > > >> > > using OperandCapacity = <br>> > ArrayRecycler<MachineOperand>::Capacity;<br>> > > >> > ><br>> > > >> > > /// Allocate an array of MachineOperands. This is only<br>> > > >> > intended for use by<br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/include/llvm/CodeGen/MachineInstr.h<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_CodeGen_MachineInstr.h-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=P1817BTam8xe_gWSipjZpR88qqNAdwIFFUXlfMpsB8I&e=<br>> > > >><br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/include/llvm/CodeGen/MachineInstr.h (original)<br>> > > >> > > +++ llvm/trunk/include/llvm/CodeGen/MachineInstr.h Mon Jul 31<br>> > > >> 20:32:15 2017<br>> > > >> > > @@ -379,6 +379,9 @@ public:<br>> > > >> > > return NumMemRefs == 1;<br>> > > >> > > }<br>> > > >> > ><br>> > > >> > > + /// Return the number of memory operands.<br>> > > >> > > + unsigned getNumMemOperands() const { return NumMemRefs; }<br>> > > >> > > +<br>> > > >> > > /// API for querying MachineInstr properties. They are the<br>> > > >> > same as MCInstrDesc<br>> > > >> > > /// queries but they are bundle aware.<br>> > > >> > ><br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Analysis_ValueTracking.cpp-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=kZRZpGyrwG9zEe89JDudBCkYnFPMzYQafnJFPQaCzCg&e=<br>> > > >><br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)<br>> > > >> > > +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Jul 31 <br>> > 20:32:15<br>> > > >> 2017<br>> > > >> > > @@ -3277,6 +3277,70 @@ void llvm::GetUnderlyingObjects(Value *V<br>> > > >> > > } while (!Worklist.empty());<br>> > > >> > > }<br>> > > >> > ><br>> > > >> > > +/// This is the function that does the work of looking through<br>> > > >> basic<br>> > > >> > > +/// ptrtoint+arithmetic+inttoptr sequences.<br>> > > >> > > +static const Value *getUnderlyingObjectFromInt(const Value *V) {<br>> > > >> > > + do {<br>> > > >> > > + if (const Operator *U = dyn_cast<Operator>(V)) {<br>> > > >> > > + // If we find a ptrtoint, we can transfer control back <br>> > to the<br>> > > >> > > + // regular getUnderlyingObjectFromInt.<br>> > > >> > > + if (U->getOpcode() == Instruction::PtrToInt)<br>> > > >> > > + return U->getOperand(0);<br>> > > >> > > + // If we find an add of a constant, a multipliedvalue, or<br>> > > >> > a phi, it's<br>> > > >> > > + // likely that the other operand will lead us to the base<br>> > > >> > > + // object. We don't have to worry about the casewhere the<br>> > > >> > > + // object address is somehow being computed by the <br>> > multiply,<br>> > > >> > > + // because our callers only care when the result is an<br>> > > >> > > + // identifiable object.<br>> > > >> > > + if (U->getOpcode() != Instruction::Add ||<br>> > > >> > > + (!isa<ConstantInt>(U->getOperand(1)) &&<br>> > > >> > > + Operator::getOpcode(U->getOperand(1)) !=<br>> > > >> Instruction::Mul &&<br>> > > >> > > + !isa<PHINode>(U->getOperand(1))))<br>> > > >> > > + return V;<br>> > > >> > > + V = U->getOperand(0);<br>> > > >> > > + } else {<br>> > > >> > > + return V;<br>> > > >> > > + }<br>> > > >> > > + assert(V->getType()->isIntegerTy() && "Unexpected operand<br>> > > >> type!");<br>> > > >> > > + } while (true);<br>> > > >> > > +}<br>> > > >> > > +<br>> > > >> > > +/// This is a wrapper around GetUnderlyingObjects and adds<br>> > > >> > support for basic<br>> > > >> > > +/// ptrtoint+arithmetic+inttoptr sequences.<br>> > > >> > > +void llvm::getUnderlyingObjectsForCodeGen(const Value *V,<br>> > > >> > > + SmallVectorImpl<Value *> &Objects,<br>> > > >> > > + const DataLayout &DL) {<br>> > > >> > > + SmallPtrSet<const Value *, 16> Visited;<br>> > > >> > > + SmallVector<const Value *, 4> Working(1, V);<br>> > > >> > > + do {<br>> > > >> > > + V = Working.pop_back_val();<br>> > > >> > > +<br>> > > >> > > + SmallVector<Value *, 4> Objs;<br>> > > >> > > + GetUnderlyingObjects(const_cast<Value *>(V), Objs, DL);<br>> > > >> > > +<br>> > > >> > > + for (Value *V : Objs) {<br>> > > >> > > + // If GetUnderlyingObjects fails to find an identifiable<br>> > > >> object,<br>> > > >> > > + // getUnderlyingObjectsForCodeGen also fails for safety.<br>> > > >> > > + if (!isIdentifiedObject(V)) {<br>> > > >> > > + Objects.clear();<br>> > > >> > > + return;<br>> > > >> > > + }<br>> > > >> > > +<br>> > > >> > > + if (!Visited.insert(V).second)<br>> > > >> > > + continue;<br>> > > >> > > + if (Operator::getOpcode(V) == Instruction::IntToPtr) {<br>> > > >> > > + const Value *O =<br>> > > >> > > + <br>> > getUnderlyingObjectFromInt(cast<User>(V)->getOperand(0));<br>> > > >> > > + if (O->getType()->isPointerTy()) {<br>> > > >> > > + Working.push_back(O);<br>> > > >> > > + continue;<br>> > > >> > > + }<br>> > > >> > > + }<br>> > > >> > > + Objects.push_back(const_cast<Value *>(V));<br>> > > >> > > + }<br>> > > >> > > + } while (!Working.empty());<br>> > > >> > > +}<br>> > > >> > > +<br>> > > >> > > /// Return true if the only users of this pointer are lifetime<br>> > > >> markers.<br>> > > >> > > bool llvm::onlyUsedByLifetimeMarkers(const Value *V) {<br>> > > >> > > for (const User *U : V->users()) {<br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/lib/CodeGen/MachineFunction.cpp<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_MachineFunction.cpp-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=allS8M0h249MGa_Gx6zUREWD57UANqNdgqwsYI7vnBc&e=<br>> > > >><br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/lib/CodeGen/MachineFunction.cpp (original)<br>> > > >> > > +++ llvm/trunk/lib/CodeGen/MachineFunction.cpp Mon Jul 31<br>> > > >> 20:32:15 2017<br>> > > >> > > @@ -330,6 +330,20 @@ MachineFunction::getMachineMemOperand(co<br>> > > >> > > MMO->getOrdering(),<br>> > > >> > MMO->getFailureOrdering());<br>> > > >> > > }<br>> > > >> > ><br>> > > >> > > +MachineMemOperand *<br>> > > >> > > +MachineFunction::getMachineMemOperand(const <br>> > MachineMemOperand *MMO,<br>> > > >> > > + const AAMDNodes &AAInfo) {<br>> > > >> > > + MachinePointerInfo MPI = MMO->getValue() ?<br>> > > >> > > + MachinePointerInfo(MMO->getValue(),<br>> > > >> MMO->getOffset()) :<br>> > > >> > > + MachinePointerInfo(MMO->getPseudoValue(),<br>> > > >> MMO->getOffset());<br>> > > >> > > +<br>> > > >> > > + return new (Allocator)<br>> > > >> > > + MachineMemOperand(MPI, MMO->getFlags(),<br>> > > >> MMO->getSize(),<br>> > > >> > > + MMO->getBaseAlignment(), AAInfo,<br>> > > >> > > + MMO->getRanges(),<br>> > > >> MMO->getSyncScopeID(),<br>> > > >> > > + MMO->getOrdering(),<br>> > > >> > MMO->getFailureOrdering());<br>> > > >> > > +}<br>> > > >> > > +<br>> > > >> > > MachineInstr::mmo_iterator<br>> > > >> > > MachineFunction::allocateMemRefsArray(unsigned long Num) {<br>> > > >> > > return Allocator.Allocate<MachineMemOperand *>(Num);<br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_ScheduleDAGInstrs.cpp-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >> <br>> > YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=Ivq088QzLr4wNpGbN-<br>> > > >> > k6QDpkRPbrJE6Z8WYbQgbAfXE&e=<br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp (original)<br>> > > >> > > +++ llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp Mon Jul 31<br>> > > >> 20:32:15 2017<br>> > > >> > > @@ -121,63 +121,6 @@ ScheduleDAGInstrs::ScheduleDAGInstrs(Mac<br>> > > >> > > SchedModel.init(ST.getSchedModel(), &ST, TII);<br>> > > >> > > }<br>> > > >> > ><br>> > > >> > > -/// This is the function that does the work of looking through<br>> > > >> basic<br>> > > >> > > -/// ptrtoint+arithmetic+inttoptr sequences.<br>> > > >> > > -static const Value *getUnderlyingObjectFromInt(const Value *V) {<br>> > > >> > > - do {<br>> > > >> > > - if (const Operator *U = dyn_cast<Operator>(V)) {<br>> > > >> > > - // If we find a ptrtoint, we can transfer control back <br>> > to the<br>> > > >> > > - // regular getUnderlyingObjectFromInt.<br>> > > >> > > - if (U->getOpcode() == Instruction::PtrToInt)<br>> > > >> > > - return U->getOperand(0);<br>> > > >> > > - // If we find an add of a constant, a multipliedvalue, or<br>> > > >> > a phi, it's<br>> > > >> > > - // likely that the other operand will lead us to the base<br>> > > >> > > - // object. We don't have to worry about the casewhere the<br>> > > >> > > - // object address is somehow being computed by the <br>> > multiply,<br>> > > >> > > - // because our callers only care when the result is an<br>> > > >> > > - // identifiable object.<br>> > > >> > > - if (U->getOpcode() != Instruction::Add ||<br>> > > >> > > - (!isa<ConstantInt>(U->getOperand(1)) &&<br>> > > >> > > - Operator::getOpcode(U->getOperand(1)) !=<br>> > > >> Instruction::Mul &&<br>> > > >> > > - !isa<PHINode>(U->getOperand(1))))<br>> > > >> > > - return V;<br>> > > >> > > - V = U->getOperand(0);<br>> > > >> > > - } else {<br>> > > >> > > - return V;<br>> > > >> > > - }<br>> > > >> > > - assert(V->getType()->isIntegerTy() && "Unexpected operand<br>> > > >> type!");<br>> > > >> > > - } while (true);<br>> > > >> > > -}<br>> > > >> > > -<br>> > > >> > > -/// This is a wrapper around GetUnderlyingObjects and adds<br>> > > >> > support for basic<br>> > > >> > > -/// ptrtoint+arithmetic+inttoptr sequences.<br>> > > >> > > -static void getUnderlyingObjects(const Value *V,<br>> > > >> > > - SmallVectorImpl<Value *> <br>> > &Objects,<br>> > > >> > > - const DataLayout &DL) {<br>> > > >> > > - SmallPtrSet<const Value *, 16> Visited;<br>> > > >> > > - SmallVector<const Value *, 4> Working(1, V);<br>> > > >> > > - do {<br>> > > >> > > - V = Working.pop_back_val();<br>> > > >> > > -<br>> > > >> > > - SmallVector<Value *, 4> Objs;<br>> > > >> > > - GetUnderlyingObjects(const_cast<Value *>(V), Objs, DL);<br>> > > >> > > -<br>> > > >> > > - for (Value *V : Objs) {<br>> > > >> > > - if (!Visited.insert(V).second)<br>> > > >> > > - continue;<br>> > > >> > > - if (Operator::getOpcode(V) == Instruction::IntToPtr) {<br>> > > >> > > - const Value *O =<br>> > > >> > > - <br>> > getUnderlyingObjectFromInt(cast<User>(V)->getOperand(0));<br>> > > >> > > - if (O->getType()->isPointerTy()) {<br>> > > >> > > - Working.push_back(O);<br>> > > >> > > - continue;<br>> > > >> > > - }<br>> > > >> > > - }<br>> > > >> > > - Objects.push_back(const_cast<Value *>(V));<br>> > > >> > > - }<br>> > > >> > > - } while (!Working.empty());<br>> > > >> > > -}<br>> > > >> > > -<br>> > > >> > > /// If this machine instr has memory reference information and<br>> > > >> > it can be tracked<br>> > > >> > > /// to a normal reference to a known object, return the Value<br>> > > >> > for that object.<br>> > > >> > > static void getUnderlyingObjectsForInstr(const MachineInstr <br>> > *MI,<br>> > > >> > > @@ -208,12 +151,10 @@ static void getUnderlyingObjectsForInstr<br>> > > >> > ><br>> > > >> > Objects.push_back(UnderlyingObjectsVector::value_type(PSV, <br>> > MayAlias));<br>> > > >> > > } else if (const Value *V = MMO->getValue()) {<br>> > > >> > > SmallVector<Value *, 4> Objs;<br>> > > >> > > - getUnderlyingObjects(V, Objs, DL);<br>> > > >> > > + getUnderlyingObjectsForCodeGen(V, Objs, DL);<br>> > > >> > ><br>> > > >> > > for (Value *V : Objs) {<br>> > > >> > > - if (!isIdentifiedObject(V))<br>> > > >> > > - return false;<br>> > > >> > > -<br>> > > >> > > + assert(isIdentifiedObject(V));<br>> > > >> > > Objects.push_back(UnderlyingObjectsVector::value_type(V, true));<br>> > > >> > > }<br>> > > >> > > } else<br>> > > >> > ><br>> > > >> > > Modified: llvm/trunk/lib/CodeGen/StackColoring.cpp<br>> > > >> > > URL: <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >></font></tt><br><tt><font size="2">> > > <br>> > <br>> u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_StackColoring.cpp-3Frev-3D309651-26r1-3D309650-26r2-3D309651-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >&
gt; ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=GtnObcIhIwZTCrC2KWaTzADR6e8JEcuoyWSr5_hAM1Q&e=<br>> > > >><br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> ==============================================================================<br>> > > >><br>> > > >> > > --- llvm/trunk/lib/CodeGen/StackColoring.cpp (original)<br>> > > >> > > +++ llvm/trunk/lib/CodeGen/StackColoring.cpp Mon Jul 31 20:32:15<br>> > > >> 2017<br>> > > >> > > @@ -37,6 +37,7 @@<br>> > > >> > > #include "llvm/CodeGen/MachineRegisterInfo.h"<br>> > > >> > > #include "llvm/CodeGen/Passes.h"<br>> > > >> > > #include "llvm/CodeGen/PseudoSourceValue.h"<br>> > > >> > > +#include "llvm/CodeGen/SelectionDAGNodes.h"<br>> > > >> > > #include "llvm/CodeGen/SlotIndexes.h"<br>> > > >> > > #include "llvm/CodeGen/StackProtector.h"<br>> > > >> > > #include "llvm/CodeGen/WinEHFuncInfo.h"<br>> > > >> > > @@ -889,6 +890,10 @@ void StackColoring::remapInstructions(De<br>> > > >> > ><br>> > > >> > > // Keep a list of *allocas* which need to be remapped.<br>> > > >> > > DenseMap<const AllocaInst*, const AllocaInst*> Allocas;<br>> > > >> > > +<br>> > > >> > > + // Keep a list of allocas which has been affected by the <br>> > remap.<br>> > > >> > > + SmallPtrSet<const AllocaInst*, 32> MergedAllocas;<br>> > > >> > > +<br>> > > >> > > for (const std::pair<int, int> &SI : SlotRemap) {<br>> > > >> > > const AllocaInst *From = <br>> > MFI->getObjectAllocation(SI.first);<br>> > > >> > > const AllocaInst *To = MFI->getObjectAllocation(SI.second);<br>> > > >> > > @@ -908,6 +913,10 @@ void StackColoring::remapInstructions(De<br>> > > >> > > Inst = Cast;<br>> > > >> > > }<br>> > > >> > ><br>> > > >> > > + // We keep both slots to maintain AliasAnalysis metadata <br>> > later.<br>> > > >> > > + MergedAllocas.insert(From);<br>> > > >> > > + MergedAllocas.insert(To);<br>> > > >> > > +<br>> > > >> > > // Allow the stack protector to adjust its value map to<br>> > > >> > account for the<br>> > > >> > > // upcoming replacement.<br>> > > >> > > SP->adjustForColoring(From, To);<br>> > > >> > > @@ -939,13 +948,6 @@ void StackColoring::remapInstructions(De<br>> > > >> > ><br>> > > >> > > // Update the MachineMemOperand to use the new alloca.<br>> > > >> > > for (MachineMemOperand *MMO : I.memoperands()) {<br>> > > >> > > - // FIXME: In order to enable the use of TBAA when using<br>> > > >> > AA in CodeGen,<br>> > > >> > > - // we'll also need to update the TBAA nodes in MMOs <br>> > with<br>> > > >> values<br>> > > >> > > - // derived from the merged allocas. When doing this,<br>> > > >> > we'll need to use<br>> > > >> > > - // the same variant of GetUnderlyingObjects that is <br>> > used<br>> > > >> by the<br>> > > >> > > - // instruction scheduler (that can look through<br>> > > >> ptrtoint/inttoptr<br>> > > >> > > - // pairs).<br>> > > >> > > -<br>> > > >> > > // We've replaced IR-level uses of the remapped<br>> > > >> allocas,so we only<br>> > > >> > > // need to replace direct uses here.<br>> > > >> > > const AllocaInst *AI =<br>> > > >> > dyn_cast_or_null<AllocaInst>(MMO->getValue());<br>> > > >> > > @@ -997,6 +999,48 @@ void StackColoring::remapInstructions(De<br>> > > >> > > MO.setIndex(ToSlot);<br>> > > >> > > FixedInstr++;<br>> > > >> > > }<br>> > > >> > > +<br>> > > >> > > + // We adjust AliasAnalysis information for merged stack<br>> > > >> slots.<br>> > > >> > > + MachineSDNode::mmo_iterator MemOps =<br>> > > >> > > + MF->allocateMemRefsArray(I.getNumMemOperands());<br>> > > >> > > + unsigned MemOpIdx = 0;<br>> > > >> > > + bool ReplaceMemOps = false;<br>> > > >> > > + for (MachineMemOperand *MMO : I.memoperands()) {<br>> > > >> > > + // If this memory location can be a slot remapped here,<br>> > > >> > > + // we remove AA information.<br>> > > >> > > + bool MayHaveConflictingAAMD = false;<br>> > > >> > > + if (MMO->getAAInfo()) {<br>> > > >> > > + if (const Value *MMOV = MMO->getValue()) {<br>> > > >> > > + SmallVector<Value *, 4> Objs;<br>> > > >> > > + getUnderlyingObjectsForCodeGen(MMOV, Objs,<br>> > > >> > MF->getDataLayout());<br>> > > >> > > +<br>> > > >> > > + if (Objs.empty())<br>> > > >> > > + MayHaveConflictingAAMD = true;<br>> > > >> > > + else<br>> > > >> > > + for (Value *V : Objs) {<br>> > > >> > > + // If this memory location comes from a known<br>> > > >> stack slot<br>> > > >> > > + // that is not remapped, we continue checking.<br>> > > >> > > + // Otherwise, we need to invalidate AA <br>> > infomation.<br>> > > >> > > + const AllocaInst *AI =<br>> > > >> dyn_cast_or_null<AllocaInst>(V);<br>> > > >> > > + if (AI && MergedAllocas.count(AI)) {<br>> > > >> > > + MayHaveConflictingAAMD = true;<br>> > > >> > > + break;<br>> > > >> > > + }<br>> > > >> > > + }<br>> > > >> > > + }<br>> > > >> > > + }<br>> > > >> > > + if (MayHaveConflictingAAMD) {<br>> > > >> > > + MemOps[MemOpIdx++] = MF->getMachineMemOperand(MMO,<br>> > > >> AAMDNodes());<br>> > > >> > > + ReplaceMemOps = true;<br>> > > >> > > + }<br>> > > >> > > + else<br>> > > >> > > + MemOps[MemOpIdx++] = MMO;<br>> > > >> > > + }<br>> > > >> > > +<br>> > > >> > > + // If any memory operand is updated, set memory <br>> > references of<br>> > > >> > > + // this instruction.<br>> > > >> > > + if (ReplaceMemOps)<br>> > > >> > > + I.setMemRefs(std::make_pair(MemOps,<br>> > > >> I.getNumMemOperands()));<br>> > > >> > > }<br>> > > >> > ><br>> > > >> > > // Update the location of C++ catch objects for the MSVC<br>> > > >> > personality routine.<br>> > > >> > ><br>> > > >> > ><br>> > > >> > > _______________________________________________<br>> > > >> > > llvm-commits mailing list<br>> > > >> > > llvm-commits@lists.llvm.org<br>> > > >> > > <a href="https://urldefense.proofpoint.com/v2/url?">https://urldefense.proofpoint.com/v2/url?</a><br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> > > >><br>> > > >> > siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> > > >> ><br>> > > >><br>> > > <br>> > <br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=ybOAOXxp0X9Ut7wbe4GMXq94sxexsNZIn0-<br>> > > >><br>> > > >> > _6pvOmdI&e=<br>> > > >> > ><br>> > > >> ><br>> > > >><br>> > > ><br>> > > ><br>> > ><br>> > <br>> <br></font></tt><BR>
</body></html>