[llvm] r321364 - [DAG] Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI.

Nirav Davé via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 22 11:45:31 PST 2017


Very odd. The only change was to alias analysis of memory operations to
constants so it should all be very local.

I've reverted the change for now and will look at it over the holidays.

-Nirav



On Fri, Dec 22, 2017 at 1:45 PM, Michael Kruse <llvm-commits at meinersbur.de>
wrote:

> Hi,
>
> this commit caused the miscompilation of three test-suite programs:
>
> LinearDependence-dbl.execution_time
> tramp3d-v4.execution_time
> pointer_member.execution_time
>
> See:
> http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-
> polly-unprofitable/builds/10187
>
> (Obsequi.execution_time was fixed by me in r321371)
>
> Could you have a look why this NFCI commit caused this? Thank you.
>
> Michael
>
>
>
>
> 2017-12-22 17:59 GMT+01:00 Nirav Dave via llvm-commits
> <llvm-commits at lists.llvm.org>:
> > Author: niravd
> > Date: Fri Dec 22 08:59:09 2017
> > New Revision: 321364
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=321364&view=rev
> > Log:
> > [DAG] Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI.
> >
> > BaseIndexOffset supercedes findBaseOffset analysis save only Constant
> > Pool addresses. Migrate analysis to BaseIndexOffset.
> >
> > Modified:
> >     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> >     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/
> SelectionDAG/DAGCombiner.cpp?rev=321364&r1=321363&r2=321364&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Fri Dec 22
> 08:59:09 2017
> > @@ -17405,43 +17405,6 @@ SDValue DAGCombiner::buildSqrtEstimate(S
> >    return buildSqrtEstimateImpl(Op, Flags, false);
> >  }
> >
> > -/// Return true if base is a frame index, which is known not to alias
> with
> > -/// anything but itself.  Provides base object and offset as results.
> > -static bool findBaseOffset(SDValue Ptr, SDValue &Base, int64_t &Offset,
> > -                           const GlobalValue *&GV, const void *&CV) {
> > -  // Assume it is a primitive operation.
> > -  Base = Ptr; Offset = 0; GV = nullptr; CV = nullptr;
> > -
> > -  // If it's an adding a simple constant then integrate the offset.
> > -  if (Base.getOpcode() == ISD::ADD) {
> > -    if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Base.getOperand(1)))
> {
> > -      Base = Base.getOperand(0);
> > -      Offset += C->getSExtValue();
> > -    }
> > -  }
> > -
> > -  // Return the underlying GlobalValue, and update the Offset.  Return
> false
> > -  // for GlobalAddressSDNode since the same GlobalAddress may be
> represented
> > -  // by multiple nodes with different offsets.
> > -  if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Base)) {
> > -    GV = G->getGlobal();
> > -    Offset += G->getOffset();
> > -    return false;
> > -  }
> > -
> > -  // Return the underlying Constant value, and update the Offset.
> Return false
> > -  // for ConstantSDNodes since the same constant pool entry may be
> represented
> > -  // by multiple nodes with different offsets.
> > -  if (ConstantPoolSDNode *C = dyn_cast<ConstantPoolSDNode>(Base)) {
> > -    CV = C->isMachineConstantPoolEntry() ? (const void
> *)C->getMachineCPVal()
> > -                                         : (const void
> *)C->getConstVal();
> > -    Offset += C->getOffset();
> > -    return false;
> > -  }
> > -  // If it's any of the following then it can't alias with anything but
> itself.
> > -  return isa<FrameIndexSDNode>(Base);
> > -}
> > -
> >  /// Return true if there is any possibility that the two addresses
> overlap.
> >  bool DAGCombiner::isAlias(LSBaseSDNode *Op0, LSBaseSDNode *Op1) const {
> >    // If they are the same then they must be aliases.
> > @@ -17483,39 +17446,16 @@ bool DAGCombiner::isAlias(LSBaseSDNode *
> >          return false;
> >      }
> >
> > -  // FIXME: findBaseOffset and ConstantValue/GlobalValue/FrameIndex
> analysis
> > -  // modified to use BaseIndexOffset.
> > -
> > -  // Gather base node and offset information.
> > -  SDValue Base0, Base1;
> > -  int64_t Offset0, Offset1;
> > -  const GlobalValue *GV0, *GV1;
> > -  const void *CV0, *CV1;
> > -  bool IsFrameIndex0 = findBaseOffset(Op0->getBasePtr(),
> > -                                      Base0, Offset0, GV0, CV0);
> > -  bool IsFrameIndex1 = findBaseOffset(Op1->getBasePtr(),
> > -                                      Base1, Offset1, GV1, CV1);
> > -
> > -  // If they have the same base address, then check to see if they
> overlap.
> > -  if (Base0 == Base1 || (GV0 && (GV0 == GV1)) || (CV0 && (CV0 == CV1)))
> > -    return !((Offset0 + NumBytes0) <= Offset1 ||
> > -             (Offset1 + NumBytes1) <= Offset0);
> > -
> > -  // It is possible for different frame indices to alias each other,
> mostly
> > -  // when tail call optimization reuses return address slots for
> arguments.
> > -  // To catch this case, look up the actual index of frame indices to
> compute
> > -  // the real alias relationship.
> > -  if (IsFrameIndex0 && IsFrameIndex1) {
> > -    MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
> > -    Offset0 += MFI.getObjectOffset(cast<Frame
> IndexSDNode>(Base0)->getIndex());
> > -    Offset1 += MFI.getObjectOffset(cast<Frame
> IndexSDNode>(Base1)->getIndex());
> > -    return !((Offset0 + NumBytes0) <= Offset1 ||
> > -             (Offset1 + NumBytes1) <= Offset0);
> > -  }
> > -
> > -  // Otherwise, if we know what the bases are, and they aren't
> identical, then
> > -  // we know they cannot alias.
> > -  if ((IsFrameIndex0 || CV0 || GV0) && (IsFrameIndex1 || CV1 || GV1))
> > +  bool IsFI0 = isa<FrameIndexSDNode>(BasePtr0.getBase());
> > +  bool IsFI1 = isa<FrameIndexSDNode>(BasePtr1.getBase());
> > +  bool IsGV0 = isa<GlobalAddressSDNode>(BasePtr0.getBase());
> > +  bool IsGV1 = isa<GlobalAddressSDNode>(BasePtr1.getBase());
> > +  bool IsCV0 = isa<ConstantPoolSDNode>(BasePtr0.getBase());
> > +  bool IsCV1 = isa<ConstantPoolSDNode>(BasePtr1.getBase());
> > +
> > +  // Matched types are already handled above. So if this is true they
> > +  // cannot alias.
> > +  if ((IsFI0 || IsGV0 || IsCV0) && (IsFI1 || IsGV1 || IsCV1))
> >      return false;
> >
> >    // If we know required SrcValue1 and SrcValue2 have relatively large
> alignment
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnaly
> sis.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/
> SelectionDAG/SelectionDAGAddressAnalysis.cpp?rev=321364&r1=
> 321363&r2=321364&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
> (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
> Fri Dec 22 08:59:09 2017
> > @@ -37,6 +37,23 @@ bool BaseIndexOffset::equalBaseIndex(Bas
> >            return true;
> >          }
> >
> > +    // Match Constants
> > +    if (auto *A = dyn_cast<ConstantPoolSDNode>(Base))
> > +      if (auto *B = dyn_cast<ConstantPoolSDNode>(Other.Base)) {
> > +        bool IsMatch =
> > +            A->isMachineConstantPoolEntry() ==
> B->isMachineConstantPoolEntry();
> > +        if (IsMatch) {
> > +          if (A->isMachineConstantPoolEntry())
> > +            IsMatch = A->getMachineCPVal() == B->getMachineCPVal();
> > +          else
> > +            IsMatch = A->getConstVal() == B->getConstVal();
> > +        }
> > +        if (IsMatch) {
> > +          Off += B->getOffset() - A->getOffset();
> > +          return true;
> > +        }
> > +      }
> > +
> >      const MachineFrameInfo &MFI = DAG.getMachineFunction().getFr
> ameInfo();
> >
> >      // Match non-equal FrameIndexes - If both frame indices are fixed
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171222/58655f92/attachment.html>


More information about the llvm-commits mailing list