"H.J. Lu" <hjl.to...@gmail.com> writes: > On Tue, Aug 13, 2024 at 4:57 AM Manolis Tsamis <manolis.tsa...@vrull.eu> > wrote: >> >> Now that more operations are allowed for noce_convert_multiple_sets, we need >> to >> check noce_can_force_operand on the sequence before calling >> try_emit_cmove_seq. >> Otherwise an inappropriate argument may be given to copy_to_mode_reg and >> result >> in an ICE. >> >> Fix-up for the recent ifcvt commit 72c9b5f438f22cca493b4e2a8a2a31ff61bf1477 >> >> PR tree-optimization/116353 >> >> gcc/ChangeLog: >> >> * ifcvt.cc (bb_ok_for_noce_convert_multiple_sets): Check >> noce_can_force_operand. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/i386/pr116353.c: New test. >> >> Tested-by: Christoph Müllner <christoph.muell...@vrull.eu> >> Signed-off-by: Manolis Tsamis <manolis.tsa...@vrull.eu> >> --- >> >> gcc/ifcvt.cc | 6 ++- >> gcc/testsuite/gcc.target/i386/pr116353.c | 55 ++++++++++++++++++++++++ >> 2 files changed, 59 insertions(+), 2 deletions(-) >> create mode 100644 gcc/testsuite/gcc.target/i386/pr116353.c >> >> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc >> index 3e25f30b67e..da59c907891 100644 >> --- a/gcc/ifcvt.cc >> +++ b/gcc/ifcvt.cc >> @@ -3938,8 +3938,10 @@ bb_ok_for_noce_convert_multiple_sets (basic_block >> test_bb, unsigned *cost) >> rtx src = SET_SRC (set); >> >> /* Do not handle anything involving memory loads/stores since it might >> - violate data-race-freedom guarantees. */ >> - if (!REG_P (dest) || contains_mem_rtx_p (src)) >> + violate data-race-freedom guarantees. Make sure we can force SRC >> + to a register as that may be needed in try_emit_cmove_seq. */ >> + if (!REG_P (dest) || contains_mem_rtx_p (src) >> + || !noce_can_force_operand (src)) >> return false; >> >> /* Destination and source must be appropriate. */ >> diff --git a/gcc/testsuite/gcc.target/i386/pr116353.c >> b/gcc/testsuite/gcc.target/i386/pr116353.c >> new file mode 100644 >> index 00000000000..8e254653d5d >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/i386/pr116353.c > > Does this test contain x86 specific code? >
Yes, please move it to the generic suite (maybe even torture, as it has
a few nice properties).
>> @@ -0,0 +1,55 @@
>> +/* PR tree-optimization/116353 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2" } */
>> +
>> +enum desmode { C };
>> +struct {
>> + unsigned char des_ivec[];
>> +} _des_crypt_desp;
>> +int des_SPtrans_6_0, des_SPtrans_4_0, des_encrypt_encrypt, des_encrypt_i;
>> +long des_encrypt_s_0, _des_crypt_tin1, _des_crypt_tout0, _des_crypt_tout1,
>> + _des_crypt_tin0;
>> +enum desmode _des_crypt_desp_0;
>> +unsigned long _des_crypt_tbuf[2];
>> +char _des_crypt_out;
>> +void des_encrypt(unsigned long *buf) {
>> + long l, r, t;
>> + l = buf[0];
>> + r = buf[1];
>> + t = r;
>> + r ^= l ^= t < 6;
>> + if (des_encrypt_encrypt)
>> + for (;; des_encrypt_i += 4)
>> + des_encrypt_s_0 ^= des_SPtrans_4_0 | des_SPtrans_6_0;
>> + buf[1] = r;
>> +}
>> +void _des_crypt() {
>> + long xor0, xor1;
>> + unsigned char *in;
>> + int cbc_mode = _des_crypt_desp_0;
>> + in = _des_crypt_desp.des_ivec;
>> + xor0 = xor1 = 0;
>> + for (;;) {
>> + _des_crypt_tin0 = *in++;
>> + _des_crypt_tin0 |= *in++ << 8;
>> + _des_crypt_tin0 |= *in++ << 16;
>> + _des_crypt_tin0 |= (long)*in << 24;
>> + _des_crypt_tin1 = *in++;
>> + _des_crypt_tin1 |= *in++ << 8;
>> + _des_crypt_tin1 |= *in++ << 16;
>> + _des_crypt_tin1 |= (long)*in << 24;
>> + _des_crypt_tbuf[0] = _des_crypt_tin0;
>> + _des_crypt_tbuf[1] = _des_crypt_tin1;
>> + des_encrypt(_des_crypt_tbuf);
>> + if (cbc_mode) {
>> + _des_crypt_tout0 = xor0;
>> + _des_crypt_tout1 = _des_crypt_tbuf[1] ^ xor1;
>> + xor0 = _des_crypt_tin0;
>> + xor1 = _des_crypt_tin1;
>> + } else {
>> + _des_crypt_tout0 = _des_crypt_tbuf[0];
>> + _des_crypt_tout1 = _des_crypt_tbuf[1];
>> + }
>> + _des_crypt_out = _des_crypt_tout0 * _des_crypt_tout1;
>> + }
>> +}
>> --
>> 2.34.1
>>
signature.asc
Description: PGP signature
