Legalize Types - Crash in soften float UINT_TO_FP result.

[bcardosolopes]

Bugzilla Link	2667
Resolution	FIXED
Resolved on	Nov 10, 2008 11:39
Version	trunk
OS	Linux
Attachments	bytecode from newlib lcong48.c
CC	@bcardosolopes,@efriedma-quic

Extended Description

The problem occurs when we have, for example a i16,ch = load... followed by an f64 = uint_to_fp...

$ mips-llc lib_a-erand48.bc -f -enable-legalize-types -debug
Promote integer result: 0xf54748: i16,ch = load 0xf52a48, 0xf52448, 0xf52b48 <0xf39510:0> alignment=2
Soften float result 0: 0xf54848: f64 = uint_to_fp 0xf54748
mips-llc: /p/llvm/svn/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:290: llvm::SDValue llvm::DAGTypeLegalizer::SoftenFloatRes_UINT_TO_FP(llvm::SDNode*): Assertion `LC != RTLIB::UNKNOWN_LIBCALL && "Unsupported UINT_TO_FP!"' failed.

The problem here, is that in method DAGTypeLegalizer::SoftenFloatRes_UINT_TO_FP,
the helper RTLIB::getUINTTOFP doesn't know how to soften i16 -> f64. This happens because the i16 load result is promoted, but it only replaces the chain Uses, not the i16 ones, and since the uint_to_fp result is legalized before its operands, we get this crash.

All targets with native support for f64 doesnt crash, since they dont legalize the result, but only the operand.

One quick hack is to replace the unsupported types (in this case i16) Uses with the new promoted type in PromoteIntRes_LOAD, but this break PromoteOp for other test cases, and IMHO, considering the LegalizeTypes design, it doesnt seems like the right approach.

It's worth mentioning that this bug doesn't exist when legalize types is disabled.

I volunteer to fix this, I would just appreciate some opinions.
Thanks,

[efriedma-quic]

This is a bit messy... the issue is that libgcc only has UINT_TO_FP libcalls for widths >= 32. What we probably want to be doing here is to detect this case in DAGTypeLegalizer::SoftenFloatRes_UINT_TO_FP and promote the operand appropriately. The fact that i16 is illegal on your target is merely a coincidence.

Although actually, the solution to this ought to be more general; it seems quite possible to run into situations like this with almost any libcall, depending on the target.

[bcardosolopes]

What I tried to mean was just what you said: the fact uint_to_fp legalize result step depends on a source operand that was not yet promoted is what generates the problem. You're right, this demands a more general solution.
Thanks,

[llvmbot]

Libcalls exist for source operand of type i32, i64 and i128.
If the operand is to be promoted, there are three cases:
(a) the source is smaller than i32, eg: i16 (probably promoted to i32).
(b) the source is larger than i128, eg: i129 (promoted to i256).
(c) the source is between two valid types, eg: i40 (promoted to i64).

Solutions:

(a) sext or zext as appropriate the operand to i32,
then do the i32 libcall. It doesn't matter if i16/i32
are legal or not: the legalize types machinery will take
care of everything.

(b) I don't know what the semantics of xinttofp are
supposed to be in this case. The obvious (but perhaps
wrong) solution is to truncate the operand to i128 and
do the i128 libcall.

(c) sext or zext as appropriate to the next biggest
round integer, eg: i40 -> i64 then do that libcall.
This is much the same as (a).

[efriedma-quic]

(b) I don't know what the semantics of xinttofp are
supposed to be in this case. The obvious (but perhaps
wrong) solution is to truncate the operand to i128 and
do the i128 libcall.

Truncating won't give the right answer; it'll cut off the top bits. There are possible solutions, but it's not particularly cheap; for an arbitrary-width uinttofp to double, something like the following is possible:
uinttofp(top64bits(n << clz(n))) * powi(2, bitwidth-64-clz(n))

That said, it might be better to just give up; we can't really provide full support for integers > 128 bits without a proper runtime.

[llvmbot]

Fixed in commit 58994.