[llvm] r309651 - [StackColoring] Update AliasAnalysis information in stack coloring pass
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 2 22:22:19 PDT 2017
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://reviews.llvm.org/rL309849.
>
> 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 multiplied value, 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 case where 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 multiplied value, 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 case where 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-
>> >
>> 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