NativeAOT-LLVM: cast unsigned ints to floats using uitofp #1994
Conversation
|
@SingleAccretion Could you please take a look at this change? I guess it might have slipped through your inbox. |
| @@ -4035,7 +4037,15 @@ private void ImportConvert(WellKnownType wellKnownType, bool checkOverflow, bool | |||
| } | |||
| else | |||
| { | |||
| LLVMValueRef converted = CastIfNecessary(loadedValue, GetLLVMTypeForTypeDesc(destType), value.Name(), wellKnownType == WellKnownType.UInt64 /* unsigned is always false, so check for the type explicitly */); | |||
| TypeDesc sourceType = value.Type; | |||
There was a problem hiding this comment.
Who calls ImportConvert?
Edit: I see, it is called from the shared logic.
| LLVMValueRef converted = CastIfNecessary(loadedValue, GetLLVMTypeForTypeDesc(destType), value.Name(), | ||
| wellKnownType == WellKnownType.UInt64 /* unsigned is always false, so check for the type explicitly */ | ||
| || sourceType.IsWellKnownType(WellKnownType.Byte) | ||
| || sourceType.IsWellKnownType(WellKnownType.UInt16) | ||
| || sourceType.IsWellKnownType(WellKnownType.UInt32) | ||
| || sourceType.IsWellKnownType(WellKnownType.UInt64) | ||
| ); |
There was a problem hiding this comment.
How does the ILImporter stack work -- does it keep the precise types?
If it does, it does not look correct to look at them. E. g.
ldobj int64
conv.r.un
Should still be treated as an unsigned cast.
/* unsigned is always false, so check for the type explicitly */
Is that true? I see it should be true for conv.r.un.
There was a problem hiding this comment.
Hmm, no that's not true, let me try to clean this up.
There was a problem hiding this comment.
There were 2 reported problems, using a byte as an index and casting a byte to a float (for an alpha channel). But looking at the code I don't see why I can't just pass the right value from the op code.
| LLVMValueRef converted = CastIfNecessary(loadedValue, GetLLVMTypeForTypeDesc(destType), value.Name(), | ||
| wellKnownType == WellKnownType.UInt64 /* unsigned is always false, so check for the type explicitly */ | ||
| || sourceType.IsWellKnownType(WellKnownType.Byte) | ||
| || sourceType.IsWellKnownType(WellKnownType.UInt16) | ||
| || sourceType.IsWellKnownType(WellKnownType.UInt32) | ||
| || sourceType.IsWellKnownType(WellKnownType.UInt64) | ||
| ); |
There was a problem hiding this comment.
| LLVMValueRef converted = CastIfNecessary(loadedValue, GetLLVMTypeForTypeDesc(destType), value.Name(), | |
| wellKnownType == WellKnownType.UInt64 /* unsigned is always false, so check for the type explicitly */ | |
| || sourceType.IsWellKnownType(WellKnownType.Byte) | |
| || sourceType.IsWellKnownType(WellKnownType.UInt16) | |
| || sourceType.IsWellKnownType(WellKnownType.UInt32) | |
| || sourceType.IsWellKnownType(WellKnownType.UInt64) | |
| ); | |
| LLVMValueRef converted = CastIfNecessary(loadedValue, GetLLVMTypeForTypeDesc(destType), value.Name(), unsigned); |
Presumably that should now work?
I also see there is:
else if ((toStoreKind == LLVMTypeKind.LLVMDoubleTypeKind || toStoreKind == LLVMTypeKind.LLVMFloatTypeKind) &&
valueTypeKind == LLVMTypeKind.LLVMIntegerTypeKind)
{
//TODO: keep track of the TypeDesc so we can call BuildFPToUI when the integer is unsigned
typedToStore = builder.BuildFPToSI(source, valueType, "CastFloatSI" + (name ?? ""));
}
In CastIfNecessary that, could, presumably also be fixed.
|
I've put code to widen the bytes/shorts to ints before the Example LLVM is now store i8 -1, i8* %alpha_local0_, align 1, !dbg !389570
%Loadloc0_ = load i8, i8* %alpha_local0_, align 1, !dbg !389571
%CastZIntloc0_ = zext i8 %Loadloc0_ to i32, !dbg !389571
%CastSIToFloatloc0_ = sitofp i32 %CastZIntloc0_ to float, !dbg !389571
%fpextop2 = fpext float %CastSIToFloatloc0_ to double, !dbg !38957 |
|
ugg, I've messed up the branches somehow... moving to WIP |
yowl
force-pushed
the
unsigned-to-float
branch
from
b1d8400 to
9b11458
Compare
…ntimelab into unsigned-to-float
| TypeDesc destType = GetWellKnownType(wellKnownType); | ||
|
|
||
| // Load the value and then convert it instead of using ValueAsType to avoid loading the incorrect size | ||
| LLVMValueRef loadedValue = value.ValueAsType(value.Type, _builder); | ||
| LLVMValueRef widenedStackValue = CastIfNecessary(loadedValue, GetLLVMTypeForTypeDesc(WidenBytesAndShorts(value.Type)), value.Name(), | ||
| stackType.IsWellKnownType(WellKnownType.Byte) || |
|
@yowl @SingleAccretion Thank you! |
This PR fixes a bug when casting ints to floating points. It assumed signed. If the source is an unsigned int, then make the cast unsigned (https://llvm.org/docs/LangRef.html#uitofp-to-instruction) .
Adds a test using a byte > 0x7f
cc @SingleAccretion