Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing support for asm! on s390x #85931

Closed
uweigand opened this issue Jun 2, 2021 · 15 comments
Closed

Missing support for asm! on s390x #85931

uweigand opened this issue Jun 2, 2021 · 15 comments
Labels
A-inline-assembly Area: inline asm!(..) F-asm `#![feature(asm)]` (not `llvm_asm`) O-SystemZ Target: SystemZ processors (s390x)

Comments

@uweigand
Copy link
Contributor

uweigand commented Jun 2, 2021

The new asm! feature implemented in #69171 is currently not supported on the s390x target. It looks like we need an implementation of the platform-specific registers and register classes in compiler/rustc_target/src/asm/s390x.rs

@jonas-schievink jonas-schievink added A-inline-assembly Area: inline asm!(..) F-asm `#![feature(asm)]` (not `llvm_asm`) O-SystemZ Target: SystemZ processors (s390x) labels Jun 2, 2021
@edelsohn
Copy link

A financial bounty from IBM to solve this issue is open for discussion.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Jul 31, 2021

Im interested in tackling this. Is there anywhere I can find documentation about s390x architecture? Online resources aren't yielding much..

@edelsohn
Copy link

IBMz ISA

@uweigand
Copy link
Contributor Author

Im interested in tackling this. Is there anywhere I can find documentation about s390x architecture? Online resources aren't yielding much..

(Sorry for the late reply, I've been out on vacation ...)

Thanks for looking into this! In addition to the ISA document David linked to above, here's the ABI documentation (if needed):
https://github.com/IBM/s390x-abi/releases

And here is the list of assembler constraints supported by GCC for the platform:
https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html
(look for the section "S/390 and zSeries").

If you have any further questions, I'll be happy to help.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Aug 18, 2021

I do have a question/problem, but am unsure exactly what the issue is, and where to begin to look for help. I have added register definitions for r[0-15] and f[0-15], and am testing the general purpose register definitions using LLVM's FileCheck tool. The only constraint codes I have added so far are r and f.

definitions can be found here
test can be found here

When I test s390x-types.rs I receive this error:

/home/linux1/rust/src/test/assembly/asm/s390x-types.rs:82:17: error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: reg_i32:
                ^
/home/linux1/rust/build/s390x-unknown-linux-gnu/test/assembly/asm/s390x-types.s390x/s390x-types.s:1:2: note: scanning from here
 .text
 ^
/home/linux1/rust/build/s390x-unknown-linux-gnu/test/assembly/asm/s390x-types.s390x/s390x-types.s:23:34: note: possible intended match here
 .section .text._ZN11s390x_types7reg_i3217h8eb3eb08541cc025E,"ax",@progbits


           1:  .text
label:82'0      X~~~~ error: no match found
            2:  .file "s390x_types.63e18542-cgu.0"
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            3:  .section .text._ZN11s390x_types9sym_fn_3217h48d94ea6ff47c559E,"ax",@progbits
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            4:  .globl _ZN11s390x_types9sym_fn_3217h48d94ea6ff47c559E
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            5:  .p2align 4
label:82'0     ~~~~~~~~~~~
            6:  .type _ZN11s390x_types9sym_fn_3217h48d94ea6ff47c559E,@function
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
            .
           18:  br %r14
label:82'0     ~~~~~~~~
           19: .Lfunc_end0:
label:82'0     ~~~~~~~~~~~~
           20:  .size _ZN11s390x_types9sym_fn_3217h48d94ea6ff47c559E, .Lfunc_end0-_ZN11s390x_types9sym_fn_3217h48d94ea6ff47c559E
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           21:  .cfi_endproc
label:82'0     ~~~~~~~~~~~~~
           22: 
label:82'0     ~
           23:  .section .text._ZN11s390x_types7reg_i3217h8eb3eb08541cc025E,"ax",@progbits
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
label:82'1                                      ?                                          possible intended match
           24:  .globl _ZN11s390x_types7reg_i3217h8eb3eb08541cc025E
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           25:  .p2align 4
label:82'0     ~~~~~~~~~~~
           26:  .type _ZN11s390x_types7reg_i3217h8eb3eb08541cc025E,@function
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           27: _ZN11s390x_types7reg_i3217h8eb3eb08541cc025E:
label:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           28:  .cfi_startproc
label:82'0     ~~~~~~~~~~~~~~~
            .
            .
            .

From what I can gather, I think the issue is that the assembler isn't able to find external symbols; I think this is happening for one of two reasons:

  1. The code I have added to the rust compiler for defining the s390x registers is not working correctly.
  2. src/test/assembly/asm/s390x-types.rs has logic or syntax errors that my inexperience hinders me from seeing.

I am not sure how much help you can provide, but obviously any would be appreciated. I don't have much experience with assembly in general, and have never programmed on an s390x before. So if this is a stupid question, or there is an obvious answer, forgive me.

@Amanieu
Copy link
Member

Amanieu commented Aug 18, 2021

You are missing #[no_mangle] on the functions.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Aug 18, 2021

I added #[no_mangle] above both sym_fn_32 and reg_i32; it seems to fix the output for sym_fn_32, but nothing changes for reg_i32.

Here is the stderr after the change:

            1:  .text
label:83'0      X~~~~ error: no match found
            2:  .file "s390x_types.63e18542-cgu.0"
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            3:  .section .text.sym_fn_32,"ax",@progbits
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            4:  .globl sym_fn_32
label:83'0     ~~~~~~~~~~~~~~~~~
            5:  .p2align 4
label:83'0     ~~~~~~~~~~~
            6:  .type sym_fn_32,@function
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
            .
           18:  br %r14
label:83'0     ~~~~~~~~
           19: .Lfunc_end0:
label:83'0     ~~~~~~~~~~~~
           20:  .size sym_fn_32, .Lfunc_end0-sym_fn_32
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           21:  .cfi_endproc
label:83'0     ~~~~~~~~~~~~~
           22: 
label:83'0     ~
           23:  .section .text._ZN11s390x_types7reg_i3217h8eb3eb08541cc025E,"ax",@progbits
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
label:83'1                                      ?                                          possible intended match
           24:  .globl _ZN11s390x_types7reg_i3217h8eb3eb08541cc025E
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           25:  .p2align 4
label:83'0     ~~~~~~~~~~~
           26:  .type _ZN11s390x_types7reg_i3217h8eb3eb08541cc025E,@function
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           27: _ZN11s390x_types7reg_i3217h8eb3eb08541cc025E:
label:83'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           28:  .cfi_startproc
label:83'0     ~~~~~~~~~~~~~~~
            .
            .
            .

@uweigand
Copy link
Contributor Author

Well, it's still mangled ... did you add the #[no_mangle] inside the check! macro that actually emits the function? See how this is done e.g. in x86-types.rs.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Aug 18, 2021

That fixes that, but of course there's another issue. I'll post more if I end up needing help.

Thanks for the hand-holding. 😃

@Amanieu
Copy link
Member

Amanieu commented Aug 18, 2021

You need to add it in check_reg as well.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Aug 21, 2021

I am assuming that when FileCheck says match not found, that means that the CHECK ... directives above the code snippets they correspond to are wrong. Is this correct?

For example, this is the code being tested, and the corresponding error:

(In this case, the asm code lgr r{{[0-15]+}}, ... is wrong, and is why FileCheck is failing)

// CHECK-LABEL: reg_i32:
// CHECK: #APP
// CHECK: lgr r{{[0-15]+}}, r{{[0-15]+}}
// CHECK: #NO_APP
check!(reg_i32, i32, reg, "lgr");
       39:  #APP
         40:  lgr %r2, %r13
check:84      X~~~~~~~~~~~~ error: no match found
         41:  #NO_APP
check:84     ~~~~~~~~
         42:  lmg %r13, %r15, 264(%r15)
check:84     ~~~~~~~~~~~~~~~~~~~~~~~~~~
         43:  br %r14
check:84     ~~~~~~~~
         44: .Lfunc_end1:
check:84     ~~~~~~~~~~~~
         45:  .size reg_i32, .Lfunc_end1-reg_i32
check:84     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If the assembly in the check directive is wrong, is there an easy way to determine what the correct assembly is? If this isn't why FileCheck is failing, I am unsure of what else the error could be.

Again, thanks for the help.

@uweigand
Copy link
Contributor Author

The regular expression is incorrect, it should be %r[0-9]+ instead of r[0-15]+.

Note that regular expressions work on characters, they don't understand integer numbers. [0-9]+ matches a sequence of at least one character in the range '0' .. '9', which matches any integer number. [0-15]+ matches a sequence of at least one character in the range '0' .. '1' or '5', which (for example) does not match "13".

Also note the missing '%' character - on s390x, all register numbers are printed with a leading '%', so the regular expression has to match that as well.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Aug 22, 2021

I am curious what further testing should be done before I submit a PR.

These are the tests I have that pass:

  • branching to an external function
  • loading an external symbol
  • 32/64-bit general registers
  • 32/64-bit fp registers

I am also curious about eventual support for the vector/access registers; I have noticed that the LLVM configuration Rust uses for the s390x does not have support for vector registers turned on, and access registers can only be used if access mode is enabled on the psw. Does IBM want support for these registers in inline asm or are they unimportant for now?

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 26, 2021
S390x inline asm

This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests.

Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 26, 2021
S390x inline asm

This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests.

Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
Manishearth added a commit to Manishearth/rust that referenced this issue Aug 27, 2021
S390x inline asm

This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests.

Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 27, 2021
S390x inline asm

This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests.

Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 28, 2021
S390x inline asm

This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests.

Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Aug 28, 2021

Can this issue be closed now?

@nagisa
Copy link
Member

nagisa commented Aug 28, 2021

Yeah, thanks.

@nagisa nagisa closed this as completed Aug 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: inline asm!(..) F-asm `#![feature(asm)]` (not `llvm_asm`) O-SystemZ Target: SystemZ processors (s390x)
Projects
None yet
Development

No branches or pull requests

6 participants