Add Three Codegen Tests #134626

veera-sivarajan · 2024-12-21T18:19:06Z

Closes #123216
Closes #49572
Closes #93514

This PR adds three codegen tests.

rustbot · 2024-12-21T18:19:14Z

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Mark-Simulacrum · 2024-12-26T23:52:18Z

@bors r+ rollup=iffy

bors · 2024-12-26T23:52:21Z

📌 Commit 62fedef has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors · 2024-12-27T06:08:45Z

⌛ Testing commit 62fedef with merge 655cf53...

…r=Mark-Simulacrum Add Four Codegen Tests Closes rust-lang#74615 Closes rust-lang#123216 Closes rust-lang#49572 Closes rust-lang#93514 This PR adds four codegen tests. The FileCheck assertions were generated with the help of `update_test_checks.py` and `update_llc_test_checks.py` from the LLVM project.

bors · 2024-12-27T06:36:21Z

💔 Test failed - checks-actions

tests/assembly/indexing-with-bools-no-redundant-instructions.rs

Mark-Simulacrum · 2025-01-11T22:24:13Z

@bors r+

bors · 2025-01-11T22:24:15Z

📌 Commit 0ec717b has been approved by Mark-Simulacrum

It is now in the queue for this repository.

…r=Mark-Simulacrum Add Four Codegen Tests Closes rust-lang#74615 Closes rust-lang#123216 Closes rust-lang#49572 Closes rust-lang#93514 This PR adds four codegen tests. The FileCheck assertions were generated with the help of `update_test_checks.py` and `update_llc_test_checks.py` from the LLVM project.

bors · 2025-01-12T00:30:56Z

⌛ Testing commit 0ec717b with merge 159ccc2...

bors · 2025-01-12T01:25:58Z

💔 Test failed - checks-actions

veera-sivarajan · 2025-02-12T03:27:09Z

@rustbot author

dianqk

This PR adds four codegen tests. The FileCheck assertions were generated with the help of update_test_checks.py and update_llc_test_checks.py from the LLVM project.

Should we use them? Or should we add so many CHECK?

Unlike LLVM, we test multiple platforms in Rust, and this IR also comes from codegen. These IRs can vary.

dianqk · 2025-02-25T05:27:04Z

tests/assembly/indexing-with-bools-no-redundant-instructions.rs

+    // CHECK: andl   $1, %eax
+    // CHECK: shll   $4, %eax
+    // CHECK: orb    $1, (%rcx,%rax)
+    // CHECK-NOT:     jmp


Just a question: should this be an assembly test?
The IR change was introduced by llvm/llvm-project#84628, and this is not a backend change.

Yeah, I think assembly test would be better because there isn't a big difference in the IR: https://rust.godbolt.org/z/sd7G1rsa7

Thanks

…, r=Mark-Simulacrum Add Four Codegen Tests Closes rust-lang#74615 Closes rust-lang#123216 Closes rust-lang#49572 Closes rust-lang#93514 This PR adds four codegen tests. The FileCheck assertions were generated with the help of `update_test_checks.py` and `update_llc_test_checks.py` from the LLVM project.

Dylan-DPC · 2025-03-10T19:15:26Z

@veera-sivarajan any updates on this? thanks

veera-sivarajan · 2025-03-11T03:20:48Z

Should we use them? Or should we add so many CHECK?

Yeah, I realized we don't need such stringent checks. Thanks for letting me know.

veera-sivarajan · 2025-03-11T03:40:48Z

Removed a test since it regressed in nightly: https://godbolt.org/z/G3989xP1G

veera-sivarajan · 2025-03-11T21:12:31Z

@rustbot review

Mark-Simulacrum · 2025-03-15T13:58:57Z

@bors r+

bors · 2025-03-15T13:59:00Z

📌 Commit 8c80d2e has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors · 2025-03-15T14:44:14Z

⌛ Testing commit 8c80d2e with merge 1035592...

…r=Mark-Simulacrum Add Three Codegen Tests Closes rust-lang#123216 Closes rust-lang#49572 Closes rust-lang#93514 This PR adds three codegen tests.

rust-log-analyzer · 2025-03-15T16:25:32Z

The job x86_64-msvc-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [assembly] tests\assembly\indexing-with-bools-no-redundant-instructions.rs stdout ----

error: verification with 'FileCheck' failed
status: exit code: 1
command: PATH=";C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.26100.0\x64;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.43.34808\bin\HostX64\x64;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.43.34808\bin\HostX64\x64;D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage0-bootstrap-tools\x86_64-pc-windows-msvc\release\deps;D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage0\bin;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Users\runneradmin\bin;D:\a\rust\rust\ninja;D:\a\rust\rust\sccache;C:\Program Files\MongoDB\Server\7.0\bin;C:\vcpkg;C:\tools\zstd;C:\hostedtoolcache\windows\stack\3.3.1\x64;C:\cabal\bin;C:\ghcup\bin;C:\mingw64\bin;C:\Program Files\dotnet;C:\Program Files\MySQL\MySQL Server 8.0\bin;C:\Program Files\R\R-4.4.2\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\SeleniumWebDrivers\ChromeDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\usr\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.23.7\x64\bin;C:\hostedtoolcache\windows\Python\3.9.13\x64\Scripts;C:\hostedtoolcache\windows\Python\3.9.13\x64;C:\hostedtoolcache\windows\Ruby\3.3.7\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\17.0.14-7\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\ProgramData\Chocolatey\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files\Microsoft SQL Server\150\Tools\Binn;C:\Program Files\dotnet;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\WiX Toolset v3.14\bin;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\140\DTS\Binn;C:\Program Files\Microsoft SQL Server\150\DTS\Binn;C:\Program Files\Microsoft SQL Server\160\DTS\Binn;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\CMake\bin;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.9.9\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\mongosh;C:\Program Files\LLVM\bin;C:\Program Files (x86)\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.43.34808\bin\HostX64\x64" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\llvm\\build\\bin\\FileCheck.exe" "--input-file" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\test\\assembly\\indexing-with-bools-no-redundant-instructions\\indexing-with-bools-no-redundant-instructions.s" "D:\\a\\rust\\rust\\tests\\assembly\\indexing-with-bools-no-redundant-instructions.rs" "--check-prefix=CHECK" "--allow-unused-prefixes" "--dump-input-context" "100"
stdout: none
--- stderr -------------------------------
D:\a\rust\rust\tests\assembly\indexing-with-bools-no-redundant-instructions.rs:14:12: error: CHECK: expected string not found in input
 // CHECK: testl %esi, %esi
           ^
D:\a\rust\rust\build\x86_64-pc-windows-msvc\test\assembly\indexing-with-bools-no-redundant-instructions\indexing-with-bools-no-redundant-instructions.s:15:3: note: scanning from here
f:
  ^
D:\a\rust\rust\build\x86_64-pc-windows-msvc\test\assembly\indexing-with-bools-no-redundant-instructions\indexing-with-bools-no-redundant-instructions.s:16:2: note: possible intended match here
 testb %dl, %dl
 ^

Input file: D:\a\rust\rust\build\x86_64-pc-windows-msvc\test\assembly\indexing-with-bools-no-redundant-instructions\indexing-with-bools-no-redundant-instructions.s
Check file: D:\a\rust\rust\tests\assembly\indexing-with-bools-no-redundant-instructions.rs

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            1:  .def @feat.00; 
            2:  .scl 3; 
            3:  .type 0; 
            4:  .endef 
            5:  .globl @feat.00 
            6: .set @feat.00, 0 
            7:  .file "indexing_with_bools_no_redundant_instructions.43d038d86a482d16-cgu.0" 
            8:  .def f; 
            9:  .scl 2; 
           10:  .type 32; 
           11:  .endef 
           12:  .section .text,"xr",one_only,f 
           13:  .globl f 
           14:  .p2align 4 
           15: f: 
check:14'0       X error: no match found
           16:  testb %dl, %dl 
check:14'0     ~~~~~~~~~~~~~~~~
check:14'1      ?               possible intended match
           17:  je .LBB0_2 
check:14'0     ~~~~~~~~~~~~
           18:  xorb %r8b, %cl 
check:14'0     ~~~~~~~~~~~~~~~~
           19:  orb $1, (%r9) 
check:14'0     ~~~~~~~~~~~~~~~
           20: .LBB0_2: 
check:14'0     ~~~~~~~~~
           21:  movzbl %cl, %eax 
check:14'0     ~~~~~~~~~~~~~~~~~~
           22:  andl $1, %eax 
check:14'0     ~~~~~~~~~~~~~~~
           23:  shll $4, %eax 
check:14'0     ~~~~~~~~~~~~~~~
           24:  orb $1, (%r9,%rax) 
check:14'0     ~~~~~~~~~~~~~~~~~~~~
           25:  retq 
check:14'0     ~~~~~~
           26:  
check:14'0     ~
>>>>>>
------------------------------------------

---
test result: FAILED. 495 passed; 1 failed; 40 ignored; 0 measured; 0 filtered out; finished in 27.52s

Some tests failed in compiletest suite=assembly mode=assembly host=x86_64-pc-windows-msvc target=x86_64-pc-windows-msvc
Build completed unsuccessfully in 1:34:06
make: *** [Makefile:113: ci-msvc-py] Error 1
  local time: Sat Mar 15 16:25:18 CUT 2025
  network time: Sat, 15 Mar 2025 16:25:18 GMT
##[error]Process completed with exit code 2.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version

bors · 2025-03-15T16:26:09Z

💔 Test failed - checks-actions

veera-sivarajan · 2025-03-18T05:09:35Z

@rustbot author

scottmcm · 2025-03-20T20:11:14Z

tests/assembly/indexing-with-bools-no-redundant-instructions.rs

+    let mut a = a & 1 != 0;
+    if b {
+        a ^= c;
+        d[0] |= 1;
+    }
+    d[a as usize] |= 1;


This is a bunch of kinda-random unjustified code. What's special about it? What makes it worth us having a test for it? Can we focus it way more somehow?

I ask because this is the kind of test that tends to rot -- people look at it, there's one assembly instruction different, go "eh, probably fine" and just update the CHECKs, quickly leading to it not really testing anything that matters any more.

What changed? Where was it fixed? Can we add a more specific test somehow?

If we don't already have a test for "we can eliminate the bounds check for indexing with bool into a [T; 2]" that sounds like a good test. But this particular combination with xors and such? I'm just not convinced.

Especially since, as you can see by the failure in bors, doing this in assembly coupled it to the calling convention, so it can't pass on both linux and windows with hard-coded register names like this.

rustbot assigned Mark-Simulacrum Dec 21, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 21, 2024