| Message ID | 20200529213109.44674-1-acsawdey@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: PR target/95347 Correctly identify stfs if prefixed | expand |
Hi! Re: [PATCH] rs6000: PR target/95347 Correctly identify stfs if prefixed Please put the PR id at the end of the subject (it is the least important information). You can also shorten it to "PR95347" -- total subject length ideally is maybe 50 chars, so something like "rtl-optimization" would be extremely long, for no good reason. On Fri, May 29, 2020 at 04:31:09PM -0500, Aaron Sawdey via Gcc-patches wrote: > Because reg_to_non_prefixed() only looks at the register being used, it > doesn't get the right answer for stfs, which leads to us not seeing > that it has a PCREL symbol ref. This patch works around this by > introducing a helper function that inspects the insn to see if it is in > fact a stfs. Then if we use NON_PREFIXED_DEFAULT, address_to_insn_form() > can see that it has the PCREL symbol ref. > +/* Helper function to see if we're potentially looking at stfs that > + could be pstfs. */ "That could be pstfs" is only confusing here, I think? It has nothing to do with this function itself. "Return true if INSN is a "movsi_from_sf" to memory"? > +static bool > +is_stfs_insn (rtx_insn *insn) > +{ > + rtx pattern=PATTERN (insn); Spaces on both sides of binary operators (like "="). > + if (GET_CODE (pattern) != PARALLEL) > + return false; > + > + /* This should be a parallel with exactly one set and one clobber. */ You could simplify this: it has to be a parallel of exactly two things, the first a SET, the second a CLOBBER? > + int i; > + rtx set=NULL, clobber=NULL; > + for (i = 0; i < XVECLEN (pattern, 0); i++) rtx set = NULL; rtx clobber = NULL; for (int i = 0; i < XVECLEN (pattern, 0); i++) (Declarations with initialiser go on a line by their own; "i" doesn't need declaring before the loop). > + /* All we care is that the destination of the SET is a mem:SI, > + the source should be an UNSPEC_SI_FROM_SF, and the clobber > + should be a scratch:V4SF. */ > + > + rtx dest = XEXP (set, 0); rtx dest = SET_DEST (set); > + rtx src = XEXP (set, 1); rtx src = SET_SRC (set); > + rtx scratch = XEXP (clobber, 0); rtx scratch = SET_DEST (clobber); > @@ -25119,8 +25171,14 @@ prefixed_store_p (rtx_insn *insn) > return false; > > machine_mode mem_mode = GET_MODE (mem); > + rtx addr = XEXP (mem, 0); > enum non_prefixed_form non_prefixed = reg_to_non_prefixed (reg, mem_mode); > - return address_is_prefixed (XEXP (mem, 0), mem_mode, non_prefixed); > + /* Need to make sure we aren't looking at a stfs which doesn't > + looking like the other things that we are looking for. */ s/looking/look/ I guess? > + if (non_prefixed == NON_PREFIXED_X && is_stfs_insn (insn)) > + return address_is_prefixed (addr, mem_mode, NON_PREFIXED_DEFAULT); > + else > + return address_is_prefixed (addr, mem_mode, non_prefixed); Rest looks fine :-) Okay for trunk with the nits fixed and the suggestions looked at. Also okay for 10, if wanted there? Thanks! Segher
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 8435bc15d72..d58fceeeea4 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -24980,6 +24980,58 @@ address_to_insn_form (rtx addr, return INSN_FORM_BAD; } +/* Helper function to see if we're potentially looking at stfs that + could be pstfs. */ + +static bool +is_stfs_insn (rtx_insn *insn) +{ + rtx pattern=PATTERN (insn); + if (GET_CODE (pattern) != PARALLEL) + return false; + + /* This should be a parallel with exactly one set and one clobber. */ + int i; + rtx set=NULL, clobber=NULL; + for (i = 0; i < XVECLEN (pattern, 0); i++) + { + rtx elt = XVECEXP (pattern, 0, i); + if (GET_CODE (elt) == SET) + { + if (set) + return false; + set = elt; + } + else if (GET_CODE (elt) == CLOBBER) + { + if (clobber) + return false; + clobber = elt; + } + else + return false; + } + + /* All we care is that the destination of the SET is a mem:SI, + the source should be an UNSPEC_SI_FROM_SF, and the clobber + should be a scratch:V4SF. */ + + rtx dest = XEXP (set, 0); + rtx src = XEXP (set, 1); + rtx scratch = XEXP (clobber, 0); + + if (GET_CODE (src) != UNSPEC || XINT (src, 1) != UNSPEC_SI_FROM_SF) + return false; + + if (GET_CODE (dest) != MEM || GET_MODE (dest) != SImode) + return false; + + if (GET_CODE (scratch) != SCRATCH || GET_MODE (scratch) != V4SFmode) + return false; + + return true; +} + /* Helper function to take a REG and a MODE and turn it into the non-prefixed instruction format (D/DS/DQ) used for offset memory. */ @@ -25119,8 +25171,14 @@ prefixed_store_p (rtx_insn *insn) return false; machine_mode mem_mode = GET_MODE (mem); + rtx addr = XEXP (mem, 0); enum non_prefixed_form non_prefixed = reg_to_non_prefixed (reg, mem_mode); - return address_is_prefixed (XEXP (mem, 0), mem_mode, non_prefixed); + /* Need to make sure we aren't looking at a stfs which doesn't + looking like the other things that we are looking for. */ + if (non_prefixed == NON_PREFIXED_X && is_stfs_insn (insn)) + return address_is_prefixed (addr, mem_mode, NON_PREFIXED_DEFAULT); + else + return address_is_prefixed (addr, mem_mode, non_prefixed); } /* Whether a load immediate or add instruction is a prefixed instruction. This