On 4/8/24 13:43, Ilya Leoshkevich wrote: > On Sat, 2024-04-06 at 18:58 +0200, Jakub Jelinek wrote: >> Hi! >> >> The following testcase is miscompiled, because we have initially >> a movti which loads the 0x3f8000003f800000ULL TImode constant >> from constant pool. Later on we split it into a pair of DImode >> loads. Now, for the first load (why just that?, though not stage4 >> material) we trigger the peephole2 which uses >> s390_const_int_pool_entry_p. >> That function doesn't check at all the constant pool mode though, >> sees >> the constant pool at that address has a CONST_INT value and just >> assumes >> that is the value to return, which is especially wrong for big- >> endian, >> if it is a DImode load from offset 0, it should be loading 0 rather >> than >> 0x3f8000003f800000ULL. >> The following patch adds checks if we are extracing a MODE_INT mode, >> if the constant pool has MODE_INT mode as well, punts if constant >> pool >> has smaller mode size than the extraction one (then it would be UB), >> if it has the same mode as before keeps using what it did before, >> if constant pool has a larger mode than the one being extracted, uses >> simplify_subreg. I'd have used avoid_constant_pool_reference >> instead which can handle also offsets into the constant pool >> constants, >> but it can't handle UNSPEC_LTREF. >> >> Another thing is that once that is fixed, we ICE when we extract >> constant >> like 0, ior insn predicate require non-0 constant. So, the patch >> also >> fixes the peephole2 so that if either 32-bit half is zero, it uses a >> mere >> load of the constant into register rather than a pair of such load >> and ior. >> >> Bootstrapped/regtested on s390x-linux, ok for trunk? > > Hi Jakub, thanks for the patch, it looks good to me. > Since I'm not a maintainer, we need to wait for Andreas' opinion.
Ok. Thank you very much Jakub for fixing this! Andreas > >> >> 2024-04-06 Jakub Jelinek <ja...@redhat.com> >> >> PR target/114605 >> * config/s390/s390.cc (s390_const_int_pool_entry_p): Punt >> if mem doesn't have MODE_INT mode, or pool constant doesn't >> have MODE_INT mode, or if pool constant mode is smaller than >> mem mode. If mem mode is different from pool constant mode, >> try to simplify subreg. If that doesn't work, punt, if it >> does, use the simplified constant instead of the constant >> pool >> constant. >> * config/s390/s390.md (movdi from const pool peephole): If >> either low or high 32-bit part is zero, just emit move insn >> instead of move + ior. >> >> * gcc.dg/pr114605.c: New test.
