This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 28/28] New -fcompare-elim pass.
+ /* Find the DEF of the flags register. It must be there. */
+ for (use_rec = DF_INSN_DEFS (insn); ; use_rec++)
+ {
+ use = *use_rec;
+ if (DF_REF_TYPE (use) == DF_REF_REG_DEF
+ && DF_REF_REGNO (use) == targetm.flags_regnum)
+ break;
+ }
Can you rename these to def/def_rec? For a moment I thought you were
not using def-use chains because I found only DF_REF_CHAIN (use).
(My observations below actually override this comment).
+ /* Note that df does not create use chains that cross basic blocks.
I don't think this is correct, as this is the very thing that is
responsible for the chains problem's potential quadratic behavior. Have
you seen it in practice (the chain that doesn't cross basic blocks, not
the quadratic behavior)?
This is basically the only case in which you'd rely on non-singleton
chains, and you're solving it by punting anyway (i.e. via missing_uses).
This means this pass could be done just as easily without def-use chains.
During the forward walk you can record the last definition of CC in the
basic block (which could start at last_cmp for an extended basic block).
Then, when you walk each insn's uses and look for a CC use. If you
find it, you know what it's last definition is. That is,
find_flags_uses_in_bb becomes something like maybe_record_flags_use and
you would call it for all instructions, passing the last CC def.
+ if (DF_REF_CHAIN (use) == NULL)
+ return false;
+
+ def = DF_REF_CHAIN (use)->ref;
Here you should probably bail out if the use has multiple reaching
definitions (i.e. DF_REF_CHAIN (use)->next != NULL). It probably won't
happen given how cbranch/cstore splitters work, but you never know.
+ Note that this doesn't follow the USE-DEF chain from X, but
+ since we already have to search for the previous clobber of
+ the flags register, this wouldn't really be a problem. */
+
+ /* Make sure that the flags are not clobbered in between the two
+ instructions. Unfortunately, we don't get DEF-DEF chains, so
+ do this the old fashioned way. */
Again, this is probably handled better without the use-def chains.
(Chains are in the DF framework, but are rarely the best
solution---especially if you're not limiting them to a region, e.g. a loop).
For a full-blown solution, we could generalize the
singleton-use-def-chains code from fwprop and reuse it here. Actually,
I don't think you plan to move instructions in compare-elim, just like
NOTICE_UPDATE_CC didn't. So, the forward scan can remember, together
with the last comparison insn, the last instruction that clobbered the
flags. Then whenever you find a comparison, you can already look at the
shape of the last clobber; you then store if the comparison instruction
is "mergeable", and if so with which insn.
Non-mergeable comparisons would still be recorded for the sake of
eliminating duplicates.
+ /* Succeed if the new instruction is valid. */
+ validate_change (dinsn, &XVECEXP (dpat, 0, 1), x, true);
+ if (!apply_change_group ())
+ return false;
Here you can test the return value of validate_change directly.
Thanks very much for this work, it is a very nice and readable pass, and
probably one of the biggest steps towards eradication of cc0.
HTH,
Paolo