tests: add sanity-check assembly test for every target by davidtwco · Pull Request #118708 · rust-lang/rust
rustbot
added
A-testsuite
labels
Dec 7, 2023
bors
added
S-waiting-on-bors
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.labels
Dec 11, 2023matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
Dec 11, 2023…t, r=Mark-Simulacrum tests: add sanity-check assembly test for every target Adds a basic assembly test checking that each target can produce assembly and update the target tier policy to require this. cc rust-lang/compiler-team#655 r? `@wesleywiser`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
Dec 11, 2023…t, r=Mark-Simulacrum tests: add sanity-check assembly test for every target Adds a basic assembly test checking that each target can produce assembly and update the target tier policy to require this. cc rust-lang/compiler-team#655 r? ``@wesleywiser``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
Dec 11, 2023…t, r=Mark-Simulacrum tests: add sanity-check assembly test for every target Adds a basic assembly test checking that each target can produce assembly and update the target tier policy to require this. cc rust-lang/compiler-team#655 r? ```@wesleywiser```
bors
added
S-waiting-on-author
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.labels
Dec 11, 2023
bors
added
S-waiting-on-review
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.labels
Jan 15, 2024Adds a basic assembly test checking that each target can produce assembly and update the target tier policy to require this. Signed-off-by: David Wood <david@davidtw.co>
In LLVM 17, PowerPC targets started including function pointer alignments in data layouts, and in Rust's update to that version (rust-lang#114048), we added the function pointer alignments. `powerpc64-unknown-linux-musl` had `Fi64` set but this seems incorrect, and the code in LLVM would always have computed `Fn32` because it is a MUSL target. Signed-off-by: David Wood <david@davidtw.co>
bors
added
S-waiting-on-bors
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.labels
Jan 17, 2024davidtwco added a commit to davidtwco/rust that referenced this pull request
Jan 17, 2024Don't skip the inconsistent data layout check for custom LLVMs. With rust-lang#118708, all targets will have a simple test that would trigger this check if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this check won't trigger with our LLVM because each target will have been confirmed to work. With non-builtin targets, this check is probably useful to have because you can change the data layout in your target and if its wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. Signed-off-by: David Wood <david@davidtw.co>
davidtwco added a commit to davidtwco/rust that referenced this pull request
Jan 17, 2024Don't skip the inconsistent data layout check for custom LLVMs. With rust-lang#118708, all targets will have a simple test that would trigger this check if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this check won't trigger with our LLVM because each target will have been confirmed to work. With non-builtin targets, this check is probably useful to have because you can change the data layout in your target and if its wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. Signed-off-by: David Wood <david@davidtw.co>
davidtwco added a commit to davidtwco/rust that referenced this pull request
Jan 18, 2024Don't skip the inconsistent data layout check for custom LLVMs. With rust-lang#118708, all targets will have a simple test that would trigger this check if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this check won't trigger with our LLVM because each target will have been confirmed to work. With non-builtin targets, this check is probably useful to have because you can change the data layout in your target and if its wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. Signed-off-by: David Wood <david@davidtw.co>
fmease added a commit to fmease/rust that referenced this pull request
Jan 23, 2024…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - `@Mark-Simulacrum` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
fmease added a commit to fmease/rust that referenced this pull request
Jan 24, 2024…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - ``@Mark-Simulacrum`` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
fmease added a commit to fmease/rust that referenced this pull request
Jan 24, 2024…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - ```@Mark-Simulacrum``` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
fmease added a commit to fmease/rust that referenced this pull request
Jan 24, 2024…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - ````@Mark-Simulacrum```` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Jan 27, 2024…wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - `@Mark-Simulacrum` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters