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

Michael Kruse via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 22 10:45:46 PST 2017


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<FrameIndexSDNode>(Base0)->getIndex());
> -    Offset1 += MFI.getObjectOffset(cast<FrameIndexSDNode>(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/SelectionDAGAddressAnalysis.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().getFrameInfo();
>
>      // 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


More information about the llvm-commits mailing list