| Message ID | ZgP0XF+rTMrcfVdo@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c-family: Cast __atomic_load_*/__atomic_exchange_* result to _BitInt rather then VCE it [PR114469] | expand |
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.
--- gcc/c-family/c-common.cc.jj 2024-03-08 09:29:30.257747832 +0100 +++ gcc/c-family/c-common.cc 2024-03-26 14:30:59.493031026 +0100 @@ -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;
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