merge main into amd-staging

.ci/premerge_advisor_explain.py

@@ -4,20 +4,90 @@
 """Script for getting explanations from the premerge advisor."""
 
 import argparse
-import os
 import platform
 import sys
+import json
+
+# TODO(boomanaiden154): Remove the optional call once we can require Python
+# 3.10.
+from typing import Optional
 
 import requests
+import github
+import github.PullRequest
 
 import generate_test_report_lib
 
 PREMERGE_ADVISOR_URL = (
     "http://premerge-advisor.premerge-advisor.svc.cluster.local:5000/explain"
 )
+COMMENT_TAG = "<!--PREMERGE ADVISOR COMMENT: {platform}-->"
+
+
+def get_comment_id(platform: str, pr: github.PullRequest.PullRequest) -> Optional[int]:
+    platform_comment_tag = COMMENT_TAG.format(platform=platform)
+    for comment in pr.as_issue().get_comments():
+        if platform_comment_tag in comment.body:
+            return comment.id
+    return None
+
+
+def get_comment(
+    github_token: str,
+    pr_number: int,
+    body: str,
+) -> dict[str, str]:
+    repo = github.Github(github_token).get_repo("llvm/llvm-project")
+    pr = repo.get_issue(pr_number).as_pull_request()
+    comment = {"body": body}
+    comment_id = get_comment_id(platform.system(), pr)
+    if comment_id:
+        comment["id"] = comment_id
+    return comment
 
 
-def main(commit_sha: str, build_log_files: list[str]):
+def main(
+    commit_sha: str,
+    build_log_files: list[str],
+    github_token: str,
+    pr_number: int,
+    return_code: int,
+):
+    """The main entrypoint for the script.
+
+    This function parses failures from files, requests information from the
+    premerge advisor, and may write a Github comment depending upon the output.
+    There are four different scenarios:
+    1. There has never been a previous failure and the job passes - We do not
+       create a comment. We write out an empty file to the comment path so the
+       issue-write workflow knows not to create anything.
+    2. There has never been a previous failure and the job fails - We create a
+       new comment containing the failure information and any possible premerge
+       advisor findings.
+    3. There has been a previous failure and the job passes - We update the
+       existing comment by passing its ID and a passed message to the
+       issue-write workflow.
+    4. There has been a previous failure and the job fails - We update the
+       existing comment in the same manner as above, but generate the comment
+       as if we have a failure.
+
+    Args:
+      commit_sha: The base commit SHA for this PR run.
+      build_log_files: The list of JUnit XML files and ninja logs.
+      github_token: The token to use to access the Github API.
+      pr_number: The number of the PR associated with this run.
+      return_code: The numerical return code of ninja/CMake.
+    """
+    if return_code == 0:
+        with open("comment", "w") as comment_file_handle:
+            comment = get_comment(
+                github_token,
+                pr_number,
+                ":white_check_mark: With the latest revision this PR passed "
+                "the premerge checks.",
+            )
+            if "id" in comment:
+                json.dump([comment], comment_file_handle)
     junit_objects, ninja_logs = generate_test_report_lib.load_info_from_files(
         build_log_files
     )
@@ -45,13 +115,31 @@ def main(commit_sha: str, build_log_files: list[str]):
     )
     if advisor_response.status_code == 200:
         print(advisor_response.json())
+        comments = [
+            get_comment(
+                github_token,
+                pr_number,
+                generate_test_report_lib.generate_report(
+                    generate_test_report_lib.compute_platform_title(),
+                    return_code,
+                    junit_objects,
+                    ninja_logs,
+                    failure_explanations_list=advisor_response.json(),
+                ),
+            )
+        ]
+        with open("comment", "w") as comment_file_handle:
+            json.dump(comments, comment_file_handle)
     else:
         print(advisor_response.reason)
 
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
     parser.add_argument("commit_sha", help="The base commit SHA for the test.")
+    parser.add_argument("return_code", help="The build's return code", type=int)
+    parser.add_argument("github_token", help="Github authentication token", type=str)
+    parser.add_argument("pr_number", help="The PR number", type=int)
     parser.add_argument(
         "build_log_files", help="Paths to JUnit report files and ninja logs.", nargs="*"
     )
@@ -62,4 +150,10 @@ def main(commit_sha: str, build_log_files: list[str]):
     if platform.machine() == "arm64":
         sys.exit(0)
 
-    main(args.commit_sha, args.build_log_files)
+    main(
+        args.commit_sha,
+        args.build_log_files,
+        args.github_token,
+        args.pr_number,
+        args.return_code,
+    )

.ci/requirements.txt

@@ -1,2 +1,3 @@
 junitparser==3.2.0
 google-cloud-storage==3.3.0
+PyGithub==2.8.1

.ci/utils.sh

@@ -33,17 +33,18 @@ function at-exit {
   # If building fails there will be no results files.
   shopt -s nullglob
 
-  if [[ "$GITHUB_STEP_SUMMARY" != "" ]]; then
+  if [[ "$GITHUB_ACTIONS" != "" ]]; then
     python "${MONOREPO_ROOT}"/.ci/generate_test_report_github.py \
       $retcode "${BUILD_DIR}"/test-results.*.xml "${MONOREPO_ROOT}"/ninja*.log \
       >> $GITHUB_STEP_SUMMARY
+    python "${MONOREPO_ROOT}"/.ci/premerge_advisor_explain.py \
+      $(git rev-parse HEAD~1) $retcode "${GITHUB_TOKEN}" \
+      $GITHUB_PR_NUMBER "${BUILD_DIR}"/test-results.*.xml \
+      "${MONOREPO_ROOT}"/ninja*.log
   fi
 
   if [[ "$retcode" != "0" ]]; then
     if [[ "$GITHUB_ACTIONS" != "" ]]; then
-      python "${MONOREPO_ROOT}"/.ci/premerge_advisor_explain.py \
-        $(git rev-parse HEAD~1) "${BUILD_DIR}"/test-results.*.xml \
-        "${MONOREPO_ROOT}"/ninja*.log
       python "${MONOREPO_ROOT}"/.ci/premerge_advisor_upload.py \
         $(git rev-parse HEAD~1) $GITHUB_RUN_NUMBER \
         "${BUILD_DIR}"/test-results.*.xml "${MONOREPO_ROOT}"/ninja*.log

.github/workflows/build-ci-container-tooling.yml

@@ -36,6 +36,9 @@ jobs:
             test-command: 'cd $HOME && clang-format --version | grep version && git-clang-format -h | grep usage && black --version | grep black'
           - container-name: lint
             test-command: 'cd $HOME && clang-tidy --version | grep version && clang-tidy-diff.py -h | grep usage'
+          - container-name: abi-tests
+            test-command: 'cd $HOME && abi-compliance-checker --help'
+            target: abi-tests
     steps:
       - name: Checkout LLVM
         uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
@@ -52,7 +55,7 @@ jobs:
         with:
           container-name: ci-ubuntu-24.04-${{ matrix.container-name }}
           dockerfile: .github/workflows/containers/github-action-ci-tooling/Dockerfile
-          target: ci-container-code-${{ matrix.container-name }}
+          target: ci-container-${{ matrix.target || format('code-{0}', matrix.container-name) }}
           test-command: ${{ matrix.test-command }}
 
   push-ci-container:

.github/workflows/containers/github-action-ci-tooling/Dockerfile

@@ -47,6 +47,28 @@ RUN echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
 # as root in 'ci-container-code-format' and 'ci-container-code-lint' containers
 
 
+FROM base AS ci-container-build-tools
+ARG LLVM_VERSION
+ARG LLVM_VERSION_MAJOR
+
+COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/clang-${LLVM_VERSION_MAJOR} \
+                            ${LLVM_SYSROOT}/bin/
+COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/lib/clang/${LLVM_VERSION_MAJOR}/include \
+                            ${LLVM_SYSROOT}/lib/clang/${LLVM_VERSION_MAJOR}/include
+RUN ln -s ${LLVM_SYSROOT}/bin/clang-${LLVM_VERSION_MAJOR} ${LLVM_SYSROOT}/bin/clang && \
+    ln -s ${LLVM_SYSROOT}/bin/clang ${LLVM_SYSROOT}/bin/clang++
+
+RUN apt-get update && \
+    DEBIAN_FRONTEND=noninteractive apt-get install -y \
+    cmake \
+    ninja-build && \
+    apt-get clean && \
+    rm -rf /var/lib/apt/lists/*
+
+ENV CC=${LLVM_SYSROOT}/bin/clang
+ENV CXX=${LLVM_SYSROOT}/bin/clang++
+
+
 FROM base AS ci-container-code-format
 ARG LLVM_VERSION
 
@@ -63,31 +85,37 @@ USER gha
 WORKDIR /home/gha
 
 
-FROM base AS ci-container-code-lint
+FROM ci-container-build-tools AS ci-container-code-lint
 ARG LLVM_VERSION
 ARG LLVM_VERSION_MAJOR
 
 COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/clang-tidy \
-                            /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/clang-${LLVM_VERSION_MAJOR} \
                             ${LLVM_SYSROOT}/bin/
-COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/lib/clang/${LLVM_VERSION_MAJOR}/include \
-                            ${LLVM_SYSROOT}/lib/clang/${LLVM_VERSION_MAJOR}/include
 COPY clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py ${LLVM_SYSROOT}/bin/clang-tidy-diff.py
 
-RUN ln -s ${LLVM_SYSROOT}/bin/clang-${LLVM_VERSION_MAJOR} ${LLVM_SYSROOT}/bin/clang && \
-    ln -s ${LLVM_SYSROOT}/bin/clang ${LLVM_SYSROOT}/bin/clang++
+# Install dependencies for 'pr-code-lint.yml' job
+COPY llvm/utils/git/requirements_linting.txt requirements_linting.txt
+RUN pip install -r requirements_linting.txt --break-system-packages && \
+    rm requirements_linting.txt
+USER gha
+WORKDIR /home/gha
+
 
+FROM ci-container-build-tools as ci-container-abi-tests
 
 RUN apt-get update && \
     DEBIAN_FRONTEND=noninteractive apt-get install -y \
-    cmake \
-    ninja-build && \
+    abi-compliance-checker \
+    abi-dumper \
+    autoconf \
+    pkg-config && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists/*
 
-# Install dependencies for 'pr-code-lint.yml' job
-COPY llvm/utils/git/requirements_linting.txt requirements_linting.txt
-RUN pip install -r requirements_linting.txt --break-system-packages && \
-    rm requirements_linting.txt
-USER gha
-WORKDIR /home/gha
+RUN git clone https://github.com/universal-ctags/ctags.git && \
+    cd ctags && \
+    ./autogen.sh && \
+    ./configure && \
+    sudo make install && \
+    rm -Rf ../ctags
+

.github/workflows/premerge.yaml

@@ -64,6 +64,9 @@ jobs:
       - name: Build and Test
         timeout-minutes: 120
         continue-on-error: ${{ runner.arch == 'ARM64' }}
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+          GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
         run: |
           git config --global --add safe.directory '*'
 
@@ -153,6 +156,9 @@ jobs:
         timeout-minutes: 180
         if: ${{ steps.vars.outputs.windows-projects != '' }}
         shell: cmd
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+          GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
         run: |
           call C:\\BuildTools\\Common7\\Tools\\VsDevCmd.bat -arch=amd64 -host_arch=amd64
           # See the comments above in the Linux job for why we define each of

bolt/include/bolt/Core/MCPlusBuilder.h

@@ -784,6 +784,11 @@ class MCPlusBuilder {
 
   virtual bool isPop(const MCInst &Inst) const { return false; }
 
+  /// Determine if a basic block looks like an epilogue. For now it is only
+  /// called at the final stage of building CFG to check basic block ending
+  /// with an indirect call that has unknown control flow attribute.
+  virtual bool isEpilogue(const BinaryBasicBlock &BB) const { return false; }
+
   /// Return true if the instruction is used to terminate an indirect branch.
   virtual bool isTerminateBranch(const MCInst &Inst) const {
     llvm_unreachable("not implemented");
@@ -1371,20 +1376,13 @@ class MCPlusBuilder {
   /// Return true if \p Inst has RestoreState annotation.
   bool hasRestoreState(const MCInst &Inst) const;
 
-  /// Stores RA Signed annotation on \p Inst.
-  void setRASigned(MCInst &Inst) const;
-
-  /// Return true if \p Inst has Signed RA annotation.
-  bool isRASigned(const MCInst &Inst) const;
-
-  /// Stores RA Unsigned annotation on \p Inst.
-  void setRAUnsigned(MCInst &Inst) const;
-
-  /// Return true if \p Inst has Unsigned RA annotation.
-  bool isRAUnsigned(const MCInst &Inst) const;
+  /// Sets kRASigned or kRAUnsigned annotation on \p Inst.
+  /// Fails if \p Inst has either annotation already set.
+  void setRAState(MCInst &Inst, bool State) const;
 
-  /// Return true if \p Inst doesn't have any annotation related to RA state.
-  bool isRAStateUnknown(const MCInst &Inst) const;
+  /// Return true if \p Inst has kRASigned annotation, false if it has
+  /// kRAUnsigned annotation, and std::nullopt if neither annotation is set.
+  std::optional<bool> getRAState(const MCInst &Inst) const;
 
   /// Return true if the instruction is a call with an exception handling info.
   virtual bool isInvoke(const MCInst &Inst) const {

bolt/lib/Core/BinaryFunction.cpp

@@ -2167,13 +2167,10 @@ bool BinaryFunction::postProcessIndirectBranches(
         continue;
       }
 
-      // If this block contains an epilogue code and has an indirect branch,
-      // then most likely it's a tail call. Otherwise, we cannot tell for sure
-      // what it is and conservatively reject the function's CFG.
-      bool IsEpilogue = llvm::any_of(BB, [&](const MCInst &Instr) {
-        return BC.MIB->isLeave(Instr) || BC.MIB->isPop(Instr);
-      });
-      if (IsEpilogue) {
+      // If this block contains epilogue code and has an indirect branch,
+      // then most likely it's a tail call. Otherwise, we cannot tell for
+      // sure what it is and conservatively reject the function's CFG.
+      if (BC.MIB->isEpilogue(BB)) {
         BC.MIB->convertJmpToTailCall(Instr);
         BB.removeAllSuccessors();
         continue;

bolt/lib/Core/MCPlusBuilder.cpp

@@ -186,26 +186,21 @@ bool MCPlusBuilder::hasRestoreState(const MCInst &Inst) const {
   return hasAnnotation(Inst, MCAnnotation::kRestoreState);
 }
 
-void MCPlusBuilder::setRASigned(MCInst &Inst) const {
+void MCPlusBuilder::setRAState(MCInst &Inst, bool State) const {
   assert(!hasAnnotation(Inst, MCAnnotation::kRASigned));
-  setAnnotationOpValue(Inst, MCAnnotation::kRASigned, true);
-}
-
-bool MCPlusBuilder::isRASigned(const MCInst &Inst) const {
-  return hasAnnotation(Inst, MCAnnotation::kRASigned);
-}
-
-void MCPlusBuilder::setRAUnsigned(MCInst &Inst) const {
   assert(!hasAnnotation(Inst, MCAnnotation::kRAUnsigned));
-  setAnnotationOpValue(Inst, MCAnnotation::kRAUnsigned, true);
+  if (State)
+    setAnnotationOpValue(Inst, MCAnnotation::kRASigned, true);
+  else
+    setAnnotationOpValue(Inst, MCAnnotation::kRAUnsigned, true);
 }
 
-bool MCPlusBuilder::isRAUnsigned(const MCInst &Inst) const {
-  return hasAnnotation(Inst, MCAnnotation::kRAUnsigned);
-}
-
-bool MCPlusBuilder::isRAStateUnknown(const MCInst &Inst) const {
-  return !(isRAUnsigned(Inst) || isRASigned(Inst));
+std::optional<bool> MCPlusBuilder::getRAState(const MCInst &Inst) const {
+  if (hasAnnotation(Inst, MCAnnotation::kRASigned))
+    return true;
+  if (hasAnnotation(Inst, MCAnnotation::kRAUnsigned))
+    return false;
+  return std::nullopt;
 }
 
 std::optional<MCLandingPad> MCPlusBuilder::getEHInfo(const MCInst &Inst) const {