Merge branch 'main' into users/kparzysz/block-construct-structure

Author: kparzysz

.ci/metrics/metrics_test.py

@@ -0,0 +1,75 @@
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+"""Tests for metrics.py"""
+
+from dataclasses import dataclass
+import requests
+import unittest
+import unittest.mock
+
+import metrics
+
+
+class TestMetrics(unittest.TestCase):
+    def test_upload_gauge_metric(self):
+        """Test that we can upload a gauge metric correctly.
+
+        Also verify that we pass around parameters like API keys and user IDs
+        correctly to the HTTP POST request.
+        """
+        test_metrics = [metrics.GaugeMetric("gauge_test", 5, 1000)]
+        return_value = requests.Response()
+        return_value.status_code = 204
+        with unittest.mock.patch(
+            "requests.post", return_value=return_value
+        ) as post_mock:
+            metrics.upload_metrics(test_metrics, "test_userid", "test_api_key")
+            self.assertSequenceEqual(post_mock.call_args.args, [metrics.GRAFANA_URL])
+            self.assertEqual(
+                post_mock.call_args.kwargs["data"], "gauge_test value=5 1000"
+            )
+            self.assertEqual(
+                post_mock.call_args.kwargs["auth"], ("test_userid", "test_api_key")
+            )
+
+    def test_upload_job_metric(self):
+        """Test that we can upload a job metric correctly."""
+        test_metrics = [
+            metrics.JobMetrics("test_job", 5, 10, 1, 1000, 7, "test_workflow")
+        ]
+        return_value = requests.Response()
+        return_value.status_code = 204
+        with unittest.mock.patch(
+            "requests.post", return_value=return_value
+        ) as post_mock:
+            metrics.upload_metrics(test_metrics, "test_userid", "test_aoi_key")
+            self.assertEqual(
+                post_mock.call_args.kwargs["data"],
+                "test_job queue_time=5,run_time=10,status=1 1000",
+            )
+
+    def test_upload_unknown_metric(self):
+        """Test we report an error if we encounter an unknown metric type."""
+
+        @dataclass
+        class FakeMetric:
+            fake_data: str
+
+        test_metrics = [FakeMetric("test")]
+
+        with self.assertRaises(ValueError):
+            metrics.upload_metrics(test_metrics, "test_userid", "test_api_key")
+
+    def test_bad_response_code(self):
+        """Test that we gracefully handle HTTP response errors."""
+        test_metrics = [metrics.GaugeMetric("gauge_test", 5, 1000)]
+        return_value = requests.Response()
+        return_value.status_code = 403
+        # Just assert that we continue running here and do not raise anything.
+        with unittest.mock.patch("requests.post", return_value=return_value) as _:
+            metrics.upload_metrics(test_metrics, "test_userid", "test_api_key")
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/workflows/check-ci.yml

@@ -31,6 +31,7 @@ jobs:
       - name: Install Python Dependencies
         run: |
           pip3 install -r .ci/all_requirements.txt
+          pip3 install -r .ci/metrics/requirements.lock.txt
           pip3 install pytest==8.4.1
       - name: Run Tests
         working-directory: .ci

.github/workflows/premerge.yaml

@@ -62,6 +62,12 @@ jobs:
           # why we do not hardcode it.
           export SCCACHE_GCS_BUCKET=$CACHE_GCS_BUCKET
           export SCCACHE_GCS_RW_MODE=READ_WRITE
+
+          # Set the idle timeout to zero to ensure sccache runs for the
+          # entire duration of the job. Otherwise it might stop if we run
+          # several test suites in a row and discard statistics that we want
+          # to save in the end.
+          export SCCACHE_IDLE_TIMEOUT=0
           sccache --start-server
 
           ./.ci/monolithic-linux.sh "${projects_to_build}" "${project_check_targets}" "${runtimes_to_build}" "${runtimes_check_targets}" "${runtimes_check_targets_needs_reconfig}" "${enable_cir}"
@@ -110,7 +116,9 @@ jobs:
         shell: cmd
         run: |
           call C:\\BuildTools\\Common7\\Tools\\VsDevCmd.bat -arch=amd64 -host_arch=amd64
-          bash -c "export SCCACHE_GCS_BUCKET=$CACHE_GCS_BUCKET; export SCCACHE_GCS_RW_MODE=READ_WRITE; sccache --start-server; .ci/monolithic-windows.sh \"${{ steps.vars.outputs.windows-projects }}\" \"${{ steps.vars.outputs.windows-check-targets }}\""
+          # See the comments above in the Linux job for why we define each of
+          # these environment variables.
+          bash -c "export SCCACHE_GCS_BUCKET=$CACHE_GCS_BUCKET; export SCCACHE_GCS_RW_MODE=READ_WRITE; export SCCACHE_IDLE_TIMEOUT=0; sccache --start-server; .ci/monolithic-windows.sh \"${{ steps.vars.outputs.windows-projects }}\" \"${{ steps.vars.outputs.windows-check-targets }}\""
       - name: Upload Artifacts
         if: '!cancelled()'
         uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0

bolt/lib/Profile/DataAggregator.cpp

@@ -906,11 +906,10 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, const Trace &Trace,
   if (BF.isPseudo())
     return Branches;
 
-  if (!BF.isSimple())
+  // Can only record traces in CFG state
+  if (!BF.hasCFG())
     return std::nullopt;
 
-  assert(BF.hasCFG() && "can only record traces in CFG state");
-
   const BinaryBasicBlock *FromBB = BF.getBasicBlockContainingOffset(From);
   const BinaryBasicBlock *ToBB = BF.getBasicBlockContainingOffset(To);
 

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -714,21 +714,6 @@ Error RewriteInstance::run() {
 
   preprocessProfileData();
 
-  // Skip disassembling if we have a translation table and we are running an
-  // aggregation job.
-  if (opts::AggregateOnly && BAT->enabledFor(InputFile)) {
-    // YAML profile in BAT mode requires CFG for .bolt.org.text functions
-    if (!opts::SaveProfile.empty() ||
-        opts::ProfileFormat == opts::ProfileFormatKind::PF_YAML) {
-      selectFunctionsToProcess();
-      disassembleFunctions();
-      processMetadataPreCFG();
-      buildFunctionsCFG();
-    }
-    processProfileData();
-    return Error::success();
-  }
-
   selectFunctionsToProcess();
 
   readDebugInfo();
@@ -4260,31 +4245,25 @@ void RewriteInstance::patchELFPHDRTable() {
   const ELFFile<ELF64LE> &Obj = ELF64LEFile->getELFFile();
   raw_fd_ostream &OS = Out->os();
 
-  // Write/re-write program headers.
   Phnum = Obj.getHeader().e_phnum;
-  if (PHDRTableOffset) {
-    // Writing new pheader table and adding one new entry for R+X segment.
-    Phnum += 1;
-    if (NewWritableSegmentSize) {
-      // Adding one more entry for R+W segment.
-      Phnum += 1;
-    }
-  } else {
+
+  if (BC->NewSegments.empty()) {
+    BC->outs() << "BOLT-INFO: not adding new segments\n";
+    return;
+  }
+
+  if (opts::UseGnuStack) {
     assert(!PHDRTableAddress && "unexpected address for program header table");
-    PHDRTableOffset = Obj.getHeader().e_phoff;
-    if (NewWritableSegmentSize) {
+    if (BC->NewSegments.size() > 1) {
       BC->errs() << "BOLT-ERROR: unable to add writable segment\n";
       exit(1);
     }
+  } else {
+    Phnum += BC->NewSegments.size();
   }
 
-  if (opts::Instrument)
-    Phnum += 2;
-
-  if (BC->NewSegments.empty()) {
-    BC->outs() << "BOLT-INFO: not adding new segments\n";
-    return;
-  }
+  if (!PHDRTableOffset)
+    PHDRTableOffset = Obj.getHeader().e_phoff;
 
   const uint64_t SavedPos = OS.tell();
   OS.seek(PHDRTableOffset);

bolt/test/X86/unclaimed-jt-entries.s

@@ -18,6 +18,18 @@
 
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
 # RUN: %clang %cflags -no-pie %t.o -o %t.exe -Wl,-q
+
+## Check that non-simple function profile is emitted in perf2bolt mode
+# RUN: link_fdata %s %t.exe %t.pa PREAGG
+# RUN: llvm-strip -N L5 -N L5_ret %t.exe
+# RUN: perf2bolt %t.exe -p %t.pa --pa -o %t.fdata -strict=0 -print-profile \
+# RUN:   -print-only=main | FileCheck %s --check-prefix=CHECK-P2B
+# CHECK-P2B: PERF2BOLT: traces mismatching disassembled function contents: 0
+# CHECK-P2B: Binary Function "main"
+# CHECK-P2B: IsSimple : 0
+# RUN: FileCheck %s --input-file %t.fdata --check-prefix=CHECK-FDATA
+# CHECK-FDATA: 1 main 0 1 main 7 0 1
+
 # RUN: llvm-bolt %t.exe -v=1 -o %t.out 2>&1 | FileCheck %s
 
 # CHECK: BOLT-WARNING: unclaimed data to code reference (possibly an unrecognized jump table entry) to .Ltmp[[#]] in main
@@ -33,8 +45,10 @@
   .size main, .Lend-main
 main:
   jmp *L4-24(,%rdi,8)
-.L5:
+# PREAGG: T #main# #L5# #L5_ret# 1
+L5:
   movl $4, %eax
+L5_ret:
   ret
 .L9:
   movl $2, %eax
@@ -58,7 +72,7 @@ L4:
   .quad .L3
   .quad .L6
   .quad .L3
-  .quad .L5
+  .quad L5
   .quad .L3
   .quad .L3
   .quad .L3

clang-tools-extra/clang-doc/JSONGenerator.cpp

@@ -83,7 +83,27 @@ serializeLocation(const Location &Loc,
   return LocationObj;
 }
 
-static json::Value serializeComment(const CommentInfo &I) {
+static void insertComment(Object &Description, json::Value &Comment,
+                          StringRef Key) {
+  auto DescriptionIt = Description.find(Key);
+
+  if (DescriptionIt == Description.end()) {
+    auto CommentsArray = json::Array();
+    CommentsArray.push_back(Comment);
+    Description[Key] = std::move(CommentsArray);
+    Description["Has" + Key.str()] = true;
+  } else {
+    DescriptionIt->getSecond().getAsArray()->push_back(Comment);
+  }
+}
+
+static json::Value extractTextComments(Object *ParagraphComment) {
+  if (!ParagraphComment)
+    return json::Object();
+  return *ParagraphComment->get("Children");
+}
+
+static Object serializeComment(const CommentInfo &I, Object &Description) {
   // taken from PR #142273
   Object Obj = Object();
 
@@ -94,7 +114,7 @@ static json::Value serializeComment(const CommentInfo &I) {
   auto &CARef = *ChildArr.getAsArray();
   CARef.reserve(I.Children.size());
   for (const auto &C : I.Children)
-    CARef.emplace_back(serializeComment(*C));
+    CARef.emplace_back(serializeComment(*C, Description));
 
   switch (I.Kind) {
   case CommentKind::CK_TextComment: {
@@ -103,9 +123,9 @@ static json::Value serializeComment(const CommentInfo &I) {
   }
 
   case CommentKind::CK_BlockCommandComment: {
-    Child.insert({"Command", I.Name});
-    Child.insert({"Children", ChildArr});
-    Obj.insert({commentKindToString(I.Kind), ChildVal});
+    auto TextCommentsArray = extractTextComments(CARef.front().getAsObject());
+    if (I.Name == "brief")
+      insertComment(Description, TextCommentsArray, "BriefComments");
     return Obj;
   }
 
@@ -137,7 +157,10 @@ static json::Value serializeComment(const CommentInfo &I) {
     if (!I.CloseName.empty())
       Child.insert({"CloseName", I.CloseName});
     Child.insert({"Children", ChildArr});
-    Obj.insert({commentKindToString(I.Kind), ChildVal});
+    if (I.CloseName == "endcode")
+      insertComment(Description, ChildVal, "CodeComments");
+    else if (I.CloseName == "endverbatim")
+      insertComment(Description, ChildVal, "VerbatimComments");
     return Obj;
   }
 
@@ -179,8 +202,8 @@ static json::Value serializeComment(const CommentInfo &I) {
   case CommentKind::CK_FullComment:
   case CommentKind::CK_ParagraphComment: {
     Child.insert({"Children", ChildArr});
-    Obj.insert({commentKindToString(I.Kind), ChildVal});
-    return Obj;
+    Child["ParagraphComment"] = true;
+    return Child;
   }
 
   case CommentKind::CK_Unknown: {
@@ -210,12 +233,20 @@ serializeCommonAttributes(const Info &I, json::Object &Obj,
   }
 
   if (!I.Description.empty()) {
-    json::Value DescArray = json::Array();
-    auto &DescArrayRef = *DescArray.getAsArray();
-    DescArrayRef.reserve(I.Description.size());
-    for (const auto &Comment : I.Description)
-      DescArrayRef.push_back(serializeComment(Comment));
-    Obj["Description"] = DescArray;
+    Object Description = Object();
+    // Skip straight to the FullComment's children
+    auto &Comments = I.Description.at(0).Children;
+    for (const auto &CommentInfo : Comments) {
+      json::Value Comment = serializeComment(*CommentInfo, Description);
+      // if a ParagraphComment is returned, then it is a top-level comment that
+      // needs to be inserted manually.
+      if (auto *ParagraphComment = Comment.getAsObject();
+          ParagraphComment->get("ParagraphComment")) {
+        auto TextCommentsArray = extractTextComments(ParagraphComment);
+        insertComment(Description, TextCommentsArray, "ParagraphComments");
+      }
+    }
+    Obj["Description"] = std::move(Description);
   }
 
   // Namespaces aren't SymbolInfos, so they dont have a DefLoc

clang-tools-extra/clang-doc/assets/class-template.mustache

@@ -128,11 +128,11 @@
             <section class="hero section-container">
                 <div class="hero__title">
                     <h1 class="hero__title-large">{{TagType}} {{Name}}</h1>
-                    {{#RecordComments}}
+                    {{#Description}}
                     <div class="hero__subtitle">
                         {{>Comments}}
                     </div>
-                    {{/RecordComments}}
+                    {{/Description}}
                 </div>
             </section>
             {{#HasPublicMembers}}

clang-tools-extra/clang-doc/assets/comment-template.mustache

@@ -5,11 +5,20 @@
     
     This file defines templates for generating comments
 }}
-{{#FullComment}}
-    {{#Children}}
-    {{>Comments}}
-    {{/Children}}
-{{/FullComment}}
+{{#BriefComments}}
+    <div>
+    {{#.}}
+        <p>{{TextComment}}</p>
+    {{/.}}
+    </div>
+{{/BriefComments}}
+{{#ParagraphComments}}
+    <div>
+    {{#.}}
+        <p>{{TextComment}}</p>
+    {{/.}}
+    </div>
+{{/ParagraphComments}}
 {{#ParagraphComment}}
     {{#Children}}
     {{>Comments}}

clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py

@@ -177,7 +177,7 @@ def main():
     parser.add_argument(
         "-j",
         type=int,
-        default=1,
+        default=0,
         help="number of tidy instances to be run in parallel.",
     )
     parser.add_argument(
@@ -318,6 +318,7 @@ def main():
     if max_task_count == 0:
         max_task_count = multiprocessing.cpu_count()
     max_task_count = min(len(lines_by_file), max_task_count)
+    print(f"Running clang-tidy in {max_task_count} threads...")
 
     combine_fixes = False
     export_fixes_dir = None