Slightly change the way stackmap and patchpoint intrinsics are lowered.
authorAndrew Trick <atrick@apple.com>
Tue, 5 Nov 2013 22:44:04 +0000 (22:44 +0000)
committerAndrew Trick <atrick@apple.com>
Tue, 5 Nov 2013 22:44:04 +0000 (22:44 +0000)
MorphNodeTo is not safe to call during DAG building. It eagerly
deletes dependent DAG nodes which invalidates the NodeMap. We could
expose a safe interface for morphing nodes, but I don't think it's
worth it. Just create a new MachineNode and replaceAllUsesWith.

My understaning of the SD design has been that we want to support
early target opcode selection. That isn't very well supported, but
generally works. It seems reasonable to rely on this feature even if
it isn't widely used.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@194102 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
test/CodeGen/X86/patchpoint.ll

index a0b965c6976227fb450889be26daf8df9b5dec61..d8a2cfb64a2665e0298810a28541f3fd0a0f1586 100644 (file)
@@ -6806,11 +6806,19 @@ void SelectionDAGBuilder::visitStackmap(const CallInst &CI) {
   if (hasGlue)
     Ops.push_back(*(Call->op_end()-1));
 
-  // Replace the target specific call node with STACKMAP in-place. This way we
-  // don't have to call ReplaceAllUsesWith and STACKMAP will take the call's
-  // place in the chain.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
-  DAG.SelectNodeTo(Call, TargetOpcode::STACKMAP, NodeTys, &Ops[0], Ops.size());
+
+  // Replace the target specific call node with a STACKMAP node.
+  MachineSDNode *MN = DAG.getMachineNode(TargetOpcode::STACKMAP, getCurSDLoc(),
+                                         NodeTys, Ops);
+
+  // StackMap generates no value, so nothing goes in the NodeMap.
+
+  // Fixup the consumers of the intrinsic. The chain and glue may be used in the
+  // call sequence.
+  DAG.ReplaceAllUsesWith(Call, MN);
+
+  DAG.DeleteNode(Call);
 }
 
 /// \brief Lower llvm.experimental.patchpoint directly to its target opcode.
@@ -6898,12 +6906,22 @@ void SelectionDAGBuilder::visitPatchpoint(const CallInst &CI) {
   if (hasGlue)
     Ops.push_back(*(Call->op_end()-1));
 
-  // Replace the target specific call node with PATCHPOINT in-place. This
-  // way we don't have to call ReplaceAllUsesWith and PATCHPOINT will
-  // take the call's place in the chain.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
-  DAG.SelectNodeTo(Call, TargetOpcode::PATCHPOINT, NodeTys, &Ops[0],
-                   Ops.size());
+
+  // Replace the target specific call node with a STACKMAP node.
+  MachineSDNode *MN = DAG.getMachineNode(TargetOpcode::PATCHPOINT,
+                                         getCurSDLoc(), NodeTys, Ops);
+
+  // PatchPoint generates no value, so nothing goes in the NodeMap.
+  //
+  // FIXME: with anyregcc calling convention it will need to be in the NodeMap
+  // and replace values.
+
+  // Fixup the consumers of the intrinsic. The chain and glue may be used in the
+  // call sequence.
+  DAG.ReplaceAllUsesWith(Call, MN);
+
+  DAG.DeleteNode(Call);
 }
 
 /// TargetLowering::LowerCallTo - This is the default LowerCallTo
index 0fe55f784fcf5d4ff1ea40480850044b21debab6..92bda7c18669f9e59f62639530d7222e63128f58 100644 (file)
@@ -63,6 +63,26 @@ entry:
   ret void
 }
 
+; Test patchpoints reusing the same TargetConstant.
+; <rdar:15390785> Assertion failed: (CI.getNumArgOperands() >= NumArgs + 4)
+; There is no way to verify this, since it depends on memory allocation.
+; But I think it's useful to include as a working example.
+define i64 @testLowerConstant(i64 %arg, i64 %tmp2, i64 %tmp10, i64* %tmp33, i64 %tmp79) {
+entry:
+  %tmp80 = add i64 %tmp79, -16
+  %tmp81 = inttoptr i64 %tmp80 to i64*
+  %tmp82 = load i64* %tmp81, align 8
+  tail call void (i32, i32, ...)* @llvm.experimental.stackmap(i32 14, i32 5, i64 %arg, i64 %tmp2, i64 %tmp10, i64 %tmp82)
+  tail call void (i32, i32, i8*, i32, ...)* @llvm.experimental.patchpoint.void(i32 15, i32 30, i8* null, i32 3, i64 %arg, i64 %tmp10, i64 %tmp82)
+  %tmp83 = load i64* %tmp33, align 8
+  %tmp84 = add i64 %tmp83, -24
+  %tmp85 = inttoptr i64 %tmp84 to i64*
+  %tmp86 = load i64* %tmp85, align 8
+  tail call void (i32, i32, ...)* @llvm.experimental.stackmap(i32 17, i32 5, i64 %arg, i64 %tmp10, i64 %tmp86)
+  tail call void (i32, i32, i8*, i32, ...)* @llvm.experimental.patchpoint.void(i32 18, i32 30, i8* null, i32 3, i64 %arg, i64 %tmp10, i64 %tmp86)
+  ret i64 10
+}
+
 declare void @llvm.experimental.stackmap(i32, i32, ...)
 declare void @llvm.experimental.patchpoint.void(i32, i32, i8*, i32, ...)
 declare i64 @llvm.experimental.patchpoint.i64(i32, i32, i8*, i32, ...)