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

Float16T mismatch casts (LLVM-325) #91

Open
kassane opened this issue Feb 26, 2024 · 15 comments
Open

Float16T mismatch casts (LLVM-325) #91

kassane opened this issue Feb 26, 2024 · 15 comments
Assignees
Labels
Status: In Progress work is in progress

Comments

@kassane
Copy link

kassane commented Feb 26, 2024

I was trying to understand the Xtensa floating point alignment, but I got an llvm-ir error about converting floatFromInt on esp32|esp32s3|cnl targets, only esp32s2 had no problem.

I could work around this problem (not solved) by removing the fp (Single FP) and s32c1i or by removing the compiler-rt.

I didn't see any reference to f16 or u16 in the Xtensa target in the llvm-17.0.1 source.

This issue only occurs when creating an executable. Unlike static objects and libraries.

Reference

@github-actions github-actions bot changed the title Float16T mismatch casts Float16T mismatch casts (LLVM-325) Feb 26, 2024
@gerekon
Copy link
Collaborator

gerekon commented Feb 27, 2024

Hi @kassane !

What llvm-project branch do you use? LLVM 17.x release branch? I suppose that floating point support has not been upstreamed yet. You can try using our repo instead (https://github.com/espressif/llvm-project/tree/xtensa_release_17.0.1).
@andreisfr Am I correct?

@kassane
Copy link
Author

kassane commented Feb 27, 2024

I'm using this branch exactly xtensa_release_17.0.1.

@andreisfr
Copy link
Collaborator

@kassane , thank you very much for report, we will investigate the problem.

@kassane
Copy link
Author

kassane commented Mar 23, 2024

Do you know if xtensa enables conversion intrinsic fp16?

Missing on:
https://github.com/espressif/llvm-project/blob/xtensa_release_17.0.1/clang/lib/Basic/Targets/Xtensa.h

bool useFP16ConversionIntrinsics() const override { return /*false or true?*/; }

search link: https://github.com/search?q=repo%3Aespressif%2Fllvm-project%20FP16Conversion&type=code

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

@kassane I am looking into this issue.
Could you try with #92?

@kassane
Copy link
Author

kassane commented Mar 28, 2024

Could you try with #92?

Hi @gerekon,
I had tried before, but the error still persists. XtensaISD::PCREL_WRAPPER TargetConstantPool:i32<i32 31744>

LLVM Emit Object... LLVM ERROR: Cannot select: 0x7d7a4f7d0770: f32 = fp16_to_fp 0x7d7a50359440, float_from_int.zig:46:9 @[ floatsihf.zig:11:24 ]
  0x7d7a50359440: i32 = or 0x7d7a4f84cfc0, 0x7d7a503598a0, float_from_int.zig:46:9 @[ floatsihf.zig:11:24 ]
    0x7d7a4f84cfc0: i32 = AssertZext 0x7d7a50359d70, ValueType:ch:i16, float_from_int.zig:46:9 @[ floatsihf.zig:11:24 ]
      0x7d7a50359d70: i32,ch = CopyFromReg 0x7d7a4f9973e0, Register:i32 %1, float_from_int.zig:46:9 @[ floatsihf.zig:11:24 ]
        0x7d7a4f0c40b0: i32 = Register %1
    0x7d7a503598a0: i32 = XtensaISD::PCREL_WRAPPER TargetConstantPool:i32<i32 31744> 0
      0x7d7a4f0ca650: i32 = TargetConstantPool<i32 31744> 0
In function: __floatsihf
``

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

ok. Sorry for confusion. Do you have a simple scenario to reproduce this?

@kassane
Copy link
Author

kassane commented Mar 28, 2024

ok. Sorry for confusion. Do you have a simple scenario to reproduce this?

No worries!
For testing, try use: https://github.com/kassane/zig-espressif-bootstrap/releases

Make anywhere freestanding example for make executable.

void _start(){}

Command:

# CPU = esp8266**|esp32s3|esp32|esp32s2*|cnl
# *: no issue
# **: no tested yet

# clang cli
zig cc start.c -target xtensa-freestanding-none -mcpu=CPU
# or zig cli
zig build-exe start.c -target xtensa-freestanding-none -mcpu=CPU
# if add -fno-compiler-rt no issue for build-exe or build-lib -dynamic
# replace build-exe to build-lib [staticlib, default] or build-obj, no get error (no compiler-rt by default)

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

Ok. Thanks.
Looks like I managed to reproduce this with Clang clang -O0 --target=xtensa-esp-elf -mcpu=esp32 fp16_test.c

static volatile __fp16 s_val;

int main()
{
  int ival = 5;
  s_val = ival;
  return 0;
}

and got

fatal error: error in backend: Cannot select: 0x5e8bc8788e80: ch = store<(volatile store (s16) into @s_val), trunc to f16> 0x5e8bc8788a90:1, 0x5e8bc8788b00, 0x5e8bc8788be0, undef:i32
  0x5e8bc8788b00: f32 = sint_to_fp 0x5e8bc8788a90
    0x5e8bc8788a90: i32,ch = load<(dereferenceable load (s32) from %ir.2)> 0x5e8bc8788a20, FrameIndex:i32<1>, undef:i32
      0x5e8bc87889b0: i32 = FrameIndex<1>
      0x5e8bc8788860: i32 = undef
  0x5e8bc8788be0: i32 = XtensaISD::PCREL_WRAPPER TargetConstantPool:i32<@s_val = internal global half 0xH0000, align 2> 0
    0x5e8bc8788b70: i32 = TargetConstantPool<@s_val = internal global half 0xH0000, align 2> 0
  0x5e8bc8788860: i32 = undef
In function: main
clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)

Looks like the same problem.

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

@andreisfr @sstefan1 Do you have any idea?

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

@maciej-czekaj Added floating support in #89. And have float intrinsics
https://github.com/espressif/llvm-project/blob/xtensa_release_17.0.1/llvm/include/llvm/IR/IntrinsicsXtensa.td#L253.
Smth wrong happens with int<->f16 conversion.

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

@kassane Ok. Looks like I made a step toward solution. Now @llvm.convert.to.fp16, @llvm.convert.from.fp16 are used for converting. This is default behaviour expanding to calls to __gnu_f2h_ieee and __gnu_h2f_ieee from compiler-rt. Looks like the code missed actions for fp32<->fp16 conversion. But in my example I have linker problems

ld.lld: error: /mnt/host/projects/esp/tmp/build_esp_clang/llvm/bin/../lib/clang-runtimes/xtensa-esp-unknown-elf/esp32/lib/libclang_rt.builtins.a(truncsfhf2.c.obj):(function __truncsfhf2: .text.__truncsfhf2+0xb6): relocation R_XTENSA_SLOT0_OP out of range: 100 is not in [-262141, 18446744073709551612]; references section '.literal.__truncsfhf2'

But that can be specific to my toolchain build only.
I have not tried your scenario yet, so maybe in your case everything works with this changes :-).

But in any case implemented fix makes use of software conversion functions. In future we will need to lower this to HW instructions if possible.

@kassane
Copy link
Author

kassane commented Mar 28, 2024

This is default behaviour expanding to calls to __gnu_f2h_ieee and __gnu_h2f_ieee

Last commit I attempted to turn false gnu_fp16 (similar to riscv32), but didn't get a different result.
https://github.com/kassane/zig/blob/528e739220f091d420f3f693d23c1c29d909dc85/lib/compiler_rt/common.zig#L49C1-L65
While the F16T function xtensa jumps in the else to u16 instead of f16.
https://github.com/kassane/zig/blob/528e739220f091d420f3f693d23c1c29d909dc85/lib/compiler_rt/common.zig#L85C8-L85C12

I have linker problems

Indeed. lld (builtin), zig doesn't allow -fuse-ld= (only -fno-lld [switch to zld]).

But in any case implemented fix makes use of software conversion functions. In future we will need to lower this to HW instructions if possible.

Ok.

@gerekon
Copy link
Collaborator

gerekon commented Mar 28, 2024

Indeed. lld (builtin), zig doesn't allow -fuse-ld= (only -fno-lld [switch to zld]).

In our toolchain problem is solved using GNU ld by passing --ld-path= to clang. Switching to LLD is in plans.

@kassane
Copy link
Author

kassane commented Apr 2, 2024

@gerekon ,

Before applying your current patch, the new error mentioned in the PR occurred when removing the -fp extension. (current toolchain available on releases)

# -mcpu=<esp32|esp32s3>-fp
error: <unknown>:0: Undefined temporary symbol .LBB385_2
# -mcpu=esp32-fp-s32c1i
lld errors: undefined references...

Your patch applied:
-mcpu=esp32-s32c1i
lld errors: undefined references...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress work is in progress
Projects
None yet
Development

No branches or pull requests

4 participants