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

[esp32s2] systimer example doesn't work #140

Closed
MabezDev opened this issue Aug 8, 2022 · 9 comments · Fixed by #167
Closed

[esp32s2] systimer example doesn't work #140

MabezDev opened this issue Aug 8, 2022 · 9 comments · Fixed by #167
Assignees
Labels
bug Something isn't working
Milestone

Comments

@MabezDev
Copy link
Member

MabezDev commented Aug 8, 2022

No description provided.

@MabezDev MabezDev added the bug Something isn't working label Aug 8, 2022
@MabezDev MabezDev self-assigned this Aug 8, 2022
@jessebraham jessebraham added this to the v0.1.0 milestone Aug 11, 2022
@MabezDev
Copy link
Member Author

AFAIK the system timer peripheral is working fine.

The interrupt mechanism for the systimer (and only the systimer, other peripheral interrupts work fine) is behaving very strangely. The interrupt fires, but inside the handler, calling xtensa_lx::interrupt::get() returns 0 (zero CPU interrupts active) and querying the status of the peripheral interrupt peripheral also returns 0. This is extremely odd, how on earth did we get into the interrupt handler if there are no active interrupts 🤔 .

@igrr
Copy link

igrr commented Aug 23, 2022

@MabezDev I think this is because the systimer peripheral on S2 generates edge interrupts, not level interrupts. This means that the interrupt signal goes high briefly and the CPU latches that signal. In order for this to work, the systimer interrupt should be attached to one of the CPU's edge interrupt inputs. In the interrupt handler, you can clear the interrupt via the INTCLR register.
From the description of the issue, it seems that you might be attaching the systimer to a level interrupt input of the CPU.

(Systimer has "edge" interrupt only on S2. On S3 and C3 it was changed to "level" interrupt.)

@MabezDev
Copy link
Member Author

So I had my suspicions it was an edge interrupt (by the way I find no reference of that in the trm, I'll make an issue about that unless you can point me to the correct place!) before your suggestion but when I tried it didn't work. I've reverted to using sys timer interrupts as edge levels and things seem to get better, but I've found an interesting bug around passing u128 into a function that doesn't work properly... it seems so far only on the esp32s2, will test the other Xtensa targets shortly (though I suspect these are working fine).

fn prints_u128(test: u128) {
    esp_println::println!("{}", test);
}

prints_u128(8852131312371371081728);
// outputs: 8856140901064495857664 which is wrong!

@MabezDev
Copy link
Member Author

Nope! They're broken on esp32 & esp32s3 too! They seem to be broken in such a way that it's correct up until the first bit is set, after that, it's seemingly random bits that are set. I guess the "happy" path that typically gets tested (1 level interrupt at a time) wouldn't fail with this current behaviour.

@MabezDev
Copy link
Member Author

Even then though, I can produce some failures.

In debug, the following assert fails, but in release, it works...

let mut one_bit = 1u128 << 65;

let fnr = one_bit.trailing_zeros();
one_bit &= !(1u128 << fnr); // unset the bit
assert_eq!(one_bit, 0);

To be honest I'm starting to wonder how on earth any of this interrupt stuff worked if this has been broken for a while...

@MabezDev
Copy link
Member Author

I've tested on a Rust STD project too, just to rule out any quirky linker script issues or bugs in esp-hal, fails on the STD project too. Looks like a codegen issue. @igrr What's the best way to test 128bit integers in clang, or am I better off trying to reduce the LLVM IR in Rust?

@igrr
Copy link

igrr commented Aug 24, 2022

I'm starting to wonder how on earth any of this interrupt stuff worked if this has been broken for a while...

Does the interrupt functionality depend on 128-bit ints anyhow? The interrupt-related registers are all 32 bit wide.

What's the best way to test 128bit integers in clang, or am I better off trying to reduce the LLVM IR in Rust?

To be honest I didn't expect 128-bit integers to work on a 32-bit target. At least GCC used to define a 128-bit type only on 64-bit targets. Clang also doesn't seem to recognize __int128 on a 32-bit architecture (e.g. armv7-a: https://godbolt.org/z/sv1GajeYs).

So if this is expected to work in the backend then you'll probably have to reduce the issue to LLVM IR...

Edit: yes, seems like 128-bit ints should work in the backend even for the 32-bit targets (e.g. https://godbolt.org/z/Yf88q8oGq).

@MabezDev
Copy link
Member Author

Hmm, I did some testing on some older compilers and it looks like "fixing" the ABI in esp-rs/rust#18 has broken 128-bit integers. I'll track down where it is going wrong.

@MabezDev
Copy link
Member Author

diff --git a/compiler/rustc_target/src/abi/call/xtensa.rs b/compiler/rustc_target/src/abi/call/xtensa.rs
index 561777dcc05..5b34691aef0 100644
--- a/compiler/rustc_target/src/abi/call/xtensa.rs
+++ b/compiler/rustc_target/src/abi/call/xtensa.rs
@@ -90,7 +90,7 @@ fn classify_arg_ty<'a, Ty, C>(arg: &mut ArgAbi<'_, Ty>, arg_gprs_left: &mut u64,
             } else if align == 64 {
                 arg.cast_to(Uniform {
                     unit: Reg::i64(),
-                    total: Size::from_bits((required_gprs / 2) * 32),
+                    total: Size::from_bits((required_gprs / 2) * 64),
                 });
             } else {
                 arg.cast_to(Uniform {

Oops... Accidentally truncated 64bit & 128bit integers by half 😓.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants