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

asm! evaluates read-write constraint ("+r") expression twice #14936

Closed
pnkfelix opened this issue Jun 16, 2014 · 1 comment · Fixed by #16606
Closed

asm! evaluates read-write constraint ("+r") expression twice #14936

pnkfelix opened this issue Jun 16, 2014 · 1 comment · Fixed by #16606
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`)

Comments

@pnkfelix
Copy link
Member

When you use "=r"(x), it evaluates x once.

When you use "+r"(x), it evaluates x twice.

You would not notice the multiple evaluation of x when it is just a variable reference. But when it is another expression, you can observe it.

I suspect this is just an oversight in the construction of the asm! macro (in that it should expand into a single occurrence of each of its operands, but it is probably expanding into two occurrences of the output operand expression when it is +r).

Here is some demo code:

#![feature(macro_rules)]
#![feature(asm)]

type History = Vec<&'static str>;

fn wrap<A>(x:A, where: &'static str, history: &mut History) -> A {
    history.push(where);
    x
}

macro_rules! demo {
    ( $output_constraint:tt ) => {
        {
            let mut x: int = 0;
            let y: int = 1;

            let mut history: History = vec!();
            unsafe {
                asm!("mov ($1), $0"
                     : $output_constraint (*wrap(&mut x, "out", &mut history))
                     : "r"(&wrap(y, "in", &mut history)));
            }
            assert_eq!((x,y), (1,1));
            assert_eq!(history.as_slice(), &["out", "in"]);
        }
    }
}

#[cfg(target_arch = "x86")]
#[cfg(target_arch = "x86_64")]
fn main() {
    fn out_write_only_expr_then_in_expr() {
        demo!("=r")
    }

    fn out_read_write_expr_then_in_expr() {
        demo!("+r")
    }

    out_write_only_expr_then_in_expr();
    out_read_write_expr_then_in_expr();
}

#[cfg(not(target_arch = "x86"), not(target_arch = "x86_64"))]
pub fn main() {}

Transcript of run (note the assertion failure shows that the out case is evaluated multiple times):

% rustc --version
rustc 0.11.0-pre (7ec7805 2014-06-16 08:16:49 +0000)
host: x86_64-apple-darwin
% rustc asm-multiple-eval.rs && ./asm-multiple-eval
task '<main>' failed at 'assertion failed: `(left == right) && (right == left)` (left: `[out, in, out]`, right: `[out, in]`)', asm-multiple-eval.rs:37
% 
pnkfelix added a commit to pnkfelix/rust that referenced this issue Jun 17, 2014
…puts.

Fix rust-lang#14962.

Also reordered the fields in libsyntax to reflect the order that the
expressions occur in an instance of `asm!`, as an attempt to remind
others of this ordering.

Finally, added a couple notes about cases that may need to be further
revised when/if rust-lang#14936 is addressed.
@pczarn
Copy link
Contributor

pczarn commented Jul 1, 2014

I'm pretty sure that trans has to take over the expanding. A bool can indicate a read-write constraint: pub outputs: Vec<(InternedString, Gc<Expr>, bool)>,

Shifting trivial code from libsyntax to librustc is unfortunate.

pczarn added a commit to pczarn/rust that referenced this issue Aug 19, 2014
Stop read+write expressions from expanding into two occurences
in the AST. Add a bool to indicate whether an operand in output
position if read+write or not.

Fixes rust-lang#14936
bors added a commit that referenced this issue Aug 20, 2014
It's unfortunate that the read+write operands need special treatment in the AST. A separate vec for all expressions is an alternative, but it doesn't play nicely with trans.

Fixes #14936
@Centril Centril added the A-inline-assembly Area: Inline assembly (`asm!(…)`) label Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants