Avoid the propagation of debug locations in SelectionDAG via CSE

[wolfgangp]

Summary

When SelectionDAG performs CSE it is possible that the context's source location is different from that of the selected node. This can lead to incorrect line number records. This patch proposes to update the debug location to the one that occurs earlier in the instruction sequence. This is in order to address PR21006 (oscillating line number records due to CSE performed on constant pool loads).

[sdmitrouk]

Hi,

Are there any reasons to implement the check in separate function instead of
changing 3-argument overload of SelectionDAG::FindNodeOrInsertPos to handle
nodes of all kinds (that would require updating its comment a bit)? If I
counted correctly, the changes cover 20 of 27 uses of that overload and it might
make sense to keep debug location erasure in one place to make it less
error-prone, unless there are reasons not to do this.

Regards,
Sergey

[wolfgangp]

In D12094#226510, @sdmitrouk wrote:

Hi,

Are there any reasons to implement the check in separate function instead of
changing 3-argument overload of SelectionDAG::FindNodeOrInsertPos to handle
nodes of all kinds (that would require updating its comment a bit)? If I
counted correctly, the changes cover 20 of 27 uses of that overload and it might
make sense to keep debug location erasure in one place to make it less
error-prone, unless there are reasons not to do this.

Regards,
Sergey

This is what I intended to do at first, but there are several contexts where the call to FindNodeOrInsertPos() is used as a query rather than with an intent to eliminate common subexpressions (e.g. all 3 versions of FindModifiedNodeSlot(), and getNodeIfExists()). It would be undesirable to possibly null out the debugLoc in those contexts, and indeed one llvm test broke by degrading the line number info when I did it this way.
That's not to say filtering the debugLoc couldn't be centralized but it would probably require some rework of the Constant node handling as well. I'm certainly open to that.

The other reason is more stylistic. I'm uncomfortable with having a function called "FindNodeOrInsertPos()", which is meant to retrieve something, have a significant side effect such as nulling out the debug location, when it's not well-defined inside the function how the result is used.

[sdmitrouk]

In D12094#226720, @wolfgangp wrote:

This is what I intended to do at first, but there are several contexts where the call to FindNodeOrInsertPos() is used as a query rather than with an intent to eliminate common subexpressions (e.g. all 3 versions of FindModifiedNodeSlot(), and getNodeIfExists()). It would be undesirable to possibly null out the debugLoc in those contexts, and indeed one llvm test broke by degrading the line number info when I did it this way.
That's not to say filtering the debugLoc couldn't be centralized but it would probably require some rework of the Constant node handling as well. I'm certainly open to that.

Not sure about getNodeIfExists(), here is part of DAGCombiner:

if (const auto *BinNode = dyn_cast<BinaryWithFlagsSDNode>(N)) {
  CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(), Ops,
                                &BinNode->Flags);
} else {
  CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(), Ops);
}

Regarding FindModifiedNodeSlot():

// See if the modified node already exists.
void *InsertPos = nullptr;
if (SDNode *Existing = FindModifiedNodeSlot(N, Op, InsertPos))
  return Existing;

So, if new node already exists, reuse it in different place. Looks like we need to drop old debug location here as well.

I guess the rest are from functions that create constant nodes.

As for the failing test, maybe it's relying on something it shouldn't? In general, cached nodes are queried for reuse, I'm not sure there are other valid cases, that's why I'm a bit suspicious about leaving some cases unhandled.

The other reason is more stylistic. I'm uncomfortable with having a function called "FindNodeOrInsertPos()", which is meant to retrieve something, have a significant side effect such as nulling out the debug location, when it's not well-defined inside the function how the result is used.

I agree, changing naming shouldn't be a problem, if you have something suitable on your mind. The interface will differ a bit for different queries, but that's not an issue as for me.

[wolfgangp]

Thank you for your comments, Sergey. If find that if I outfit getIfExists() with a debugLoc() parameter and pass it through, the failure goes away, so I have revised the patch to update the node from within the FindNode helper, dispensing with the FilterDebugLoc() routine. I've renamed the helper routine to reflect that it updates the node and took the liberty to adjust the comment a bit.

[sdmitrouk]

Thanks for the update, LGTM with returned first line of SelectionDAG.cpp.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

1 ↗

(On Diff #32779)

You probably removed this by accident.

[wolfgangp]

Added back the first line of SelectionDAG.cpp, which was accidentally removed.

[wolfgangp]

In D12094#229846, @sdmitrouk wrote:

Thanks for the update, LGTM with returned first line of SelectionDAG.cpp.

Oops, thank you for noticing that and thank you for the review.

Would it be possible for you to commit this change for me? I don't have commit access yet.

[echristo]

I've made an inline comment on this, if you could reply that would be good. Please don't commit this yet.

Thanks!

-eric

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

997–1001 ↗

(On Diff #32841)

This is an interesting perspective. I'm not sure I agree though. Things like CSE'ing of nodes can cause attribution of expressions to a line that does an equivalent computation, but this change is going to associate the expression to a line that doesn't do the expression at all in the general case. How is this better than the current status quo?

[wolfgangp]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

997–1001 ↗

(On Diff #32841)

I have to agree that it's not a good solution, and the only reason I prefer it over the status quo is that I haven't been able to come up with a case where an expression is actually attributed to a line that doesn't compute it (likely the preceding line), whereas there's at least one case that suffers from the status quo (PR21006). Anecdotal evidence, I know.

[wolfgangp]

Given the concerns about nulling out the debug location here is a different attempt: We determine from the context whether the place where the node is reused is located earlier in the sequence of instructions. To this end we're using the SDNodes' IROrder which reflects the sequence of IR instructions. This required adjustment of the interface of FindNodeOrInsertPos() because we need an SDLoc instead of just a DebugLoc.

Sergey, I wasn't sure if we can subsume Constant nodes under this change, but it seemed that it's still better to null out the DebugLocation in those cases.

[sdmitrouk]

In D12094#238549, @wolfgangp wrote:

We determine from the context whether the place where the node is reused is located earlier in the sequence of instructions.

So basically among all uses of the same CSEed node this picks debug location from the earliest one? And original issue could be rephrased as: debug location of CSEed nodes is picked non-deterministically. If that's the case, this seems to be really nice change.

Sergey, I wasn't sure if we can subsume Constant nodes under this change, but it seemed that it's still better to null out the DebugLocation in those cases.

I agree, and it's safer to leave it as is, at least until counter example is provided.

[wolfgangp]

Ping...

[wolfgangp]

Ping...

Updated the test case to catch up with recent IR changes.

[echristo]

Could you do an upload with full context please? It's a bit hard to pick out bits of the selection dag changes. In general I think the idea is fine, but the default: case could use a comment while you're at it.

Thanks!

-eric

[wolfgangp]

Thanks, Eric. Uploaded full diff and added a comment to the default: case.

[echristo]

One inline comment.

Thanks!

-eric

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

853 ↗

(On Diff #41707)

I'm unclear why we have a default SDLoc here?

[wolfgangp]

Adressed review comment on default SDLoc() in getNodeIfExists(). This routine is used in an attempt to take advantage of existing commutative binary nodes by the DAG combiner. No reason to not update the debug location if the node is reused.

[wolfgangp]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

853 ↗

(On Diff #41795)

I didn't author the original code, but I see no reason to not update the DebugLoc. This routine is called from DAGCombine in an attempt to reuse a binary commutative node. Thanks for pointing this out.

[wolfgangp]

Updated diff against r265709. No additional functional changes. There is no need to patch DAGCombiner.cpp any more because of an interface change to getNodeIfExists(). Apologies for leaving this dormant for a while.

[wolfgangp]

Ping...

[echristo]

LGTM. Thanks!

-eric

llvm/trunk/include/llvm/CodeGen/SelectionDAG.h

private:
   /// not, return the insertion token that will make insertion faster.  This
   /// overload is for nodes other than Constant or ConstantFP, use the other one
   /// for those.
   SDNode *FindNodeOrInsertPos(const FoldingSetNodeID &ID, void *&InsertPos);
 
   /// Look up the node specified by ID in CSEMap.  If it exists, return it.  If
   /// not, return the insertion token that will make insertion faster.  Performs
   /// additional processing for constant nodes.
-  SDNode *FindNodeOrInsertPos(const FoldingSetNodeID &ID, DebugLoc DL,
+  SDNode *FindNodeOrInsertPos(const FoldingSetNodeID &ID, SDLoc DL,
                               void *&InsertPos);
 
   /// List of non-single value types.
   FoldingSet<SDVTListNode> VTListMap;
 
   /// Maps to auto-CSE operations.
   std::vector<CondCodeSDNode*> CondCodeNodes;
 

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

SDNode *SelectionDAG::FindModifiedNodeSlot(SDNode *N, SDValue Op,
                                            void *&InsertPos) {
   if (doNotCSE(N))
     return nullptr;
 
   SDValue Ops[] = { Op };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, N->getOpcode(), N->getVTList(), Ops);
   AddNodeIDCustom(ID, N);
-  SDNode *Node = FindNodeOrInsertPos(ID, N->getDebugLoc(), InsertPos);
+  SDNode *Node = FindNodeOrInsertPos(ID, SDLoc(N), InsertPos);
   if (Node)
     if (const SDNodeFlags *Flags = N->getFlags())
       Node->intersectFlagsWith(Flags);
   return Node;
 }
 
 /// FindModifiedNodeSlot - Find a slot for the specified node if its operands
 /// were replaced with those specified.  If this node is never memoized,
 /// return null, otherwise return a pointer to the slot it would take.  If a
 /// node already exists with these operands, the slot will be non-null.
 SDNode *SelectionDAG::FindModifiedNodeSlot(SDNode *N,
                                            SDValue Op1, SDValue Op2,
                                            void *&InsertPos) {
   if (doNotCSE(N))
     return nullptr;
 
   SDValue Ops[] = { Op1, Op2 };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, N->getOpcode(), N->getVTList(), Ops);
   AddNodeIDCustom(ID, N);
-  SDNode *Node = FindNodeOrInsertPos(ID, N->getDebugLoc(), InsertPos);
+  SDNode *Node = FindNodeOrInsertPos(ID, SDLoc(N), InsertPos);
   if (Node)
     if (const SDNodeFlags *Flags = N->getFlags())
       Node->intersectFlagsWith(Flags);
   return Node;
 }
 
 
 /// FindModifiedNodeSlot - Find a slot for the specified node if its operands
 /// were replaced with those specified.  If this node is never memoized,
 /// return null, otherwise return a pointer to the slot it would take.  If a
 /// node already exists with these operands, the slot will be non-null.
 SDNode *SelectionDAG::FindModifiedNodeSlot(SDNode *N, ArrayRef<SDValue> Ops,
                                            void *&InsertPos) {
   if (doNotCSE(N))
     return nullptr;
 
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, N->getOpcode(), N->getVTList(), Ops);
   AddNodeIDCustom(ID, N);
-  SDNode *Node = FindNodeOrInsertPos(ID, N->getDebugLoc(), InsertPos);
+  SDNode *Node = FindNodeOrInsertPos(ID, SDLoc(N), InsertPos);
   if (Node)
     if (const SDNodeFlags *Flags = N->getFlags())
       Node->intersectFlagsWith(Flags);
   return Node;
 }
 
 /// getEVTAlignment - Compute the default alignment value for the
 /// given type.
case ISD::ConstantFP:
       llvm_unreachable("Querying for Constant and ConstantFP nodes requires "
                        "debug location.  Use another overload.");
     }
   }
   return N;
 }
 
 SDNode *SelectionDAG::FindNodeOrInsertPos(const FoldingSetNodeID &ID,
-                                          DebugLoc DL, void *&InsertPos) {
+                                          SDLoc DL, void *&InsertPos) {
   SDNode *N = CSEMap.FindNodeOrInsertPos(ID, InsertPos);
   if (N) {
     switch (N->getOpcode()) {
-    default: break; // Process only regular (non-target) constant nodes.
     case ISD::Constant:
     case ISD::ConstantFP:
       // Erase debug location from the node if the node is used at several
       // different places. Do not propagate one location to all uses as it
       // will cause a worse single stepping debugging experience.
-      if (N->getDebugLoc() != DL)
+      if (N->getDebugLoc() != DL.getDebugLoc())
         N->setDebugLoc(DebugLoc());
       break;
+    default:
+      // When the node's point of use is located earlier in the instruction
+      // sequence than its prior point of use, update its debug info to the
+      // earlier location.
+      if (DL.getIROrder() && DL.getIROrder() < N->getIROrder())
+        N->setDebugLoc(DL.getDebugLoc());
+      break;
     }
   }
   return N;
 }
 
 void SelectionDAG::clear() {
   allnodes_clear();
   OperandRecycler.clear(OperandAllocator);
assert(Elt->getBitWidth() == EltVT.getSizeInBits() &&
          "APInt size does not match type size!");
   unsigned Opc = isT ? ISD::TargetConstant : ISD::Constant;
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, Opc, getVTList(EltVT), None);
   ID.AddPointer(Elt);
   ID.AddBoolean(isO);
   void *IP = nullptr;
   SDNode *N = nullptr;
-  if ((N = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP)))
+  if ((N = FindNodeOrInsertPos(ID, DL, IP)))
     if (!VT.isVector())
       return SDValue(N, 0);
 
   if (!N) {
     N = newSDNode<ConstantSDNode>(isT, isO, Elt, DL.getDebugLoc(), EltVT);
     CSEMap.InsertNode(N, IP);
     InsertNode(N);
   }
SDValue SelectionDAG::getConstantFP(const ConstantFP& V, SDLoc DL, EVT VT,
   // value, so that we don't have problems with 0.0 comparing equal to -0.0, and
   // we don't have issues with SNANs.
   unsigned Opc = isTarget ? ISD::TargetConstantFP : ISD::ConstantFP;
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, Opc, getVTList(EltVT), None);
   ID.AddPointer(&V);
   void *IP = nullptr;
   SDNode *N = nullptr;
-  if ((N = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP)))
+  if ((N = FindNodeOrInsertPos(ID, DL, IP)))
     if (!VT.isVector())
       return SDValue(N, 0);
 
   if (!N) {
     N = newSDNode<ConstantFPSDNode>(isTarget, &V, DL.getDebugLoc(), EltVT);
     CSEMap.InsertNode(N, IP);
     InsertNode(N);
   }
SDValue SelectionDAG::getGlobalAddress(const GlobalValue *GV, SDLoc DL,
 
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, Opc, getVTList(VT), None);
   ID.AddPointer(GV);
   ID.AddInteger(Offset);
   ID.AddInteger(TargetFlags);
   ID.AddInteger(GV->getType()->getAddressSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP))
+  if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
     return SDValue(E, 0);
 
   auto *N = newSDNode<GlobalAddressSDNode>(
       Opc, DL.getIROrder(), DL.getDebugLoc(), GV, VT, Offset, TargetFlags);
   CSEMap.InsertNode(N, IP);
     InsertNode(N);
   return SDValue(N, 0);
 }
SDValue SelectionDAG::getVectorShuffle(EVT VT, SDLoc dl, SDValue N1,
 
   FoldingSetNodeID ID;
   SDValue Ops[2] = { N1, N2 };
   AddNodeIDNode(ID, ISD::VECTOR_SHUFFLE, getVTList(VT), Ops);
   for (unsigned i = 0; i != NElts; ++i)
     ID.AddInteger(MaskVec[i]);
 
   void* IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP))
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP))
     return SDValue(E, 0);
 
   // Allocate the mask array for the node out of the BumpPtrAllocator, since
   // SDNode doesn't have access to it.  This memory will be "leaked" when
   // the node is deallocated, but recovered when the NodeAllocator is released.
   int *MaskAlloc = OperandAllocator.Allocate<int>(NElts);
   memcpy(MaskAlloc, &MaskVec[0], NElts * sizeof(int));
 
SDValue SelectionDAG::getConvertRndSat(EVT VT, SDLoc dl,
   if (DTy == STy &&
       (Code == ISD::CVT_UU || Code == ISD::CVT_SS || Code == ISD::CVT_FF))
     return Val;
 
   FoldingSetNodeID ID;
   SDValue Ops[] = { Val, DTy, STy, Rnd, Sat };
   AddNodeIDNode(ID, ISD::CONVERT_RNDSAT, getVTList(VT), Ops);
   void* IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP))
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP))
     return SDValue(E, 0);
 
   auto *N =
       newSDNode<CvtRndSatSDNode>(VT, dl.getIROrder(), dl.getDebugLoc(), Code);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
   InsertNode(N);
SDValue SelectionDAG::getAddrSpaceCast(SDLoc dl, EVT VT, SDValue Ptr,
                                        unsigned SrcAS, unsigned DestAS) {
   SDValue Ops[] = {Ptr};
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::ADDRSPACECAST, getVTList(VT), Ops);
   ID.AddInteger(SrcAS);
   ID.AddInteger(DestAS);
 
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP))
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP))
     return SDValue(E, 0);
 
   auto *N = newSDNode<AddrSpaceCastSDNode>(dl.getIROrder(), dl.getDebugLoc(),
                                            VT, SrcAS, DestAS);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
   InsertNode(N);

 }
 
 /// getNode - Gets or creates the specified node.
 ///
 SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT) {
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, Opcode, getVTList(VT), None);
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP))
+  if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
     return SDValue(E, 0);
 
   auto *N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(),
                               getVTList(VT));
   CSEMap.InsertNode(N, IP);
 
   InsertNode(N);
   return SDValue(N, 0);
SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL,
 
   SDNode *N;
   SDVTList VTs = getVTList(VT);
   SDValue Ops[] = {Operand};
   if (VT != MVT::Glue) { // Don't CSE flag producing nodes
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTs, Ops);
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP))
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
       return SDValue(E, 0);
 
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
     createOperands(N, Ops);
     CSEMap.InsertNode(N, IP);
   } else {
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
     createOperands(N, Ops);
SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1,
   // Memoize this node if possible.
   SDNode *N;
   SDVTList VTs = getVTList(VT);
   if (VT != MVT::Glue) {
     SDValue Ops[] = {N1, N2};
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTs, Ops);
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP)) {
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP)) {
       if (Flags)
         E->intersectFlagsWith(Flags);
       return SDValue(E, 0);
     }
 
     N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, Flags);
     CSEMap.InsertNode(N, IP);
   } else {
SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT,
   // Memoize node if it doesn't produce a flag.
   SDNode *N;
   SDVTList VTs = getVTList(VT);
   SDValue Ops[] = {N1, N2, N3};
   if (VT != MVT::Glue) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTs, Ops);
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP))
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
       return SDValue(E, 0);
 
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
     createOperands(N, Ops);
     CSEMap.InsertNode(N, IP);
   } else {
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
     createOperands(N, Ops);
SDValue SelectionDAG::getAtomic(unsigned Opcode, SDLoc dl, EVT MemVT,
                                 AtomicOrdering SuccessOrdering,
                                 AtomicOrdering FailureOrdering,
                                 SynchronizationScope SynchScope) {
   FoldingSetNodeID ID;
   ID.AddInteger(MemVT.getRawBits());
   AddNodeIDNode(ID, Opcode, VTList, Ops);
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void* IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<AtomicSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
 
   auto *N = newSDNode<AtomicSDNode>(Opcode, dl.getIROrder(), dl.getDebugLoc(),
                                     VTList, MemVT, MMO, SuccessOrdering,
                                     FailureOrdering, SynchScope);
   createOperands(N, Ops);
SelectionDAG::getMemIntrinsicNode(unsigned Opcode, SDLoc dl, SDVTList VTList,
 
   // Memoize the node unless it returns a flag.
   MemIntrinsicSDNode *N;
   if (VTList.VTs[VTList.NumVTs-1] != MVT::Glue) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTList, Ops);
     ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+    if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
       cast<MemIntrinsicSDNode>(E)->refineAlignment(MMO);
       return SDValue(E, 0);
     }
 
     N = newSDNode<MemIntrinsicSDNode>(Opcode, dl.getIROrder(), dl.getDebugLoc(),
                                       VTList, MemVT, MMO);
     createOperands(N, Ops);
 
SelectionDAG::getLoad(ISD::MemIndexedMode AM, ISD::LoadExtType ExtType,
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::LOAD, VTs, Ops);
   ID.AddInteger(MemVT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(ExtType, AM, MMO->isVolatile(),
                                      MMO->isNonTemporal(),
                                      MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<LoadSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<LoadSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs, AM,
                                   ExtType, MemVT, MMO);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
SDValue SelectionDAG::getStore(SDValue Chain, SDLoc dl, SDValue Val,
   SDValue Ops[] = { Chain, Val, Ptr, Undef };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::STORE, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(false, ISD::UNINDEXED, MMO->isVolatile(),
                                      MMO->isNonTemporal(), MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<StoreSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<StoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                    ISD::UNINDEXED, false, VT, MMO);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
SDValue SelectionDAG::getTruncStore(SDValue Chain, SDLoc dl, SDValue Val,
   SDValue Ops[] = { Chain, Val, Ptr, Undef };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::STORE, VTs, Ops);
   ID.AddInteger(SVT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(true, ISD::UNINDEXED, MMO->isVolatile(),
                                      MMO->isNonTemporal(), MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<StoreSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<StoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                    ISD::UNINDEXED, true, SVT, MMO);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
SelectionDAG::getIndexedStore(SDValue OrigStore, SDLoc dl, SDValue Base,
   SDVTList VTs = getVTList(Base.getValueType(), MVT::Other);
   SDValue Ops[] = { ST->getChain(), ST->getValue(), Base, Offset };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::STORE, VTs, Ops);
   ID.AddInteger(ST->getMemoryVT().getRawBits());
   ID.AddInteger(ST->getRawSubclassData());
   ID.AddInteger(ST->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP))
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP))
     return SDValue(E, 0);
 
   auto *N = newSDNode<StoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs, AM,
                                    ST->isTruncatingStore(), ST->getMemoryVT(),
                                    ST->getMemOperand());
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
SelectionDAG::getMaskedLoad(EVT VT, SDLoc dl, SDValue Chain,
   AddNodeIDNode(ID, ISD::MLOAD, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(ExtTy, ISD::UNINDEXED,
                                      MMO->isVolatile(),
                                      MMO->isNonTemporal(),
                                      MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedLoadSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<MaskedLoadSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                         ExtTy, MemVT, MMO);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
SDValue SelectionDAG::getMaskedStore(SDValue Chain, SDLoc dl, SDValue Val,
   SDValue Ops[] = { Chain, Ptr, Mask, Val };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::MSTORE, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(false, ISD::UNINDEXED, MMO->isVolatile(),
                                      MMO->isNonTemporal(), MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedStoreSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<MaskedStoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                          isTrunc, MemVT, MMO);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
SelectionDAG::getMaskedGather(SDVTList VTs, EVT VT, SDLoc dl,
   AddNodeIDNode(ID, ISD::MGATHER, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(ISD::NON_EXTLOAD, ISD::UNINDEXED,
                                      MMO->isVolatile(),
                                      MMO->isNonTemporal(),
                                      MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedGatherSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
 
   auto *N = newSDNode<MaskedGatherSDNode>(dl.getIROrder(), dl.getDebugLoc(),
                                           VTs, VT, MMO);
   createOperands(N, Ops);
 
SDValue SelectionDAG::getMaskedScatter(SDVTList VTs, EVT VT, SDLoc dl,
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::MSCATTER, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
   ID.AddInteger(encodeMemSDNodeFlags(false, ISD::UNINDEXED, MMO->isVolatile(),
                                      MMO->isNonTemporal(),
                                      MMO->isInvariant()));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
-  if (SDNode *E = FindNodeOrInsertPos(ID, dl.getDebugLoc(), IP)) {
+  if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedScatterSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<MaskedScatterSDNode>(dl.getIROrder(), dl.getDebugLoc(),
                                            VTs, VT, MMO);
   createOperands(N, Ops);
 
   assert(N->getMask().getValueType().getVectorNumElements() ==
SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT,
   SDNode *N;
   SDVTList VTs = getVTList(VT);
 
   if (VT != MVT::Glue) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTs, Ops);
     void *IP = nullptr;
 
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP))
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
       return SDValue(E, 0);
 
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
     createOperands(N, Ops);
 
     CSEMap.InsertNode(N, IP);
   } else {
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);

 #endif
 
   // Memoize the node unless it returns a flag.
   SDNode *N;
   if (VTList.VTs[VTList.NumVTs-1] != MVT::Glue) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTList, Ops);
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP))
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
       return SDValue(E, 0);
 
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTList);
     createOperands(N, Ops);
     CSEMap.InsertNode(N, IP);
   } else {
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTList);
     createOperands(N, Ops);

 /// deleting things.
 SDNode *SelectionDAG::MorphNodeTo(SDNode *N, unsigned Opc,
                                   SDVTList VTs, ArrayRef<SDValue> Ops) {
   // If an identical node already exists, use it.
   void *IP = nullptr;
   if (VTs.VTs[VTs.NumVTs-1] != MVT::Glue) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opc, VTs, Ops);
-    if (SDNode *ON = FindNodeOrInsertPos(ID, N->getDebugLoc(), IP))
+    if (SDNode *ON = FindNodeOrInsertPos(ID, SDLoc(N), IP))
       return UpdadeSDLocOnMergedSDNode(ON, SDLoc(N));
   }
 
   if (!RemoveNodeFromCSEMaps(N))
     IP = nullptr;
 
   // Start the morphing.
   N->NodeType = Opc;
SelectionDAG::getMachineNode(unsigned Opcode, SDLoc DL, SDVTList VTs,
   bool DoCSE = VTs.VTs[VTs.NumVTs-1] != MVT::Glue;
   MachineSDNode *N;
   void *IP = nullptr;
 
   if (DoCSE) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, ~Opcode, VTs, Ops);
     IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL.getDebugLoc(), IP)) {
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP)) {
       return cast<MachineSDNode>(UpdadeSDLocOnMergedSDNode(E, DL));
     }
   }
 
   // Allocate a new MachineSDNode.
   N = newSDNode<MachineSDNode>(~Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
   createOperands(N, Ops);
 

 /// else return NULL.
 SDNode *SelectionDAG::getNodeIfExists(unsigned Opcode, SDVTList VTList,
                                       ArrayRef<SDValue> Ops,
                                       const SDNodeFlags *Flags) {
   if (VTList.VTs[VTList.NumVTs - 1] != MVT::Glue) {
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTList, Ops);
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DebugLoc(), IP)) {
+    if (SDNode *E = FindNodeOrInsertPos(ID, SDLoc(), IP)) {
       if (Flags)
         E->intersectFlagsWith(Flags);
       return E;
     }
   }
   return nullptr;
 }
 

llvm/trunk/test/DebugInfo/Generic/isel-cse-line.ll

+; RUN: llc -filetype=asm -fast-isel=false -O0 < %s | FileCheck %s
+;
+; Generated by:
+; clang -emit-llvm -S -g test.cpp
+
+; typedef double         fp_t;
+; typedef unsigned long  int_t;
+;
+; int_t glb_start      = 17;
+; int_t glb_end        = 42;
+;
+; int main()
+; {
+;   int_t start = glb_start;
+;   int_t end   = glb_end;
+;
+;   fp_t dbl_start = (fp_t) start;
+;   fp_t dbl_end   = (fp_t) end;
+;
+;   return 0;
+; }
+
+; SelectionDAG performs CSE on constant pool loads. Make sure line numbers
+; from such nodes are not propagated. Doing so results in oscillating line numbers.
+
+; CHECK: .loc 1 12
+; CHECK: .loc 1 13
+; CHECK-NOT: .loc 1 12
+
+; ModuleID = 't.cpp'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@glb_start = global i64 17, align 8
+@glb_end = global i64 42, align 8
+
+; Function Attrs: norecurse nounwind uwtable
+define i32 @main() !dbg !14 {
+  %1 = alloca i32, align 4
+  %2 = alloca i64, align 8
+  %3 = alloca i64, align 8
+  %4 = alloca double, align 8
+  %5 = alloca double, align 8
+  store i32 0, i32* %1, align 4
+  call void @llvm.dbg.declare(metadata i64* %2, metadata !18, metadata !19), !dbg !20
+  %6 = load i64, i64* @glb_start, align 8, !dbg !21
+  store i64 %6, i64* %2, align 8, !dbg !20
+  call void @llvm.dbg.declare(metadata i64* %3, metadata !22, metadata !19), !dbg !23
+  %7 = load i64, i64* @glb_end, align 8, !dbg !24
+  store i64 %7, i64* %3, align 8, !dbg !23
+  call void @llvm.dbg.declare(metadata double* %4, metadata !25, metadata !19), !dbg !26
+  %8 = load i64, i64* %2, align 8, !dbg !27
+  %9 = uitofp i64 %8 to double, !dbg !27
+  store double %9, double* %4, align 8, !dbg !26
+  call void @llvm.dbg.declare(metadata double* %5, metadata !28, metadata !19), !dbg !29
+  %10 = load i64, i64* %3, align 8, !dbg !30
+  %11 = uitofp i64 %10 to double, !dbg !30
+  store double %11, double* %5, align 8, !dbg !29
+  ret i32 0, !dbg !31
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!11, !12}
+!llvm.ident = !{!13}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.9.0 (trunk 268246)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !6)
+!1 = !DIFile(filename: "/home/wpieb/test/D12094.cpp", directory: "/home/wpieb/build/llvm/trunk/llvm-RelWithDebInfo")
+!2 = !{}
+!3 = !{!4}
+!4 = !DIDerivedType(tag: DW_TAG_typedef, name: "fp_t", file: !1, line: 1, baseType: !5)
+!5 = !DIBasicType(name: "double", size: 64, align: 64, encoding: DW_ATE_float)
+!6 = !{!7, !10}
+!7 = distinct !DIGlobalVariable(name: "glb_start", scope: !0, file: !1, line: 4, type: !8, isLocal: false, isDefinition: true, variable: i64* @glb_start)
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "int_t", file: !1, line: 2, baseType: !9)
+!9 = !DIBasicType(name: "long unsigned int", size: 64, align: 64, encoding: DW_ATE_unsigned)
+!10 = distinct !DIGlobalVariable(name: "glb_end", scope: !0, file: !1, line: 5, type: !8, isLocal: false, isDefinition: true, variable: i64* @glb_end)
+!11 = !{i32 2, !"Dwarf Version", i32 4}
+!12 = !{i32 2, !"Debug Info Version", i32 3}
+!13 = !{!"clang version 3.9.0 (trunk 268246)"}
+!14 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 7, type: !15, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!15 = !DISubroutineType(types: !16)
+!16 = !{!17}
+!17 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!18 = !DILocalVariable(name: "start", scope: !14, file: !1, line: 9, type: !8)
+!19 = !DIExpression()
+!20 = !DILocation(line: 9, column: 9, scope: !14)
+!21 = !DILocation(line: 9, column: 17, scope: !14)
+!22 = !DILocalVariable(name: "end", scope: !14, file: !1, line: 10, type: !8)
+!23 = !DILocation(line: 10, column: 9, scope: !14)
+!24 = !DILocation(line: 10, column: 17, scope: !14)
+!25 = !DILocalVariable(name: "dbl_start", scope: !14, file: !1, line: 12, type: !4)
+!26 = !DILocation(line: 12, column: 8, scope: !14)
+!27 = !DILocation(line: 12, column: 27, scope: !14)
+!28 = !DILocalVariable(name: "dbl_end", scope: !14, file: !1, line: 13, type: !4)
+!29 = !DILocation(line: 13, column: 8, scope: !14)
+!30 = !DILocation(line: 13, column: 27, scope: !14)
+!31 = !DILocation(line: 15, column: 3, scope: !14)