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