[XRay][tools] Support tail-call exits before we write them in the runtime

deanberris · deanberris · commit 0f84a7d355bb · 2017-09-18T06:08:46.000Z
Summary: This change adds support for explicit tail-exit records to be written by the XRay runtime. This lets us differentiate the tail exit records/events in the log, and allows us to treat those exit events especially in the future. For now we allow printing those out in YAML (and reading them in). Reviewers: kpw, pelikan Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D37964 llvm-svn: 313514
diff --git a/llvm/docs/XRayFDRFormat.rst b/llvm/docs/XRayFDRFormat.rst
@@ -165,7 +165,7 @@ Function action types are as follows.
 +---------------+--------------+-----------------------------------------------+
 | Exit          | ``1``        | Typical function exit.                        |
 +---------------+--------------+-----------------------------------------------+
-| Tail_Exit     | ``2``        | An exit from a function due to Tail call      |
+| Tail_Exit     | ``2``        | An exit from a function due to tail call      |
 |               |              | optimization.                                 |
 +---------------+--------------+-----------------------------------------------+
 | Entry_Args    | ``3``        | A function entry that records arguments.      |
diff --git a/llvm/include/llvm/XRay/XRayRecord.h b/llvm/include/llvm/XRay/XRayRecord.h
@@ -53,7 +53,7 @@ struct XRayFileHeader {
 /// This may or may not correspond to actual record types in the raw trace (as
 /// the loader implementation may synthesize this information in the process of
 /// of loading).
-enum class RecordTypes { ENTER, EXIT };
+enum class RecordTypes { ENTER, EXIT, TAIL_EXIT };
 
 struct XRayRecord {
   /// The type of record.
diff --git a/llvm/include/llvm/XRay/YAMLXRayRecord.h b/llvm/include/llvm/XRay/YAMLXRayRecord.h
@@ -54,6 +54,7 @@ template <> struct ScalarEnumerationTraits<xray::RecordTypes> {
   static void enumeration(IO &IO, xray::RecordTypes &Type) {
     IO.enumCase(Type, "function-enter", xray::RecordTypes::ENTER);
     IO.enumCase(Type, "function-exit", xray::RecordTypes::EXIT);
+    IO.enumCase(Type, "function-tail-exit", xray::RecordTypes::TAIL_EXIT);
   }
 };
 
diff --git a/llvm/lib/XRay/Trace.cpp b/llvm/lib/XRay/Trace.cpp
@@ -48,7 +48,7 @@ Error readBinaryFormatHeader(StringRef Data, XRayFileHeader &FileHeader) {
   FileHeader.NonstopTSC = Bitfield & 1uL << 1;
   FileHeader.CycleFrequency = HeaderExtractor.getU64(&OffsetPtr);
   std::memcpy(&FileHeader.FreeFormData, Data.bytes_begin() + OffsetPtr, 16);
-  if (FileHeader.Version != 1)
+  if (FileHeader.Version != 1 && FileHeader.Version != 2)
     return make_error<StringError>(
         Twine("Unsupported XRay file version: ") + Twine(FileHeader.Version),
         std::make_error_code(std::errc::invalid_argument));
@@ -94,6 +94,9 @@ Error loadNaiveFormatLog(StringRef Data, XRayFileHeader &FileHeader,
     case 1:
       Record.Type = RecordTypes::EXIT;
       break;
+    case 2:
+      Record.Type = RecordTypes::TAIL_EXIT;
+      break;
     default:
       return make_error<StringError>(
           Twine("Unknown record type '") + Twine(int{Type}) + "'",
@@ -320,9 +323,11 @@ Error processFDRFunctionRecord(FDRState &State, uint8_t RecordFirstByte,
       Record.Type = RecordTypes::ENTER;
       break;
     case static_cast<uint8_t>(RecordTypes::EXIT):
-    case 2: // TAIL_EXIT is not yet defined in RecordTypes.
       Record.Type = RecordTypes::EXIT;
       break;
+    case static_cast<uint8_t>(RecordTypes::TAIL_EXIT):
+      Record.Type = RecordTypes::TAIL_EXIT;
+      break;
     default:
       // Cast to an unsigned integer to not interpret the record type as a char.
       return make_error<StringError>(
@@ -443,7 +448,7 @@ Error loadFDRLog(StringRef Data, XRayFileHeader &FileHeader,
   // Having iterated over everything we've been given, we've either consumed
   // everything and ended up in the end state, or were told to skip the rest.
   bool Finished = State.Expects == FDRState::Token::SCAN_TO_END_OF_THREAD_BUF &&
-        State.CurrentBufferSize == State.CurrentBufferConsumed;
+                  State.CurrentBufferSize == State.CurrentBufferConsumed;
   if (State.Expects != FDRState::Token::NEW_BUFFER_RECORD_OR_EOF && !Finished)
     return make_error<StringError>(
         Twine("Encountered EOF with unexpected state expectation ") +
@@ -534,9 +539,8 @@ Expected<Trace> llvm::xray::loadTraceFile(StringRef Filename, bool Sort) {
   enum BinaryFormatType { NAIVE_FORMAT = 0, FLIGHT_DATA_RECORDER_FORMAT = 1 };
 
   Trace T;
-  if (Version == 1 && Type == NAIVE_FORMAT) {
-    if (auto E =
-            loadNaiveFormatLog(Data, T.FileHeader, T.Records))
+  if (Type == NAIVE_FORMAT && (Version == 1 || Version == 2)) {
+    if (auto E = loadNaiveFormatLog(Data, T.FileHeader, T.Records))
       return std::move(E);
   } else if (Version == 1 && Type == FLIGHT_DATA_RECORDER_FORMAT) {
     if (auto E = loadFDRLog(Data, T.FileHeader, T.Records))
diff --git a/llvm/test/tools/llvm-xray/X86/convert-fdr-to-yaml.txt b/llvm/test/tools/llvm-xray/X86/convert-fdr-to-yaml.txt
@@ -16,7 +16,7 @@
 ; CHECK-NEXT:   - { type: 0, func-id: 2, function: '2', cpu: 5, thread: 5, kind: function-exit, tsc: 7238225556407467 }
 ; CHECK-NEXT:   - { type: 0, func-id: 4, function: '4', cpu: 5, thread: 5, kind: function-enter, tsc: 7238225556407492 }
 ; CHECK-NEXT:   - { type: 0, func-id: 5, function: '5', cpu: 5, thread: 5, kind: function-enter, tsc: 7238225556407517 }
-; CHECK-NEXT:   - { type: 0, func-id: 5, function: '5', cpu: 5, thread: 5, kind: function-exit, tsc: 7238225556407542 }
+; CHECK-NEXT:   - { type: 0, func-id: 5, function: '5', cpu: 5, thread: 5, kind: function-tail-exit, tsc: 7238225556407542 }
 ; CHECK-NEXT:   - { type: 0, func-id: 268435455, function: '268435455', cpu: 5, thread: 5, kind: function-enter, tsc: 7238225556407552 }
 ; CHECK-NEXT:   - { type: 0, func-id: 268435455, function: '268435455', cpu: 5, thread: 5, kind: function-exit, tsc: 7238225556407562 }
 ; CHECK-NEXT:   - { type: 0, func-id: 6, function: '6', cpu: 6, thread: 5, kind: function-enter, tsc: 7238225556407682 }
diff --git a/llvm/tools/llvm-xray/xray-account.cc b/llvm/tools/llvm-xray/xray-account.cc
@@ -150,7 +150,8 @@ bool LatencyAccountant::accountRecord(const XRayRecord &Record) {
     ThreadStack.emplace_back(Record.FuncId, Record.TSC);
     break;
   }
-  case RecordTypes::EXIT: {
+  case RecordTypes::EXIT:
+  case RecordTypes::TAIL_EXIT: {
     if (ThreadStack.empty())
       return false;
 
@@ -419,6 +420,9 @@ template <> struct format_provider<llvm::xray::RecordTypes> {
       case RecordTypes::EXIT:
         Stream << "exit";
         break;
+      case RecordTypes::TAIL_EXIT:
+        Stream << "tail-exit";
+        break;
     }
   }
 };
diff --git a/llvm/tools/llvm-xray/xray-converter.cc b/llvm/tools/llvm-xray/xray-converter.cc
@@ -128,6 +128,9 @@ void TraceConverter::exportAsRAWv1(const Trace &Records, raw_ostream &OS) {
     case RecordTypes::EXIT:
       Writer.write(uint8_t{1});
       break;
+    case RecordTypes::TAIL_EXIT:
+      Writer.write(uint8_t{2});
+      break;
     }
     Writer.write(R.FuncId);
     Writer.write(R.TSC);
diff --git a/llvm/tools/llvm-xray/xray-graph.cc b/llvm/tools/llvm-xray/xray-graph.cc
@@ -214,7 +214,8 @@ Error GraphRenderer::accountRecord(const XRayRecord &Record) {
     ThreadStack.push_back({Record.FuncId, Record.TSC});
     break;
   }
-  case RecordTypes::EXIT: {
+  case RecordTypes::EXIT:
+  case RecordTypes::TAIL_EXIT: {
     // FIXME: Refactor this and the account subcommand to reduce code
     // duplication
     if (ThreadStack.size() == 0 || ThreadStack.back().FuncId != Record.FuncId) {
diff --git a/llvm/tools/llvm-xray/xray-stacks.cc b/llvm/tools/llvm-xray/xray-stacks.cc
@@ -352,7 +352,8 @@ class StackTrie {
       }
       return AccountRecordStatus::OK;
     }
-    case RecordTypes::EXIT: {
+    case RecordTypes::EXIT:
+    case RecordTypes::TAIL_EXIT: {
       bool wasLastRecordExit = state->wasLastRecordExit;
       state->wasLastRecordExit = true;
       // The exit case is more interesting, since we want to be able to deduce

Original file line number	Diff line number	Diff line change
`@@ -54,6 +54,7 @@ template <> struct ScalarEnumerationTraits<xray::RecordTypes> {`
`54`	`54`	`static void enumeration(IO &IO, xray::RecordTypes &Type) {`
`55`	`55`	`IO.enumCase(Type, "function-enter", xray::RecordTypes::ENTER);`
`56`	`56`	`IO.enumCase(Type, "function-exit", xray::RecordTypes::EXIT);`
	`57`	`+ IO.enumCase(Type, "function-tail-exit", xray::RecordTypes::TAIL_EXIT);`
`57`	`58`	`}`
`58`	`59`	`};`
`59`	`60`
Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,8 @@ bool LatencyAccountant::accountRecord(const XRayRecord &Record) {`
`150`	`150`	`ThreadStack.emplace_back(Record.FuncId, Record.TSC);`
`151`	`151`	`break;`
`152`	`152`	`}`
`153`		`- case RecordTypes::EXIT: {`
	`153`	`+ case RecordTypes::EXIT:`
	`154`	`+ case RecordTypes::TAIL_EXIT: {`
`154`	`155`	`if (ThreadStack.empty())`
`155`	`156`	`return false;`
`156`	`157`
`@@ -419,6 +420,9 @@ template <> struct format_provider<llvm::xray::RecordTypes> {`
`419`	`420`	`case RecordTypes::EXIT:`
`420`	`421`	`Stream << "exit";`
`421`	`422`	`break;`
	`423`	`+ case RecordTypes::TAIL_EXIT:`
	`424`	`+ Stream << "tail-exit";`
	`425`	`+ break;`
`422`	`426`	`}`
`423`	`427`	`}`
`424`	`428`	`};`
Original file line number	Diff line number	Diff line change
`@@ -352,7 +352,8 @@ class StackTrie {`
`352`	`352`	`}`
`353`	`353`	`return AccountRecordStatus::OK;`
`354`	`354`	`}`
`355`		`- case RecordTypes::EXIT: {`
	`355`	`+ case RecordTypes::EXIT:`
	`356`	`+ case RecordTypes::TAIL_EXIT: {`
`356`	`357`	`bool wasLastRecordExit = state->wasLastRecordExit;`
`357`	`358`	`state->wasLastRecordExit = true;`
`358`	`359`	`// The exit case is more interesting, since we want to be able to deduce`