<html><body><p><font size="2">Hi Mikael,</font><br><br><font size="2">Thank you for sharing your problem. I will investigate it.</font><br><font size="2">Do you have this problem with the latest code base? I have updated my first patch (r309651) in </font><a href="https://reviews.llvm.org/rL309849"><font size="2">https://reviews.llvm.org/rL309849</font></a><font size="2">.</font><br><br><font size="2">Regards,</font><br><font size="2">Hiroshi</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/02 20:34:01:<br><br>> From: Mikael Holmén <mikael.holmen@ericsson.com></font></tt><br><tt><font size="2">> To: Hiroshi Inoue <inouehrs@jp.ibm.com>, <llvm-<br>> commits@lists.llvm.org>, "Hal Finkel" <hfinkel@anl.gov>, Matthias <br>> Braun <matze@braunis.de></font></tt><br><tt><font size="2">> Cc: Mattias Eriksson V <mattias.v.eriksson@ericsson.com></font></tt><br><tt><font size="2">> Date: 2017/10/02 20:35</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 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 dependency <br>> between a store and a following load. The store _might_ store at the <br>> same address as the load loads from, but it doesn't always do 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 locations it <br>> might store to (is this weird?). They memory locations are completely <br>> unrelated and describe the two locations the pointer %r1 can point to.<br>> <br>> Before this patch, when ScheduleDAGInstrs::buildSchedGraph called<br>> <br>>      UnderlyingObjectsVector Objs;<br>>      getUnderlyingObjectsForInstr(&MI, MFI, Objs, MF.getDataLayout());<br>> <br>> it got an empty Objs vector back since getUnderlyingObjectsForInstr <br>> found one non-identified object (%_tmp9) and then cleared the whole vector.<br>> <br>> With this patch, I instead get a non-empty Objs, containing only @g_290 <br>> and because of that we don't add a chain dependency towards the load <br>> following the store, and we get wrong code as a result.<br>> <br>> I know there is code in the method 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 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 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>> u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D309651-26view-3Drev&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=e3Q5lQrWHghbIG_b8_wJ0PPVh5IF218IDKqkTOkGGV0&e=<br>> > Log:<br>> > [StackColoring] Update AliasAnalysis information in stack 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 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 this pass.<br>> > The incorrect TBAA metadata results in recent failures in <br>> bootstrap test on ppc64le (PR33928) by allowing unsafe instruction 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=KsDAR0RUJBPSTneO3i2jMCIKtjKOSAi1260oq3nZfkA&e=<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 = 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 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=WJpDpSXhK9W0O2Y7ry9udwfGJezS0aC1GhsuQrPJ2a8&e=<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 MachineMemOperand *MMO,<br>> >                                             int64_t Offset, uint64_t Size);<br>> >   <br>> > +  /// Allocate a new MachineMemOperand by copying an existing one,<br>> > +  /// replacing only AliasAnalysis information. <br>> MachineMemOperands are owned<br>> > +  /// by the MachineFunction and need not be explicitly deallocated.<br>> > +  MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,<br>> > +                                          const AAMDNodes &AAInfo);<br>> > +<br>> >     using OperandCapacity = 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=P1817BTam8xe_gWSipjZpR88qqNAdwIFFUXlfMpsB8I&e=<br>> > <br>> ==============================================================================<br>> > --- llvm/trunk/include/llvm/CodeGen/MachineInstr.h (original)<br>> > +++ llvm/trunk/include/llvm/CodeGen/MachineInstr.h Mon Jul 31 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=kZRZpGyrwG9zEe89JDudBCkYnFPMzYQafnJFPQaCzCg&e=<br>> > <br>> ==============================================================================<br>> > --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)<br>> > +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Jul 31 20:32:15 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 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 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 multiplied value, 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 case where the<br>> > +      // object address is somehow being computed by the 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)) != 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 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 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>> > +          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 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=allS8M0h249MGa_Gx6zUREWD57UANqNdgqwsYI7vnBc&e=<br>> > <br>> ==============================================================================<br>> > --- llvm/trunk/lib/CodeGen/MachineFunction.cpp (original)<br>> > +++ llvm/trunk/lib/CodeGen/MachineFunction.cpp Mon Jul 31 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 MachineMemOperand *MMO,<br>> > +                                      const AAMDNodes &AAInfo) {<br>> > +  MachinePointerInfo MPI = MMO->getValue() ?<br>> > +             MachinePointerInfo(MMO->getValue(), MMO->getOffset()) :<br>> > +             MachinePointerInfo(MMO->getPseudoValue(), MMO->getOffset());<br>> > +<br>> > +  return new (Allocator)<br>> > +             MachineMemOperand(MPI, MMO->getFlags(), MMO->getSize(),<br>> > +                               MMO->getBaseAlignment(), AAInfo,<br>> > +                               MMO->getRanges(), 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=Ivq088QzLr4wNpGbN-<br>> k6QDpkRPbrJE6Z8WYbQgbAfXE&e= <br>> > <br>> ==============================================================================<br>> > --- llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp (original)<br>> > +++ llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp Mon Jul 31 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 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 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 multiplied value, 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 case where the<br>> > -      // object address is somehow being computed by the 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)) != 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 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 *> &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>> > -          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 *MI,<br>> > @@ -208,12 +151,10 @@ static void getUnderlyingObjectsForInstr<br>> >           <br>> Objects.push_back(UnderlyingObjectsVector::value_type(PSV, 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>> 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>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=GtnObcIhIwZTCrC2KWaTzADR6e8JEcuoyWSr5_hAM1Q&e=<br>> > <br>> ==============================================================================<br>> > --- llvm/trunk/lib/CodeGen/StackColoring.cpp (original)<br>> > +++ llvm/trunk/lib/CodeGen/StackColoring.cpp Mon Jul 31 20:32:15 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 remap.<br>> > +  SmallPtrSet<const AllocaInst*, 32> MergedAllocas;<br>> > +<br>> >     for (const std::pair<int, int> &SI : SlotRemap) {<br>> >       const AllocaInst *From = 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 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 with values<br>> > -        // derived from the merged allocas. When doing this, <br>> we'll need to use<br>> > -        // the same variant of GetUnderlyingObjects that is used by the<br>> > -        // instruction scheduler (that can look through ptrtoint/inttoptr<br>> > -        // pairs).<br>> > -<br>> >           // We've replaced IR-level uses of the remapped 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 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 stack slot<br>> > +                // that is not remapped, we continue checking.<br>> > +                // Otherwise, we need to invalidate AA infomation.<br>> > +                const AllocaInst *AI = 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, AAMDNodes());<br>> > +          ReplaceMemOps = true;<br>> > +        }<br>> > +        else<br>> > +          MemOps[MemOpIdx++] = MMO;<br>> > +      }<br>> > +<br>> > +      // If any memory operand is updated, set memory references of<br>> > +      // this instruction.<br>> > +      if (ReplaceMemOps)<br>> > +        I.setMemRefs(std::make_pair(MemOps, 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>> u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwICaQ&c=jf_iaSHvJObTbx-<br>> siA1ZOg&r=m0X4DAZw5Qh-99BsgZP4_s5n1wQ8Sz4aTCPvRS-<br>> YkVI&m=mIxa9fsPE91JBDyRBM9jO3Gnmf1VjsdJcurWlZTiDmA&s=ybOAOXxp0X9Ut7wbe4GMXq94sxexsNZIn0-<br>> _6pvOmdI&e= <br>> > <br>> <br></font></tt><BR>
</body></html>