================
@@ -103,6 +103,13 @@ class TypeDescriptor {
     /// representation is that of bitcasting the floating-point value to an
     /// integer type.
     TK_Float = 0x0001,
+    /// An _BitInt(N) type. Lowest bit is 1 for a signed value, 0 for an
+    /// unsigned value. Remaining bits are log_2(bit_width). The value
----------------
bjope wrote:

The new thing in this patch is that the size of the _BitInt type being 
described is stored after the type string. It can be read using 
`getIntegerBitCount()`.

Thus, to get the size of the type being described one should no longer use  
`getIntegerBitWidth()` but instead `getIntegerBitCount()`. The latter would 
give correct result for both TK_Integer and TK_BitInt, while the former doesn't 
work for BitInt.  IMHO it had been better to keep the name 
`getIntegerBitWidth()` for this purpose. Now I figure most/all old uses of 
`getIntegerBitWidth()` actually should be replaced with calls to 
`getIntegerBitCount()`, but that has not happened as far as I can tell.

The new behavior of  `getIntegerBitWidth()` is merely to indicate the size used 
to store the Value being represented when it does not fit inside the 
ValueHandle. So there shouldn't really be any need to use that function outside 
`Value::getSIntValue()` (assuming that `getIntegerBitCount()` would know how to 
get the size of TK_Integer via the TypeInfo field by itself).

And unfortunatly there isn't much information about how those non-inlined 
values are store. The `Value::getSIntValue()` functions is just reading an s64 
or s128 from memory (and that is what it traditionally has been doing for 
TK_Integer). But then I think the IR emitted by clang should cast the value to 
i64/i128 instead of doing  
```
    RawAddress Ptr = CreateDefaultAlignTempAlloca(V->getType());
    Builder.CreateStore(V, Ptr);
```
Now we for example get things like `store i66 %x, ptr %tmp` which is storing a 
non byte-sized value to memory, which then is loaded by the runtime as a 
128-bit value.
Worth noting is that after commit 9ad72df55cb74b2919327 (#91364) the frontend 
emits byte-sized loads/stores for _BitInt. So I really think the store emitted 
by CodeGenFunction::EmitCheckValue for the BitInt values should zero/sign 
extended similarly (as if they were casted to "s64" or "s128" before being 
stored). That would at least make the code a bit more consistent (even though 
the question about supporting values larger than 128-bits would remain).

https://github.com/llvm/llvm-project/pull/96240
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to