c-family: Cast __atomic_load_*/__atomic_exchange_* result to _BitInt rather then VCE it [PR114469]
Checks
Commit Message
Hi!
As written in the PR, torture/bitint-64.c test fails with -O2 -flto
and the reason is that on _BitInt arches where the padding bits
are undefined, the padding bits in the _Atomic vars are also undefined,
but when __atomic_load or __atomic_exchange on a _BitInt _Atomic variable
with some padding bits is lowered into __atomic_load_{1,2,4,8,16} or
__atomic_exchange_*, the mode precision unsigned result is VIEW_CONVERT_EXPR
converted to _BitInt and because of the VCE nothing actually sign/zero
extends it as needed for later uses - the var is no longer addressable and
expansion assumes such automatic vars are properly extended.
The following patch fixes that by using NOP_EXPR on it (the
VIEW_CONVERT_EXPR after it will then be optimized away during
gimplification, didn't want to repeat it in the code as else result = build1
(VIEW_CONVERT_EXPR, ...); twice.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2024-03-26 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/114469
* c-common.cc (resolve_overloaded_builtin): For _BitInt result
on !extended targets convert result to the _BitInt type before
using VIEW_CONVERT_EXPR.
Jakub
Comments
On Wed, 27 Mar 2024, Jakub Jelinek wrote:
> Hi!
>
> As written in the PR, torture/bitint-64.c test fails with -O2 -flto
> and the reason is that on _BitInt arches where the padding bits
> are undefined, the padding bits in the _Atomic vars are also undefined,
> but when __atomic_load or __atomic_exchange on a _BitInt _Atomic variable
> with some padding bits is lowered into __atomic_load_{1,2,4,8,16} or
> __atomic_exchange_*, the mode precision unsigned result is VIEW_CONVERT_EXPR
> converted to _BitInt and because of the VCE nothing actually sign/zero
> extends it as needed for later uses - the var is no longer addressable and
> expansion assumes such automatic vars are properly extended.
>
> The following patch fixes that by using NOP_EXPR on it (the
> VIEW_CONVERT_EXPR after it will then be optimized away during
> gimplification, didn't want to repeat it in the code as else result = build1
> (VIEW_CONVERT_EXPR, ...); twice.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
This is OK. A natural question arising is whether anything else might
generate a VIEW_CONVERT_EXPR for code that somehow reinterprets memory as
of type _BitInt, and then later fail to ensure required sign/zero
extension, or whether this is the only place where a VIEW_CONVERT_EXPR of
_BitInt type can cause problems.
@@ -8461,7 +8461,19 @@ resolve_overloaded_builtin (location_t l
if (new_return)
{
/* Cast function result from I{1,2,4,8,16} to the required type. */
- result = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (new_return), result);
+ if (TREE_CODE (TREE_TYPE (new_return)) == BITINT_TYPE)
+ {
+ struct bitint_info info;
+ unsigned prec = TYPE_PRECISION (TREE_TYPE (new_return));
+ targetm.c.bitint_type_info (prec, &info);
+ if (!info.extended)
+ /* For _BitInt which has the padding bits undefined
+ convert to the _BitInt type rather than VCE to force
+ zero or sign extension. */
+ result = build1 (NOP_EXPR, TREE_TYPE (new_return), result);
+ }
+ result
+ = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (new_return), result);
result = build2 (MODIFY_EXPR, TREE_TYPE (new_return), new_return,
result);
TREE_SIDE_EFFECTS (result) = 1;