[Test][AutoDiff] Replace sed command with echo/cat for internal shell compatibility #85842

emirariemir · 2025-12-04T19:04:03Z

Refactored AutoDiff test files that contains REQUIRES: shell by replacing the line:

// RUN: sed -e 's/import Swift$/import Swift; import _Differentiation/' %t/tmp.sil > %t/tmp_fixed.sil

with echo and cat commands:

// RUN: echo "import _Differentiation" > %t/tmp_preamble
// RUN: cat %t/tmp_preamble %t/tmp.sil > %t/tmp_fixed.sil

to make it compatible with LLVM Lit internal shell.

Partially address #84407

…itness.sil file

…_type.swift file Convert `sed` command with `echo` and `cat` commands to achieve the same logic for compatibility with LLVM Lit internal shell.

Convert the existing `sed` commands in `AutoDiff` test files that use `REQUIRES: shell` into equivalent logic using `echo` and `cat` for compatibility with LLVM Lit’s internal shell.

asl · 2025-12-04T19:37:49Z

test/AutoDiff/SIL/Serialization/differentiable_function_type.swift

 // https://github.com/apple/swift/issues/54526
 // Workaround because import declarations are not preserved in .sib files.
-// RUN: sed -e 's/import Swift$/import Swift; import _Differentiation/' %t/tmp.sil > %t/tmp_fixed.sil
+// RUN: echo "import _Differentiation" > %t/tmp_preamble


I do not recall all all restrictions of internal shell, does one of the following work instead?

cat <(echo "import _Differentiation") tmp.sil > tmp_fixed.sil (echo "import _Differentiation"; cat tmp.sil) > tmp_fixed.sil

or even

echo "import _Differentiation" | cat - tmp.sil > tmp_fixed.sil

I think that the <(...) syntax is only supported by BASH and not by internal shell, so that test would fail in Windows CI or shells other than Bash. I think keeping it as two atomic, standard commands is the most robust approach (thanks to @hnrklssn for suggesting this approach in the first place):

// RUN: echo "import _Differentiation" > %t/tmp_fixed.sil // RUN: cat %t/tmp.sil >> %t/tmp_fixed.sil

Is this so for pipe as well? I believe this is something that should be supported (see https://github.com/llvm/llvm-project/blob/2a10e91d8f2edf20bb0e6680e668b327334dc6e8/llvm/utils/lit/tests/Inputs/shtest-shell/diff-pipes.txt)

Chiming in here, lit's shell doesn't give you much to work with. You can't use process substitution and you can't use echo if it's piped to another command. At least with the two line approach it keeps things simple.

Pipes are supported in general, the one limitation is that the built-in version of echo does not support pipes. Another option would be to create a file test/AutoDiff/SIL/import_Differentiation.txt containing import _Differentiation, and use cat %S/import_Differentiation.txt %t/tmp.sil > %t/tmp_fixed.sil. No strong preference from my end though.

ramonasuncion · 2025-12-04T21:10:27Z

LGTM!

ramonasuncion · 2025-12-04T21:10:31Z

? @swift-ci please smoke test

ramonasuncion · 2025-12-04T21:39:12Z

The bot clocked out for the day. 🪫🪫🪫

hnrklssn · 2025-12-05T00:17:38Z

@swift-ci please smoke test

hnrklssn · 2025-12-05T00:18:36Z

The bot clocked out for the day. 🪫🪫🪫

magic touch ;)

emirariemir · 2025-12-05T06:27:00Z

Oh no, Windows platform test failed 😭 ! I cannot see the cause in details, it says:

No problems were identified. If you know why this problem occurred, please add a suitable Cause for it.

How do I identify the problem that makes the test fail?

hnrklssn · 2025-12-05T06:41:25Z

Oh no, Windows platform test failed 😭 ! I cannot see the cause in details, it says:

No problems were identified. If you know why this problem occurred, please add a suitable Cause for it.

How do I identify the problem that makes the test fail?

These two tests failed:


Failed Tests (2):
  Swift(windows-x86_64) :: AutoDiff/SIL/differentiability_witness_function_inst.sil
  Swift(windows-x86_64) :: AutoDiff/SIL/sil_differentiability_witness.sil

If you click on the failed CI bot, and then go to "View as plain text" you can see the build and test log (where the above excerpt is from). I'm on the phone so I can't really see what the actual issue was, but if you cmd+f for those paths you should find the test output for them. Let me know if you need more help, I can take a look tomorrow. Normally when only Windows fails it's related to either newlines or path separators being different.

emirariemir · 2025-12-05T11:52:08Z

If you click on the failed CI bot, and then go to "View as plain text" you can see the build and test log (where the above excerpt is from).

Thanks so much for the explanation! I’ll check the logs and try to fix it. If I run into issues, I’ll reach out again.

hnrklssn · 2025-12-05T18:23:06Z

It looks like // IRGEN: @fooWJrSUUpSr = external global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] doesn't match @fooWJrSUUpSr = external dllimport global %swift.differentiability_witness, align 8 because of the extra dllimport on Windows. These tests were not running on Windows previously, since REQUIRES: shell is never fulfilled on Windows, so we'll have to make some small adjustments. Something like // IRGEN: @fooWJrSUUpSr = external {{(dllimport)?}} global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] should fix it.

asl · 2025-12-05T18:25:36Z

It looks like // IRGEN: @fooWJrSUUpSr = external global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] doesn't match @fooWJrSUUpSr = external dllimport global %swift.differentiability_witness, align 8 because of the extra dllimport on Windows. These tests were not running on Windows previously, since REQUIRES: shell is never fulfilled on Windows, so we'll have to make some small adjustments. Something like // IRGEN: @fooWJrSUUpSr = external {{(dllimport)?}} global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] should fix it.

Yeah, this is what normally done in tests. Or just ignore everything between external and global, e.g.

// IRGEN: @fooWJrSUUpSr = external {{.*}} global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]]

emirariemir · 2025-12-06T11:07:29Z

It looks like // IRGEN: @fooWJrSUUpSr = external global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] doesn't match @fooWJrSUUpSr = external dllimport global %swift.differentiability_witness, align 8 because of the extra dllimport on Windows. These tests were not running on Windows previously, since REQUIRES: shell is never fulfilled on Windows, so we'll have to make some small adjustments. Something like // IRGEN: @fooWJrSUUpSr = external {{(dllimport)?}} global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] should fix it.

Yeah, this is what normally done in tests. Or just ignore everything between external and global, e.g.
// IRGEN: @fooWJrSUUpSr = external {{.*}} global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+

Thanks for the explanation! I also checked the other failing test file, AutoDiff/SIL/sil_differentiability_witness.sil, and it looks like there is a similar Windows-specific mismatch there as well. The check // IRGEN-LABEL: @externalFn1WJrSpSr ={{( protected)?}} global { ptr, ptr } { ... does not match the Windows IR: @externalFn1WJrSpSr = dllexport global { ptr, ptr } ....

I believe this could be fixed in a similar way by allowing dllexport explicitly:

// IRGEN-LABEL: @externalFn1WJrSpSr ={{( dllexport)?( protected)?}} global { ptr, ptr } {...

But as @asl mentioned, it could also be fixed by ignoring everything between = and global:

// IRGEN-LABEL: @externalFn1WJrSpSr ={{.*}}global { ptr, ptr } {

But I couldn't figure out which is the best way to approach this. Allowing only the known platform differences seems safer to me, since a fully generic {{.*}} might hide unexpected attribute regressions. I am not sure if that could be a case so thats why I need further guidance before I proceed.

…ndows dllimport and dllexport This change updates the affected `IRGEN` and `IRGEN-LABEL` checks to explicitly allow optional `dllimport` and `dllexport` attributes using regex patterns, while keeping the rest of the checks strict.

emirariemir · 2025-12-07T10:04:43Z

It looks like // IRGEN: @fooWJrSUUpSr = external global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] doesn't match @fooWJrSUUpSr = external dllimport global %swift.differentiability_witness, align 8 because of the extra dllimport on Windows. These tests were not running on Windows previously, since REQUIRES: shell is never fulfilled on Windows, so we'll have to make some small adjustments. Something like // IRGEN: @fooWJrSUUpSr = external {{(dllimport)?}} global %swift.differentiability_witness, align [[PTR_ALIGNMENT:[0-9]+]] should fix it.

I’ve updated the failing tests by adding optional dllimport and dllexport handling to the relevant FileCheck patterns, following the approach suggested by @hnrklssn. The changes are now up to date and ready for another review.

If you’d prefer {{.*}} approach, I can switch to that as well — happy to adjust based on feedback.

hnrklssn · 2025-12-07T18:51:47Z

@swift-ci please smoke test

emirariemir added 3 commits December 4, 2025 21:24

[Test][AutoDiff] Replace sed with echo/cat in sil_differentiability_w…

5e44c04

…itness.sil file

[Test][AutoDiff] Replace sed with echo/cat in differentiable_function…

8f5d318

…_type.swift file Convert `sed` command with `echo` and `cat` commands to achieve the same logic for compatibility with LLVM Lit internal shell.

[Test][AutoDiff] Replace sed with echo/cat for Lit shell

30451f9

Convert the existing `sed` commands in `AutoDiff` test files that use `REQUIRES: shell` into equivalent logic using `echo` and `cat` for compatibility with LLVM Lit’s internal shell.

emirariemir requested a review from asl as a code owner December 4, 2025 19:04

asl reviewed Dec 4, 2025

View reviewed changes

emirariemir requested review from asl, hnrklssn and ramonasuncion December 7, 2025 10:04

[Test][AutoDiff] Replace sed command with echo/cat for internal shell compatibility #85842

Are you sure you want to change the base?

[Test][AutoDiff] Replace sed command with echo/cat for internal shell compatibility #85842

Conversation

emirariemir commented Dec 4, 2025

Uh oh!

asl Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

emirariemir Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

asl Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

ramonasuncion Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hnrklssn Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

ramonasuncion commented Dec 4, 2025

Uh oh!

ramonasuncion commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ramonasuncion commented Dec 4, 2025

Uh oh!

hnrklssn commented Dec 5, 2025

Uh oh!

hnrklssn commented Dec 5, 2025

Uh oh!

emirariemir commented Dec 5, 2025

Uh oh!

hnrklssn commented Dec 5, 2025

Uh oh!

emirariemir commented Dec 5, 2025

Uh oh!

hnrklssn commented Dec 5, 2025

Uh oh!

asl commented Dec 5, 2025

Uh oh!

emirariemir commented Dec 6, 2025

Uh oh!

emirariemir commented Dec 7, 2025

Uh oh!

hnrklssn commented Dec 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ramonasuncion Dec 4, 2025 •

edited

Loading

ramonasuncion commented Dec 4, 2025 •

edited

Loading