On Tue, May 20, 2025 at 03:44:09PM GMT, Jakub Jelinek wrote: > I'd suggest working on it incrementally rather than with a full patch set. > In one or multiple patches handle the promote_mode stuff, the atomic > extension and expr.cc changes with the feedback incorporated.
Ok. > For gimple-lower-bitint.cc I'd really like to see what testing you've done > to decide on a case by case basis. > > > > Are you sure all those changes were really necessary (rather than doing > > > them > > > just in case)? I believe most of gimple-lower-bitint.cc already should be > > > sign or zero extending the partial limbs when storing stuff, there can be > > > some corner cases (I think one of the shift directions at least). > > > > The modifications to gimple-lower-bitint.cc are based on testing, > > The tests weren't included :(. "The tests" refer to the existing regression tests in testsuite/gcc.dg and testsuite/gcc.dg/torture, specifically bitint-*.c. > > since I found that simply setting the "info.extended" flag won't work unless > > I make changes to promote_function_mode, which leads to a series of > > changes to correct all the regtests. > > > > Specifically, the tests told me to extend (thought "truncate" > > was kind of an equivalent word) the output of left shift, plus/minus, > > Truncation is the exact opposite of extension. > I can understand the need for handling of left shifts, for all the rest > I'd really like to see testcases. Again, the existing testcases would do. > > More common tests would surely be helpful, especially for new ports. > > > > However, the specific test you mentioned would not be compatible with > > the proposed LoongArch ABI, where the top 64-bit limb within the top > > 128-bit ABI-limb may be undefined. e.g. _BitInt(192). > > Ugh, that feels very creative in the psABI :(. This only concerns the large/huge _BitInt(N) which is fairly new (i.e. rarely implemented or actually used), so I think it's nice to have some discussion on the creativity (or "weirdo"-ness :D) here. The idea is simple: 1. 8-byte limbs 2. 16-byte alignment for possible vector load/store optimizations 3. The top partial limb is usually handled separately, so the top-limb extension would mostly be useless if we still handle these with non-vector instructions. > In any case, even that could be handled in the macro, although it would > need to have defined(__loongarch__) specific helpers that would perhaps > for > sizeof (x) * __CHAR_BIT__ >= 128 > && (__builtin_popcountg (~(typeof (x)) 0) & 64) == 0 > choose unsigned _BitInt smaller by 64 bits from the one mentioned > in the macro (for signed similarly using __builtin_clrsbg). > > > Perhaps it's better to leave it to target-specific tests? > > Please don't, we don't want to repeat that for all the info->extended > targets (which looks to be arm 32-bit, s390x and loongarch right now). > We want to test it on all, like the whole bitint testsuite helps to find > issues on all the arches, most of it isn't target specific. Ok.
