[PATCH 1/7] ifcvt: Check if cmovs are needed.

Robin Dapp rdapp@linux.ibm.com
Thu Jul 22 12:06:46 GMT 2021


Hi Richard,

thanks for going through the patch set.

> A hash_set might be simpler, given that the code only enters insns
> for which the bool is false.  “rtx_insn *” would be better than rtx.

Right, changed that.

> Do you mean the sets might be removed or that the checks might be
> removed?

It's actually the checks that are removed.  I clarified and amended the 
comments.

> The patch is quite hard to review on its own, since nothing actually
> uses this variable.  It's also not obvious how the
> reg_overlap_mentioned_p code works if the old target is referenced
> later.
> 
> Could you refactor the series a bit so that each patch is
> self-contained? It's OK if that means fewer patches.
The attached v2 makes use of need_cmov now, I hope this makes it a bit 
clearer.  Regarding the reg_overlap_mentioned_p: it is used to detect 
the swap idioms as well as when a cmov destination is used in the 
condition as well.  If needed this could be two separate patches (most 
of the second patch would be comments, though).

Regards
  Robin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v2-0001-ifcvt-Check-if-cmovs-are-needed.patch
Type: text/x-patch
Size: 6312 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210722/141715a2/attachment.bin>


More information about the Gcc-patches mailing list