[clang] warn for overflow on type conversion between __fp16 and char

[Quuxplusone]

Bugzilla Link	PR48665
Status	NEW
Importance	P normal
Reported by	Allen zhong (zhongyunde@tom.com)
Reported on	2021-01-05 06:09:12 -0800
Last modified on	2021-01-05 20:39:24 -0800
Version	trunk
Hardware	PC Windows NT
CC	craig.topper@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, neeilans@live.com, pengfei.wang@intel.com, richard-llvm@metafoo.co.uk, spatel+llvm@rotateright.com
Fixed by commit(s)
Attachments	test_x86.cpp (273 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 24352
clang test_x86.cpp -S -O2 -o test.s

base on the case test_x86.cpp attached and llvm10, compile on the x86 machine
with commands similar as: clang test_x86.cpp -S -O2 -o test.s

#define half __fp16

void host_expect(char * arrDst)
{
    half arrSrc[8] = {(half)0xecee, (half)0xf330, (half)0x698a, (half)0x2583, (half)0xb45d, (half)0xfc66, (half)0x3587, (half)0x2607};

    for(int i = 0; i < 8; i++)
    {
        arrDst[i] = arrSrc[i];
    }
}

we'll get an empty define of the function host_expect, which is unexpected.

        .globl  _Z11host_expectPc       # -- Begin function _Z11host_expectPc
        .p2align        4, 0x90
        .type   _Z11host_expectPc,@function
_Z11host_expectPc:                      # @_Z11host_expectPc
        .cfi_startproc
# %bb.0:                                # %entry
        retq
.Lfunc_end0:
        .size   _Z11host_expectPc, .Lfunc_end0-_Z11host_expectPc
        .cfi_endproc
                                        # -- End function
        .ident  "clang version 10.0.0

[Quuxplusone]

Attached test_x86.cpp (273 bytes, text/plain): clang test_x86.cpp -S -O2 -o test.s

[Quuxplusone]

I think it is an optimization that once compiler knows all converted values are poison, it can remove any instructions related. You can change some of them to a legal value e.g. 0 to see the difference.

[Quuxplusone]

do you mean this is invalid convention for above two type，so a compiler warning is likely need?

[Quuxplusone]

I meant the value are out of range instead of type invalid. It's user's responsibility to make sure the value are always valid. Otherwise, it is undefined behavior and compiler are free to do any optimization without warning.

[Quuxplusone]

See also the part (F.4.1) of the C/C++ standard where it says:
"if the integral part of the floating value exceeds the range of the integer
type, then the ‘‘invalid’’ floating-point exception is raised and the resulting
value is unspecified"

I didn't convert all of those hex values, but if they are all out-of-range (-
128,127) for a byte, then there is no compiler bug.

[Quuxplusone]

Should clang warn about out of range i2f/f2i constant conversions?

[Quuxplusone]

Clang has that warning in general:
https://clang.llvm.org/docs/DiagnosticsReference.html#wfloat-overflow-conversion

And I see it for a most basic float example:
char f2i() {
  return 129.0f;
}

48665.c:2:10: warning: implicit conversion of out of range value from 'float'
to 'char' is undefined [-Wliteral-conversion]

But I'm not seeing it for 'half', and I'm not sure if clang is capable of
unwinding the loop code in the description.

Let's make this a clang-front-end bug to track whether the warnings are
happening as expected. I don't see any x86-specific problems here.

[Quuxplusone]

According https://en.wikipedia.org/wiki/Half-precision_floating-point_format,
The maximum representable value of Half precision is (2−2−10) × 215 = 65504,
and it is larger than the The maximum representable value of type short.

But after I try to change the type char to short as following, I still find
some undef in the dump file with command clang ~/test_x86.cpp -S -O2 -o test.s
-mllvm -print-before-all, am I missing some thing ?

#define half __fp16

void host_expect(short * arrDst)
{
    half arrSrc[8] = {(half)0xecee, (half)0xf330, (half)0x698a, (half)0x2583, (half)0x8001, (half)0x8000, (half)0x3587, (half)0x26};

    for(int i = 0; i < 8; i++)
    {
        arrDst[i] = arrSrc[i];
    }
}

; <label>:2:                                      ; preds = %1
  store i16 undef, i16* %0, align 2, !tbaa !2
  %3 = getelementptr inbounds i16, i16* %0, i64 1
  store i16 undef, i16* %3, align 2, !tbaa !2
  %4 = getelementptr inbounds i16, i16* %0, i64 2
  store i16 27024, i16* %4, align 2, !tbaa !2
  %5 = getelementptr inbounds i16, i16* %0, i64 3
  store i16 9600, i16* %5, align 2, !tbaa !2
  %6 = getelementptr inbounds i16, i16* %0, i64 4
  store i16 undef, i16* %6, align 2, !tbaa !2
  %7 = getelementptr inbounds i16, i16* %0, i64 5
  store i16 undef, i16* %7, align 2, !tbaa !2
  %8 = getelementptr inbounds i16, i16* %0, i64 6
  store i16 13704, i16* %8, align 2, !tbaa !2
  %9 = getelementptr inbounds i16, i16* %0, i64 7
  store i16 38, i16* %9, align 2, !tbaa !2

[Quuxplusone]

> (half)0xf330
[...]
> am I missing some thing ?

0xf330 fits in a 'half' (with some loss of precision), but (the corresponding
rounded 'half' value) does not fit in a 'short'.

[Quuxplusone]

Maybe you need unsigned short :-)

[Quuxplusone]

thanks very much, this is precise answer which I want to know.