ziglang / zig-bootstrap

take off every zig
390 stars 91 forks source link

handle missing op freeze #186

Closed yxd-ym closed 2 months ago

yxd-ym commented 2 months ago

it seems that freeze is missing from PromoteFloatResult

alexrp commented 2 months ago

Do you have some LLVM IR that reproduces the issue?

nektro commented 2 months ago

patches here need to be sent to https://github.com/llvm/llvm-project or https://github.com/ziglang/zig and then pulled back in

andrewrk commented 2 months ago

This project does not carry functional patches from LLVM, only patches to help get it to build from source. You have to work with LLVM upstream if you want this change.

yxd-ym commented 2 months ago

Do you have some LLVM IR that reproduces the issue?

No, I don't.

I checked the error message Do not know how to promote this operator's result! and found the following code piece

https://github.com/ziglang/zig-bootstrap/blob/a234f004db8c5732456587a42c20a4c00913d738/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp#L2578-L2588

I found this OP by changing the error log message to output the OP name because the default branch is really suspicious.

And when bootstrapping loongarch64-linux-musl target again, the error message outputs that the OP that causing this error is FREEZE.

I think this OP is generated by compiling some code of zig (maybe musl?). But I've no idea which piece of code makes the compiler generates FREEZE op.

yxd-ym commented 2 months ago

This project does not carry functional patches from LLVM, only patches to help get it to build from source. You have to work with LLVM upstream if you want this change.

OK, got it.

yxd-ym commented 2 months ago

Do you have some LLVM IR that reproduces the issue?

@alexrp

OK, I've got some from this commit: https://github.com/llvm/llvm-project/commit/0019c2f194a5e1f4cd65c5284e204328cc40ab3d

define half @freeze_half() nounwind {
  %y1 = freeze half undef
  %t1 = fadd half %y1, %y1
  ret half %t1
}
alexrp commented 2 months ago

Seems to be a good repro: https://godbolt.org/z/PPfvWjjG5

I think your proposed fix is reasonable, especially considering that commit. I'm familiar with the LLVM contribution process now, so let me know if you'd like any help submitting the fix upstream. 🙂

yxd-ym commented 2 months ago

I created a PR to llvm https://github.com/llvm/llvm-project/pull/107791

andrewrk commented 2 months ago

Nice work. If it is merged upstream we can carry the patch

yxd-ym commented 2 months ago

I created a PR to llvm llvm/llvm-project#107791

@alexrp This PR is merged into main. However, I've no idea how to get it released in 19.x. Since llvm 19.1.0 is already released, maybe we need to figure it out how to get it into 19.1.1 (which is 2 weeks later?).

alexrp commented 2 months ago

I think you would need to ask the LLVM LoongArch maintainers (ping them on the PR) if they think it's worth backporting for the next patch release.

But if they prefer not to backport it for some reason, then as Andrew said, we can apply the patch to this repo until LLVM 20 since it has been merged upstream.