Skip to content

[llvm][AArch64] Simplify reverse iteration fix in #217752. NFC - #217979

Open
jroelofs wants to merge 1 commit into
llvm:mainfrom
jroelofs:jroelofs/ab0b549b-post-commit-feedback
Open

[llvm][AArch64] Simplify reverse iteration fix in #217752. NFC#217979
jroelofs wants to merge 1 commit into
llvm:mainfrom
jroelofs:jroelofs/ab0b549b-post-commit-feedback

Conversation

@jroelofs

Copy link
Copy Markdown
Contributor

This is a follow-up to ab0b549 where vhscampos noticed that we were skipping MBBI in the backward search for SP adjustment FrameDestroy instructions. Since MBB always points at the PAUTH_EPILOGUE pseudo, there is no functional difference in this change, but it's worth simplifying that because the intent does not look obvious from construction, and we don't lose much from iterating over one extra instruction.

This is a follow-up to ab0b549 where vhscampos
noticed that we were skipping MBBI in the backward search for SP adjustment
FrameDestroy instructions. Since MBB always points at the PAUTH_EPILOGUE pseudo,
there is no functional difference in this change, but it's worth simplifying
that because the intent does not look obvious from construction, and we don't
lose much from iterating over one extra instruction.
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-backend-aarch64

Author: Jon Roelofs (jroelofs)

Changes

This is a follow-up to ab0b549 where vhscampos noticed that we were skipping MBBI in the backward search for SP adjustment FrameDestroy instructions. Since MBB always points at the PAUTH_EPILOGUE pseudo, there is no functional difference in this change, but it's worth simplifying that because the intent does not look obvious from construction, and we don't lose much from iterating over one extra instruction.


Full diff: https://github.com/llvm/llvm-project/pull/217979.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64PointerAuth.cpp (+1-1)
diff --git a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
index 374f42a02cc08..0e4a51286cf54 100644
--- a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
+++ b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
@@ -327,7 +327,7 @@ void AArch64PointerAuthImpl::authenticateLR(
   int64_t Offset = -ArgumentStackToRestore;
   SmallVector<MachineInstr *, 2> SPMods;
   if (ArgumentStackToRestore > 0) {
-    for (MachineInstr &MI : reverse(make_range(MBB.begin(), MBBI))) {
+    for (MachineInstr &MI : make_range(MBBI.getReverse(), MBB.rend())) {
       if (!MI.getFlag(MachineInstr::FrameDestroy))
         break;
       if ((MI.getOpcode() == AArch64::ADDXri ||

@github-actions

Copy link
Copy Markdown

🐧 Linux x64 Test Results

  • 181029 tests passed
  • 3709 tests skipped
  • 1 test failed

Failed Tests

(click on a test name to see its output)

Clang

Clang.Modules/GH204633.cppm
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 1
rm -rf /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp
# executed command: rm -rf /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp
# note: command had no output on stdout or stderr
# RUN: at line 2
split-file /home/gha/actions-runner/_work/llvm-project/llvm-project/clang/test/Modules/GH204633.cppm /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp
# executed command: split-file /home/gha/actions-runner/_work/llvm-project/llvm-project/clang/test/Modules/GH204633.cppm /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp
# note: command had no output on stdout or stderr
# RUN: at line 3
/home/gha/actions-runner/_work/llvm-project/llvm-project/build/bin/clang -cc1 -internal-isystem /home/gha/actions-runner/_work/llvm-project/llvm-project/build/lib/clang/24/include -nostdsysteminc -std=c++26 -triple x86_64-pc-win32 -fsyntax-only -fmodules    -fimplicit-module-maps -fmodules-cache-path=/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp    -fmodule-map-file=/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm -verify /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm
# executed command: /home/gha/actions-runner/_work/llvm-project/llvm-project/build/bin/clang -cc1 -internal-isystem /home/gha/actions-runner/_work/llvm-project/llvm-project/build/lib/clang/24/include -nostdsysteminc -std=c++26 -triple x86_64-pc-win32 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp -fmodule-map-file=/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm -verify /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm
# .---command stderr------------
# | error: 'expected-error' diagnostics expected but not seen: 
# |   File /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm Line 4 (directive at /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm:1): unexpected preprocessing token '{' after module name
# | error: 'expected-error' diagnostics seen but not expected: 
# |   File /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/test/Modules/Output/GH204633.cppm.tmp/original.cppm Line 4: unexpected '{' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed
# | 2 errors generated.
# `-----------------------------
# error: command failed with exit status: 1

--

If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the infrastructure label.

@github-actions

Copy link
Copy Markdown

🪟 Windows x64 Test Results

  • 140155 tests passed
  • 3723 tests skipped
  • 1 test failed

Failed Tests

(click on a test name to see its output)

Clang

Clang.Modules/GH204633.cppm
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 1
rm -rf C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp
# executed command: rm -rf 'C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp'
# note: command had no output on stdout or stderr
# RUN: at line 2
split-file C:\_work\llvm-project\llvm-project\clang\test\Modules\GH204633.cppm C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp
# executed command: split-file 'C:\_work\llvm-project\llvm-project\clang\test\Modules\GH204633.cppm' 'C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp'
# note: command had no output on stdout or stderr
# RUN: at line 3
c:\_work\llvm-project\llvm-project\build\bin\clang.exe -cc1 -internal-isystem C:\_work\llvm-project\llvm-project\build\lib\clang\24\include -nostdsysteminc -std=c++26 -triple x86_64-pc-win32 -fsyntax-only -fmodules    -fimplicit-module-maps -fmodules-cache-path=C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp    -fmodule-map-file=C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm -verify C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm
# executed command: 'c:\_work\llvm-project\llvm-project\build\bin\clang.exe' -cc1 -internal-isystem 'C:\_work\llvm-project\llvm-project\build\lib\clang\24\include' -nostdsysteminc -std=c++26 -triple x86_64-pc-win32 -fsyntax-only -fmodules -fimplicit-module-maps '-fmodules-cache-path=C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp' '-fmodule-map-file=C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm' -verify 'C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm'
# .---command stderr------------
# | error: 'expected-error' diagnostics expected but not seen: 
# |   File C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm Line 4 (directive at C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm:1): unexpected preprocessing token '{' after module name
# | error: 'expected-error' diagnostics seen but not expected: 
# |   File C:\_work\llvm-project\llvm-project\build\tools\clang\test\Modules\Output\GH204633.cppm.tmp/original.cppm Line 4: unexpected '{' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed
# | 2 errors generated.
# `-----------------------------
# error: command failed with exit status: 1

--

If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the infrastructure label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant