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. > > 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.
