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

Not supported instr error #95

Closed
bugadani opened this issue Nov 29, 2021 · 12 comments
Closed

Not supported instr error #95

bugadani opened this issue Nov 29, 2021 · 12 comments
Assignees

Comments

@bugadani
Copy link

bugadani commented Nov 29, 2021

I have the following snippet:

pub enum Enum<'a> {
    A(&'a str),
    B { ptr: usize, len: usize },
    C(&'a [u8]),
    D(u8),
}

impl Enum<'_> {
    pub(crate) fn foo(&self, tmp: &mut Vec<u8>) {
        match self {
            Self::A(_) => tmp.push(0),
            Enum::B { .. } => tmp.push(1),
            Enum::C(_) => tmp.push(2),
            Enum::D(_) => tmp.push(3),
        }
    }
}

pub struct B<'a> {
    slice: &'a [Enum<'a>],
}
impl B<'_> {
    pub fn foo(&self, tmp: &mut Vec<u8>) {
        for e in self.slice {
            e.foo(tmp);
        }
    }
}

Trying to compile it, the build fails with: LLVM ERROR: Not supported instr: <MCInst 271 <MCOperand Reg:42>>. Slight modifications of this example can halt the compiler in a (seemingly, I got bored after 15 minutes of waiting) infinite loop.

This is a minimized repro. What's especially interesting is that deleting the D variant causes the code to compile. However, on the original code, having the A-B-C variants still fails.

I am trying to build for an ESP32 using the 1.56.0.1 rustc version. Build fails both locally (version below) and on a freshly set-up ubuntu GHA runner.

❯ rustc +esp -vV
rustc 1.56.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-pc-windows-msvc
release: 1.56.0-dev
LLVM version: 12.0.1
@bugadani
Copy link
Author

@MabezDev
Copy link
Member

Thank you @bugadani! The repro project is very useful, hopefully we should be able to track this down relatively quickly.

@MabezDev MabezDev self-assigned this Nov 30, 2021
@MabezDev
Copy link
Member

MabezDev commented Nov 30, 2021

Ouch, maybe not as simple as I thought. I can reproduce the error with your repo, however when I ask rustc to emit llvm ir it magically compiles...

Do you see the same behavior?

RUSTFLAGS='--emit=llvm-ir' cargo +esp-dev rustc --target=xtensa-esp32-espidf -Z build-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort

@bugadani
Copy link
Author

If this is similar enough:

❯ cargo +esp rustc --target=xtensa-esp32-espidf -Z build-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort -- --emit=llvm-ir

Then no, I have this output, without any .ll files:

Compiling foolib v0.1.0 (C:\_OpenSource\idftest\foolib)
LLVM ERROR: Not supported instr: <MCInst 271 <MCOperand Reg:42>>
error: could not compile `foolib`
warning: build failed, waiting for other jobs to finish...
error: build failed

@MabezDev
Copy link
Member

MabezDev commented Nov 30, 2021

❯ cargo +esp rustc --target=xtensa-esp32-espidf -Z build-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort -- --emit=llvm-ir

Did not emit .ll files for me either, but setting RUSTFLAGS did. Once it did, the LLVM error went away on my machine 🤔.

@bugadani
Copy link
Author

bugadani commented Dec 1, 2021

You are right, I see the same thing on my Windows machine. Interesting, I wouldn't have expected any difference between RUSTFLAGS and cli arguments.

@MabezDev
Copy link
Member

Hi @bugadani, we've fixed this issue in LLVM and should be available in the next release! Will keep this open until its available publicly.

@MabezDev
Copy link
Member

Thanks for your patience @bugadani, this should now be fixed in the 1.58 release of the fork (includes the fix inside our LLVM 13 release). Please let me know if you encounter any more issues :)

@bugadani
Copy link
Author

Thanks! I'll return with some feedback as soon as the binaries are released. In the mean time, could you please link the commit that fixed the issue? For one, it should be a good reference here, and I'm also interested in what went wrong :) Thank you!

@MabezDev
Copy link
Member

I'm glad you asked actually! I just realised the PR hasn't actually made it into the llvm13 release just yet. Will reopen this issue (sorry for the noise!).

As for what went wrong, the esp32 has hardware loop registers and in your test case it was possible to generate multiple loop entries which weren't being handled correctly in the backend.

@MabezDev MabezDev reopened this Jan 14, 2022
@bugadani
Copy link
Author

Yup, 1.58.0.0 loops without making progress. At least I cancelled the build after 38 minutes that usually takes like 5.

@MabezDev
Copy link
Member

This is now fixed in the 1.59 release, the specif commit in LLVM is espressif/llvm-project@d735e99. Thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants