On Thu, 30 Nov 2023 at 17:53, Jeff Law <jeffreya...@gmail.com> wrote: > > * ext-dce.c: Fixes for carry handling. > > > > * ext-dce.c (safe_for_live_propagation): Handle MINUS. > > (ext_dce_process_uses): Break out carry handling into .. > > (carry_backpropagate): This new function. > > Better handling of ASHIFT. > > Add handling of SMUL_HIGHPART, UMUL_HIGHPART, SIGN_EXTEND, SS_ASHIFT > > and > > US_ASHIFT. > > > > * ext-dce.c: fix SUBREG_BYTE test > > > > As mentioned in > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637486.html > > and > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638473.html > > > > > > diff --git a/gcc/ext-dce.cc b/gcc/ext-dce.cc > > index 4e4c57de117..228c50e8b73 100644 > > --- a/gcc/ext-dce.cc > > +++ b/gcc/ext-dce.cc > > @@ -38,7 +38,10 @@ along with GCC; see the file COPYING3. If not see > > bit 0..7 (least significant byte) > > bit 8..15 (second least significant byte) > > bit 16..31 > > - bit 32..BITS_PER_WORD-1 */ > > + bit 32..BITS_PER_WORD-1 > > + > > + For vector modes, we apply these bit groups to every lane; if any of the > > + bits in the group are live in any lane, we consider this group live. */ > Why add vector modes now? I realize it might help a vectorized sub*_dct > from x264, but I was thinking that would be more of a gcc-15 improvement.
Actually, we already did, but because it was unintentional, it wasn't done properly. I've been using BEG_MODE_BITSIZE(GET_MODE (x)).to_constant, thinking a mode should just have a constant size that can easily fit into an int. I was wrong. Debugging found that was a scalable vector mode. SUBREGs, shifts and other stuff has vector modes and goes through the code. Instead of adding code to bail our, I though it would be a good idea to think about how vector modes can be supported without balooning the computation time or memory. And keeping in mind the original intention of the patch - eliminating redundant sign/zero extension - that actually can applied to vectors as well, and that means we should consider how these operations work on each lane. By looking at the inner mode of a vector, we also conventiently also get a sane size. For complex numbers, it's also saner to treat them as two-element vectors, tham trying to apply the bit parts while ignoring the structure, so it makes sense to use GET_MODE_INNER in general. Something that could be done for further improvement but seems too complex for gcc 14 would be to handle vector constants as shift counts. Come to think of it, I actually applied the wrong test for the integer shift counts - it should be CONST_INT_P, not CONSTANT_P. > > > > /* Note this pass could be used to narrow memory loads too. It's > > not clear if that's profitable or not in general. */ > > > @@ -96,6 +100,8 @@ safe_for_live_propagation (rtx_code code) > > case SS_ASHIFT: > > case US_ASHIFT: > > case ASHIFT: > > + case LSHIFTRT: > > + case ASHIFTRT: > > return true; > So this starts to touch on a cleanup Richard mentioned. The codes in > there until now were supposed to be safe across the board. Saturating operations are not safe at all without explicitly computing the liveness propagation. > As we add > things like LSHIFTRT, we need to describe how to handle liveness > transfer from the destination into the source(s). I think what Richard > is asking for is to just have one place which handles both. LSHIFTRT is much simpler than the saturating operations. > Anyway, my current plan would be to pull in the formatting fixes, the > back propagation without the vector enhancement. Pretending the vector modes don't happen is not making the code safe. We have to handle them somehow, so we might as well do that in a way that is consistent and gives more potential for optimization.
